| Message ID | 20220330075051.34326-1-puranjay12@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftests/bpf: Move spintest from samples/bpf to selftests | expand |
On Wed, Mar 30, 2022 at 12:51 AM Puranjay Mohan <puranjay12@gmail.com> wrote: > > According to the discussions in [1] everything from the samples/bpf > directory should be moved out or deleted. > > Move the spintest program from BPF samples to BPF selftests. > There are no functional changes. BPF skeleton has been used for loading, > etc. > Great that you are helping with selftests consolidation, thanks! I have few comments which will require a new revision, but overall this is moving in the right direction. > [1] https://lore.kernel.org/all/CAEf4BzYv3ONhy-JnQUtknzgBSK0gpm9GBJYtbAiJQe50_eX7Uw@mail.gmail.com/ > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > --- > samples/bpf/Makefile | 3 -- > .../selftests/bpf/prog_tests/spintest.c | 43 +++++++------------ > .../testing/selftests/bpf/progs/test_spin.c | 15 +++---- > 3 files changed, 23 insertions(+), 38 deletions(-) > rename samples/bpf/spintest_user.c => tools/testing/selftests/bpf/prog_tests/spintest.c (63%) > rename samples/bpf/spintest_kern.c => tools/testing/selftests/bpf/progs/test_spin.c (86%) > [...] > > - if (load_kallsyms()) { > - printf("failed to process /proc/kallsyms\n"); > - return 2; > - } > + err = load_kallsyms(); do we need to use kallsyms? selftests generally follows the latest kernel, so whatever attach target doesn't exist in the kernel we should just remove > + if (!ASSERT_OK(err, "load_kallsyms")) > + return; > + skel = test_spin__open_and_load(); > > - snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); > - obj = bpf_object__open_file(filename, NULL); > - if (libbpf_get_error(obj)) { > - fprintf(stderr, "ERROR: opening BPF object file failed\n"); > - obj = NULL; > - goto cleanup; > - } > + if (!ASSERT_OK_PTR(skel, "test_spin__open_and_load")) > + return; > > - /* load BPF program */ > - if (bpf_object__load(obj)) { > - fprintf(stderr, "ERROR: loading BPF object file failed\n"); > - goto cleanup; > - } > - > - map_fd = bpf_object__find_map_fd_by_name(obj, "my_map"); > - if (map_fd < 0) { > - fprintf(stderr, "ERROR: finding a map in obj file failed\n"); > - goto cleanup; > - } > + map_fd = bpf_map__fd(skel->maps.my_map); > > - bpf_object__for_each_program(prog, obj) { > + bpf_object__for_each_program(prog, skel->obj) { > section = bpf_program__section_name(prog); > if (sscanf(section, "kprobe/%s", symbol) != 1) > continue; > @@ -52,7 +41,8 @@ int main(int ac, char **argv) > /* Attach prog only when symbol exists */ > if (ksym_get_addr(symbol)) { > links[j] = bpf_program__attach(prog); > - if (libbpf_get_error(links[j])) { > + err = libbpf_get_error(links[j]); > + if (!ASSERT_OK(err, "bpf_program__attach")) { > fprintf(stderr, "bpf_program__attach failed\n"); > links[j] = NULL; > goto cleanup; there are a bunch of leftover prints, let's get rid of them and/or turn them into ASSERT_xxx() there is sleep(1) in each of 5 iterations, this slows down the selftests a lot. Let's get rid of it. there is also assert(), there shouldn't be any calls to assert() crashing the process in selftests > @@ -89,5 +79,4 @@ int main(int ac, char **argv) > bpf_link__destroy(links[j]); let's use skeleton's links "storage" and simplify the cleanup significantly. > > bpf_object__close(obj); this should be test_spin__destroy() instead > - return 0; > } > diff --git a/samples/bpf/spintest_kern.c b/tools/testing/selftests/bpf/progs/test_spin.c > similarity index 86% > rename from samples/bpf/spintest_kern.c > rename to tools/testing/selftests/bpf/progs/test_spin.c > index 455da77319d9..531783fe6cb9 100644 > --- a/samples/bpf/spintest_kern.c > +++ b/tools/testing/selftests/bpf/progs/test_spin.c > @@ -4,14 +4,14 @@ > * modify it under the terms of version 2 of the GNU General Public > * License as published by the Free Software Foundation. > */ > -#include <linux/skbuff.h> > -#include <linux/netdevice.h> > -#include <linux/version.h> > -#include <uapi/linux/bpf.h> > -#include <uapi/linux/perf_event.h> > +#include <vmlinux.h> > #include <bpf/bpf_helpers.h> > #include <bpf/bpf_tracing.h> > > +#ifndef PERF_MAX_STACK_DEPTH > +#define PERF_MAX_STACK_DEPTH 127 > +#endif > + > struct { > __uint(type, BPF_MAP_TYPE_HASH); > __type(key, long); > @@ -27,8 +27,8 @@ struct { > > struct { > __uint(type, BPF_MAP_TYPE_STACK_TRACE); > - __uint(key_size, sizeof(u32)); > - __uint(value_size, PERF_MAX_STACK_DEPTH * sizeof(u64)); > + __uint(key_size, sizeof(__u32)); > + __uint(value_size, PERF_MAX_STACK_DEPTH * sizeof(__u64)); > __uint(max_entries, 10000); > } stackmap SEC(".maps"); > > @@ -66,4 +66,3 @@ SEC("kprobe/__htab_percpu_map_update_elem")PROG(p16) > SEC("kprobe/htab_map_alloc")PROG(p17) I don't see why we need this PROG(pXX) magic instead of just doing a function call. Let's have a common function called from each of the defined kprobe program. > > char _license[] SEC("license") = "GPL"; > -u32 _version SEC("version") = LINUX_VERSION_CODE; > -- > 2.35.1 >
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 38638845db9d..570f1b0d9f00 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -23,7 +23,6 @@ tprogs-y += test_probe_write_user tprogs-y += trace_output tprogs-y += lathist tprogs-y += offwaketime -tprogs-y += spintest tprogs-y += map_perf_test tprogs-y += test_overhead tprogs-y += test_cgrp2_array_pin @@ -86,7 +85,6 @@ test_probe_write_user-objs := test_probe_write_user_user.o trace_output-objs := trace_output_user.o lathist-objs := lathist_user.o offwaketime-objs := offwaketime_user.o $(TRACE_HELPERS) -spintest-objs := spintest_user.o $(TRACE_HELPERS) map_perf_test-objs := map_perf_test_user.o test_overhead-objs := test_overhead_user.o test_cgrp2_array_pin-objs := test_cgrp2_array_pin.o @@ -144,7 +142,6 @@ always-y += tcbpf1_kern.o always-y += tc_l2_redirect_kern.o always-y += lathist_kern.o always-y += offwaketime_kern.o -always-y += spintest_kern.o always-y += map_perf_test_kern.o always-y += test_overhead_tp_kern.o always-y += test_overhead_raw_tp_kern.o diff --git a/samples/bpf/spintest_user.c b/tools/testing/selftests/bpf/prog_tests/spintest.c similarity index 63% rename from samples/bpf/spintest_user.c rename to tools/testing/selftests/bpf/prog_tests/spintest.c index 0d7e1e5a8658..15b436fb3f9a 100644 --- a/samples/bpf/spintest_user.c +++ b/tools/testing/selftests/bpf/prog_tests/spintest.c @@ -6,45 +6,34 @@ #include <sys/resource.h> #include <bpf/libbpf.h> #include <bpf/bpf.h> +#include <test_progs.h> #include "trace_helpers.h" +#include "test_spin.skel.h" -int main(int ac, char **argv) +void test_spin(void) { - char filename[256], symbol[256]; struct bpf_object *obj = NULL; struct bpf_link *links[20]; long key, next_key, value; struct bpf_program *prog; + struct test_spin *skel; int map_fd, i, j = 0; const char *section; struct ksym *sym; + char symbol[256]; + int err; - if (load_kallsyms()) { - printf("failed to process /proc/kallsyms\n"); - return 2; - } + err = load_kallsyms(); + if (!ASSERT_OK(err, "load_kallsyms")) + return; + skel = test_spin__open_and_load(); - snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); - obj = bpf_object__open_file(filename, NULL); - if (libbpf_get_error(obj)) { - fprintf(stderr, "ERROR: opening BPF object file failed\n"); - obj = NULL; - goto cleanup; - } + if (!ASSERT_OK_PTR(skel, "test_spin__open_and_load")) + return; - /* load BPF program */ - if (bpf_object__load(obj)) { - fprintf(stderr, "ERROR: loading BPF object file failed\n"); - goto cleanup; - } - - map_fd = bpf_object__find_map_fd_by_name(obj, "my_map"); - if (map_fd < 0) { - fprintf(stderr, "ERROR: finding a map in obj file failed\n"); - goto cleanup; - } + map_fd = bpf_map__fd(skel->maps.my_map); - bpf_object__for_each_program(prog, obj) { + bpf_object__for_each_program(prog, skel->obj) { section = bpf_program__section_name(prog); if (sscanf(section, "kprobe/%s", symbol) != 1) continue; @@ -52,7 +41,8 @@ int main(int ac, char **argv) /* Attach prog only when symbol exists */ if (ksym_get_addr(symbol)) { links[j] = bpf_program__attach(prog); - if (libbpf_get_error(links[j])) { + err = libbpf_get_error(links[j]); + if (!ASSERT_OK(err, "bpf_program__attach")) { fprintf(stderr, "bpf_program__attach failed\n"); links[j] = NULL; goto cleanup; @@ -89,5 +79,4 @@ int main(int ac, char **argv) bpf_link__destroy(links[j]); bpf_object__close(obj); - return 0; } diff --git a/samples/bpf/spintest_kern.c b/tools/testing/selftests/bpf/progs/test_spin.c similarity index 86% rename from samples/bpf/spintest_kern.c rename to tools/testing/selftests/bpf/progs/test_spin.c index 455da77319d9..531783fe6cb9 100644 --- a/samples/bpf/spintest_kern.c +++ b/tools/testing/selftests/bpf/progs/test_spin.c @@ -4,14 +4,14 @@ * modify it under the terms of version 2 of the GNU General Public * License as published by the Free Software Foundation. */ -#include <linux/skbuff.h> -#include <linux/netdevice.h> -#include <linux/version.h> -#include <uapi/linux/bpf.h> -#include <uapi/linux/perf_event.h> +#include <vmlinux.h> #include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> +#ifndef PERF_MAX_STACK_DEPTH +#define PERF_MAX_STACK_DEPTH 127 +#endif + struct { __uint(type, BPF_MAP_TYPE_HASH); __type(key, long); @@ -27,8 +27,8 @@ struct { struct { __uint(type, BPF_MAP_TYPE_STACK_TRACE); - __uint(key_size, sizeof(u32)); - __uint(value_size, PERF_MAX_STACK_DEPTH * sizeof(u64)); + __uint(key_size, sizeof(__u32)); + __uint(value_size, PERF_MAX_STACK_DEPTH * sizeof(__u64)); __uint(max_entries, 10000); } stackmap SEC(".maps"); @@ -66,4 +66,3 @@ SEC("kprobe/__htab_percpu_map_update_elem")PROG(p16) SEC("kprobe/htab_map_alloc")PROG(p17) char _license[] SEC("license") = "GPL"; -u32 _version SEC("version") = LINUX_VERSION_CODE;
According to the discussions in [1] everything from the samples/bpf directory should be moved out or deleted. Move the spintest program from BPF samples to BPF selftests. There are no functional changes. BPF skeleton has been used for loading, etc. [1] https://lore.kernel.org/all/CAEf4BzYv3ONhy-JnQUtknzgBSK0gpm9GBJYtbAiJQe50_eX7Uw@mail.gmail.com/ Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> --- samples/bpf/Makefile | 3 -- .../selftests/bpf/prog_tests/spintest.c | 43 +++++++------------ .../testing/selftests/bpf/progs/test_spin.c | 15 +++---- 3 files changed, 23 insertions(+), 38 deletions(-) rename samples/bpf/spintest_user.c => tools/testing/selftests/bpf/prog_tests/spintest.c (63%) rename samples/bpf/spintest_kern.c => tools/testing/selftests/bpf/progs/test_spin.c (86%)