Previously, re-using pinned DEVMAP maps would always fail, because get_map_info on a DEVMAP always returns flags with BPF_F_RDONLY_PROG set, but BPF_F_RDONLY_PROG being set on a map during creation is invalid. Thus, ignore the BPF_F_RDONLY_PROG flag on both sides when checking for compatibility with an existing DEVMAP. Ignoring it on both sides ensures that it continues to work on older kernels which don't set BPF_F_RDONLY_PROG on get_map_info. The same problem is handled in a third-party ebpf library: - https://github.com/cilium/ebpf/issues/925 - https://github.com/cilium/ebpf/pull/930 Fixes: 0cdbb4b09a06 ("devmap: Allow map lookups from eBPF") Signed-off-by: Yureka Lilian --- tools/lib/bpf/libbpf.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index d41ee26b9..049b0c400 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -5076,6 +5076,7 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) { struct bpf_map_info map_info; __u32 map_info_len = sizeof(map_info); + __u32 map_flags_for_check = map->def.map_flags; int err; memset(&map_info, 0, map_info_len); @@ -5088,11 +5089,22 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd) return false; } + /* get_map_info on a DEVMAP will always return flags with + * BPF_F_RDONLY_PROG set, but it will never be set on a map + * being created. + * Thus, ignore the BPF_F_RDONLY_PROG flag on both sides when + * checking for compatibility with an existing DEVMAP. + */ + if (map->def.type == BPF_MAP_TYPE_DEVMAP || map->def.type == BPF_MAP_TYPE_DEVMAP_HASH) { + map_info.map_flags |= BPF_F_RDONLY_PROG; + map_flags_for_check |= 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 == map_flags_for_check && map_info.map_extra == map->map_extra); } -- 2.50.1 The test covers basic re-use of a pinned DEVMAP map, with both matching and mismatching parameters. Signed-off-by: Yureka Lilian --- .../bpf/prog_tests/pinning_devmap_reuse.c | 68 +++++++++++++++++++ .../selftests/bpf/progs/test_pinning_devmap.c | 20 ++++++ 2 files changed, 88 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c create mode 100644 tools/testing/selftests/bpf/progs/test_pinning_devmap.c diff --git a/tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c b/tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c new file mode 100644 index 000000000..06befb03b --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/pinning_devmap_reuse.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include +#include +#include + +void test_pinning_devmap_reuse(void) +{ + const char *pinpath1 = "/sys/fs/bpf/pinmap1"; + const char *pinpath2 = "/sys/fs/bpf/pinmap2"; + const char *file = "./test_pinning_devmap.bpf.o"; + struct bpf_object *obj1 = NULL, *obj2 = NULL; + int err; + __u32 duration = 0; + DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts); + + /* load the object a first time */ + obj1 = bpf_object__open_file(file, NULL); + err = libbpf_get_error(obj1); + if (CHECK(err, "first open", "err %d\n", err)) { + obj1 = NULL; + goto out; + } + err = bpf_object__load(obj1); + if (CHECK(err, "first load", "err %d\n", err)) + goto out; + + /* load the object a second time, re-using the pinned map */ + obj2 = bpf_object__open_file(file, NULL); + if (CHECK(err, "second open", "err %d\n", err)) { + obj2 = NULL; + goto out; + } + err = bpf_object__load(obj2); + if (CHECK(err, "second load", "err %d\n", err)) + goto out; + + /* we can close the reference safely without + * the map's refcount falling to 0 + */ + bpf_object__close(obj1); + obj1 = NULL; + + /* now, swap the pins */ + err = renameat2(0, pinpath1, 0, pinpath2, RENAME_EXCHANGE); + if (CHECK(err, "swap pins", "err %d\n", err)) + goto out; + + /* load the object again, this time the re-use should fail */ + obj1 = bpf_object__open_file(file, NULL); + err = libbpf_get_error(obj1); + if (CHECK(err, "third open", "err %d\n", err)) { + obj1 = NULL; + goto out; + } + err = bpf_object__load(obj1); + if (CHECK(err != -EINVAL, "param mismatch load", "err %d\n", err)) + goto out; + +out: + unlink(pinpath1); + unlink(pinpath2); + if (obj1) + bpf_object__close(obj1); + if (obj2) + bpf_object__close(obj2); +} diff --git a/tools/testing/selftests/bpf/progs/test_pinning_devmap.c b/tools/testing/selftests/bpf/progs/test_pinning_devmap.c new file mode 100644 index 000000000..c855f8f87 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_pinning_devmap.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include + +struct { + __uint(type, BPF_MAP_TYPE_DEVMAP); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, __u32); + __uint(pinning, LIBBPF_PIN_BY_NAME); +} pinmap1 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_DEVMAP); + __uint(max_entries, 2); + __type(key, __u32); + __type(value, __u32); + __uint(pinning, LIBBPF_PIN_BY_NAME); +} pinmap2 SEC(".maps"); -- 2.50.1