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

["Toke Høiland-Jørgensen" <toke@redhat.com>]

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

> On Tue, Sep 14, 2021 at 12:02 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
>>
>> Em Tue, Sep 14, 2021 at 11:28:28AM -0700, Andrii Nakryiko escreveu:
>> > On Tue, Sep 14, 2021 at 11:21 AM Jiri Olsa <jolsa@redhat.com> wrote:
>> > >
>> > > On Tue, Sep 14, 2021 at 10:00:04AM -0700, Andrii Nakryiko wrote:
>> > > > 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