* [PATCH bpf-next] selftests/bpf: Move spintest from samples/bpf to selftests
@ 2022-03-30 7:50 Puranjay Mohan
2022-04-03 23:30 ` Andrii Nakryiko
0 siblings, 1 reply; 2+ messages in thread
From: Puranjay Mohan @ 2022-03-30 7:50 UTC (permalink / raw)
To: ast, daniel, andrii, bpf; +Cc: Puranjay Mohan
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%)
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;
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Move spintest from samples/bpf to selftests
2022-03-30 7:50 [PATCH bpf-next] selftests/bpf: Move spintest from samples/bpf to selftests Puranjay Mohan
@ 2022-04-03 23:30 ` Andrii Nakryiko
0 siblings, 0 replies; 2+ messages in thread
From: Andrii Nakryiko @ 2022-04-03 23:30 UTC (permalink / raw)
To: Puranjay Mohan; +Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-04-03 23:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-30 7:50 [PATCH bpf-next] selftests/bpf: Move spintest from samples/bpf to selftests Puranjay Mohan
2022-04-03 23:30 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.