netdev - Re: [RFC PATCH bpf-next 6/8] libbpf: allow specifying map definitions using BTF

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: <20190604210710.GA17053@mini-arch>
Date:   Tue, 4 Jun 2019 14:07:10 -0700
From:   Stanislav Fomichev <sdf@...ichev.me>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Alexei Starovoitov <ast@...com>, Andrii Nakryiko <andriin@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 6/8] libbpf: allow specifying map
 definitions using BTF

On 06/04, Andrii Nakryiko wrote:
> On Tue, Jun 4, 2019 at 6:45 AM Stanislav Fomichev <sdf@...ichev.me> wrote:
> >
> > On 06/03, Stanislav Fomichev wrote:
> > > > BTF is mandatory for _any_ new feature.
> > > If something is easy to support without asking everyone to upgrade to
> > > a bleeding edge llvm, why not do it?
> > > So much for backwards compatibility and flexibility.
> > >
> > > > It's for introspection and debuggability in the first place.
> > > > Good debugging is not optional.
> > > Once llvm 8+ is everywhere, sure, but we are not there yet (I'm talking
> > > about upstream LTS distros like ubuntu/redhat).
> > But putting this aside, one thing that I didn't see addressed in the
> > cover letter is: what is the main motivation for the series?
> > Is it to support iproute2 map definitions (so cilium can switch to libbpf)?
> 
> In general, the motivation is to arrive at a way to support
> declaratively defining maps in such a way, that:
> - captures type information (for debuggability/introspection) in
> coherent and hard-to-screw-up way;
> - allows to support missing useful features w/ good syntax (e.g.,
> natural map-in-map case vs current completely manual non-declarative
> way for libbpf);

[..]
> - ultimately allow iproute2 to use libbpf as unified loader (and thus
> the need to support its existing features, like
> BPF_MAP_TYPE_PROG_ARRAY initialization, pinning, map-in-map);
So prog_array tail call info would be encoded in the magic struct instead of
a __section_tail(whatever) macros that iproute2 is using? Does it
mean that the programs that target iproute2 would have to be rewritten?
Or we don't have a goal to provide source-level compatibility?

In general, supporting iproute2 seems like the most compelling
reason to use BTF given current state of llvm+btf adoption.
BPF_ANNOTATE_KV_PAIR and map-in-map syntax while ugly, is not the major
paint point (imho); but I agree, with BTF both of those things
look much better.

That's why I was trying to understand whether we can start with using
BTF to support _existing_ iproute2 format and then, once it's working,
generalize it (and kill bpf_map_def or make it a subset of generic BTF).
That way we are not implementing another way to support pinning/tail
calls, but enabling iproute2 to use libbpf.

But feel free to ignore all my nonsense above; I don't really have any
major concerns with the new generic format rather than discoverability
(the docs might help) and a mandate that everyone switches to it immediately.

> The only missing feature that can be supported reasonably with
> bpf_map_def is pinning (as it's just another int field), but all the
> other use cases requires awkward approach of matching arbitrary IDs,
> which feels like a bad way forward.
> 
> 
> > If that's the case, maybe explicitly focus on that? Once we have
> > proof-of-concept working for iproute2 mode, we can extend it to everything.