* [PATCH net-next v2 0/2] Fix pinning devmaps
@ 2022-12-01 1:11 Pramukh Naduthota
2022-12-01 1:11 ` [PATCH net-next v2 1/2] Ignore RDONLY_PROG for devmaps in libbpf to allow re-loading of pinned devmaps Pramukh Naduthota
2022-12-01 1:11 ` [PATCH net-next v2 2/2] Add a selftest for devmap pinning Pramukh Naduthota
0 siblings, 2 replies; 6+ messages in thread
From: Pramukh Naduthota @ 2022-12-01 1:11 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, Pramukh Naduthota
Fix devmap pinning and reloading. The kernel adds BPF_F_RDONLY_PROG to
all devmaps when created, but libbpf checks that user flags match pinned
map flags when using LIBBPF_PIN_BY_NAME, so reusing pinned devmaps
doesn't work, failing with an error like:
libbpf: couldn't reuse pinned map at '/sys/fs/bpf/dev_map':
parameter mismatch
Work around this by ignoring RDONLY_PROG in the compat check in libbpf.
Changes since v1:
- Fixed a broken import
- Fixed style issues
Pramukh Naduthota (2):
Ignore RDONLY_PROG for devmaps in libbpf to allow re-loading of pinned
devmaps
Add a selftest for devmap pinning.
tools/lib/bpf/libbpf.c | 8 +++++++-
.../testing/selftests/bpf/prog_tests/devmap.c | 20 +++++++++++++++++++
.../selftests/bpf/progs/test_pinned_devmap.c | 17 ++++++++++++++++
3 files changed, 44 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/devmap.c
create mode 100644 tools/testing/selftests/bpf/progs/test_pinned_devmap.c
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next v2 1/2] Ignore RDONLY_PROG for devmaps in libbpf to allow re-loading of pinned devmaps 2022-12-01 1:11 [PATCH net-next v2 0/2] Fix pinning devmaps Pramukh Naduthota @ 2022-12-01 1:11 ` Pramukh Naduthota 2022-12-01 1:11 ` [PATCH net-next v2 2/2] Add a selftest for devmap pinning Pramukh Naduthota 1 sibling, 0 replies; 6+ messages in thread From: Pramukh Naduthota @ 2022-12-01 1:11 UTC (permalink / raw) To: bpf Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, Pramukh Naduthota Ignore BPF_F_RDONLY_PROG when checking for compatibility for devmaps. The kernel adds the flag to all devmap creates, and this breaks pinning behavior, as libbpf will then check the actual vs user supplied flags and see this difference. Work around this by adding RDONLY_PROG to the users's flags when testing against the pinned map Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects") Signed-off-by: Pramukh Naduthota <pnaduthota@google.com> --- tools/lib/bpf/libbpf.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 50d41815f431..a3dae26d82d6 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -4818,6 +4818,7 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) char msg[STRERR_BUFSIZE]; __u32 map_info_len; int err; + unsigned int effective_flags = map->def.map_flags; map_info_len = sizeof(map_info); @@ -4830,11 +4831,16 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) return false; } + /* The kernel adds RDONLY_PROG to devmaps */ + if (map->def.type == BPF_MAP_TYPE_DEVMAP || + map->def.type == BPF_MAP_TYPE_DEVMAP_HASH) + effective_flags |= BPF_F_RDONLY_PROG; + 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.map_flags == map->def.map_flags && + map_info.map_flags == effective_flags && map_info.map_extra == map->map_extra); } -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next v2 2/2] Add a selftest for devmap pinning. 2022-12-01 1:11 [PATCH net-next v2 0/2] Fix pinning devmaps Pramukh Naduthota 2022-12-01 1:11 ` [PATCH net-next v2 1/2] Ignore RDONLY_PROG for devmaps in libbpf to allow re-loading of pinned devmaps Pramukh Naduthota @ 2022-12-01 1:11 ` Pramukh Naduthota 2022-12-05 22:31 ` Daniel Borkmann 1 sibling, 1 reply; 6+ messages in thread From: Pramukh Naduthota @ 2022-12-01 1:11 UTC (permalink / raw) To: bpf Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, Pramukh Naduthota Add a selftest Signed-off-by: Pramukh Naduthota <pnaduthota@google.com> --- .../testing/selftests/bpf/prog_tests/devmap.c | 20 +++++++++++++++++++ .../selftests/bpf/progs/test_pinned_devmap.c | 17 ++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/devmap.c create mode 100644 tools/testing/selftests/bpf/progs/test_pinned_devmap.c diff --git a/tools/testing/selftests/bpf/prog_tests/devmap.c b/tools/testing/selftests/bpf/prog_tests/devmap.c new file mode 100644 index 000000000000..50c5006c1416 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/devmap.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2022 Google */ +#include "testing_helpers.h" +#include "test_progs.h" +#include "test_pinned_devmap.skel.h" + +void test_devmap_pinning(void) +{ + struct test_pinned_devmap *ptr; + + ptr = test_pinned_devmap__open_and_load() + ASSERT_OK_PTR(ptr, "first load"); + test_pinned_devmap__destroy(ptr); + ASSERT_OK_PTR(test_pinned_devmap__open_and_load(), "re-load"); +} + +void test_devmap(void) +{ + test_devmap_pinning(); +} diff --git a/tools/testing/selftests/bpf/progs/test_pinned_devmap.c b/tools/testing/selftests/bpf/progs/test_pinned_devmap.c new file mode 100644 index 000000000000..2e9b25fe657c --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_pinned_devmap.c @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2022 Google */ +#include <stddef.h> +#include <linux/bpf.h> +#include <linux/types.h> +#include <bpf/bpf_helpers.h> + +struct { + __uint(type, BPF_MAP_TYPE_DEVMAP_HASH); + __uint(max_entries, 32); + __type(key, int); + __type(value, int); + __uint(pinning, LIBBPF_PIN_BY_NAME); +} repinned_dev_map SEC(".maps"); + + +char _license[] SEC("license") = "GPL"; -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/2] Add a selftest for devmap pinning. 2022-12-01 1:11 ` [PATCH net-next v2 2/2] Add a selftest for devmap pinning Pramukh Naduthota @ 2022-12-05 22:31 ` Daniel Borkmann 2022-12-07 0:42 ` Pramukh Naduthota 0 siblings, 1 reply; 6+ messages in thread From: Daniel Borkmann @ 2022-12-05 22:31 UTC (permalink / raw) To: Pramukh Naduthota, bpf Cc: ast, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa On 12/1/22 2:11 AM, Pramukh Naduthota wrote: > Add a selftest > > Signed-off-by: Pramukh Naduthota <pnaduthota@google.com> > --- > .../testing/selftests/bpf/prog_tests/devmap.c | 20 +++++++++++++++++++ > .../selftests/bpf/progs/test_pinned_devmap.c | 17 ++++++++++++++++ > 2 files changed, 37 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/devmap.c > create mode 100644 tools/testing/selftests/bpf/progs/test_pinned_devmap.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/devmap.c b/tools/testing/selftests/bpf/prog_tests/devmap.c > new file mode 100644 > index 000000000000..50c5006c1416 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/devmap.c > @@ -0,0 +1,20 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2022 Google */ > +#include "testing_helpers.h" > +#include "test_progs.h" > +#include "test_pinned_devmap.skel.h" > + > +void test_devmap_pinning(void) > +{ > + struct test_pinned_devmap *ptr; > + > + ptr = test_pinned_devmap__open_and_load() > + ASSERT_OK_PTR(ptr, "first load"); Looks like you never actually compiled your selftest? :( [...] TEST-OBJ [test_progs] rcu_read_lock.test.o TEST-OBJ [test_progs] btf_dump.test.o In file included from /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c:4: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c: In function ‘test_devmap_pinning’: ./test_progs.h:352:35: error: expected expression before ‘{’ token 352 | #define ASSERT_OK_PTR(ptr, name) ({ \ | ^ /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c:12:2: note: in expansion of macro ‘ASSERT_OK_PTR’ 12 | ASSERT_OK_PTR(ptr, "first load"); | ^~~~~~~~~~~~~ /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c:11:8: error: called object is not a function or function pointer 11 | ptr = test_pinned_devmap__open_and_load() | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ make: *** [Makefile:539: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/devmap.test.o] Error 1 make: *** Waiting for unfinished jobs.... make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf' Error: Process completed with exit code 2. > + test_pinned_devmap__destroy(ptr); > + ASSERT_OK_PTR(test_pinned_devmap__open_and_load(), "re-load"); > +} > + > +void test_devmap(void) > +{ > + test_devmap_pinning(); > +} > diff --git a/tools/testing/selftests/bpf/progs/test_pinned_devmap.c b/tools/testing/selftests/bpf/progs/test_pinned_devmap.c > new file mode 100644 > index 000000000000..2e9b25fe657c > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_pinned_devmap.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2022 Google */ > +#include <stddef.h> > +#include <linux/bpf.h> > +#include <linux/types.h> > +#include <bpf/bpf_helpers.h> > + > +struct { > + __uint(type, BPF_MAP_TYPE_DEVMAP_HASH); > + __uint(max_entries, 32); > + __type(key, int); > + __type(value, int); > + __uint(pinning, LIBBPF_PIN_BY_NAME); > +} repinned_dev_map SEC(".maps"); > + > + > +char _license[] SEC("license") = "GPL"; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/2] Add a selftest for devmap pinning. 2022-12-05 22:31 ` Daniel Borkmann @ 2022-12-07 0:42 ` Pramukh Naduthota 2022-12-07 6:43 ` John Fastabend 0 siblings, 1 reply; 6+ messages in thread From: Pramukh Naduthota @ 2022-12-07 0:42 UTC (permalink / raw) To: Daniel Borkmann Cc: bpf, ast, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa Sorry, looks like I didn't run the tests again after fixing my checkpatch errors. Still new to this, and am quite mortified. Is there a better way to fix this than sending out a v3 of my patch? On Mon, Dec 5, 2022 at 2:31 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 12/1/22 2:11 AM, Pramukh Naduthota wrote: > > Add a selftest > > > > Signed-off-by: Pramukh Naduthota <pnaduthota@google.com> > > --- > > .../testing/selftests/bpf/prog_tests/devmap.c | 20 +++++++++++++++++++ > > .../selftests/bpf/progs/test_pinned_devmap.c | 17 ++++++++++++++++ > > 2 files changed, 37 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/devmap.c > > create mode 100644 tools/testing/selftests/bpf/progs/test_pinned_devmap.c > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/devmap.c b/tools/testing/selftests/bpf/prog_tests/devmap.c > > new file mode 100644 > > index 000000000000..50c5006c1416 > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/prog_tests/devmap.c > > @@ -0,0 +1,20 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright (c) 2022 Google */ > > +#include "testing_helpers.h" > > +#include "test_progs.h" > > +#include "test_pinned_devmap.skel.h" > > + > > +void test_devmap_pinning(void) > > +{ > > + struct test_pinned_devmap *ptr; > > + > > + ptr = test_pinned_devmap__open_and_load() > > + ASSERT_OK_PTR(ptr, "first load"); > > Looks like you never actually compiled your selftest? :( > > [...] > TEST-OBJ [test_progs] rcu_read_lock.test.o > TEST-OBJ [test_progs] btf_dump.test.o > In file included from /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c:4: > /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c: In function ‘test_devmap_pinning’: > ./test_progs.h:352:35: error: expected expression before ‘{’ token > 352 | #define ASSERT_OK_PTR(ptr, name) ({ \ > | ^ > /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c:12:2: note: in expansion of macro ‘ASSERT_OK_PTR’ > 12 | ASSERT_OK_PTR(ptr, "first load"); > | ^~~~~~~~~~~~~ > /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/devmap.c:11:8: error: called object is not a function or function pointer > 11 | ptr = test_pinned_devmap__open_and_load() > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > make: *** [Makefile:539: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/devmap.test.o] Error 1 > make: *** Waiting for unfinished jobs.... > make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf' > Error: Process completed with exit code 2. > > > + test_pinned_devmap__destroy(ptr); > > + ASSERT_OK_PTR(test_pinned_devmap__open_and_load(), "re-load"); > > +} > > + > > +void test_devmap(void) > > +{ > > + test_devmap_pinning(); > > +} > > diff --git a/tools/testing/selftests/bpf/progs/test_pinned_devmap.c b/tools/testing/selftests/bpf/progs/test_pinned_devmap.c > > new file mode 100644 > > index 000000000000..2e9b25fe657c > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/test_pinned_devmap.c > > @@ -0,0 +1,17 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright (c) 2022 Google */ > > +#include <stddef.h> > > +#include <linux/bpf.h> > > +#include <linux/types.h> > > +#include <bpf/bpf_helpers.h> > > + > > +struct { > > + __uint(type, BPF_MAP_TYPE_DEVMAP_HASH); > > + __uint(max_entries, 32); > > + __type(key, int); > > + __type(value, int); > > + __uint(pinning, LIBBPF_PIN_BY_NAME); > > +} repinned_dev_map SEC(".maps"); > > + > > + > > +char _license[] SEC("license") = "GPL"; > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v2 2/2] Add a selftest for devmap pinning. 2022-12-07 0:42 ` Pramukh Naduthota @ 2022-12-07 6:43 ` John Fastabend 0 siblings, 0 replies; 6+ messages in thread From: John Fastabend @ 2022-12-07 6:43 UTC (permalink / raw) To: Pramukh Naduthota, Daniel Borkmann Cc: bpf, ast, andrii, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa Pramukh Naduthota wrote: > Sorry, looks like I didn't run the tests again after fixing my > checkpatch errors. Still new to this, and am quite mortified. > > Is there a better way to fix this than sending out a v3 of my patch? You'll need to send a v3 and be sure to build/run the selftests. Also for future reference please don't top post it. And thanks for working on the fix. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-07 6:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-01 1:11 [PATCH net-next v2 0/2] Fix pinning devmaps Pramukh Naduthota 2022-12-01 1:11 ` [PATCH net-next v2 1/2] Ignore RDONLY_PROG for devmaps in libbpf to allow re-loading of pinned devmaps Pramukh Naduthota 2022-12-01 1:11 ` [PATCH net-next v2 2/2] Add a selftest for devmap pinning Pramukh Naduthota 2022-12-05 22:31 ` Daniel Borkmann 2022-12-07 0:42 ` Pramukh Naduthota 2022-12-07 6:43 ` John Fastabend
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox