Re: [PATCH v2 bpf-next 2/4] bpftool: use bpf_obj_get_info_by_fd directly

[Quentin Monnet <quentin@isovalent.com>]

On Wed, 20 Oct 2021 at 18:37, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Oct 11, 2021 at 1:20 AM Dave Marchevsky <davemarchevsky@fb.com> wrote:
> >
> > To prepare for impending deprecation of libbpf's
> > bpf_program__get_prog_info_linear, migrate uses of this function to use
> > bpf_obj_get_info_by_fd.
> >
> > Since the profile_target_name and dump_prog_id_as_func_ptr helpers were
> > only looking at the first func_info, avoid grabbing the rest to save a
> > malloc. For do_dump, add a more full-featured helper, but avoid
> > free/realloc of buffer when possible for multi-prog dumps.
> >
> > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> > ---
> >  tools/bpf/bpftool/btf_dumper.c |  40 +++++----
> >  tools/bpf/bpftool/prog.c       | 154 +++++++++++++++++++++++++--------
> >  2 files changed, 144 insertions(+), 50 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> > index 9c25286a5c73..0f85704628bf 100644
> > --- a/tools/bpf/bpftool/btf_dumper.c
> > +++ b/tools/bpf/bpftool/btf_dumper.c
> > @@ -32,14 +32,16 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
> >                                     const struct btf_type *func_proto,
> >                                     __u32 prog_id)
> >  {
> > -       struct bpf_prog_info_linear *prog_info = NULL;
> >         const struct btf_type *func_type;
> > +       int prog_fd = -1, func_sig_len;
> > +       struct bpf_prog_info info = {};
> > +       __u32 info_len = sizeof(info);
> >         const char *prog_name = NULL;
> > -       struct bpf_func_info *finfo;
> >         struct btf *prog_btf = NULL;
> > -       struct bpf_prog_info *info;
> > -       int prog_fd, func_sig_len;
> > +       struct bpf_func_info finfo;
> > +       __u32 finfo_rec_size;
> >         char prog_str[1024];
> > +       int err;
> >
> >         /* Get the ptr's func_proto */
> >         func_sig_len = btf_dump_func(d->btf, prog_str, func_proto, NULL, 0,
> > @@ -55,22 +57,27 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d,
> >         if (prog_fd == -1)
>
> please change this to (prog_fd < 0), see [0] for why
>
> we should check all the other places in bpftool to see if there are
> any patterns like this that would break on libbpf 1.0 (cc Quentin as
> well)
>
>   [0] https://github.com/libbpf/libbpf/wiki/Libbpf-1.0-migration-guide#direct-error-code-returning-libbpf_strict_direct_errs

Hi! Looking at bpftool's code (looking for "-1"), I could find only
two occurrences of that pattern, one in btf_dumper.c as noted above,
and another one in struct_ops.c:

    fd = bpf_map_get_fd_by_id(id);
    if (fd == -1) {
        [...]
    }

Dave, are you willing to address it as well? I can send a patch later
this week otherwise.

Thanks,
Quentin