| Message ID | 20220923060614.4025371-2-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [v4,bpf-next,1/2] bpf: Allow ringbuf memory to be used as map key | expand |
On Thu, Sep 22, 2022 at 11:06 PM Dave Marchevsky <davemarchevsky@fb.com> 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@fb.com> > Acked-by: Yonghong Song <yhs@fb.com> > --- > v3->v4: lore.kernel.org/bpf/20220922142208.3009672-2-davemarchevsky@fb.com > > * Fix some nits (Yonghong) > * make subtest runner functions static > * don't goto cleanup if -EDONE check fails > * add 'workaround' to comment in test to ease future grepping > * Add Yonghong ack > > v2->v3: lore.kernel.org/bpf/20220914123600.927632-2-davemarchevsky@fb.com > > * Test that ring_buffer__poll returns -EDONE (Alexei) > > v1->v2: lore.kernel.org/bpf/20220912101106.2765921-1-davemarchevsky@fb.com > > * 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 | 64 ++++++++++++++++- > .../bpf/progs/test_ringbuf_map_key.c | 71 +++++++++++++++++++ > 3 files changed, 139 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 5898d3828b82..28bd482f34a1 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -358,9 +358,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 > [...] > +static 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); if (!ASSERT_OK_PTR(ringbuf, ...)) is missing > + > + 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"); > + err = ring_buffer__poll(ringbuf, -1); > + ASSERT_EQ(err, -EDONE, "ring_buffer__poll"); > + > +cleanup_ringbuf: > + ring_buffer__free(ringbuf); > +cleanup: > + test_ringbuf_map_key_lskel__destroy(skel_map_key); > +} > + > +void test_ringbuf(void) > +{ > + if (test__start_subtest("ringbuf")) > + ringbuf_subtest(); > + if (test__start_subtest("ringbuf_map_key")) > + ringbuf_map_key_subtest(); > +} > diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c b/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c > new file mode 100644 > index 000000000000..44f89c4d1f9e > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c > @@ -0,0 +1,71 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */ > + > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include "bpf_misc.h" > + > +char _license[] SEC("license") = "GPL"; > + > +struct sample { > + int pid; > + int seq; > + long value; > + char comm[16]; > +}; > + > +struct { > + __uint(type, BPF_MAP_TYPE_RINGBUF); > + __uint(max_entries, 4096); you are setting max_entries from user-space, so best drop this line here > +} ringbuf SEC(".maps"); > + > +struct { > + __uint(type, BPF_MAP_TYPE_HASH); > + __uint(max_entries, 1000); > + __type(key, struct sample); > + __type(value, int); > +} hash_map SEC(".maps"); > + > +/* inputs */ > +int pid = 0; > + > +/* inner state */ > +long seq = 0; > + > +SEC("fentry/" SYS_PREFIX "sys_getpgid") if we use SEC("ksyscall/getpgid") this will work on s390x as well. We don't seem to gain anything from using fentry here, do we? Currently entire ringbuf selftests is denied on s390x, but you are switching it to use subtest, so we can enable at least the new subtest? > +int test_ringbuf_mem_map_key(void *ctx) > +{ > + int cur_pid = bpf_get_current_pid_tgid() >> 32; > + struct sample *sample, sample_copy; > + int *lookup_val; > + > + if (cur_pid != pid) > + return 0; > + > + sample = bpf_ringbuf_reserve(&ringbuf, sizeof(*sample), 0); > + if (!sample) > + return 0; > + > + sample->pid = pid; > + bpf_get_current_comm(sample->comm, sizeof(sample->comm)); > + sample->seq = ++seq; > + sample->value = 42; > + > + /* test using 'sample' (PTR_TO_MEM | MEM_ALLOC) as map key arg > + */ > + lookup_val = (int *)bpf_map_lookup_elem(&hash_map, sample); > + > + /* workaround - memcpy is necessary so that verifier doesn't > + * complain with: > + * verifier internal error: more than one arg with ref_obj_id R3 > + * when trying to do bpf_map_update_elem(&hash_map, sample, &sample->seq, BPF_ANY); > + * > + * Since bpf_map_lookup_elem above uses 'sample' as key, test using > + * sample field as value below > + */ > + __builtin_memcpy(&sample_copy, sample, sizeof(struct sample)); > + bpf_map_update_elem(&hash_map, &sample_copy, &sample->seq, BPF_ANY); > + > + bpf_ringbuf_submit(sample, 0); > + return 0; > +} > -- > 2.30.2 >
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 5898d3828b82..28bd482f34a1 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -358,9 +358,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..8c7d1467696c 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) +static void ringbuf_subtest(void) { const size_t rec_sz = BPF_RINGBUF_HDR_SZ + sizeof(struct sample); pthread_t thread; @@ -297,3 +299,63 @@ 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; + } +} + +static 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"); + err = ring_buffer__poll(ringbuf, -1); + ASSERT_EQ(err, -EDONE, "ring_buffer__poll"); + +cleanup_ringbuf: + ring_buffer__free(ringbuf); +cleanup: + test_ringbuf_map_key_lskel__destroy(skel_map_key); +} + +void test_ringbuf(void) +{ + if (test__start_subtest("ringbuf")) + ringbuf_subtest(); + if (test__start_subtest("ringbuf_map_key")) + ringbuf_map_key_subtest(); +} diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c b/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c new file mode 100644 index 000000000000..44f89c4d1f9e --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */ + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include "bpf_misc.h" + +char _license[] SEC("license") = "GPL"; + +struct sample { + int pid; + int seq; + long value; + char comm[16]; +}; + +struct { + __uint(type, BPF_MAP_TYPE_RINGBUF); + __uint(max_entries, 4096); +} ringbuf SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_HASH); + __uint(max_entries, 1000); + __type(key, struct sample); + __type(value, int); +} hash_map SEC(".maps"); + +/* inputs */ +int pid = 0; + +/* inner state */ +long seq = 0; + +SEC("fentry/" SYS_PREFIX "sys_getpgid") +int test_ringbuf_mem_map_key(void *ctx) +{ + int cur_pid = bpf_get_current_pid_tgid() >> 32; + struct sample *sample, sample_copy; + int *lookup_val; + + if (cur_pid != pid) + return 0; + + sample = bpf_ringbuf_reserve(&ringbuf, sizeof(*sample), 0); + if (!sample) + return 0; + + sample->pid = pid; + bpf_get_current_comm(sample->comm, sizeof(sample->comm)); + sample->seq = ++seq; + sample->value = 42; + + /* test using 'sample' (PTR_TO_MEM | MEM_ALLOC) as map key arg + */ + lookup_val = (int *)bpf_map_lookup_elem(&hash_map, sample); + + /* workaround - memcpy is necessary so that verifier doesn't + * complain with: + * verifier internal error: more than one arg with ref_obj_id R3 + * when trying to do bpf_map_update_elem(&hash_map, sample, &sample->seq, BPF_ANY); + * + * Since bpf_map_lookup_elem above uses 'sample' as key, test using + * sample field as value below + */ + __builtin_memcpy(&sample_copy, sample, sizeof(struct sample)); + bpf_map_update_elem(&hash_map, &sample_copy, &sample->seq, BPF_ANY); + + bpf_ringbuf_submit(sample, 0); + return 0; +}