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

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

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.

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?

>
> -Toke
>