On Tue, Jun 23, 2020 at 11:47 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Tue, Jun 23, 2020 at 5:34 PM Andrii Nakryiko <andriin@xxxxxx> wrote:
> >
> > Similar message for map creation is extremely useful, so add similar for BPF
> > programs.
>
> 'extremely useful' is quite subjective.
> If we land this patch then everyone will be allowed to add pr_debug()
> everywhere in libbpf with the same reasoning: "it's extremely useful pr_debug".
We print this for maps, making it clear which maps and with which FD
were created. Having this for programs is just as useful. It doesn't
overwhelm output (and it's debug one either way). "everyone will be
allowed to add pr_debug()" is a big stretch, you can't just sneak in
or force random pr_debug, we do review patches and if something
doesn't make sense we can and we do reject it, regardless of claimed
usefulness by the patch author.
So far, libbpf debug logs were extremely helpful (subjective, of
course, but what isn't?) to debug "remotely" various issues that BPF
users had. They don't feel overwhelmingly verbose and don't have a lot
of unnecessary info. Adding a few lines (how many BPF programs are
there per each BPF object?) for listing BPF programs is totally ok.
But I'm not going to fight it, up to you, of course.
>
> > Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> > ---
> > tools/lib/bpf/libbpf.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 18461deb1b19..f24a90c86c58 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -5379,8 +5379,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > }
> >
> > ret = bpf_load_program_xattr(&load_attr, log_buf, log_buf_size);
> > -
> > if (ret >= 0) {
> > + pr_debug("prog '%s' ('%s'): created successfully, fd=%d\n",
> > + prog->name, prog->section_name, ret);
> > if (log_buf && load_attr.log_level)
> > pr_debug("verifier log:\n%s", log_buf);
> > *pfd = ret;
> > --
> > 2.24.1
> >
