Re: [PATCH bpf-next 09/10] libbpf: simplify look up by name of internal maps - Toke Høiland-Jørgensen

From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Stanislav Fomichev <sdf@google.com>,
	Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@fb.com>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 09/10] libbpf: simplify look up by name of internal maps
Date: Thu, 21 Oct 2021 00:03:22 +0200	[thread overview]
Message-ID: <87cznze4lx.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzbhE0JMzap6OJf1vstyFi+qRTn1UjhUz+AoaUJYazA2BA@mail.gmail.com>

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

> On Wed, Oct 20, 2021 at 11:09 AM Stanislav Fomichev <sdf@google.com> wrote:
>>
>> On Wed, Oct 20, 2021 at 10:59 AM Andrii Nakryiko
>> <andrii.nakryiko@gmail.com> wrote:
>> >
>> > On Tue, Oct 12, 2021 at 8:29 AM Stanislav Fomichev <sdf@google.com> wrote:
>> > >
>> > > On Mon, Oct 11, 2021 at 8:45 PM Andrii Nakryiko
>> > > <andrii.nakryiko@gmail.com> wrote:
>> > > >
>> > > > On Mon, Oct 11, 2021 at 11:24 PM Alexei Starovoitov <ast@fb.com> wrote:
>> > > > >
>> > > > > On 10/8/21 2:44 PM, Toke Høiland-Jørgensen wrote:
>> > > > > >
>> > > > > > 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!
>> > > > >
>> > > > > Let's start storing full map names in BTF instead.
>> > > > > Like we do already for progs.
>> > > > > Some tools already fetch full prog names this way.
>> > > >
>> > > > We do have those names in BTF. Each map has either corresponding VAR
>> > > > or DATASEC. The problem is that we don't know which.
>> > > >
>> > > > Are you proposing to add some extra "btf_def_type_id" field to specify
>> > > > BTF type ID of what "defines" the map (VAR or DATASEC)? That would
>> > > > work. Would still require UAPI and kernel changes, of course.
>> > > >
>> > > > The reason Toke and others were asking to preserve that object name
>> > > > prefix for .rodata/.data maps was different though, and won't be
>> > > > addressed by the above. Even if you know the BTF VAR/DATASEC, you
>> > > > don't know the "object name" associated with the map. And the kernel
>> > > > doesn't know because it's purely libbpf's abstraction. And sometimes
>> > > > that abstraction doesn't make sense (e.g., if we create a map that's
>> > > > pinned and reused from multiple BPF apps/objects).
>> > >
>> > > [..]
>> > >
>> > > > We do have BPF metadata that Stanislav added a while ago, so maybe we
>> > > > should just punt that problem to that? I'd love to have clean
>> > > > ".rodata" and ".data" names, of course.
>> > >
>> > > Are you suggesting we add some option to associate the metadata with
>> > > the maps (might be an option)? IIRC, the metadata can only be
>> > > associated with the progs right now.
>> >
>> > Well, maps have associated BTF fd, when they are created, no? So you
>> > can find all the same metadata for the map, no?
>>
>> I guess that's true, we can store this metadata in the map itself
>> using something like existing bpf_metadata_ prefix.
>
> We had a discussion during the inaugural BSC meeting about having a
> small set of "standardized" metadata strings. "owner" and
> "description" (or maybe "app" for "application name") were two that
> were clearly useful and generally useful. So if we update bpftool and
> other tooling to recognize bpf_metadata_owner and bpf_metadata_app and
> print them in some nice and meaningful way in bpftool output (in
> addition to general btf_metadata dump), it would be great.

I like the idea of specifying some well-known metadata names, especially
if libbpf can auto-populate them if the user doesn't.

Also, couldn't bpftool just print out all bpf_metadata_* fields? At
least in a verbose mode...

-Toke