On Wed, Apr 7, 2021 at 9:17 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Wed, Apr 7, 2021 at 11:05 AM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Wed, Apr 7, 2021 at 9:36 AM Yauheni Kaliuta
> > <yauheni.kaliuta@xxxxxxxxxx> wrote:
> > >
> > > Hi, Andrii!
> > >
> > > On Wed, Mar 31, 2021 at 9:52 PM Andrii Nakryiko
> > > <andrii.nakryiko@xxxxxxxxx> wrote:
> > > >
> > > > On Wed, Mar 31, 2021 at 9:45 AM Yauheni Kaliuta
> > > > <yauheni.kaliuta@xxxxxxxxxx> wrote:
> > > > >
> > > > > Set bpf table sizes dynamically according to the runtime page size
> > > > > value.
> > > > >
> > > > > Do not switch to ASSERT macros, keep CHECK, for consistency with the
> > > > > rest of the test. Can be a separate cleanup patch.
> > > > >
> > > > > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@xxxxxxxxxx>
> > > > > ---
> > > > > .../selftests/bpf/prog_tests/ringbuf_multi.c | 23 ++++++++++++++++---
> > > > > .../selftests/bpf/progs/test_ringbuf_multi.c | 1 -
> > > > > 2 files changed, 20 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > > index d37161e59bb2..159de99621c7 100644
> > > > > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > > > @@ -41,13 +41,30 @@ 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;
> > > > > + int page_size = getpagesize();
> > > > >
> > > > > - skel = test_ringbuf_multi__open_and_load();
> > > > > - if (CHECK(!skel, "skel_open_load", "skeleton open&load failed\n"))
> > > > > + skel = test_ringbuf_multi__open();
> > > > > + if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > > > > return;
> > > > >
> > > > > + err = bpf_map__set_max_entries(skel->maps.ringbuf1, page_size);
> > > > > + if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > > + goto cleanup;
> > > > > +
> > > > > + err = bpf_map__set_max_entries(skel->maps.ringbuf2, page_size);
> > > > > + if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > > + goto cleanup;
> > > > > +
> > > > > + err = bpf_map__set_max_entries(bpf_map__inner_map(skel->maps.ringbuf_arr), page_size);
> > > > > + if (CHECK(err != 0, "bpf_map__set_max_entries", "bpf_map__set_max_entries failed\n"))
> > > > > + goto cleanup;
> > > > > +
> > > > > + err = test_ringbuf_multi__load(skel);
> > > > > + if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > > > > + goto cleanup;
> > > > > +
> > > >
> > > > To test bpf_map__set_inner_map_fd() interaction with map-in-map
> > > > initialization, can you extend the test to have another map-in-map
> > > > (could be HASHMAP, just for fun), which is initialized with either
> > > > ringbuf1 or ringbuf2, but then from user-space use a different way to
> > > > override inner map definition:
> > > >
> > > > int proto_fd = bpf_create_map(... RINGBUF of page_size ...);
> > > > bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > > > close(proto_fd);
> > > >
> > > > /* perform load, it should succeed */
> > > >
> > > > Important is to use a different map-in-map from ringbuf_arr, so that
> > > > load fails, unless set_inner_map_fd() properly updates internals of a
> > > > map.
> > >
> > > Is that what you mean?
> >
> > yes
> >
> >
> > > https://github.com/ykaliuta/linux/commit/59fedd3b00678023d04b74bac61581a04896602e
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > index 159de99621c7..5794317d05dd 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf_multi.c
> > > @@ -44,6 +44,7 @@ void test_ringbuf_multi(void)
> > > struct ring_buffer *ringbuf = NULL;
> > > int err;
> > > int page_size = getpagesize();
> > > + int proto_fd;
> > >
> > > skel = test_ringbuf_multi__open();
> > > if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
> > > @@ -61,6 +62,16 @@ void test_ringbuf_multi(void)
> > > if (CHECK(err != 0, "bpf_map__set_max_entries",
> > > "bpf_map__set_max_entries failed\n"))
> > > goto cleanup;
> > >
> > > + proto_fd = bpf_create_map(BPF_MAP_TYPE_RINGBUF, 0, 0, page_size, 0);
> > > + if (CHECK(proto_fd == -1, "bpf_create_map", "bpf_create_map failed\n"))
> > > + goto cleanup;
> > > +
> > > + err = bpf_map__set_inner_map_fd(skel->maps.ringbuf_hash, proto_fd);
> > > + if (CHECK(err != 0, "bpf_map__set_inner_map_fd",
> > > "bpf_map__set_inner_map_fd failed\n"))
> > > + goto cleanup;
> > > +
> > > + close(proto_fd);
>
> I think the problem is this, you are deleting that inner map too soon,
> before the kernel can get it. Try moving close(proto_fd) after the
> load step.
Ah, of course, stupid me :) Thanks, it works!
>
> > > +
> > > err = test_ringbuf_multi__load(skel);
> > > if (CHECK(err != 0, "skel_load", "skeleton load failed\n"))
> > > goto cleanup;
> > > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > index 055c10b2ff80..197b86546dca 100644
> > > --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c
> > > @@ -30,6 +30,17 @@ struct {
> > > },
> > > };
> > >
> > > +struct {
> > > + __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
> > > + __uint(max_entries, 1);
> > > + __type(key, int);
> > > + __array(values, struct ringbuf_map);
> > > +} ringbuf_hash SEC(".maps") = {
> > > + .values = {
> > > + [0] = &ringbuf1,
> > > + },
> > > +};
> > > +
> > >
> > >
> > > I get with it:
> > >
> > > test_ringbuf_multi:PASS:bpf_map__set_inner_map_fd 0 nsec
> > > libbpf: Error in bpf_create_map_xattr(ringbuf_arr):Invalid
> > > argument(-22). Retrying without BTF.
> > > libbpf: Error in bpf_create_map_xattr(ringbuf_hash):Invalid
> > > argument(-22). Retrying without BTF.
> > > libbpf: map 'ringbuf_hash': 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
> > > #90 ringbuf_multi:FAIL
> >
> > Did you try to investigate why it doesn't work?
> >
> > >
> > >
> > > >
> > > > > /* 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");
> > > > >
> > > > > --
> > > > > 2.31.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > WBR, Yauheni
> > >
>
--
WBR, Yauheni
