* [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
@ 2021-12-06 23:08 Song Liu
2021-12-07 2:37 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Song Liu @ 2021-12-06 23:08 UTC (permalink / raw)
To: bpf, netdev; +Cc: ast, daniel, andrii, kernel-team, acme, acme, Song Liu
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()")
Signed-off-by: Song Liu <song@kernel.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,
sizeof(struct perf_event_attr),
sizeof(struct perf_event_attr_map_entry),
- ATTR_MAP_SIZE, 0);
+ ATTR_MAP_SIZE, NULL);
if (map_fd < 0)
return -1;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
2021-12-06 23:08 [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map Song Liu
@ 2021-12-07 2:37 ` Andrii Nakryiko
2021-12-07 4:32 ` Song Liu
0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2021-12-07 2:37 UTC (permalink / raw)
To: Song Liu
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo
On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.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.
> Signed-off-by: Song Liu <song@kernel.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().
> sizeof(struct perf_event_attr),
> sizeof(struct perf_event_attr_map_entry),
> - ATTR_MAP_SIZE, 0);
> + ATTR_MAP_SIZE, NULL);
> if (map_fd < 0)
> return -1;
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
2021-12-07 2:37 ` Andrii Nakryiko
@ 2021-12-07 4:32 ` Song Liu
2021-12-07 5:13 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Song Liu @ 2021-12-07 4:32 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Song Liu, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo
> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.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
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?
>
>> Signed-off-by: Song Liu <song@kernel.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?
Thanks,
Song
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
2021-12-07 4:32 ` Song Liu
@ 2021-12-07 5:13 ` Andrii Nakryiko
2021-12-07 6:30 ` Song Liu
0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2021-12-07 5:13 UTC (permalink / raw)
To: Song Liu
Cc: Song Liu, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo
On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.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@kernel.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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
2021-12-07 5:13 ` Andrii Nakryiko
@ 2021-12-07 6:30 ` Song Liu
2021-12-07 23:02 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Song Liu @ 2021-12-07 6:30 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Song Liu, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo
> On Dec 6, 2021, at 9:13 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.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/
I guess the default build test doesn't enable BUILD_BPF_SKEL?
>
>> 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@kernel.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.
Do you mean something like this?
int __weak
bpf_map_create(enum bpf_map_type map_type,
const char *map_name __maybe_unused,
__u32 key_size,
__u32 value_size,
__u32 max_entries,
const struct bpf_map_create_opts *opts __maybe_unused)
{
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
return bpf_create_map(map_type, key_size, value_size, max_entries, 0);
#pragma GCC diagnostic pop
}
I guess this won't work when bpf_create_map() is eventually removed, as
__weak function are still compiled, no?
Thanks,
Song
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
2021-12-07 6:30 ` Song Liu
@ 2021-12-07 23:02 ` Andrii Nakryiko
2021-12-07 23:10 ` Song Liu
0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2021-12-07 23:02 UTC (permalink / raw)
To: Song Liu
Cc: Song Liu, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo
On Mon, Dec 6, 2021 at 10:30 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Dec 6, 2021, at 9:13 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >>>
> >>> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.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/
>
> I guess the default build test doesn't enable BUILD_BPF_SKEL?
I see, right, I think I already asked about that before :( Is it
possible to set Makefile such that it will do BUILD_BPF_SKEL=1 if
Clang version is recent enough and other conditions are satisfied
(unless overridden or something)?
>
> >
> >> 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@kernel.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.
>
> Do you mean something like this?
>
> int __weak
> bpf_map_create(enum bpf_map_type map_type,
> const char *map_name __maybe_unused,
> __u32 key_size,
> __u32 value_size,
> __u32 max_entries,
> const struct bpf_map_create_opts *opts __maybe_unused)
> {
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> return bpf_create_map(map_type, key_size, value_size, max_entries, 0);
> #pragma GCC diagnostic pop
> }
>
> I guess this won't work when bpf_create_map() is eventually removed, as
> __weak function are still compiled, no?
Yes and yes. I'm not sure what would be perf's plan w.r.t. libbpf 1.0,
we'll need to work together to figure this out. At some point perf
will need to say that the minimum version of supported libbpf is v0.6
or something and just assume all those newer APIs are there (no need
to bump it all the way to libbpf 1.0, btw).
>
> Thanks,
> Song
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map
2021-12-07 23:02 ` Andrii Nakryiko
@ 2021-12-07 23:10 ` Song Liu
0 siblings, 0 replies; 7+ messages in thread
From: Song Liu @ 2021-12-07 23:10 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Song Liu, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team, Arnaldo Carvalho de Melo,
Arnaldo Carvalho de Melo
> On Dec 7, 2021, at 3:02 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Dec 6, 2021 at 10:30 PM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Dec 6, 2021, at 9:13 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Mon, Dec 6, 2021 at 8:32 PM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Dec 6, 2021, at 6:37 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>>>
>>>>> On Mon, Dec 6, 2021 at 3:08 PM Song Liu <song@kernel.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/
>>
>> I guess the default build test doesn't enable BUILD_BPF_SKEL?
>
> I see, right, I think I already asked about that before :( Is it
> possible to set Makefile such that it will do BUILD_BPF_SKEL=1 if
> Clang version is recent enough and other conditions are satisfied
> (unless overridden or something)?
Arnaldo is working on this. I guess we can flip the default soon.
>
>>
>>>
>>>> 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@kernel.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.
>>
>> Do you mean something like this?
>>
>> int __weak
>> bpf_map_create(enum bpf_map_type map_type,
>> const char *map_name __maybe_unused,
>> __u32 key_size,
>> __u32 value_size,
>> __u32 max_entries,
>> const struct bpf_map_create_opts *opts __maybe_unused)
>> {
>> #pragma GCC diagnostic push
>> #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>> return bpf_create_map(map_type, key_size, value_size, max_entries, 0);
>> #pragma GCC diagnostic pop
>> }
>>
>> I guess this won't work when bpf_create_map() is eventually removed, as
>> __weak function are still compiled, no?
>
> Yes and yes. I'm not sure what would be perf's plan w.r.t. libbpf 1.0,
> we'll need to work together to figure this out. At some point perf
> will need to say that the minimum version of supported libbpf is v0.6
> or something and just assume all those newer APIs are there (no need
> to bump it all the way to libbpf 1.0, btw).
OK. I will send this version. And we can decide the next step when we
remove bpf_create_map().
Thanks,
Song
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-07 23:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-06 23:08 [PATCH bpf-next] perf/bpf_counter: use bpf_map_create instead of bpf_create_map Song Liu
2021-12-07 2:37 ` Andrii Nakryiko
2021-12-07 4:32 ` Song Liu
2021-12-07 5:13 ` Andrii Nakryiko
2021-12-07 6:30 ` Song Liu
2021-12-07 23:02 ` Andrii Nakryiko
2021-12-07 23:10 ` Song Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.