Re: [PATCH perf] perf: ignore deprecation warning when using libbpf's btf__get_from_id()

Andrii Nakryiko [off-list ref] writes:

On Tue, Sep 14, 2021 at 12:02 PM Arnaldo Carvalho de Melo
[off-list ref] wrote:

quoted

Em Tue, Sep 14, 2021 at 11:28:28AM -0700, Andrii Nakryiko escreveu:

quoted

On Tue, Sep 14, 2021 at 11:21 AM Jiri Olsa [off-list ref] wrote:

quoted

On Tue, Sep 14, 2021 at 10:00:04AM -0700, Andrii Nakryiko wrote:

quoted

Perf code re-implements libbpf's btf__load_from_kernel_by_id() API as
a weak function, presumably to dynamically link against old version of
libbpf shared library. Unfortunately this causes compilation warning
when perf is compiled against libbpf v0.6+.

For now, just ignore deprecation warning, but there might be a better
solution, depending on perf's needs.

HI,
the problem we tried to solve is when perf is using symbols
which are not yet available in released libbpf.. but it all
linkes in default perf build because it's linked statically
libbpf.a in the tree

If you are always statically linking libbpf into perf, there is no
need to implement this __weak shim. Libbpf is never going to deprecate
an API if a new/replacement API hasn't been at least in a previous
released version. So in this case btf__load_from_kernel_by_id() was
added in libbpf 0.5, and btf__get_from_id() was marked deprecated in
libbpf 0.6 (not yet released, of course). So with that, do you still
think we need this __weak re-implementation?

I was wondering if this was done to make latest perf code compile
against some old libbpf source code or dynamically linked against old
libbpf. But if that's not the case, the fix should be a removal of
__weak btf__load_from_kernel_by_id().

It was made to build against the libbpf that comes with fedora 34, the
distro I'm using, which is:

⬢[acme@toolbox perf]$ sudo dnf install libbpf-devel
Package libbpf-devel-2:0.4.0-1.fc34.x86_64 is already installed.
Dependencies resolved.
Nothing to do.
Complete!
⬢[acme@toolbox perf]$ cat /etc/redhat-release
Fedora release 34 (Thirty Four)

And we have 'make -C tools/perf build-test' that has one entry to build
with LIBBPF_EXTERNAL=1, i.e. using whatever libbpf-devel package is
installed in the distro, in addtion to statically linking with the
libbpf in the kernel sources.

That is done because several distros are linking perf with the libbpf
they ship.

When I merged the latest upstream this test failed, and I realized that
some files in tools/perf/ had changed to make use of a new function and
that was the reason for the build test failure.

So I tried to provide a transition help for these cases, initially as a
feature test that would look if that new function was available and if
not, provide the fallback, but then ended up following Jiri's suggestion
for a __weak function, as that involved less coding.

Ok, that's cool, then my "fix" should be fine for now. Can you please
land it in perf/core to unblock Stephen's (cc'ed) build failure when
merging perf and bpf-next trees?

Also it's good to keep in mind that libbpf is now providing
LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION macro, so when
statically linking you should be able to use that to detect libbpf
version. For shared library cases we should probably also add runtime
APIs (e.g., int libbpf_major_version(void), int
libbpf_minor_version(void), const char *libbpf_version(void)) so that
you can do more detection based on libbpf version at runtime. Let me
know if it's something that would be helpful.

Yes, please! We're currently using this horror to be able to print the
libbpf version being used by xdp-tools:

https://github.com/xdp-project/xdp-tools/blob/master/lib/util/util.c#L100

Would be awesome to have an API function we could just call instead :)

-Toke