* [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning
@ 2022-02-22 20:42 Stijn Tintel
2022-02-23 2:36 ` Song Liu
0 siblings, 1 reply; 6+ messages in thread
From: Stijn Tintel @ 2022-02-22 20:42 UTC (permalink / raw)
To: toke, netdev
Cc: bpf, linux-kernel, kpsingh, john.fastabend, yhs, songliubraving,
kafai, andrii, daniel, ast
When a BPF map of type BPF_MAP_TYPE_PERF_EVENT_ARRAY doesn't have the
max_entries parameter set, this parameter will be set to the number of
possible CPUs. Due to this, the map_is_reuse_compat function will return
false, causing the following error when trying to reuse the map:
libbpf: couldn't reuse pinned map at '/sys/fs/bpf/m_logging': parameter mismatch
Fix this by checking against the number of possible CPUs if the
max_entries parameter is not set in the map definition.
Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
---
tools/lib/bpf/libbpf.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7f10dd501a52..b076ab728f0e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4750,7 +4750,7 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
struct bpf_map_info map_info = {};
char msg[STRERR_BUFSIZE];
__u32 map_info_len;
- int err;
+ int def_max_entries, err;
map_info_len = sizeof(map_info);
@@ -4763,10 +4763,15 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
return false;
}
+ if (map->def.type == BPF_MAP_TYPE_PERF_EVENT_ARRAY && !map->def.max_entries)
+ def_max_entries = libbpf_num_possible_cpus();
+ else
+ def_max_entries = map->def.max_entries;
+
return (map_info.type == map->def.type &&
map_info.key_size == map->def.key_size &&
map_info.value_size == map->def.value_size &&
- map_info.max_entries == map->def.max_entries &&
+ map_info.max_entries == def_max_entries &&
map_info.map_flags == map->def.map_flags &&
map_info.map_extra == map->map_extra);
}
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning
2022-02-22 20:42 [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning Stijn Tintel
@ 2022-02-23 2:36 ` Song Liu
2022-02-23 23:15 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2022-02-23 2:36 UTC (permalink / raw)
To: Stijn Tintel
Cc: Toke Høiland-Jørgensen, Networking, bpf, open list,
KP Singh, John Fastabend, Yonghong Song, Song Liu,
Martin KaFai Lau, Andrii Nakryiko, Daniel Borkmann,
Alexei Starovoitov
On Tue, Feb 22, 2022 at 12:51 PM Stijn Tintel <stijn@linux-ipv6.be> wrote:
>
> When a BPF map of type BPF_MAP_TYPE_PERF_EVENT_ARRAY doesn't have the
> max_entries parameter set, this parameter will be set to the number of
> possible CPUs. Due to this, the map_is_reuse_compat function will return
> false, causing the following error when trying to reuse the map:
>
> libbpf: couldn't reuse pinned map at '/sys/fs/bpf/m_logging': parameter mismatch
>
> Fix this by checking against the number of possible CPUs if the
> max_entries parameter is not set in the map definition.
>
> Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
> Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Song Liu <songliubraving@fb.com>
I think the following fix would be more future proof, but the patch
as-is is better for
stable backport? How about we add a follow up patch on top of current
patch to fix
def->max_entries once for all?
Thanks,
Song
diff --git i/tools/lib/bpf/libbpf.c w/tools/lib/bpf/libbpf.c
index ad43b6ce825e..a1bc1c80bc69 100644
--- i/tools/lib/bpf/libbpf.c
+++ w/tools/lib/bpf/libbpf.c
@@ -4881,10 +4881,9 @@ static int bpf_object__create_map(struct
bpf_object *obj, struct bpf_map *map, b
return nr_cpus;
}
pr_debug("map '%s': setting size to %d\n", map->name, nr_cpus);
- max_entries = nr_cpus;
- } else {
- max_entries = def->max_entries;
+ def->max_entries = nr_cpus;
}
+ max_entries = def->max_entries;
if (bpf_map__is_struct_ops(map))
create_attr.btf_vmlinux_value_type_id =
map->btf_vmlinux_value_type_id;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning
2022-02-23 2:36 ` Song Liu
@ 2022-02-23 23:15 ` Andrii Nakryiko
2022-02-24 10:08 ` Stijn Tintel
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2022-02-23 23:15 UTC (permalink / raw)
To: Song Liu
Cc: Stijn Tintel, Toke Høiland-Jørgensen, Networking, bpf,
open list, KP Singh, John Fastabend, Yonghong Song, Song Liu,
Martin KaFai Lau, Andrii Nakryiko, Daniel Borkmann,
Alexei Starovoitov
On Tue, Feb 22, 2022 at 6:37 PM Song Liu <song@kernel.org> wrote:
>
> On Tue, Feb 22, 2022 at 12:51 PM Stijn Tintel <stijn@linux-ipv6.be> wrote:
> >
> > When a BPF map of type BPF_MAP_TYPE_PERF_EVENT_ARRAY doesn't have the
> > max_entries parameter set, this parameter will be set to the number of
> > possible CPUs. Due to this, the map_is_reuse_compat function will return
> > false, causing the following error when trying to reuse the map:
> >
> > libbpf: couldn't reuse pinned map at '/sys/fs/bpf/m_logging': parameter mismatch
> >
> > Fix this by checking against the number of possible CPUs if the
> > max_entries parameter is not set in the map definition.
> >
> > Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
> > Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
>
> Acked-by: Song Liu <songliubraving@fb.com>
>
> I think the following fix would be more future proof, but the patch
> as-is is better for
> stable backport? How about we add a follow up patch on top of current
> patch to fix
> def->max_entries once for all?
Keeping special logic for PERF_EVENT_ARRAY in one place is
preferrable. With this, the changes in map_is_reuse_compat() shouldn't
be necessary at all. Stijn, can you please send v2 with Song's
proposed changes?
>
> Thanks,
> Song
>
> diff --git i/tools/lib/bpf/libbpf.c w/tools/lib/bpf/libbpf.c
> index ad43b6ce825e..a1bc1c80bc69 100644
> --- i/tools/lib/bpf/libbpf.c
> +++ w/tools/lib/bpf/libbpf.c
> @@ -4881,10 +4881,9 @@ static int bpf_object__create_map(struct
> bpf_object *obj, struct bpf_map *map, b
> return nr_cpus;
> }
> pr_debug("map '%s': setting size to %d\n", map->name, nr_cpus);
> - max_entries = nr_cpus;
> - } else {
> - max_entries = def->max_entries;
> + def->max_entries = nr_cpus;
> }
> + max_entries = def->max_entries;
>
> if (bpf_map__is_struct_ops(map))
> create_attr.btf_vmlinux_value_type_id =
> map->btf_vmlinux_value_type_id;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning
2022-02-23 23:15 ` Andrii Nakryiko
@ 2022-02-24 10:08 ` Stijn Tintel
2022-02-24 15:38 ` Stijn Tintel
0 siblings, 1 reply; 6+ messages in thread
From: Stijn Tintel @ 2022-02-24 10:08 UTC (permalink / raw)
To: Andrii Nakryiko, Song Liu
Cc: Toke Høiland-Jørgensen, Networking, bpf, open list,
KP Singh, John Fastabend, Yonghong Song, Song Liu,
Martin KaFai Lau, Andrii Nakryiko, Daniel Borkmann,
Alexei Starovoitov
On 24/02/2022 01:15, Andrii Nakryiko wrote:
> On Tue, Feb 22, 2022 at 6:37 PM Song Liu <song@kernel.org> wrote:
>> On Tue, Feb 22, 2022 at 12:51 PM Stijn Tintel <stijn@linux-ipv6.be> wrote:
>>> When a BPF map of type BPF_MAP_TYPE_PERF_EVENT_ARRAY doesn't have the
>>> max_entries parameter set, this parameter will be set to the number of
>>> possible CPUs. Due to this, the map_is_reuse_compat function will return
>>> false, causing the following error when trying to reuse the map:
>>>
>>> libbpf: couldn't reuse pinned map at '/sys/fs/bpf/m_logging': parameter mismatch
>>>
>>> Fix this by checking against the number of possible CPUs if the
>>> max_entries parameter is not set in the map definition.
>>>
>>> Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
>>> Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
>> Acked-by: Song Liu <songliubraving@fb.com>
>>
>> I think the following fix would be more future proof, but the patch
>> as-is is better for
>> stable backport? How about we add a follow up patch on top of current
>> patch to fix
>> def->max_entries once for all?
> Keeping special logic for PERF_EVENT_ARRAY in one place is
> preferrable. With this, the changes in map_is_reuse_compat() shouldn't
> be necessary at all. Stijn, can you please send v2 with Song's
> proposed changes?
>
Will do!
Thanks,
Stijn
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning
2022-02-24 10:08 ` Stijn Tintel
@ 2022-02-24 15:38 ` Stijn Tintel
2022-02-25 3:48 ` Song Liu
0 siblings, 1 reply; 6+ messages in thread
From: Stijn Tintel @ 2022-02-24 15:38 UTC (permalink / raw)
To: Andrii Nakryiko, Song Liu
Cc: Toke Høiland-Jørgensen, Networking, bpf, open list,
KP Singh, John Fastabend, Yonghong Song, Song Liu,
Martin KaFai Lau, Andrii Nakryiko, Daniel Borkmann,
Alexei Starovoitov
On 24/02/2022 12:08, Stijn Tintel wrote:
> On 24/02/2022 01:15, Andrii Nakryiko wrote:
>> On Tue, Feb 22, 2022 at 6:37 PM Song Liu <song@kernel.org> wrote:
>>> On Tue, Feb 22, 2022 at 12:51 PM Stijn Tintel <stijn@linux-ipv6.be> wrote:
>>>> When a BPF map of type BPF_MAP_TYPE_PERF_EVENT_ARRAY doesn't have the
>>>> max_entries parameter set, this parameter will be set to the number of
>>>> possible CPUs. Due to this, the map_is_reuse_compat function will return
>>>> false, causing the following error when trying to reuse the map:
>>>>
>>>> libbpf: couldn't reuse pinned map at '/sys/fs/bpf/m_logging': parameter mismatch
>>>>
>>>> Fix this by checking against the number of possible CPUs if the
>>>> max_entries parameter is not set in the map definition.
>>>>
>>>> Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
>>>> Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
>>> Acked-by: Song Liu <songliubraving@fb.com>
>>>
>>> I think the following fix would be more future proof, but the patch
>>> as-is is better for
>>> stable backport? How about we add a follow up patch on top of current
>>> patch to fix
>>> def->max_entries once for all?
>> Keeping special logic for PERF_EVENT_ARRAY in one place is
>> preferrable. With this, the changes in map_is_reuse_compat() shouldn't
>> be necessary at all. Stijn, can you please send v2 with Song's
>> proposed changes?
>>
> Will do!
Unfortunately that doesn't work. In bpf_object__create_maps, we call
bpf_object__reuse_map and map_is_reuse_compat before
bpf_object__create_map, so we check map_info.max_entries ==
map->def.max_entries before the latter is being overwritten.
So I propose to send a v2 based on my initial submission, but use __u32
for def_max_entries instead of int, unless someone has another suggestion?
Thanks,
Stijn
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning
2022-02-24 15:38 ` Stijn Tintel
@ 2022-02-25 3:48 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2022-02-25 3:48 UTC (permalink / raw)
To: Stijn Tintel
Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Networking,
bpf, open list, KP Singh, John Fastabend, Yonghong Song,
Song Liu, Martin KaFai Lau, Andrii Nakryiko, Daniel Borkmann,
Alexei Starovoitov
On Thu, Feb 24, 2022 at 7:38 AM Stijn Tintel <stijn@linux-ipv6.be> wrote:
>
> On 24/02/2022 12:08, Stijn Tintel wrote:
> > On 24/02/2022 01:15, Andrii Nakryiko wrote:
> >> On Tue, Feb 22, 2022 at 6:37 PM Song Liu <song@kernel.org> wrote:
> >>> On Tue, Feb 22, 2022 at 12:51 PM Stijn Tintel <stijn@linux-ipv6.be> wrote:
> >>>> When a BPF map of type BPF_MAP_TYPE_PERF_EVENT_ARRAY doesn't have the
> >>>> max_entries parameter set, this parameter will be set to the number of
> >>>> possible CPUs. Due to this, the map_is_reuse_compat function will return
> >>>> false, causing the following error when trying to reuse the map:
> >>>>
> >>>> libbpf: couldn't reuse pinned map at '/sys/fs/bpf/m_logging': parameter mismatch
> >>>>
> >>>> Fix this by checking against the number of possible CPUs if the
> >>>> max_entries parameter is not set in the map definition.
> >>>>
> >>>> Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
> >>>> Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
> >>> Acked-by: Song Liu <songliubraving@fb.com>
> >>>
> >>> I think the following fix would be more future proof, but the patch
> >>> as-is is better for
> >>> stable backport? How about we add a follow up patch on top of current
> >>> patch to fix
> >>> def->max_entries once for all?
> >> Keeping special logic for PERF_EVENT_ARRAY in one place is
> >> preferrable. With this, the changes in map_is_reuse_compat() shouldn't
> >> be necessary at all. Stijn, can you please send v2 with Song's
> >> proposed changes?
> >>
> > Will do!
>
> Unfortunately that doesn't work. In bpf_object__create_maps, we call
> bpf_object__reuse_map and map_is_reuse_compat before
> bpf_object__create_map, so we check map_info.max_entries ==
> map->def.max_entries before the latter is being overwritten.
>
> So I propose to send a v2 based on my initial submission, but use __u32
> for def_max_entries instead of int, unless someone has another suggestion?
How about we move
if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY && !def->max_entries) {
...
}
to bpf_object__create_maps() before bpf_object__reuse_map(). And maybe add
a helper function with it?
Thanks,
Song
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-25 3:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 20:42 [PATCH] libbpf: fix BPF_MAP_TYPE_PERF_EVENT_ARRAY auto-pinning Stijn Tintel
2022-02-23 2:36 ` Song Liu
2022-02-23 23:15 ` Andrii Nakryiko
2022-02-24 10:08 ` Stijn Tintel
2022-02-24 15:38 ` Stijn Tintel
2022-02-25 3:48 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®