Re: [PATCH bpf-next v2 0/5] libbpf: rename btf__get_from_id() and btf__load() APIs, support split BTF

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v2 0/5] libbpf: rename btf__get_from_id() and btf__load() APIs, support split BTF
Date: Thu, 22 Jul 2021 17:58:12 -0700	[thread overview]
Message-ID: <CAEf4Bzb30BNeLgio52OrxHk2VWfKitnbNUnO0sAXZTA94bYfmg@mail.gmail.com> (raw)
In-Reply-To: <20210721153808.6902-1-quentin@isovalent.com>

On Wed, Jul 21, 2021 at 8:38 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> As part of the effort to move towards a v1.0 for libbpf [0], this set
> improves some confusing function names related to BTF loading from and to
> the kernel:
>
> - btf__load() becomes btf__load_into_kernel().
> - btf__get_from_id becomes btf__load_from_kernel_by_id().
> - A new version btf__load_from_kernel_by_id_split() extends the former to
>   add support for split BTF.
>
> The old functions are not removed or marked as deprecated yet, there
> should be in a future libbpf version.

Oh, and I was thinking about this whole deprecation having to be done
in two steps. It's super annoying to keep track of that. Ideally, we'd
have some macro that can mark API deprecated "in the future", when
actual libbpf version is >= to defined version. So something like
this:

LIBBPF_DEPRECATED_AFTER(V(0,5), "API that will be marked deprecated in v0.6")

We'd need to make sure that during the build time we have some
LIBBPF_VERSION macro available against which we compare the expected
version and add or not the __attribute__((deprecated)).

Does this make sense? WDYT? I haven't looked into how hard it would be
to implement this, but it should be easy enough, so if you'd like some
macro challenge, please take a stab at it.

Having this it would be possible to make all the deprecations at the
same time that we add replacement APIs and not ask anyone to follow-up
potentially a month or two later, right?

>
> The last patch is a trivial change to bpftool to add support for dumping
> split BTF objects by referencing them by their id (and not only by their
> BTF path).
>
> [0] https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#btfh-apis
>
> v2:
> - Remove deprecation marking of legacy functions (patch 4/6 from v1).
> - Make btf__load_from_kernel_by_id{,_split}() return the btf struct.
> - Add new functions to v0.5.0 API (and not v0.6.0).
>
> Quentin Monnet (5):
>   libbpf: rename btf__load() as btf__load_into_kernel()
>   libbpf: rename btf__get_from_id() as btf__load_from_kernel_by_id()
>   tools: replace btf__get_from_id() with btf__load_from_kernel_by_id()
>   libbpf: add split BTF support for btf__load_from_kernel_by_id()
>   tools: bpftool: support dumping split BTF by id
>
>  tools/bpf/bpftool/btf.c                      |  8 ++---
>  tools/bpf/bpftool/btf_dumper.c               |  6 ++--
>  tools/bpf/bpftool/map.c                      | 16 +++++-----
>  tools/bpf/bpftool/prog.c                     | 29 +++++++++++------
>  tools/lib/bpf/btf.c                          | 33 ++++++++++++++------
>  tools/lib/bpf/btf.h                          |  4 +++
>  tools/lib/bpf/libbpf.c                       |  7 +++--
>  tools/lib/bpf/libbpf.map                     |  3 ++
>  tools/perf/util/bpf-event.c                  | 11 ++++---
>  tools/perf/util/bpf_counter.c                | 12 +++++--
>  tools/testing/selftests/bpf/prog_tests/btf.c |  4 ++-
>  11 files changed, 86 insertions(+), 47 deletions(-)
>
> --
> 2.30.2
>