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

[Lorenz Bauer <lmb@cloudflare.com>]

On Mon, 17 Jun 2019 at 22:00, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > In my mind, BPF loaders should be able to pass through BTF to the kernel
> > as a binary blob as much as possible. That's why I want the format to
> > be "self describing". Compatibility then becomes a question of: what
> > feature are you using on which kernel. The kernel itself can then still be
> > strict-by-default or what have you.
>
> That would work in ideal world, where kernel is updated frequently
> (and BTF is self-describing, which it is not). In practice, though,
> libbpf is far more up-to-date and lends its hand on "sanitizing" .BTF
> from kernel-unsupported features (so far we manage to pull this off
> very reasonably). If you have a good proposal how to make .BTF
> self-describing, that would be great!

I think sanitizing is going to become a problem, but we've been around
that argument a few times :)

Making .BTF self describing need at least adding length to certain fields,
as I mentioned in another thread. Plus an interface to interrogate the
kernel about a loaded BTF blob.

> > I agree with you, the syntax probably has to be different. I'd just like it to
> > differ by more than a "*" in the struct definition, because that is too small
> > to notice.
>
> So let's lay out how it will be done in practice:
>
> 1. Simple map w/ custom key/value
>
> struct my_key { ... };
> struct my_value { ... };
>
> struct {
>     __u32 type;
>     __u32 max_entries;
>     struct my_key *key;
>     struct my_value *value;
> } my_simple_map BPF_MAP = {
>     .type = BPF_MAP_TYPE_ARRAY,
>     .max_entries = 16,
> };
>
> 2. Now map-in-map:
>
> struct {
>     __u32 type;
>     __u32 max_entries;
>     struct my_key *key;
>     struct {
>         __u32 type;
>         __u32 max_entries;
>         __u64 *key;
>         struct my_value *value;
>     } value;
> } my_map_in_map BPF_MAP = {
>     .type = BPF_MAP_TYPE_HASH_OF_MAPS,
>     .max_entries = 16,
>     .value = {
>         .type = BPF_MAP_TYPE_ARRAY,
>         .max_entries = 100,
>     },
> };
>
> It's clearly hard to misinterpret inner map definition for a custom
> anonymous struct type, right?

That's not what I'm concerned about. My point is: sometimes you
have to use a pointer, sometimes you don't. Every user has to learn this.
Chance is, they'll probably get it wrong first. Is there a way to give a
reasonable error message for this?

> > I kind of assumed that BTF support for those maps would at some point
> > appear, maybe I should have checked that.
>
> It will. Current situation with maps not supporting specifying BTF for
> key and/or value looks more like a bug, than feature and we should fix
> that. But even if we fix it today, kernels are updated much slower
> than libbpf, so by not supporting key_size/value_size, we force people
> to get stuck with legacy bpf_map_def for a really long time.

OK.

I'll go and look at the newest revision of the patch set now :o)

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com