On 7/16/21 7:35 AM, Andrii Nakryiko wrote:
On Wed, Jul 14, 2021 at 9:52 AM Martynas Pumputis <m@xxxxxxxxx> wrote:
Add a test case to check whether an unsuccessful creation of an outer
map of a BTF-defined map-in-map destroys the inner map.
As bpf_object__create_map() is a static function, we cannot just call it
from the test case and then check whether a map accessible via
map->inner_map_fd has been removed. Instead, we iterate over all maps
and check whether the map "$MAP_NAME.inner" does not exist.
Signed-off-by: Martynas Pumputis <m@xxxxxxxxx>
---
.../bpf/progs/test_map_in_map_invalid.c | 27 +++++++++
tools/testing/selftests/bpf/test_maps.c | 58 ++++++++++++++++++-
2 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c
diff --git a/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c b/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c
new file mode 100644
index 000000000000..03601779e4ed
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_map_in_map_invalid.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Isovalent, Inc. */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+struct inner {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __type(key, __u32);
+ __type(value, int);
+ __uint(max_entries, 4);
+};
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
+ __uint(max_entries, 0); /* This will make map creation to fail */
+ __uint(key_size, sizeof(__u32));
+ __array(values, struct inner);
+} mim SEC(".maps");
+
+SEC("xdp_noop")
+int xdp_noop0(struct xdp_md *ctx)
+{
+ return XDP_PASS;
+}
+
+int _version SEC("version") = 1;
please don't add new uses of version, it's completely unnecessary on
modern kernels
Sure.
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 30cbf5d98f7d..48f6c6dfd188 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1153,12 +1153,16 @@ static void test_sockmap(unsigned int tasks, void *data)
}
#define MAPINMAP_PROG "./test_map_in_map.o"
+#define MAPINMAP_INVALID_PROG "./test_map_in_map_invalid.o"
static void test_map_in_map(void)
{
struct bpf_object *obj;
struct bpf_map *map;
int mim_fd, fd, err;
int pos = 0;
+ struct bpf_map_info info = {};
+ __u32 len = sizeof(info);
+ __u32 id = 0;
obj = bpf_object__open(MAPINMAP_PROG);
@@ -1229,10 +1233,62 @@ static void test_map_in_map(void)
close(fd);
bpf_object__close(obj);
+
+
+ /* Test that failing bpf_object__create_map() destroys the inner map */
+
+ obj = bpf_object__open(MAPINMAP_INVALID_PROG);
you didn't check bpf_object__open() succeeded here...
For the sake of brevity, I didn't add the check. If the opening fails,
then we will catch it anyway with the bpf_object__find_map_by_name()
invocation below: it will log "libbpf: elf: failed to open $PROG_NAME:
No such file or directory" and then segfault.
+
+ map = bpf_object__find_map_by_name(obj, "mim");
... and crash will happen here on error
+ if (!map) {
+ printf("Failed to load array of maps from test prog\n");
+ goto out_map_in_map;
+ }
+
[...]
