On Thu, Jun 20, 2019 at 2:28 AM Lorenz Bauer [off-list ref] wrote:

On Mon, 17 Jun 2019 at 22:00, Andrii Nakryiko [off-list ref] wrote:

quoted

quoted

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 :)

Yep :)

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.

quoted

quoted

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?

Right now pointer is always required. My initial intent for map-in-map
was to not use pointer, but since then I've proposed a slightly
different approach, which eliminates all these concerns you mentioned.
As for messaging, yeah, that the simplest part, which can always be
improved.

quoted

quoted

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