On Wed, 26 Sep 2018 23:54:17 +0000, Andrey Ignatov wrote:
> Jakub Kicinski <jakub.kicin...@netronome.com> [Wed, 2018-09-26 16:20 -0700]:
> > On Wed, 26 Sep 2018 15:24:52 -0700, Andrey Ignatov wrote:
> > > This patch set introduces libbpf_attach_type_by_name function in libbpf to
> > > identify attach type by section name.
> > >
> > > This is useful to avoid writing same logic over and over again in user
> > > space applications that leverage libbpf.
> > >
> > > Patch 1 has more details on the new function and problem being solved.
> > > Patches 2 and 3 add support for new section names.
> > > Patch 4 uses new function in a selftest.
> > > Patch 5 adds selftest for libbpf_{prog,attach}_type_by_name.
> > >
> > > As a side note there are a lot of inconsistencies now between names used
> > > by
> > > libbpf and bpftool (e.g. cgroup/skb vs cgroup_skb, cgroup_device and
> > > device
> > > vs cgroup/dev, sockops vs sock_ops, etc). This patch set does not address
> > > it but it tries not to make it harder to address it in the future.
> >
> > I was wondering a few times whether I should point it out to people
> > during review, but thought it would be nit picking. Maybe we should be
> > more strict.
> >
> > Your series LGTM!
>
> Thanks for review!
>
> IMO having it consistent would be great, e.g. one writes a program with
> section name X and bpftool shows/accepts it in exactly same way in all
> its sub-commands (w/o maybe custom suffix added by program writer).
>
> But I doubt that keeping a few places in sync manually will work long
> term since it's easy to miss such a thing.
>
> What do you think of having one source of truth in libbpf so that a
> string for prog_type or attach_type is defined once and all other places
> (e.g. bpftool prog show, bpftool cgroup show) use only corresponding
> enum-s to get those strings, but don't introduce any new strings?
>
> Keeping already existing names in a backward compatible way is a pain
> though.
One source of truth would be nice IMO. The backward compat ties our
hands a tiny bit, but we could do a fallback strategy, where bpftool
checks if it has a name defined for a type, and if it doesn't (for new
types) it will ask libbpf.
There is another place actually where names are hard coded - for program
load command we use the types as they appear in libbpf already. And
that requires putting them in help, man page and bash completions, too.
> Another thing, I was wondering, is if there is a way to bypass strings
> completely (at least in libbpf, since bpftool still has to print
> human-readable names) and keep actual bpf_prog_type and bpf_attach_type
> as metadata for a program in ELF file. Maybe some compiler magic ..
Possibly. I'm far from an ELF expert, but I think attaching arbitrary
metadata to sections may be harder, name is already there and is free
form.
