[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACAyw98hwj5hpT00P5JiW3V+QPdyddKfN_yQj=okXvg89eTgsA@mail.gmail.com>
Date: Thu, 20 Jun 2019 10:27:52 +0100
From: Lorenz Bauer <lmb@...udflare.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Andrii Nakryiko <andriin@...com>,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
Alexei Starovoitov <ast@...com>,
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 Mon, 17 Jun 2019 at 22:00, Andrii Nakryiko <andrii.nakryiko@...il.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
Powered by blists - more mailing lists