netdev - Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180926235413.GA38328@rdna-mbp.dhcp.thefacebook.com>
Date:   Wed, 26 Sep 2018 23:54:17 +0000
From:   Andrey Ignatov <rdna@...com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next 0/5] Introduce libbpf_attach_type_by_name

Jakub Kicinski <jakub.kicinski@...ronome.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.

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 ..

-- 
Andrey Ignatov