Re: [PATCH v4 bpf-next 2/3] veristat: add -d debug mode option to see debug libbpf log

Andrii Nakryiko
2023-03-29T18:36:00Z

On Wed, Mar 29, 2023 at 10:37 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2023-03-27 at 11:52 -0700, Andrii Nakryiko wrote:
> > Add -d option to allow requesting libbpf debug logs from veristat.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/testing/selftests/bpf/veristat.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> > index 83231456d3c5..263df32fbda8 100644
> > --- a/tools/testing/selftests/bpf/veristat.c
> > +++ b/tools/testing/selftests/bpf/veristat.c
> > @@ -135,6 +135,7 @@ static struct env {
> > char **filenames;
> > int filename_cnt;
> > bool verbose;
> > + bool debug;
> > bool quiet;
> > int log_level;
>
> Nitpick:
> it is now three booleans that control verbosity level, would it be
> better to use numerical level instead?
>

I don't think so, because bool fields make checks cleaner in specific
places in the code.

> > enum resfmt out_fmt;
> > @@ -169,7 +170,7 @@ static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va
> > {
> > if (!env.verbose)
> > return 0;
> > - if (level == LIBBPF_DEBUG /* && !env.verbose */)
> > + if (level == LIBBPF_DEBUG && !env.debug)
> > return 0;
> > return vfprintf(stderr, format, args);
> > }
> > @@ -186,6 +187,7 @@ static const struct argp_option opts[] = {
> > { NULL, 'h', NULL, OPTION_HIDDEN, "Show the full help" },
> > { "verbose", 'v', NULL, 0, "Verbose mode" },
> > { "log-level", 'l', "LEVEL", 0, "Verifier log level (default 0 for normal mode, 1 for verbose mode)" },
> > + { "debug", 'd', NULL, 0, "Debug mode (turns on libbpf debug logging)" },
> > { "quiet", 'q', NULL, 0, "Quiet mode" },
> > { "emit", 'e', "SPEC", 0, "Specify stats to be emitted" },
> > { "sort", 's', "SPEC", 0, "Specify sort order" },
> > @@ -212,6 +214,10 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> > case 'v':
> > env.verbose = true;
> > break;
> > + case 'd':
> > + env.debug = true;
> > + env.verbose = true;
> > + break;
> > case 'q':
> > env.quiet = true;
> > break;
>