Re: [PATCH bpf-next 09/10] libbpf: simplify look up by name of internal maps

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

> On Fri, Oct 8, 2021 at 10:31 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> andrii.nakryiko@gmail.com writes:
>>
>> > From: Andrii Nakryiko <andrii@kernel.org>
>> >
>> > Map name that's assigned to internal maps (.rodata, .data, .bss, etc)
>> > consist of a small prefix of bpf_object's name and ELF section name as
>> > a suffix. This makes it hard for users to "guess" the name to use for
>> > looking up by name with bpf_object__find_map_by_name() API.
>> >
>> > One proposal was to drop object name prefix from the map name and just
>> > use ".rodata", ".data", etc, names. One downside called out was that
>> > when multiple BPF applications are active on the host, it will be hard
>> > to distinguish between multiple instances of .rodata and know which BPF
>> > object (app) they belong to. Having few first characters, while quite
>> > limiting, still can give a bit of a clue, in general.
>> >
>> > Another downside of such approach is that it is not backwards compatible
>> > and, among direct use of bpf_object__find_map_by_name() API, will break
>> > any BPF skeleton generated using bpftool that was compiled with older
>> > libbpf version.
>> >
>> > Instead of causing all this pain, libbpf will still generate map name
>> > using a combination of object name and ELF section name, but it will
>> > allow looking such maps up by their natural names, which correspond to
>> > their respective ELF section names. This means non-truncated ELF section
>> > names longer than 15 characters are going to be expected and supported.
>> >
>> > With such set up, we get the best of both worlds: leave small bits of
>> > a clue about BPF application that instantiated such maps, as well as
>> > making it easy for user apps to lookup such maps at runtime. In this
>> > sense it closes corresponding libbpf 1.0 issue ([0]).
>>
>> I like this approach. Only possible problem I can see is that it might
>> be confusing that a map can be looked up with one name, but that it
>> disappears once it's loaded into the kernel (and the BPF object is
>> closed).
>>
>> Hmm, couldn't we just extend the kernel to accept longer names? Kinda
>> like with the netdev name aliases: support a secondary label that can be
>> longer, and have bpftool display both?
>
> Yes, this discrepancy can be confusing. I'd like all those internal
> maps to be named after their corresponding ELF sections, tbh. We have
> a mechanism now to make this transition (libbpf_set_strict_mode()),
> but people have complained before that just seeing ".data" won't give
> them enough information.

Yeah, I do also sympathise with that complaint :)

> But if we are going to extend the kernel with longer map names, then
> I'd rather stick to clean ".data.custom" naming from the very
> beginning, and then switch all existing .data/.rodata/.bss/.kconfig
> map naming to the same convention as well (guarded by opt-in flag in
> libbpf_set_strict_mode() until libbpf 1.0). In the kernel, though,
> instead of having two names (i.e., one is alias), I'd just allow to
> provide one long name and then all existing UAPIs that have char[16]
> everywhere would just be a potentially truncated prefix of such a
> longer name. All the tooling can be updated to use long name when
> available, of course. WDYT?

Hmm, so introduce a new 'map_name_long' field, and on query the kernel
will fill in the old map_name with a truncated version, and put the full
name in the new field? Yeah, I guess that would work too!

-Toke