| Message ID | 20190523105426.3938-2-quentin.monnet@netronome.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | tools: bpftool: add an option for debug output from libbpf and verifier | expand |
On Thu, May 23, 2019 at 3:54 AM Quentin Monnet <quentin.monnet@netronome.com> wrote: > > libbpf has three levels of priority for output messages: warn, info, > debug. By default, debug output is not printed to the console. > > Add a new "--debug" (short name: "-d") option to bpftool to print libbpf > logs for all three levels. > > Internally, we simply use the function provided by libbpf to replace the > default printing function by one that prints logs regardless of their > level. > > v2: > - Remove the possibility to select the log-levels to use (v1 offered a > combination of "warn", "info" and "debug"). > - Rename option and offer a short name: -d|--debug. Such and option in CLI tools is usually called -v|--verbose, I'm wondering if it might be a better name choice? Btw, some tools also use -v, -vv and -vvv to define different levels of verbosity, which is something we can consider in the future, as it's backwards compatible. > - Add option description to all bpftool manual pages (instead of > bpftool-prog.rst only), as all commands use libbpf. > > Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > --- > tools/bpf/bpftool/Documentation/bpftool-btf.rst | 4 ++++ > tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 4 ++++ > .../bpf/bpftool/Documentation/bpftool-feature.rst | 4 ++++ > tools/bpf/bpftool/Documentation/bpftool-map.rst | 4 ++++ > tools/bpf/bpftool/Documentation/bpftool-net.rst | 4 ++++ > tools/bpf/bpftool/Documentation/bpftool-perf.rst | 4 ++++ > tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 ++++ > tools/bpf/bpftool/Documentation/bpftool.rst | 3 +++ > tools/bpf/bpftool/bash-completion/bpftool | 2 +- > tools/bpf/bpftool/main.c | 14 +++++++++++++- > 10 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst > index 2dbc1413fabd..00668df1bf7a 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst > @@ -67,6 +67,10 @@ OPTIONS > -p, --pretty > Generate human-readable JSON output. Implies **-j**. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > EXAMPLES > ======== > **# bpftool btf dump id 1226** > diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst > index ac26876389c2..36807735e2a5 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst > @@ -113,6 +113,10 @@ OPTIONS > -f, --bpffs > Show file names of pinned programs. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > EXAMPLES > ======== > | > diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst > index 14180e887082..4d08f35034a2 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst > @@ -73,6 +73,10 @@ OPTIONS > -p, --pretty > Generate human-readable JSON output. Implies **-j**. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > SEE ALSO > ======== > **bpf**\ (2), > diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst > index 13ef27b39f20..490b4501cb6e 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst > @@ -152,6 +152,10 @@ OPTIONS > Do not automatically attempt to mount any virtual file system > (such as tracefs or BPF virtual file system) when necessary. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > EXAMPLES > ======== > **# bpftool map show** > diff --git a/tools/bpf/bpftool/Documentation/bpftool-net.rst b/tools/bpf/bpftool/Documentation/bpftool-net.rst > index 934580850f42..d8e5237a2085 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-net.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-net.rst > @@ -65,6 +65,10 @@ OPTIONS > -p, --pretty > Generate human-readable JSON output. Implies **-j**. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > EXAMPLES > ======== > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-perf.rst b/tools/bpf/bpftool/Documentation/bpftool-perf.rst > index 0c7576523a21..e252bd0bc434 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-perf.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-perf.rst > @@ -53,6 +53,10 @@ OPTIONS > -p, --pretty > Generate human-readable JSON output. Implies **-j**. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > EXAMPLES > ======== > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst > index e8118544d118..9a92614569e6 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst > @@ -174,6 +174,10 @@ OPTIONS > Do not automatically attempt to mount any virtual file system > (such as tracefs or BPF virtual file system) when necessary. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > + > EXAMPLES > ======== > **# bpftool prog show** > diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst > index 3e562d7fd56f..43dba0717953 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool.rst > @@ -66,6 +66,9 @@ OPTIONS > Do not automatically attempt to mount any virtual file system > (such as tracefs or BPF virtual file system) when necessary. > > + -d, --debug > + Print all logs available from libbpf, including debug-level > + information. > > SEE ALSO > ======== > diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool > index 50e402a5a9c8..3a476e25d046 100644 > --- a/tools/bpf/bpftool/bash-completion/bpftool > +++ b/tools/bpf/bpftool/bash-completion/bpftool > @@ -181,7 +181,7 @@ _bpftool() > > # Deal with options > if [[ ${words[cword]} == -* ]]; then > - local c='--version --json --pretty --bpffs --mapcompat' > + local c='--version --json --pretty --bpffs --mapcompat --debug' > COMPREPLY=( $( compgen -W "$c" -- "$cur" ) ) > return 0 > fi > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c > index 1ac1fc520e6a..d74293938a05 100644 > --- a/tools/bpf/bpftool/main.c > +++ b/tools/bpf/bpftool/main.c > @@ -10,6 +10,7 @@ > #include <string.h> > > #include <bpf.h> > +#include <libbpf.h> > > #include "main.h" > > @@ -77,6 +78,13 @@ static int do_version(int argc, char **argv) > return 0; > } > > +static int __printf(2, 0) > +print_all_levels(__maybe_unused enum libbpf_print_level level, > + const char *format, va_list args) > +{ > + return vfprintf(stderr, format, args); > +} > + > int cmd_select(const struct cmd *cmds, int argc, char **argv, > int (*help)(int argc, char **argv)) > { > @@ -317,6 +325,7 @@ int main(int argc, char **argv) > { "bpffs", no_argument, NULL, 'f' }, > { "mapcompat", no_argument, NULL, 'm' }, > { "nomount", no_argument, NULL, 'n' }, > + { "debug", no_argument, NULL, 'd' }, > { 0 } > }; > int opt, ret; > @@ -332,7 +341,7 @@ int main(int argc, char **argv) > hash_init(map_table.table); > > opterr = 0; > - while ((opt = getopt_long(argc, argv, "Vhpjfmn", > + while ((opt = getopt_long(argc, argv, "Vhpjfmnd", > options, NULL)) >= 0) { > switch (opt) { > case 'V': > @@ -362,6 +371,9 @@ int main(int argc, char **argv) > case 'n': > block_mount = true; > break; > + case 'd': > + libbpf_set_print(print_all_levels); > + break; > default: > p_err("unrecognized option '%s'", argv[optind - 1]); > if (json_output) > -- > 2.17.1 >
On Thu, 23 May 2019 09:20:52 -0700, Andrii Nakryiko wrote: > On Thu, May 23, 2019 at 3:54 AM Quentin Monnet wrote: > > > > libbpf has three levels of priority for output messages: warn, info, > > debug. By default, debug output is not printed to the console. > > > > Add a new "--debug" (short name: "-d") option to bpftool to print libbpf > > logs for all three levels. > > > > Internally, we simply use the function provided by libbpf to replace the > > default printing function by one that prints logs regardless of their > > level. > > > > v2: > > - Remove the possibility to select the log-levels to use (v1 offered a > > combination of "warn", "info" and "debug"). > > - Rename option and offer a short name: -d|--debug. > > Such and option in CLI tools is usually called -v|--verbose, I'm > wondering if it might be a better name choice? > > Btw, some tools also use -v, -vv and -vvv to define different levels > of verbosity, which is something we can consider in the future, as > it's backwards compatible. That was my weak suggestion. Sometimes -v is used for version, e.g. GCC. -d is sometimes used for debug, e.g. man, iproute2 uses it as short for "detailed". If the consensus is that -v is better I don't really mind.
On Thu, May 23, 2019 at 1:44 PM Jakub Kicinski <jakub.kicinski@netronome.com> wrote: > > On Thu, 23 May 2019 09:20:52 -0700, Andrii Nakryiko wrote: > > On Thu, May 23, 2019 at 3:54 AM Quentin Monnet wrote: > > > > > > libbpf has three levels of priority for output messages: warn, info, > > > debug. By default, debug output is not printed to the console. > > > > > > Add a new "--debug" (short name: "-d") option to bpftool to print libbpf > > > logs for all three levels. > > > > > > Internally, we simply use the function provided by libbpf to replace the > > > default printing function by one that prints logs regardless of their > > > level. > > > > > > v2: > > > - Remove the possibility to select the log-levels to use (v1 offered a > > > combination of "warn", "info" and "debug"). > > > - Rename option and offer a short name: -d|--debug. > > > > Such and option in CLI tools is usually called -v|--verbose, I'm > > wondering if it might be a better name choice? > > > > Btw, some tools also use -v, -vv and -vvv to define different levels > > of verbosity, which is something we can consider in the future, as > > it's backwards compatible. > > That was my weak suggestion. Sometimes -v is used for version, e.g. > GCC. -d is sometimes used for debug, e.g. man, iproute2 uses it as > short for "detailed". If the consensus is that -v is better I don't > really mind. It's minor, so I'm not insisting at all, just wasn't sure it was brought up. bpftool is sufficiently different in its conventions from other modern CLIs anyways. As for -v for version. It seems like the trend is to use -V|--version for version, and -v|--verbose for verbosity. I've also seen some tools option for `cli version` (subcommand) for version. Anyway, no strong preferences from me either.
2019-05-23 13:57 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com> > On Thu, May 23, 2019 at 1:44 PM Jakub Kicinski > <jakub.kicinski@netronome.com> wrote: >> >> On Thu, 23 May 2019 09:20:52 -0700, Andrii Nakryiko wrote: >>> On Thu, May 23, 2019 at 3:54 AM Quentin Monnet wrote: >>>> >>>> libbpf has three levels of priority for output messages: warn, info, >>>> debug. By default, debug output is not printed to the console. >>>> >>>> Add a new "--debug" (short name: "-d") option to bpftool to print libbpf >>>> logs for all three levels. >>>> >>>> Internally, we simply use the function provided by libbpf to replace the >>>> default printing function by one that prints logs regardless of their >>>> level. >>>> >>>> v2: >>>> - Remove the possibility to select the log-levels to use (v1 offered a >>>> combination of "warn", "info" and "debug"). >>>> - Rename option and offer a short name: -d|--debug. >>> >>> Such and option in CLI tools is usually called -v|--verbose, I'm >>> wondering if it might be a better name choice? >>> >>> Btw, some tools also use -v, -vv and -vvv to define different levels >>> of verbosity, which is something we can consider in the future, as >>> it's backwards compatible. >> >> That was my weak suggestion. Sometimes -v is used for version, e.g. >> GCC. -d is sometimes used for debug, e.g. man, iproute2 uses it as >> short for "detailed". If the consensus is that -v is better I don't >> really mind. > > It's minor, so I'm not insisting at all, just wasn't sure it was > brought up. bpftool is sufficiently different in its conventions from > other modern CLIs anyways. > > As for -v for version. It seems like the trend is to use -V|--version > for version, and -v|--verbose for verbosity. I've also seen some tools > option for `cli version` (subcommand) for version. Anyway, no strong > preferences from me either. > For the record, bpftool has both "bpftool -V" and "bpftool version" to dump the version number. This leaves us with "-v" free to do something like "--verbose", but just as Jakub said we wanted to limit the risks of confusion... I don't mind changing, but since nobody has expressed a strong opinion on the matter, I'll stick to "-d|--debug" for now. Thanks Andrii for the review! Quentin
diff --git a/tools/bpf/bpftool/Documentation/bpftool-btf.rst b/tools/bpf/bpftool/Documentation/bpftool-btf.rst index 2dbc1413fabd..00668df1bf7a 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-btf.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-btf.rst @@ -67,6 +67,10 @@ OPTIONS -p, --pretty Generate human-readable JSON output. Implies **-j**. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + EXAMPLES ======== **# bpftool btf dump id 1226** diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst index ac26876389c2..36807735e2a5 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst @@ -113,6 +113,10 @@ OPTIONS -f, --bpffs Show file names of pinned programs. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + EXAMPLES ======== | diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst index 14180e887082..4d08f35034a2 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst @@ -73,6 +73,10 @@ OPTIONS -p, --pretty Generate human-readable JSON output. Implies **-j**. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + SEE ALSO ======== **bpf**\ (2), diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst index 13ef27b39f20..490b4501cb6e 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst @@ -152,6 +152,10 @@ OPTIONS Do not automatically attempt to mount any virtual file system (such as tracefs or BPF virtual file system) when necessary. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + EXAMPLES ======== **# bpftool map show** diff --git a/tools/bpf/bpftool/Documentation/bpftool-net.rst b/tools/bpf/bpftool/Documentation/bpftool-net.rst index 934580850f42..d8e5237a2085 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-net.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-net.rst @@ -65,6 +65,10 @@ OPTIONS -p, --pretty Generate human-readable JSON output. Implies **-j**. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + EXAMPLES ======== diff --git a/tools/bpf/bpftool/Documentation/bpftool-perf.rst b/tools/bpf/bpftool/Documentation/bpftool-perf.rst index 0c7576523a21..e252bd0bc434 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-perf.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-perf.rst @@ -53,6 +53,10 @@ OPTIONS -p, --pretty Generate human-readable JSON output. Implies **-j**. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + EXAMPLES ======== diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst index e8118544d118..9a92614569e6 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst @@ -174,6 +174,10 @@ OPTIONS Do not automatically attempt to mount any virtual file system (such as tracefs or BPF virtual file system) when necessary. + -d, --debug + Print all logs available from libbpf, including debug-level + information. + EXAMPLES ======== **# bpftool prog show** diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst index 3e562d7fd56f..43dba0717953 100644 --- a/tools/bpf/bpftool/Documentation/bpftool.rst +++ b/tools/bpf/bpftool/Documentation/bpftool.rst @@ -66,6 +66,9 @@ OPTIONS Do not automatically attempt to mount any virtual file system (such as tracefs or BPF virtual file system) when necessary. + -d, --debug + Print all logs available from libbpf, including debug-level + information. SEE ALSO ======== diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index 50e402a5a9c8..3a476e25d046 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -181,7 +181,7 @@ _bpftool() # Deal with options if [[ ${words[cword]} == -* ]]; then - local c='--version --json --pretty --bpffs --mapcompat' + local c='--version --json --pretty --bpffs --mapcompat --debug' COMPREPLY=( $( compgen -W "$c" -- "$cur" ) ) return 0 fi diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 1ac1fc520e6a..d74293938a05 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -10,6 +10,7 @@ #include <string.h> #include <bpf.h> +#include <libbpf.h> #include "main.h" @@ -77,6 +78,13 @@ static int do_version(int argc, char **argv) return 0; } +static int __printf(2, 0) +print_all_levels(__maybe_unused enum libbpf_print_level level, + const char *format, va_list args) +{ + return vfprintf(stderr, format, args); +} + int cmd_select(const struct cmd *cmds, int argc, char **argv, int (*help)(int argc, char **argv)) { @@ -317,6 +325,7 @@ int main(int argc, char **argv) { "bpffs", no_argument, NULL, 'f' }, { "mapcompat", no_argument, NULL, 'm' }, { "nomount", no_argument, NULL, 'n' }, + { "debug", no_argument, NULL, 'd' }, { 0 } }; int opt, ret; @@ -332,7 +341,7 @@ int main(int argc, char **argv) hash_init(map_table.table); opterr = 0; - while ((opt = getopt_long(argc, argv, "Vhpjfmn", + while ((opt = getopt_long(argc, argv, "Vhpjfmnd", options, NULL)) >= 0) { switch (opt) { case 'V': @@ -362,6 +371,9 @@ int main(int argc, char **argv) case 'n': block_mount = true; break; + case 'd': + libbpf_set_print(print_all_levels); + break; default: p_err("unrecognized option '%s'", argv[optind - 1]); if (json_output)