Message ID | 20211115193105.1952656-1-sdf@google.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | [bpf-next,v2] bpftool: add current libbpf_strict mode to version output | expand |
On Mon, 15 Nov 2021 at 19:31, Stanislav Fomichev <sdf@google.com> wrote: > > + bpftool --legacy --version > bpftool v5.15.0 > features: libbfd, skeletons > + bpftool --version > bpftool v5.15.0 > features: libbfd, libbpf_strict, skeletons > > + bpftool --legacy --help > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > bpftool batch file FILE > bpftool version > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > {-V|--version} } > + bpftool --help > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > bpftool batch file FILE > bpftool version > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > {-V|--version} } > > + bpftool --legacy > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > bpftool batch file FILE > bpftool version > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > {-V|--version} } > + bpftool > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > bpftool batch file FILE > bpftool version > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > {-V|--version} } > > + bpftool --legacy version > bpftool v5.15.0 > features: libbfd, skeletons > + bpftool version > bpftool v5.15.0 > features: libbfd, libbpf_strict, skeletons > > + bpftool --json --legacy version > {"version":"5.15.0","features":{"libbfd":true,"libbpf_strict":false,"skeletons":true}} > + bpftool --json version > {"version":"5.15.0","features":{"libbfd":true,"libbpf_strict":true,"skeletons":true}} > > v2: > - fixes for -h and -V (Quentin Monnet) > > Suggested-by: Quentin Monnet <quentin@isovalent.com> > Signed-off-by: Stanislav Fomichev <sdf@google.com> The behaviour will change in a few cases where both the help and version commands and/or options are provided, e.g. "bpftool -h version" used to print the help and will do the version instead, "bpftool -V help" changes in an opposite fashion. Given that there's no practical interest in having both commands/options, and that the behaviour was not really consistent so far, I consider that this is not an issue. However, we now have "bpftool --version" returning -1 (instead of 0). Any chance we can fix that? Maybe simply something like the change below instead? ------ diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 473791e87f7d..b2e67b0f02cf 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -400,6 +400,7 @@ int main(int argc, char **argv) { "legacy", no_argument, NULL, 'l' }, { 0 } }; + bool version_requested = false; int opt, ret; last_do_help = do_help; @@ -414,7 +415,8 @@ int main(int argc, char **argv) options, NULL)) >= 0) { switch (opt) { case 'V': - return do_version(argc, argv); + version_requested = true; + break; case 'h': return do_help(argc, argv); case 'p': @@ -479,6 +481,9 @@ int main(int argc, char **argv) if (argc < 0) usage(); + if (version_requested) + return do_version(argc, argv); + ret = cmd_select(cmds, argc, argv, do_help); if (json_output)
On Mon, Nov 15, 2021 at 3:34 PM Quentin Monnet <quentin@isovalent.com> wrote: > > On Mon, 15 Nov 2021 at 19:31, Stanislav Fomichev <sdf@google.com> wrote: > > > > + bpftool --legacy --version > > bpftool v5.15.0 > > features: libbfd, skeletons > > + bpftool --version > > bpftool v5.15.0 > > features: libbfd, libbpf_strict, skeletons > > > > + bpftool --legacy --help > > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > > bpftool batch file FILE > > bpftool version > > > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > > {-V|--version} } > > + bpftool --help > > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > > bpftool batch file FILE > > bpftool version > > > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > > {-V|--version} } > > > > + bpftool --legacy > > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > > bpftool batch file FILE > > bpftool version > > > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > > {-V|--version} } > > + bpftool > > Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } > > bpftool batch file FILE > > bpftool version > > > > OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } > > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | > > {-V|--version} } > > > > + bpftool --legacy version > > bpftool v5.15.0 > > features: libbfd, skeletons > > + bpftool version > > bpftool v5.15.0 > > features: libbfd, libbpf_strict, skeletons > > > > + bpftool --json --legacy version > > {"version":"5.15.0","features":{"libbfd":true,"libbpf_strict":false,"skeletons":true}} > > + bpftool --json version > > {"version":"5.15.0","features":{"libbfd":true,"libbpf_strict":true,"skeletons":true}} > > > > v2: > > - fixes for -h and -V (Quentin Monnet) > > > > Suggested-by: Quentin Monnet <quentin@isovalent.com> > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > The behaviour will change in a few cases where both the help and > version commands and/or options are provided, e.g. "bpftool -h > version" used to print the help and will do the version instead, > "bpftool -V help" changes in an opposite fashion. Given that there's > no practical interest in having both commands/options, and that the > behaviour was not really consistent so far, I consider that this is > not an issue. > > However, we now have "bpftool --version" returning -1 (instead of 0). > Any chance we can fix that? Maybe simply something like the change > below instead? That works as well. I didn't want to special case it, but agreed that changing exit value might not be a good idea (I was assuming they already return -1 and didn't check). Will resend shortly with your version.
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 473791e87f7d..601d0ee5e6d3 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -93,6 +93,7 @@ static int do_version(int argc, char **argv) jsonw_name(json_wtr, "features"); jsonw_start_object(json_wtr); /* features */ jsonw_bool_field(json_wtr, "libbfd", has_libbfd); + jsonw_bool_field(json_wtr, "libbpf_strict", !legacy_libbpf); jsonw_bool_field(json_wtr, "skeletons", has_skeletons); jsonw_end_object(json_wtr); /* features */ @@ -106,6 +107,10 @@ static int do_version(int argc, char **argv) printf(" libbfd"); nb_features++; } + if (!legacy_libbpf) { + printf("%s libbpf_strict", nb_features++ ? "," : ""); + nb_features++; + } if (has_skeletons) printf("%s skeletons", nb_features++ ? "," : ""); printf("\n"); @@ -414,9 +419,11 @@ int main(int argc, char **argv) options, NULL)) >= 0) { switch (opt) { case 'V': - return do_version(argc, argv); + last_do_help = do_version; + break; case 'h': - return do_help(argc, argv); + last_do_help = do_help; + break; case 'p': pretty_output = true; /* fall through */ @@ -476,7 +483,7 @@ int main(int argc, char **argv) argc -= optind; argv += optind; - if (argc < 0) + if (argc < 1) usage(); ret = cmd_select(cmds, argc, argv, do_help);
+ bpftool --legacy --version bpftool v5.15.0 features: libbfd, skeletons + bpftool --version bpftool v5.15.0 features: libbfd, libbpf_strict, skeletons + bpftool --legacy --help Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } bpftool batch file FILE bpftool version OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | {-V|--version} } + bpftool --help Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } bpftool batch file FILE bpftool version OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | {-V|--version} } + bpftool --legacy Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } bpftool batch file FILE bpftool version OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | {-V|--version} } + bpftool Usage: bpftool [OPTIONS] OBJECT { COMMAND | help } bpftool batch file FILE bpftool version OBJECT := { prog | map | link | cgroup | perf | net | feature | btf | gen | struct_ops | iter } OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy} | {-V|--version} } + bpftool --legacy version bpftool v5.15.0 features: libbfd, skeletons + bpftool version bpftool v5.15.0 features: libbfd, libbpf_strict, skeletons + bpftool --json --legacy version {"version":"5.15.0","features":{"libbfd":true,"libbpf_strict":false,"skeletons":true}} + bpftool --json version {"version":"5.15.0","features":{"libbfd":true,"libbpf_strict":true,"skeletons":true}} v2: - fixes for -h and -V (Quentin Monnet) Suggested-by: Quentin Monnet <quentin@isovalent.com> Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/bpf/bpftool/main.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)