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

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Stanislav Fomichev <sdf@google.com>,
	Quentin Monnet <quentin@isovalent.com>
Cc: "Alexei Starovoitov" <ast@fb.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.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: Wed, 20 Oct 2021 11:20:32 -0700	[thread overview]
Message-ID: <CAEf4BzbhE0JMzap6OJf1vstyFi+qRTn1UjhUz+AoaUJYazA2BA@mail.gmail.com> (raw)
In-Reply-To: <CAKH8qBsazaRM+ACSJc69Tt9RXR95twtG-T+Sn8LmV9sKWkhu1Q@mail.gmail.com>

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.

Which is a long way to say that once we have bpf_metadat_app, you
already have associated bpf_object name (or whatever user will decide
to name their BPF application). Which solves this map naming problem
as well (with tooling support, of course).

cc Quentin also. Thoughts?