| Message ID | 20220922142208.3009672-2-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [v3,bpf-next,1/2] bpf: Allow ringbuf memory to be used as map key | expand |
On 9/22/22 7:22 AM, Dave Marchevsky 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> Ack with a few nits below. Acked-by: Yonghong Song <yhs@fb.com> > --- > 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 | 65 ++++++++++++++++- > .../bpf/progs/test_ringbuf_map_key.c | 70 +++++++++++++++++++ > 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 > > -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..2d54ceac9417 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) static? > { > const size_t rec_sz = BPF_RINGBUF_HDR_SZ + sizeof(struct sample); > pthread_t thread; > @@ -297,3 +299,64 @@ 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) static? > +{ > + 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); > + if (!ASSERT_EQ(err, -EDONE, "ring_buffer__poll")) > + goto cleanup_ringbuf; there is no need for 'goto' above. > + > +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..495f85c6e120 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c > @@ -0,0 +1,70 @@ > +// 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); > + > + /* memcpy is necessary so that verifier doesn't complain with: could you add a keyword 'workaround' so it can be easily searchable if we forgot to make this enhancement in the future? > + * 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; > +}
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..2d54ceac9417 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,64 @@ 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"); + err = ring_buffer__poll(ringbuf, -1); + if (!ASSERT_EQ(err, -EDONE, "ring_buffer__poll")) + goto cleanup_ringbuf; + +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..495f85c6e120 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c @@ -0,0 +1,70 @@ +// 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); + + /* 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; +}
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> --- 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 | 65 ++++++++++++++++- .../bpf/progs/test_ringbuf_map_key.c | 70 +++++++++++++++++++ 3 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/test_ringbuf_map_key.c