On Wed, Sep 14, 2022 at 1:36 PM Dave Marchevsky <davemarchevsky@xxxxxx> wrote:
>
> Add a test_ringbuf_map_key test prog, borrowing heavily from extant
> test_ringbuf.c. The program tries to use the result of
> bpf_ringbuf_reserve as map_key, which was not possible before previouis
> commits in this series. The test runner added to prog_tests/ringbuf.c
> verifies that the program loads and does basic sanity checks to confirm
> that it runs as expected.
>
> Also, refactor test_ringbuf such that runners for existing test_ringbuf
> and newly-added test_ringbuf_map_key are subtests of 'ringbuf' top-level
> test.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> ---
> v1->v2: lore.kernel.org/bpf/20220912101106.2765921-1-davemarchevsky@xxxxxx
>
> * Actually run the program instead of just loading (Yonghong)
> * Add a bpf_map_update_elem call to the test (Yonghong)
> * Refactor runner such that existing test and newly-added test are
> subtests of 'ringbuf' top-level test (Yonghong)
> * Remove unused globals in test prog (Yonghong)
>
> tools/testing/selftests/bpf/Makefile | 8 ++-
> .../selftests/bpf/prog_tests/ringbuf.c | 63 ++++++++++++++++-
> .../bpf/progs/test_ringbuf_map_key.c | 70 +++++++++++++++++++
> 3 files changed, 137 insertions(+), 4 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 6cd327f1f216..231d9c1364c9 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -351,9 +351,11 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \
> test_subskeleton.skel.h test_subskeleton_lib.skel.h \
> test_usdt.skel.h
>
> -LSKELS := fentry_test.c fexit_test.c fexit_sleep.c \
> - test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \
> - map_ptr_kern.c core_kern.c core_kern_overflow.c
> +LSKELS := fentry_test.c fexit_test.c fexit_sleep.c atomics.c \
> + trace_printk.c trace_vprintk.c map_ptr_kern.c \
> + core_kern.c core_kern_overflow.c test_ringbuf.c \
> + test_ringbuf_map_key.c
> +
> # Generate both light skeleton and libbpf skeleton for these
> LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test.c \
> kfunc_call_test_subprog.c
> diff --git a/tools/testing/selftests/bpf/prog_tests/ringbuf.c b/tools/testing/selftests/bpf/prog_tests/ringbuf.c
> index 9a80fe8a6427..e0f8db69cb77 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ringbuf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ringbuf.c
> @@ -13,6 +13,7 @@
> #include <linux/perf_event.h>
> #include <linux/ring_buffer.h>
> #include "test_ringbuf.lskel.h"
> +#include "test_ringbuf_map_key.lskel.h"
>
> #define EDONE 7777
>
> @@ -58,6 +59,7 @@ static int process_sample(void *ctx, void *data, size_t len)
> }
> }
>
> +static struct test_ringbuf_map_key_lskel *skel_map_key;
> static struct test_ringbuf_lskel *skel;
> static struct ring_buffer *ringbuf;
>
> @@ -81,7 +83,7 @@ static void *poll_thread(void *input)
> return (void *)(long)ring_buffer__poll(ringbuf, timeout);
> }
>
> -void test_ringbuf(void)
> +void ringbuf_subtest(void)
> {
> const size_t rec_sz = BPF_RINGBUF_HDR_SZ + sizeof(struct sample);
> pthread_t thread;
> @@ -297,3 +299,62 @@ void test_ringbuf(void)
> ring_buffer__free(ringbuf);
> test_ringbuf_lskel__destroy(skel);
> }
> +
> +static int process_map_key_sample(void *ctx, void *data, size_t len)
> +{
> + struct sample *s;
> + int err, val;
> +
> + s = data;
> + switch (s->seq) {
> + case 1:
> + ASSERT_EQ(s->value, 42, "sample_value");
> + err = bpf_map_lookup_elem(skel_map_key->maps.hash_map.map_fd,
> + s, &val);
> + ASSERT_OK(err, "hash_map bpf_map_lookup_elem");
> + ASSERT_EQ(val, 1, "hash_map val");
> + return -EDONE;
> + default:
> + return 0;
> + }
> +}
> +
> +void ringbuf_map_key_subtest(void)
> +{
> + int err;
> +
> + skel_map_key = test_ringbuf_map_key_lskel__open();
> + if (!ASSERT_OK_PTR(skel_map_key, "test_ringbuf_map_key_lskel__open"))
> + return;
> +
> + skel_map_key->maps.ringbuf.max_entries = getpagesize();
> + skel_map_key->bss->pid = getpid();
> +
> + err = test_ringbuf_map_key_lskel__load(skel_map_key);
> + if (!ASSERT_OK(err, "test_ringbuf_map_key_lskel__load"))
> + goto cleanup;
> +
> + ringbuf = ring_buffer__new(skel_map_key->maps.ringbuf.map_fd,
> + process_map_key_sample, NULL, NULL);
> +
> + err = test_ringbuf_map_key_lskel__attach(skel_map_key);
> + if (!ASSERT_OK(err, "test_ringbuf_map_key_lskel__attach"))
> + goto cleanup_ringbuf;
> +
> + syscall(__NR_getpgid);
> + ASSERT_EQ(skel_map_key->bss->seq, 1, "skel_map_key->bss->seq");
> + ring_buffer__poll(ringbuf, -1);
Why is there no err == EDONE check here?
Without the check the prog could have skipped
ringbuf_submit and process_map_key_sample() above would not
be called.
