netdev - Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzbN17eviD18-_C2UN+P5gMm4vFXVrdLd9UHx0ev+gJsjw@mail.gmail.com>
Date:   Mon, 6 Dec 2021 21:13:20 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Song Liu <songliubraving@...com>
Cc:     Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Kernel Team <Kernel-team@...com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map

On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@...com> wrote:
>
>
>
> > On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@...il.com> wrote:
> >
> > On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@...nel.org> wrote:
> >>
> >> bpf_create_map is deprecated. Replace it with bpf_map_create.
> >>
> >> Fixes: 992c4225419a ("libbpf: Unify low-level map creation APIs w/ new bpf_map_create()")
> >
> > This is not a bug fix, it's an improvement. So I don't think "Fixes: "
> > is warranted here, tbh.
>
> I got compilation errors before this change, like
>
> util/bpf_counter.c: In function ‘bperf_lock_attr_map’:
> util/bpf_counter.c:323:3: error: ‘bpf_create_map’ is deprecated: libbpf v0.7+: use bpf_map_create() instead [-Werror=deprecated-declarations]
>    map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
>    ^~~~~~
> In file included from util/bpf_counter.h:7,
>                  from util/bpf_counter.c:15:
> /data/users/songliubraving/kernel/linux-git/tools/lib/bpf/bpf.h:91:16: note: declared here
>  LIBBPF_API int bpf_create_map(enum bpf_map_type map_type, int key_size,
>                 ^~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make[4]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:96: util/bpf_counter.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [/data/users/songliubraving/kernel/linux-git/tools/build/Makefile.build:139: util] Error 2
> make[2]: *** [Makefile.perf:665: perf-in.o] Error 2
> make[1]: *** [Makefile.perf:240: sub-make] Error 2
> make: *** [Makefile:70: all] Error 2
>

Hmm.. is util/bpf_counter.h guarded behind some Makefile arguments?
I've sent #pragma temporary workarounds just a few days ago ([0]), but
this one didn't come up during the build.

  [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211203004640.2455717-1-andrii@kernel.org/

> Do we plan to remove bpf_create_map in the future? If not, we can probably just
> add '#pragma GCC diagnostic ignored "-Wdeprecated-declarations"' can call it done?

Yes, it will be removed in a few libbpf releases when we switch to the
1.0 version. So suppressing a warning is a temporary work-around.

>
> >
> >> Signed-off-by: Song Liu <song@...nel.org>
> >> ---
> >> tools/perf/util/bpf_counter.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> >> index c17d4a43ce065..ed150a9b3a0c0 100644
> >> --- a/tools/perf/util/bpf_counter.c
> >> +++ b/tools/perf/util/bpf_counter.c
> >> @@ -320,10 +320,10 @@ static int bperf_lock_attr_map(struct target *target)
> >>        }
> >>
> >>        if (access(path, F_OK)) {
> >> -               map_fd = bpf_create_map(BPF_MAP_TYPE_HASH,
> >> +               map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, NULL,
> >
> > I think perf is trying to be linkable with libbpf as a shared library,
> > so on some older versions of libbpf bpf_map_create() won't be (yet)
> > available. So to make this work, I think you'll need to define your
> > own weak bpf_map_create function that will use bpf_create_map().
>
> Hmm... I didn't know the plan to link libbpf as shared library. In this case,
> maybe the #pragma solution is preferred?

See "perf tools: Add more weak libbpf functions" sent by Jiri not so
long ago about what they did with some other used APIs that are now
marked deprecated.

>
> Thanks,
> Song
>