Re: [PATCH v2 4/4] selftests/bpf: ringbuf, mmap: bump up page size to 64K

[Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>]

On Mon, Mar 29, 2021 at 8:20 AM Yauheni Kaliuta
<yauheni.kaliuta@xxxxxxxxxx> wrote:
>
> On Sun, Mar 28, 2021 at 8:03 AM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
>
> [...]
>
> > >
> > >  struct {
> > >         __uint(type, BPF_MAP_TYPE_ARRAY);
> > > -       __uint(max_entries, 4096);
> > > +       __uint(max_entries, PAGE_SIZE);
> >
> >
> > so you can set map size at runtime before bpf_object__load (or
> > skeleton's load) with bpf_map__set_max_entries. That way you don't
> > have to do any assumptions. Just omit max_entries in BPF source code,
> > and always set it in userspace.
>
> Will it work for ringbuf_multi? If I just set max_entries for ringbuf1
> and ringbuf2 that way, it gives me
>
> libbpf: map 'ringbuf_arr': failed to create inner map: -22
> libbpf: map 'ringbuf_arr': failed to create: Invalid argument(-22)
> libbpf: failed to load object 'test_ringbuf_multi'
> libbpf: failed to load BPF skeleton 'test_ringbuf_multi': -22
> test_ringbuf_multi:FAIL:skel_load skeleton load failed
>

You are right, it won't work. We'd need to add something like
bpf_map__inner_map() accessor to allow to adjust the inner map
definition:

bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);

And some more fixes. Here's minimal diff that made it work, but
probably needs a bit more testing:

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7aad78dbb4b4..ed5586cce227 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2194,6 +2194,7 @@ static int parse_btf_map_def(struct bpf_object *obj,
             map->inner_map = calloc(1, sizeof(*map->inner_map));
             if (!map->inner_map)
                 return -ENOMEM;
+            map->inner_map->fd = -1;
             map->inner_map->sec_idx = obj->efile.btf_maps_shndx;
             map->inner_map->name = malloc(strlen(map->name) +
                               sizeof(".inner") + 1);
@@ -3845,6 +3846,14 @@ __u32 bpf_map__max_entries(const struct bpf_map *map)
     return map->def.max_entries;
 }

+struct bpf_map *bpf_map__inner_map(struct bpf_map *map)
+{
+    if (!bpf_map_type__is_map_in_map(map->def.type))
+        return NULL;
+
+    return map->inner_map;
+}
+
 int bpf_map__set_max_entries(struct bpf_map *map, __u32 max_entries)
 {
     if (map->fd >= 0)
@@ -9476,6 +9485,7 @@ int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd)
         pr_warn("error: inner_map_fd already specified\n");
         return -EINVAL;
     }
+    zfree(&map->inner_map);
     map->inner_map_fd = fd;
     return 0;
 }
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index f500621d28e5..bec4e6a6e31d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -480,6 +480,7 @@ LIBBPF_API int bpf_map__pin(struct bpf_map *map,
const char *path);
 LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);

 LIBBPF_API int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd);
+LIBBPF_API struct bpf_map *bpf_map__inner_map(struct bpf_map *map);

 LIBBPF_API long libbpf_get_error(const void *ptr);

diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index f5990f7208ce..eeb6d5ebd1cc 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -360,4 +360,5 @@ LIBBPF_0.4.0 {
         bpf_linker__free;
         bpf_linker__new;
         bpf_object__set_kversion;
+        bpf_map__inner_map;
 } LIBBPF_0.3.0;
diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
index d37161e59bb2..cdc9c9b1d0e1 100644
--- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
@@ -41,13 +41,23 @@ static int process_sample(void *ctx, void *data, size_t len)
 void test_ringbuf_multi(void)
 {
     struct test_ringbuf_multi *skel;
-    struct ring_buffer *ringbuf;
+    struct ring_buffer *ringbuf = NULL;
     int err;

-    skel = test_ringbuf_multi__open_and_load();
+    skel = test_ringbuf_multi__open();
     if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
         return;

+    bpf_map__set_max_entries(skel->maps.ringbuf1, 4096);
+    bpf_map__set_max_entries(skel->maps.ringbuf2, 4096);
+    bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), 4096);
+
+    err = test_ringbuf_multi__load(skel);
+    if (!ASSERT_OK(err, "skel_load"))
+        goto cleanup;
+
     /* only trigger BPF program for current process */
     skel->bss->pid = getpid();

diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
index edf3b6953533..055c10b2ff80 100644
--- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
+++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
@@ -15,7 +15,6 @@ struct sample {

 struct ringbuf_map {
     __uint(type, BPF_MAP_TYPE_RINGBUF);
-    __uint(max_entries, 1 << 12);
 } ringbuf1 SEC(".maps"),
   ringbuf2 SEC(".maps");