Re: [PATCH bpf-next v4 2/3] bpftool: conditionally append / to the progtype

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 21, 2021 at 12:55 PM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote:
>
> 2021-10-21 09:56 UTC-0700 ~ Stanislav Fomichev <sdf@xxxxxxxxxx>
> > Otherwise, attaching with bpftool doesn't work with strict section names.
> >
> > Also, switch to libbpf strict mode to use the latest conventions
> > (note, I don't think we have any cli api guarantees?).
> >
> > Cc: Quentin Monnet <quentin@xxxxxxxxxxxxx>
> > Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
> > ---
> >  tools/bpf/bpftool/main.c | 4 ++++
> >  tools/bpf/bpftool/prog.c | 9 +++++++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> > index 02eaaf065f65..8223bac1e401 100644
> > --- a/tools/bpf/bpftool/main.c
> > +++ b/tools/bpf/bpftool/main.c
> > @@ -409,6 +409,10 @@ int main(int argc, char **argv)
> >       block_mount = false;
> >       bin_name = argv[0];
> >
> > +     ret = libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
> > +     if (ret)
> > +             p_err("failed to enable libbpf strict mode: %d", ret);
> > +
> >       hash_init(prog_table.table);
> >       hash_init(map_table.table);
> >       hash_init(link_table.table);
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index 277d51c4c5d9..b04990588ccf 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -1420,8 +1420,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> >                       err = get_prog_type_by_name(type, &common_prog_type,
> >                                                   &expected_attach_type);
> >                       free(type);
> > -                     if (err < 0)
> > -                             goto err_free_reuse_maps;
>
> Thanks a lot for the change! Can you please test it for e.g. an XDP
> program? You should see that "bpftool prog load prog.o <path> type xdp"
> prints a debug message from libbpf about the first attempt (above)
> failing, before the second attempt (below) succeeds.
>
> We need to get rid of this message. I think it should be easy, because
> we explicitly "ask" for that message in get_prog_type_by_name(), in the
> same file, if it fails to load in the first place.
>
> Could you please update get_prog_type_by_name() to take an additional
> switch as an argument, to tell if the debug-info should be retrieved
> (then first attempt here would skip it, second would keep it)?
> An alternative could be to move all the '/' and retries handling to that
> function, and I think it would end up in bpftool keeping support for the
> legacy object files with the former convention - but that would somewhat
> defeat the objectives of the strict mode, so maybe not the best option.

How about we call libbpf_prog_type_by_name with the provided argv
first and then, if it doesn't work, we fallback to appending '\' and
using get_prog_type_by_name ?

> > +                     if (err < 0) {
>
> We could run the second attempt only on libbpf returning -ESRCH, maybe?

Not sure it matters here, why not always retry on error?

> > +                             err = get_prog_type_by_name(*argv, &common_prog_type,
> > +                                                         &expected_attach_type);
> > +                             if (err < 0)
> > +
> > +                                     goto err_free_reuse_maps;
> > +                     }
> >
> >                       NEXT_ARG();
> >               } else if (is_prefix(*argv, "map")) {
> >
>



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux