* [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod
@ 2023-01-30 8:55 Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h Jiri Olsa
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
hi,
I noticed several times in discussions that we should move test kfuncs
into kernel module, now perhaps even more pressing with all the kfunc
effort. This patchset moves all the test kfuncs into bpf_testmod.
I added bpf_testmod/bpf_testmod_kfunc.h header that is shared between
bpf_testmod kernel module and BPF programs, which brings some difficulties
with __ksym define. But I'm not sure having separate headers for BPF
programs and for kernel module would be better.
This patchset also needs:
74bc3a5acc82 bpf: Add missing btf_put to register_btf_id_dtor_kfuncs
which is only in bpf/master now.
v2 changes:
- add 74bc3a5acc82 into bpf-next/master CI, so the test would pass
https://github.com/kernel-patches/vmtest/pull/192
- remove extra externs [Artem]
- using un/load_bpf_testmod in other tests
- rebased
thanks,
jirka
---
Jiri Olsa (7):
selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h
selftests/bpf: Move test_progs helpers to testing_helpers object
selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod
selftests/bpf: Use un/load_bpf_testmod functions in tests
selftests/bpf: Load bpf_testmod for verifier test
selftests/bpf: Allow to use kfunc from testmod.ko in test_verifier
bpf: Move kernel test kfuncs to bpf_testmod
net/bpf/test_run.c | 262 +------------------------------------------------------------------------------------------
tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h | 92 ++++++++++++++++++++++++++++++++
tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c | 34 ++----------
tools/testing/selftests/bpf/prog_tests/module_attach.c | 12 ++---
tools/testing/selftests/bpf/progs/cb_refs.c | 4 +-
tools/testing/selftests/bpf/progs/jit_probe_mem.c | 4 +-
tools/testing/selftests/bpf/progs/kfunc_call_destructive.c | 3 +-
tools/testing/selftests/bpf/progs/kfunc_call_fail.c | 9 +---
tools/testing/selftests/bpf/progs/kfunc_call_race.c | 3 +-
tools/testing/selftests/bpf/progs/kfunc_call_test.c | 16 +-----
tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c | 17 ++++--
tools/testing/selftests/bpf/progs/map_kptr.c | 6 +--
tools/testing/selftests/bpf/progs/map_kptr_fail.c | 5 +-
tools/testing/selftests/bpf/test_progs.c | 76 ++++-----------------------
tools/testing/selftests/bpf/test_progs.h | 1 -
tools/testing/selftests/bpf/test_verifier.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
tools/testing/selftests/bpf/testing_helpers.c | 61 +++++++++++++++++++++
tools/testing/selftests/bpf/testing_helpers.h | 10 ++++
19 files changed, 549 insertions(+), 434 deletions(-)
create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
2023-01-30 15:15 ` David Vernet
2023-01-30 8:55 ` [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object Jiri Olsa
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Move all kfunc exports into separate header file and include it
in tests that need it.
We will move all test kfuncs into bpf_testmod in following change,
so it's convenient to have declarations in single place.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../bpf/bpf_testmod/bpf_testmod_kfunc.h | 92 +++++++++++++++++++
tools/testing/selftests/bpf/progs/cb_refs.c | 4 +-
.../selftests/bpf/progs/jit_probe_mem.c | 4 +-
.../bpf/progs/kfunc_call_destructive.c | 3 +-
.../selftests/bpf/progs/kfunc_call_fail.c | 9 +-
.../selftests/bpf/progs/kfunc_call_race.c | 3 +-
.../selftests/bpf/progs/kfunc_call_test.c | 16 +---
.../bpf/progs/kfunc_call_test_subprog.c | 17 +++-
tools/testing/selftests/bpf/progs/map_kptr.c | 6 +-
.../selftests/bpf/progs/map_kptr_fail.c | 5 +-
10 files changed, 114 insertions(+), 45 deletions(-)
create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
new file mode 100644
index 000000000000..164cbae2b0d7
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _BPF_TESTMOD_KFUNC_H
+#define _BPF_TESTMOD_KFUNC_H
+
+#ifndef __ksym
+#define __ksym __attribute__((section(".ksyms")))
+#endif
+
+struct prog_test_pass1 {
+ int x0;
+ struct {
+ int x1;
+ struct {
+ int x2;
+ struct {
+ int x3;
+ };
+ };
+ };
+};
+
+struct prog_test_pass2 {
+ int len;
+ short arr1[4];
+ struct {
+ char arr2[4];
+ unsigned long arr3[8];
+ } x;
+};
+
+struct prog_test_fail1 {
+ void *p;
+ int x;
+};
+
+struct prog_test_fail2 {
+ int x8;
+ struct prog_test_pass1 x;
+};
+
+struct prog_test_fail3 {
+ int len;
+ char arr1[2];
+ char arr2[];
+};
+
+struct prog_test_member1 {
+ int a;
+};
+
+struct prog_test_member {
+ struct prog_test_member1 m;
+ int c;
+};
+
+struct prog_test_ref_kfunc {
+ int a;
+ int b;
+ struct prog_test_member memb;
+ struct prog_test_ref_kfunc *next;
+ refcount_t cnt;
+};
+
+extern struct prog_test_ref_kfunc *
+bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
+extern struct prog_test_ref_kfunc *
+bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
+extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
+
+extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
+extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
+extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
+extern int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
+extern void bpf_kfunc_call_int_mem_release(int *p) __ksym;
+
+extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
+
+extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
+ __u32 c, __u64 d) __ksym;
+extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
+extern struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
+extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
+
+extern void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
+extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
+extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
+extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
+
+extern void bpf_kfunc_call_test_destructive(void) __ksym;
+
+#endif /* _BPF_TESTMOD_KFUNC_H */
diff --git a/tools/testing/selftests/bpf/progs/cb_refs.c b/tools/testing/selftests/bpf/progs/cb_refs.c
index 7653df1bc787..823901c1b839 100644
--- a/tools/testing/selftests/bpf/progs/cb_refs.c
+++ b/tools/testing/selftests/bpf/progs/cb_refs.c
@@ -2,6 +2,7 @@
#include <vmlinux.h>
#include <bpf/bpf_tracing.h>
#include <bpf/bpf_helpers.h>
+#include "bpf_testmod/bpf_testmod_kfunc.h"
struct map_value {
struct prog_test_ref_kfunc __kptr_ref *ptr;
@@ -14,9 +15,6 @@ struct {
__uint(max_entries, 16);
} array_map SEC(".maps");
-extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
-
static __noinline int cb1(void *map, void *key, void *value, void *ctx)
{
void *p = *(void **)ctx;
diff --git a/tools/testing/selftests/bpf/progs/jit_probe_mem.c b/tools/testing/selftests/bpf/progs/jit_probe_mem.c
index 2d2e61470794..96207f126054 100644
--- a/tools/testing/selftests/bpf/progs/jit_probe_mem.c
+++ b/tools/testing/selftests/bpf/progs/jit_probe_mem.c
@@ -3,13 +3,11 @@
#include <vmlinux.h>
#include <bpf/bpf_tracing.h>
#include <bpf/bpf_helpers.h>
+#include "bpf_testmod/bpf_testmod_kfunc.h"
static struct prog_test_ref_kfunc __kptr_ref *v;
long total_sum = -1;
-extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
-
SEC("tc")
int test_jit_probe_mem(struct __sk_buff *ctx)
{
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
index 767472bc5a97..6a9b13a79ae8 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
@@ -1,8 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
-
-extern void bpf_kfunc_call_test_destructive(void) __ksym;
+#include "bpf_testmod/bpf_testmod_kfunc.h"
SEC("tc")
int kfunc_destructive_test(void)
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_fail.c b/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
index b98313d391c6..e857d1c4cf5b 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
@@ -2,14 +2,7 @@
/* Copyright (c) 2021 Facebook */
#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
-
-extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
-extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
-extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
-extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
-extern int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
-extern void bpf_kfunc_call_int_mem_release(int *p) __ksym;
+#include "bpf_testmod/bpf_testmod_kfunc.h"
struct syscall_test_args {
__u8 data[16];
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_race.c b/tools/testing/selftests/bpf/progs/kfunc_call_race.c
index 4e8fed75a4e0..a9558e434611 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_race.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_race.c
@@ -1,8 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
-
-extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
+#include "bpf_testmod/bpf_testmod_kfunc.h"
SEC("tc")
int kfunc_call_fail(struct __sk_buff *ctx)
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
index d91c58d06d38..d2fe17e2b433 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -2,21 +2,7 @@
/* Copyright (c) 2021 Facebook */
#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
-
-extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
-extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
-extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
- __u32 c, __u64 d) __ksym;
-
-extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
-extern void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
-extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
-extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
-extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
-extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
-extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
-extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
+#include "bpf_testmod/bpf_testmod_kfunc.h"
SEC("tc")
int kfunc_call_test4(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
index c1fdecabeabf..f74c78bb5efd 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
@@ -4,10 +4,21 @@
#include <bpf/bpf_helpers.h>
#include "bpf_tcp_helpers.h"
+/*
+ * We can't include vmlinux.h, because it conflicts with bpf_tcp_helpers.h,
+ * but we need refcount_t typedef for bpf_testmod_kfunc.h.
+ * Adding it directly.
+ */
+typedef struct {
+ int counter;
+} atomic_t;
+typedef struct refcount_struct {
+ atomic_t refs;
+} refcount_t;
+
+#include "bpf_testmod/bpf_testmod_kfunc.h"
+
extern const int bpf_prog_active __ksym;
-extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
- __u32 c, __u64 d) __ksym;
-extern struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
int active_res = -1;
int sk_state_res = -1;
diff --git a/tools/testing/selftests/bpf/progs/map_kptr.c b/tools/testing/selftests/bpf/progs/map_kptr.c
index eb8217803493..d53474f5b05b 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr.c
@@ -2,6 +2,7 @@
#include <vmlinux.h>
#include <bpf/bpf_tracing.h>
#include <bpf/bpf_helpers.h>
+#include "bpf_testmod/bpf_testmod_kfunc.h"
struct map_value {
struct prog_test_ref_kfunc __kptr *unref_ptr;
@@ -57,11 +58,6 @@ DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, hash_map, hash_of_hash_maps);
DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, hash_malloc_map, hash_of_hash_malloc_maps);
DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, lru_hash_map, hash_of_lru_hash_maps);
-extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
-extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
-
static void test_kptr_unref(struct map_value *v)
{
struct prog_test_ref_kfunc *p;
diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
index 760e41e1a632..1358a7c9e399 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
@@ -4,6 +4,7 @@
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_core_read.h>
#include "bpf_misc.h"
+#include "bpf_testmod/bpf_testmod_kfunc.h"
struct map_value {
char buf[8];
@@ -19,10 +20,6 @@ struct array_map {
__uint(max_entries, 1);
} array_map SEC(".maps");
-extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
-
SEC("?tc")
__failure __msg("kptr access size must be BPF_DW")
int size_not_bpf_dw(struct __sk_buff *ctx)
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
2023-01-30 15:23 ` David Vernet
2023-01-30 8:55 ` [PATCHv2 bpf-next 3/7] selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod Jiri Olsa
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Moving test_progs helpers to testing_helpers object so they can be
used from test_verifier in following changes.
Also adding missing ifndef header guard to testing_helpers.h header.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/test_progs.c | 67 +------------------
tools/testing/selftests/bpf/test_progs.h | 1 -
tools/testing/selftests/bpf/testing_helpers.c | 63 +++++++++++++++++
tools/testing/selftests/bpf/testing_helpers.h | 10 +++
4 files changed, 75 insertions(+), 66 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 6d5e3022c75f..a150c35516ef 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -11,7 +11,6 @@
#include <signal.h>
#include <string.h>
#include <execinfo.h> /* backtrace */
-#include <linux/membarrier.h>
#include <sys/sysinfo.h> /* get_nprocs */
#include <netinet/in.h>
#include <sys/select.h>
@@ -616,68 +615,6 @@ int extract_build_id(char *build_id, size_t size)
return -1;
}
-static int finit_module(int fd, const char *param_values, int flags)
-{
- return syscall(__NR_finit_module, fd, param_values, flags);
-}
-
-static int delete_module(const char *name, int flags)
-{
- return syscall(__NR_delete_module, name, flags);
-}
-
-/*
- * Trigger synchronize_rcu() in kernel.
- */
-int kern_sync_rcu(void)
-{
- return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0);
-}
-
-static void unload_bpf_testmod(void)
-{
- if (kern_sync_rcu())
- fprintf(env.stderr, "Failed to trigger kernel-side RCU sync!\n");
- if (delete_module("bpf_testmod", 0)) {
- if (errno == ENOENT) {
- if (verbose())
- fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
- return;
- }
- fprintf(env.stderr, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
- return;
- }
- if (verbose())
- fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
-}
-
-static int load_bpf_testmod(void)
-{
- int fd;
-
- /* ensure previous instance of the module is unloaded */
- unload_bpf_testmod();
-
- if (verbose())
- fprintf(stdout, "Loading bpf_testmod.ko...\n");
-
- fd = open("bpf_testmod.ko", O_RDONLY);
- if (fd < 0) {
- fprintf(env.stderr, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
- return -ENOENT;
- }
- if (finit_module(fd, "", 0)) {
- fprintf(env.stderr, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
- close(fd);
- return -EINVAL;
- }
- close(fd);
-
- if (verbose())
- fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
- return 0;
-}
-
/* extern declarations for test funcs */
#define DEFINE_TEST(name) \
extern void test_##name(void) __weak; \
@@ -1655,7 +1592,7 @@ int main(int argc, char **argv)
env.stderr = stderr;
env.has_testmod = true;
- if (!env.list_test_names && load_bpf_testmod()) {
+ if (!env.list_test_names && load_bpf_testmod(env.stderr, verbose())) {
fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
env.has_testmod = false;
}
@@ -1754,7 +1691,7 @@ int main(int argc, char **argv)
close(env.saved_netns_fd);
out:
if (!env.list_test_names && env.has_testmod)
- unload_bpf_testmod();
+ unload_bpf_testmod(env.stderr, verbose());
free_test_selector(&env.test_selector);
free_test_selector(&env.subtest_selector);
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index d5d51ec97ec8..b9dac3c32712 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -390,7 +390,6 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name);
int compare_map_keys(int map1_fd, int map2_fd);
int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len);
int extract_build_id(char *build_id, size_t size);
-int kern_sync_rcu(void);
int trigger_module_test_read(int read_sz);
int trigger_module_test_write(int write_sz);
int write_sysctl(const char *sysctl, const char *value);
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index 9695318e8132..c0eb54bf08b3 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -8,6 +8,7 @@
#include <bpf/libbpf.h>
#include "test_progs.h"
#include "testing_helpers.h"
+#include <linux/membarrier.h>
int parse_num_list(const char *s, bool **num_set, int *num_set_len)
{
@@ -229,3 +230,65 @@ int bpf_test_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
return bpf_prog_load(type, NULL, license, insns, insns_cnt, &opts);
}
+
+static int finit_module(int fd, const char *param_values, int flags)
+{
+ return syscall(__NR_finit_module, fd, param_values, flags);
+}
+
+static int delete_module(const char *name, int flags)
+{
+ return syscall(__NR_delete_module, name, flags);
+}
+
+void unload_bpf_testmod(FILE *err, bool verbose)
+{
+ if (kern_sync_rcu())
+ fprintf(err, "Failed to trigger kernel-side RCU sync!\n");
+ if (delete_module("bpf_testmod", 0)) {
+ if (errno == ENOENT) {
+ if (verbose)
+ fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
+ return;
+ }
+ fprintf(err, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
+ return;
+ }
+ if (verbose)
+ fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
+}
+
+int load_bpf_testmod(FILE *err, bool verbose)
+{
+ int fd;
+
+ /* ensure previous instance of the module is unloaded */
+ unload_bpf_testmod(err, verbose);
+
+ if (verbose)
+ fprintf(stdout, "Loading bpf_testmod.ko...\n");
+
+ fd = open("bpf_testmod.ko", O_RDONLY);
+ if (fd < 0) {
+ fprintf(err, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
+ return -ENOENT;
+ }
+ if (finit_module(fd, "", 0)) {
+ fprintf(err, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
+ close(fd);
+ return -EINVAL;
+ }
+ close(fd);
+
+ if (verbose)
+ fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
+ return 0;
+}
+
+/*
+ * Trigger synchronize_rcu() in kernel.
+ */
+int kern_sync_rcu(void)
+{
+ return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0);
+}
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index 6ec00bf79cb5..2f80ca5b5f54 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -1,5 +1,9 @@
/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
/* Copyright (C) 2020 Facebook, Inc. */
+
+#ifndef __TRACING_HELPERS_H
+#define __TRACING_HELPERS_H
+
#include <stdbool.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
@@ -20,3 +24,9 @@ struct test_filter_set;
int parse_test_list(const char *s,
struct test_filter_set *test_set,
bool is_glob_pattern);
+
+int load_bpf_testmod(FILE *err, bool verbose);
+void unload_bpf_testmod(FILE *err, bool verbose);
+int kern_sync_rcu(void);
+
+#endif /* __TRACING_HELPERS_H */
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 3/7] selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
2023-01-30 15:28 ` David Vernet
2023-01-30 8:55 ` [PATCHv2 bpf-next 4/7] selftests/bpf: Use un/load_bpf_testmod functions in tests Jiri Olsa
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Do not unload bpf_testmod in load_bpf_testmod, instead call
unload_bpf_testmod separatelly.
This way we will be able use un/load_bpf_testmod functions
in other tests that un/load bpf_testmod module.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/test_progs.c | 11 ++++++++---
tools/testing/selftests/bpf/testing_helpers.c | 3 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index a150c35516ef..9ca718c84890 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1592,9 +1592,14 @@ int main(int argc, char **argv)
env.stderr = stderr;
env.has_testmod = true;
- if (!env.list_test_names && load_bpf_testmod(env.stderr, verbose())) {
- fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
- env.has_testmod = false;
+ if (!env.list_test_names) {
+ /* ensure previous instance of the module is unloaded */
+ unload_bpf_testmod(env.stderr, verbose());
+
+ if (load_bpf_testmod(env.stderr, verbose())) {
+ fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
+ env.has_testmod = false;
+ }
}
/* initializing tests */
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index c0eb54bf08b3..ade6208b4a69 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -262,9 +262,6 @@ int load_bpf_testmod(FILE *err, bool verbose)
{
int fd;
- /* ensure previous instance of the module is unloaded */
- unload_bpf_testmod(err, verbose);
-
if (verbose)
fprintf(stdout, "Loading bpf_testmod.ko...\n");
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 4/7] selftests/bpf: Use un/load_bpf_testmod functions in tests
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
` (2 preceding siblings ...)
2023-01-30 8:55 ` [PATCHv2 bpf-next 3/7] selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
2023-01-30 15:32 ` David Vernet
2023-01-30 8:55 ` [PATCHv2 bpf-next 5/7] selftests/bpf: Load bpf_testmod for verifier test Jiri Olsa
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Now that we have un/load_bpf_testmod helpers in testing_helpers.h,
we can use it in other tests and save some lines.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/bpf_mod_race.c | 34 +++----------------
.../selftests/bpf/prog_tests/module_attach.c | 12 +++----
tools/testing/selftests/bpf/testing_helpers.c | 7 ++--
tools/testing/selftests/bpf/testing_helpers.h | 2 +-
4 files changed, 14 insertions(+), 41 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c b/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
index a4d0cc9d3367..40c5b3b5ff78 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
@@ -11,6 +11,7 @@
#include "ksym_race.skel.h"
#include "bpf_mod_race.skel.h"
#include "kfunc_call_race.skel.h"
+#include "testing_helpers.h"
/* This test crafts a race between btf_try_get_module and do_init_module, and
* checks whether btf_try_get_module handles the invocation for a well-formed
@@ -44,35 +45,10 @@ enum bpf_test_state {
static _Atomic enum bpf_test_state state = _TS_INVALID;
-static int sys_finit_module(int fd, const char *param_values, int flags)
-{
- return syscall(__NR_finit_module, fd, param_values, flags);
-}
-
-static int sys_delete_module(const char *name, unsigned int flags)
-{
- return syscall(__NR_delete_module, name, flags);
-}
-
-static int load_module(const char *mod)
-{
- int ret, fd;
-
- fd = open("bpf_testmod.ko", O_RDONLY);
- if (fd < 0)
- return fd;
-
- ret = sys_finit_module(fd, "", 0);
- close(fd);
- if (ret < 0)
- return ret;
- return 0;
-}
-
static void *load_module_thread(void *p)
{
- if (!ASSERT_NEQ(load_module("bpf_testmod.ko"), 0, "load_module_thread must fail"))
+ if (!ASSERT_NEQ(load_bpf_testmod(stderr, false), 0, "load_module_thread must fail"))
atomic_store(&state, TS_MODULE_LOAD);
else
atomic_store(&state, TS_MODULE_LOAD_FAIL);
@@ -124,7 +100,7 @@ static void test_bpf_mod_race_config(const struct test_config *config)
if (!ASSERT_NEQ(fault_addr, MAP_FAILED, "mmap for uffd registration"))
return;
- if (!ASSERT_OK(sys_delete_module("bpf_testmod", 0), "unload bpf_testmod"))
+ if (!ASSERT_OK(unload_bpf_testmod(stderr, false), "unload bpf_testmod"))
goto end_mmap;
skel = bpf_mod_race__open();
@@ -202,8 +178,8 @@ static void test_bpf_mod_race_config(const struct test_config *config)
bpf_mod_race__destroy(skel);
ASSERT_OK(kern_sync_rcu(), "kern_sync_rcu");
end_module:
- sys_delete_module("bpf_testmod", 0);
- ASSERT_OK(load_module("bpf_testmod.ko"), "restore bpf_testmod");
+ unload_bpf_testmod(stderr, false);
+ ASSERT_OK(load_bpf_testmod(stderr, false), "restore bpf_testmod");
end_mmap:
munmap(fault_addr, 4096);
atomic_store(&state, _TS_INVALID);
diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
index 7fc01ff490db..54dee902a30a 100644
--- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
@@ -4,6 +4,7 @@
#include <test_progs.h>
#include <stdbool.h>
#include "test_module_attach.skel.h"
+#include "testing_helpers.h"
static int duration;
@@ -32,11 +33,6 @@ static int trigger_module_test_writable(int *val)
return 0;
}
-static int delete_module(const char *name, int flags)
-{
- return syscall(__NR_delete_module, name, flags);
-}
-
void test_module_attach(void)
{
const int READ_SZ = 456;
@@ -93,21 +89,21 @@ void test_module_attach(void)
if (!ASSERT_OK_PTR(link, "attach_fentry"))
goto cleanup;
- ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
+ ASSERT_ERR(unload_bpf_testmod(stderr, false), "unload_bpf_testmod");
bpf_link__destroy(link);
link = bpf_program__attach(skel->progs.handle_fexit);
if (!ASSERT_OK_PTR(link, "attach_fexit"))
goto cleanup;
- ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
+ ASSERT_ERR(unload_bpf_testmod(stderr, false), "unload_bpf_testmod");
bpf_link__destroy(link);
link = bpf_program__attach(skel->progs.kprobe_multi);
if (!ASSERT_OK_PTR(link, "attach_kprobe_multi"))
goto cleanup;
- ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
+ ASSERT_ERR(unload_bpf_testmod(stderr, false), "unload_bpf_testmod");
bpf_link__destroy(link);
cleanup:
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index ade6208b4a69..c8326d2355da 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -241,7 +241,7 @@ static int delete_module(const char *name, int flags)
return syscall(__NR_delete_module, name, flags);
}
-void unload_bpf_testmod(FILE *err, bool verbose)
+int unload_bpf_testmod(FILE *err, bool verbose)
{
if (kern_sync_rcu())
fprintf(err, "Failed to trigger kernel-side RCU sync!\n");
@@ -249,13 +249,14 @@ void unload_bpf_testmod(FILE *err, bool verbose)
if (errno == ENOENT) {
if (verbose)
fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
- return;
+ return -1;
}
fprintf(err, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
- return;
+ return -1;
}
if (verbose)
fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
+ return 0;
}
int load_bpf_testmod(FILE *err, bool verbose)
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index 2f80ca5b5f54..dd725c02b31f 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -26,7 +26,7 @@ int parse_test_list(const char *s,
bool is_glob_pattern);
int load_bpf_testmod(FILE *err, bool verbose);
-void unload_bpf_testmod(FILE *err, bool verbose);
+int unload_bpf_testmod(FILE *err, bool verbose);
int kern_sync_rcu(void);
#endif /* __TRACING_HELPERS_H */
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 5/7] selftests/bpf: Load bpf_testmod for verifier test
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
` (3 preceding siblings ...)
2023-01-30 8:55 ` [PATCHv2 bpf-next 4/7] selftests/bpf: Use un/load_bpf_testmod functions in tests Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 6/7] selftests/bpf: Allow to use kfunc from testmod.ko in test_verifier Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 7/7] bpf: Move kernel test kfuncs to bpf_testmod Jiri Olsa
6 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Loading bpf_testmod kernel module for verifier test. We will
move all the tests kfuncs into bpf_testmod in following change.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/test_verifier.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 8c808551dfd7..ec7bad90595a 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -45,6 +45,7 @@
#include "bpf_util.h"
#include "test_btf.h"
#include "../../../include/linux/filter.h"
+#include "testing_helpers.h"
#ifndef ENOTSUPP
#define ENOTSUPP 524
@@ -1705,6 +1706,12 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
{
int i, passes = 0, errors = 0;
+ /* ensure previous instance of the module is unloaded */
+ unload_bpf_testmod(stderr, verbose);
+
+ if (load_bpf_testmod(stderr, verbose))
+ return EXIT_FAILURE;
+
for (i = from; i < to; i++) {
struct bpf_test *test = &tests[i];
@@ -1732,6 +1739,8 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
}
}
+ unload_bpf_testmod(stderr, verbose);
+
printf("Summary: %d PASSED, %d SKIPPED, %d FAILED\n", passes,
skips, errors);
return errors ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 6/7] selftests/bpf: Allow to use kfunc from testmod.ko in test_verifier
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
` (4 preceding siblings ...)
2023-01-30 8:55 ` [PATCHv2 bpf-next 5/7] selftests/bpf: Load bpf_testmod for verifier test Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 7/7] bpf: Move kernel test kfuncs to bpf_testmod Jiri Olsa
6 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Currently the test_verifier allows test to specify kfunc symbol
and search for it in the kernel BTF.
Adding the possibility to search for kfunc also in bpf_testmod
module when it's not found in kernel BTF.
To find bpf_testmod btf we need to get back SYS_ADMIN cap.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/test_verifier.c | 161 +++++++++++++++++---
1 file changed, 139 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index ec7bad90595a..3a6ecd5e4d45 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -879,8 +879,140 @@ static int create_map_kptr(void)
return fd;
}
+static void set_root(bool set)
+{
+ __u64 caps;
+
+ if (set) {
+ if (cap_enable_effective(1ULL << CAP_SYS_ADMIN, &caps))
+ perror("cap_disable_effective(CAP_SYS_ADMIN)");
+ } else {
+ if (cap_disable_effective(1ULL << CAP_SYS_ADMIN, &caps))
+ perror("cap_disable_effective(CAP_SYS_ADMIN)");
+ }
+}
+
+static inline __u64 ptr_to_u64(const void *ptr)
+{
+ return (__u64) (unsigned long) ptr;
+}
+
+static struct btf *btf__load_testmod_btf(struct btf *vmlinux)
+{
+ struct bpf_btf_info info;
+ __u32 len = sizeof(info);
+ struct btf *btf = NULL;
+ char name[64];
+ __u32 id = 0;
+ int err, fd;
+
+ /* Iterate all loaded BTF objects and find bpf_testmod,
+ * we need SYS_ADMIN cap for that.
+ */
+ set_root(true);
+
+ while (true) {
+ err = bpf_btf_get_next_id(id, &id);
+ if (err) {
+ if (errno == ENOENT)
+ break;
+ perror("bpf_btf_get_next_id failed");
+ break;
+ }
+
+ fd = bpf_btf_get_fd_by_id(id);
+ if (fd < 0) {
+ if (errno == ENOENT)
+ continue;
+ perror("bpf_btf_get_fd_by_id failed");
+ break;
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.name_len = sizeof(name);
+ info.name = ptr_to_u64(name);
+ len = sizeof(info);
+
+ err = bpf_obj_get_info_by_fd(fd, &info, &len);
+ if (err) {
+ close(fd);
+ perror("bpf_obj_get_info_by_fd failed");
+ break;
+ }
+
+ if (strcmp("bpf_testmod", name)) {
+ close(fd);
+ continue;
+ }
+
+ btf = btf__load_from_kernel_by_id_split(id, vmlinux);
+ if (!btf) {
+ close(fd);
+ break;
+ }
+
+ /* We need the fd to stay open so it can be used in fd_array.
+ * The final cleanup call to btf__free will free btf object
+ * and close the file descriptor.
+ */
+ btf__set_fd(btf, fd);
+ break;
+ }
+
+ set_root(false);
+ return btf;
+}
+
+static struct btf *testmod_btf;
+static struct btf *vmlinux_btf;
+
+static void kfuncs_cleanup(void)
+{
+ btf__free(testmod_btf);
+ btf__free(vmlinux_btf);
+}
+
+static void fixup_prog_kfuncs(struct bpf_insn *prog, int *fd_array,
+ struct kfunc_btf_id_pair *fixup_kfunc_btf_id)
+{
+ /* Patch in kfunc BTF IDs */
+ while (fixup_kfunc_btf_id->kfunc) {
+ int btf_id = 0;
+
+ /* try to find kfunc in kernel BTF */
+ vmlinux_btf = vmlinux_btf ?: btf__load_vmlinux_btf();
+ if (vmlinux_btf) {
+ btf_id = btf__find_by_name_kind(vmlinux_btf,
+ fixup_kfunc_btf_id->kfunc,
+ BTF_KIND_FUNC);
+ btf_id = btf_id < 0 ? 0 : btf_id;
+ }
+
+ /* kfunc not found in kernel BTF, try bpf_testmod BTF */
+ if (!btf_id) {
+ testmod_btf = testmod_btf ?: btf__load_testmod_btf(vmlinux_btf);
+ if (testmod_btf) {
+ btf_id = btf__find_by_name_kind(testmod_btf,
+ fixup_kfunc_btf_id->kfunc,
+ BTF_KIND_FUNC);
+ btf_id = btf_id < 0 ? 0 : btf_id;
+ if (btf_id) {
+ /* We put bpf_testmod module fd into fd_array
+ * and its index 1 into instruction 'off'.
+ */
+ *fd_array = btf__fd(testmod_btf);
+ prog[fixup_kfunc_btf_id->insn_idx].off = 1;
+ }
+ }
+ }
+
+ prog[fixup_kfunc_btf_id->insn_idx].imm = btf_id;
+ fixup_kfunc_btf_id++;
+ }
+}
+
static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
- struct bpf_insn *prog, int *map_fds)
+ struct bpf_insn *prog, int *map_fds, int *fd_array)
{
int *fixup_map_hash_8b = test->fixup_map_hash_8b;
int *fixup_map_hash_48b = test->fixup_map_hash_48b;
@@ -905,7 +1037,6 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_ringbuf = test->fixup_map_ringbuf;
int *fixup_map_timer = test->fixup_map_timer;
int *fixup_map_kptr = test->fixup_map_kptr;
- struct kfunc_btf_id_pair *fixup_kfunc_btf_id = test->fixup_kfunc_btf_id;
if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -1106,25 +1237,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
} while (*fixup_map_kptr);
}
- /* Patch in kfunc BTF IDs */
- if (fixup_kfunc_btf_id->kfunc) {
- struct btf *btf;
- int btf_id;
-
- do {
- btf_id = 0;
- btf = btf__load_vmlinux_btf();
- if (btf) {
- btf_id = btf__find_by_name_kind(btf,
- fixup_kfunc_btf_id->kfunc,
- BTF_KIND_FUNC);
- btf_id = btf_id < 0 ? 0 : btf_id;
- }
- btf__free(btf);
- prog[fixup_kfunc_btf_id->insn_idx].imm = btf_id;
- fixup_kfunc_btf_id++;
- } while (fixup_kfunc_btf_id->kfunc);
- }
+ fixup_prog_kfuncs(prog, fd_array, test->fixup_kfunc_btf_id);
}
struct libcap {
@@ -1451,6 +1564,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
int run_errs, run_successes;
int map_fds[MAX_NR_MAPS];
const char *expected_err;
+ int fd_array[2] = { -1, -1 };
int saved_errno;
int fixup_skips;
__u32 pflags;
@@ -1464,7 +1578,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
if (!prog_type)
prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
fixup_skips = skips;
- do_test_fixup(test, prog_type, prog, map_fds);
+ do_test_fixup(test, prog_type, prog, map_fds, &fd_array[1]);
if (test->fill_insns) {
prog = test->fill_insns;
prog_len = test->prog_len;
@@ -1498,6 +1612,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
else
opts.log_level = DEFAULT_LIBBPF_LOG_LEVEL;
opts.prog_flags = pflags;
+ if (fd_array[1] != -1)
+ opts.fd_array = &fd_array[0];
if ((prog_type == BPF_PROG_TYPE_TRACING ||
prog_type == BPF_PROG_TYPE_LSM) && test->kfunc) {
@@ -1740,6 +1856,7 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
}
unload_bpf_testmod(stderr, verbose);
+ kfuncs_cleanup();
printf("Summary: %d PASSED, %d SKIPPED, %d FAILED\n", passes,
skips, errors);
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 bpf-next 7/7] bpf: Move kernel test kfuncs to bpf_testmod
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
` (5 preceding siblings ...)
2023-01-30 8:55 ` [PATCHv2 bpf-next 6/7] selftests/bpf: Allow to use kfunc from testmod.ko in test_verifier Jiri Olsa
@ 2023-01-30 8:55 ` Jiri Olsa
6 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 8:55 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, David Vernet,
Kumar Kartikeya Dwivedi, Artem Savkov
Moving kernel test kfuncs into bpf_testmod kernel module,
and adding necessary init calls and BTF IDs records.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
net/bpf/test_run.c | 262 +-----------------
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 198 ++++++++++++-
2 files changed, 198 insertions(+), 262 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 7dbefa4fd2eb..6e203fcbc016 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -535,209 +535,6 @@ int noinline bpf_modify_return_test(int a, int *b)
return a + *b;
}
-u64 noinline bpf_kfunc_call_test1(struct sock *sk, u32 a, u64 b, u32 c, u64 d)
-{
- return a + b + c + d;
-}
-
-int noinline bpf_kfunc_call_test2(struct sock *sk, u32 a, u32 b)
-{
- return a + b;
-}
-
-struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
-{
- return sk;
-}
-
-long noinline bpf_kfunc_call_test4(signed char a, short b, int c, long d)
-{
- /* Provoke the compiler to assume that the caller has sign-extended a,
- * b and c on platforms where this is required (e.g. s390x).
- */
- return (long)a + (long)b + (long)c + d;
-}
-
-struct prog_test_member1 {
- int a;
-};
-
-struct prog_test_member {
- struct prog_test_member1 m;
- int c;
-};
-
-struct prog_test_ref_kfunc {
- int a;
- int b;
- struct prog_test_member memb;
- struct prog_test_ref_kfunc *next;
- refcount_t cnt;
-};
-
-static struct prog_test_ref_kfunc prog_test_struct = {
- .a = 42,
- .b = 108,
- .next = &prog_test_struct,
- .cnt = REFCOUNT_INIT(1),
-};
-
-noinline struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr)
-{
- refcount_inc(&prog_test_struct.cnt);
- return &prog_test_struct;
-}
-
-noinline struct prog_test_member *
-bpf_kfunc_call_memb_acquire(void)
-{
- WARN_ON_ONCE(1);
- return NULL;
-}
-
-noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p)
-{
- if (!p)
- return;
-
- refcount_dec(&p->cnt);
-}
-
-noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p)
-{
-}
-
-noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
-{
- WARN_ON_ONCE(1);
-}
-
-static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
-{
- if (size > 2 * sizeof(int))
- return NULL;
-
- return (int *)p;
-}
-
-noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
-{
- return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
-}
-
-noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
-{
- return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
-}
-
-/* the next 2 ones can't be really used for testing expect to ensure
- * that the verifier rejects the call.
- * Acquire functions must return struct pointers, so these ones are
- * failing.
- */
-noinline int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
-{
- return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
-}
-
-noinline void bpf_kfunc_call_int_mem_release(int *p)
-{
-}
-
-noinline struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
-{
- struct prog_test_ref_kfunc *p = READ_ONCE(*pp);
-
- if (!p)
- return NULL;
- refcount_inc(&p->cnt);
- return p;
-}
-
-struct prog_test_pass1 {
- int x0;
- struct {
- int x1;
- struct {
- int x2;
- struct {
- int x3;
- };
- };
- };
-};
-
-struct prog_test_pass2 {
- int len;
- short arr1[4];
- struct {
- char arr2[4];
- unsigned long arr3[8];
- } x;
-};
-
-struct prog_test_fail1 {
- void *p;
- int x;
-};
-
-struct prog_test_fail2 {
- int x8;
- struct prog_test_pass1 x;
-};
-
-struct prog_test_fail3 {
- int len;
- char arr1[2];
- char arr2[];
-};
-
-noinline void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb)
-{
-}
-
-noinline void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p)
-{
-}
-
-noinline void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p)
-{
-}
-
-noinline void bpf_kfunc_call_test_fail1(struct prog_test_fail1 *p)
-{
-}
-
-noinline void bpf_kfunc_call_test_fail2(struct prog_test_fail2 *p)
-{
-}
-
-noinline void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p)
-{
-}
-
-noinline void bpf_kfunc_call_test_mem_len_pass1(void *mem, int mem__sz)
-{
-}
-
-noinline void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len)
-{
-}
-
-noinline void bpf_kfunc_call_test_mem_len_fail2(u64 *mem, int len)
-{
-}
-
-noinline void bpf_kfunc_call_test_ref(struct prog_test_ref_kfunc *p)
-{
-}
-
-noinline void bpf_kfunc_call_test_destructive(void)
-{
-}
-
__diag_pop();
BTF_SET8_START(bpf_test_modify_return_ids)
@@ -750,34 +547,6 @@ static const struct btf_kfunc_id_set bpf_test_modify_return_set = {
.set = &bpf_test_modify_return_ids,
};
-BTF_SET8_START(test_sk_check_kfunc_ids)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
-BTF_ID_FLAGS(func, bpf_kfunc_call_memb_acquire, KF_ACQUIRE | KF_RET_NULL)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
-BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE)
-BTF_ID_FLAGS(func, bpf_kfunc_call_memb1_release, KF_RELEASE)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_get_rdwr_mem, KF_RET_NULL)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_get_rdonly_mem, KF_RET_NULL)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_acq_rdonly_mem, KF_ACQUIRE | KF_RET_NULL)
-BTF_ID_FLAGS(func, bpf_kfunc_call_int_mem_release, KF_RELEASE)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_kptr_get, KF_ACQUIRE | KF_RET_NULL | KF_KPTR_GET)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass_ctx)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass1)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass2)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail1)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail2)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail3)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
-BTF_SET8_END(test_sk_check_kfunc_ids)
-
static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
u32 size, u32 headroom, u32 tailroom)
{
@@ -1660,37 +1429,8 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
return err;
}
-static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
- .owner = THIS_MODULE,
- .set = &test_sk_check_kfunc_ids,
-};
-
-BTF_ID_LIST(bpf_prog_test_dtor_kfunc_ids)
-BTF_ID(struct, prog_test_ref_kfunc)
-BTF_ID(func, bpf_kfunc_call_test_release)
-BTF_ID(struct, prog_test_member)
-BTF_ID(func, bpf_kfunc_call_memb_release)
-
static int __init bpf_prog_test_run_init(void)
{
- const struct btf_id_dtor_kfunc bpf_prog_test_dtor_kfunc[] = {
- {
- .btf_id = bpf_prog_test_dtor_kfunc_ids[0],
- .kfunc_btf_id = bpf_prog_test_dtor_kfunc_ids[1]
- },
- {
- .btf_id = bpf_prog_test_dtor_kfunc_ids[2],
- .kfunc_btf_id = bpf_prog_test_dtor_kfunc_ids[3],
- },
- };
- int ret;
-
- ret = register_btf_fmodret_id_set(&bpf_test_modify_return_set);
- ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_prog_test_kfunc_set);
- ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_prog_test_kfunc_set);
- ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_prog_test_kfunc_set);
- return ret ?: register_btf_id_dtor_kfuncs(bpf_prog_test_dtor_kfunc,
- ARRAY_SIZE(bpf_prog_test_dtor_kfunc),
- THIS_MODULE);
+ return register_btf_fmodret_id_set(&bpf_test_modify_return_set);
}
late_initcall(bpf_prog_test_run_init);
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 5085fea3cac5..9307aa60a088 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -9,6 +9,8 @@
#include <linux/sysfs.h>
#include <linux/tracepoint.h>
#include "bpf_testmod.h"
+#define __ksym
+#include "bpf_testmod_kfunc.h"
#define CREATE_TRACE_POINTS
#include "bpf_testmod-events.h"
@@ -220,7 +222,180 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
.write = bpf_testmod_test_write,
};
+noinline u64 bpf_kfunc_call_test1(struct sock *sk, u32 a, u64 b, u32 c, u64 d)
+{
+ return a + b + c + d;
+}
+
+noinline int bpf_kfunc_call_test2(struct sock *sk, u32 a, u32 b)
+{
+ return a + b;
+}
+
+struct sock *noinline bpf_kfunc_call_test3(struct sock *sk)
+{
+ return sk;
+}
+
+noinline long bpf_kfunc_call_test4(signed char a, short b, int c, long d)
+{
+ /* Provoke the compiler to assume that the caller has sign-extended a,
+ * b and c on platforms where this is required (e.g. s390x).
+ */
+ return (long)a + (long)b + (long)c + d;
+}
+
+static struct prog_test_ref_kfunc prog_test_struct = {
+ .a = 42,
+ .b = 108,
+ .next = &prog_test_struct,
+ .cnt = REFCOUNT_INIT(1),
+};
+
+noinline struct prog_test_ref_kfunc *
+bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr)
+{
+ refcount_inc(&prog_test_struct.cnt);
+ return &prog_test_struct;
+}
+
+noinline struct prog_test_member *
+bpf_kfunc_call_memb_acquire(void)
+{
+ WARN_ON_ONCE(1);
+ return NULL;
+}
+
+noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p)
+{
+ if (!p)
+ return;
+
+ refcount_dec(&p->cnt);
+}
+
+noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p)
+{
+}
+
+noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
+{
+ WARN_ON_ONCE(1);
+}
+
+static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
+{
+ if (size > 2 * sizeof(int))
+ return NULL;
+
+ return (int *)p;
+}
+
+noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
+{
+ return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
+}
+
+noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
+{
+ return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
+}
+
+/* the next 2 ones can't be really used for testing expect to ensure
+ * that the verifier rejects the call.
+ * Acquire functions must return struct pointers, so these ones are
+ * failing.
+ */
+noinline int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
+{
+ return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
+}
+
+noinline void bpf_kfunc_call_int_mem_release(int *p)
+{
+}
+
+noinline struct prog_test_ref_kfunc *
+bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
+{
+ struct prog_test_ref_kfunc *p = READ_ONCE(*pp);
+
+ if (!p)
+ return NULL;
+ refcount_inc(&p->cnt);
+ return p;
+}
+
+noinline void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb)
+{
+}
+
+noinline void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_fail1(struct prog_test_fail1 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_fail2(struct prog_test_fail2 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_mem_len_pass1(void *mem, int mem__sz)
+{
+}
+
+noinline void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len)
+{
+}
+
+noinline void bpf_kfunc_call_test_mem_len_fail2(u64 *mem, int len)
+{
+}
+
+noinline void bpf_kfunc_call_test_ref(struct prog_test_ref_kfunc *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_destructive(void)
+{
+}
+
BTF_SET8_START(bpf_testmod_check_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_memb_acquire, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_memb1_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_get_rdwr_mem, KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_get_rdonly_mem, KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_acq_rdonly_mem, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_int_mem_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_kptr_get, KF_ACQUIRE | KF_RET_NULL | KF_KPTR_GET)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass_ctx)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail3)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
BTF_SET8_END(bpf_testmod_check_kfunc_ids)
@@ -229,13 +404,34 @@ static const struct btf_kfunc_id_set bpf_testmod_kfunc_set = {
.set = &bpf_testmod_check_kfunc_ids,
};
+BTF_ID_LIST(bpf_prog_test_dtor_kfunc_ids)
+BTF_ID(struct, prog_test_ref_kfunc)
+BTF_ID(func, bpf_kfunc_call_test_release)
+BTF_ID(struct, prog_test_member)
+BTF_ID(func, bpf_kfunc_call_memb_release)
+
extern int bpf_fentry_test1(int a);
-static int bpf_testmod_init(void)
+static int __init bpf_testmod_init(void)
{
+ const struct btf_id_dtor_kfunc bpf_prog_test_dtor_kfunc[] = {
+ {
+ .btf_id = bpf_prog_test_dtor_kfunc_ids[0],
+ .kfunc_btf_id = bpf_prog_test_dtor_kfunc_ids[1]
+ },
+ {
+ .btf_id = bpf_prog_test_dtor_kfunc_ids[2],
+ .kfunc_btf_id = bpf_prog_test_dtor_kfunc_ids[3],
+ },
+ };
int ret;
ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_testmod_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
+ ret = ret ?: register_btf_id_dtor_kfuncs(bpf_prog_test_dtor_kfunc,
+ ARRAY_SIZE(bpf_prog_test_dtor_kfunc),
+ THIS_MODULE);
if (ret < 0)
return ret;
if (bpf_fentry_test1(0) < 0)
--
2.39.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h
2023-01-30 8:55 ` [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h Jiri Olsa
@ 2023-01-30 15:15 ` David Vernet
2023-01-30 23:16 ` Jiri Olsa
0 siblings, 1 reply; 14+ messages in thread
From: David Vernet @ 2023-01-30 15:15 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Kumar Kartikeya Dwivedi,
Artem Savkov
On Mon, Jan 30, 2023 at 09:55:34AM +0100, Jiri Olsa wrote:
> Move all kfunc exports into separate header file and include it
> in tests that need it.
>
> We will move all test kfuncs into bpf_testmod in following change,
> so it's convenient to have declarations in single place.
Nice, good call.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../bpf/bpf_testmod/bpf_testmod_kfunc.h | 92 +++++++++++++++++++
> tools/testing/selftests/bpf/progs/cb_refs.c | 4 +-
> .../selftests/bpf/progs/jit_probe_mem.c | 4 +-
> .../bpf/progs/kfunc_call_destructive.c | 3 +-
> .../selftests/bpf/progs/kfunc_call_fail.c | 9 +-
> .../selftests/bpf/progs/kfunc_call_race.c | 3 +-
> .../selftests/bpf/progs/kfunc_call_test.c | 16 +---
> .../bpf/progs/kfunc_call_test_subprog.c | 17 +++-
> tools/testing/selftests/bpf/progs/map_kptr.c | 6 +-
> .../selftests/bpf/progs/map_kptr_fail.c | 5 +-
> 10 files changed, 114 insertions(+), 45 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> new file mode 100644
> index 000000000000..164cbae2b0d7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
Should we update the selftests Makefile to rebuild progs when the testmod
changes? Something like:
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index e79039726173..ed0fb32aa855 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -438,6 +438,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \
$(TRUNNER_BPF_PROGS_DIR)/%.c \
$(TRUNNER_BPF_PROGS_DIR)/*.h \
$$(INCLUDE_DIR)/vmlinux.h \
+ $(OUTPUT)/bpf_testmod.ko \
$(wildcard $(BPFDIR)/bpf_*.h) \
$(wildcard $(BPFDIR)/*.bpf.h) \
| $(TRUNNER_OUTPUT) $$(BPFOBJ)
> @@ -0,0 +1,92 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _BPF_TESTMOD_KFUNC_H
> +#define _BPF_TESTMOD_KFUNC_H
> +
> +#ifndef __ksym
> +#define __ksym __attribute__((section(".ksyms")))
> +#endif
What about doing something like this:
#ifndef __KERNEL__
#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
#else
#define __ksym
#endif
Thoughts?
> +
> +struct prog_test_pass1 {
> + int x0;
> + struct {
> + int x1;
> + struct {
> + int x2;
> + struct {
> + int x3;
> + };
> + };
> + };
> +};
> +
> +struct prog_test_pass2 {
> + int len;
> + short arr1[4];
> + struct {
> + char arr2[4];
> + unsigned long arr3[8];
> + } x;
> +};
> +
> +struct prog_test_fail1 {
> + void *p;
> + int x;
> +};
> +
> +struct prog_test_fail2 {
> + int x8;
> + struct prog_test_pass1 x;
> +};
> +
> +struct prog_test_fail3 {
> + int len;
> + char arr1[2];
> + char arr2[];
> +};
> +
> +struct prog_test_member1 {
> + int a;
> +};
> +
> +struct prog_test_member {
> + struct prog_test_member1 m;
> + int c;
> +};
> +
> +struct prog_test_ref_kfunc {
> + int a;
> + int b;
> + struct prog_test_member memb;
> + struct prog_test_ref_kfunc *next;
> + refcount_t cnt;
> +};
> +
> +extern struct prog_test_ref_kfunc *
> +bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
> +extern struct prog_test_ref_kfunc *
> +bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
> +extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> +
> +extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
> +extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
> +extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> +extern int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> +extern void bpf_kfunc_call_int_mem_release(int *p) __ksym;
> +
> +extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
> +
> +extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
> + __u32 c, __u64 d) __ksym;
> +extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
> +extern struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
> +extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
> +
> +extern void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
> +extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
> +extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
> +extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
> +
> +extern void bpf_kfunc_call_test_destructive(void) __ksym;
nit: Can we remove extern from all of these function signatures? Doesn't
really matter much to leave it there, but given that the keyword does
nothing for functions it feels unnecessary / noisy.
> +
> +#endif /* _BPF_TESTMOD_KFUNC_H */
> diff --git a/tools/testing/selftests/bpf/progs/cb_refs.c b/tools/testing/selftests/bpf/progs/cb_refs.c
> index 7653df1bc787..823901c1b839 100644
> --- a/tools/testing/selftests/bpf/progs/cb_refs.c
> +++ b/tools/testing/selftests/bpf/progs/cb_refs.c
> @@ -2,6 +2,7 @@
> #include <vmlinux.h>
> #include <bpf/bpf_tracing.h>
> #include <bpf/bpf_helpers.h>
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> struct map_value {
> struct prog_test_ref_kfunc __kptr_ref *ptr;
> @@ -14,9 +15,6 @@ struct {
> __uint(max_entries, 16);
> } array_map SEC(".maps");
>
> -extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
> -extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> -
> static __noinline int cb1(void *map, void *key, void *value, void *ctx)
> {
> void *p = *(void **)ctx;
> diff --git a/tools/testing/selftests/bpf/progs/jit_probe_mem.c b/tools/testing/selftests/bpf/progs/jit_probe_mem.c
> index 2d2e61470794..96207f126054 100644
> --- a/tools/testing/selftests/bpf/progs/jit_probe_mem.c
> +++ b/tools/testing/selftests/bpf/progs/jit_probe_mem.c
> @@ -3,13 +3,11 @@
> #include <vmlinux.h>
> #include <bpf/bpf_tracing.h>
> #include <bpf/bpf_helpers.h>
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> static struct prog_test_ref_kfunc __kptr_ref *v;
> long total_sum = -1;
>
> -extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
> -extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> -
> SEC("tc")
> int test_jit_probe_mem(struct __sk_buff *ctx)
> {
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
> index 767472bc5a97..6a9b13a79ae8 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_destructive.c
> @@ -1,8 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <vmlinux.h>
> #include <bpf/bpf_helpers.h>
> -
> -extern void bpf_kfunc_call_test_destructive(void) __ksym;
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> SEC("tc")
> int kfunc_destructive_test(void)
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_fail.c b/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
> index b98313d391c6..e857d1c4cf5b 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_fail.c
> @@ -2,14 +2,7 @@
> /* Copyright (c) 2021 Facebook */
> #include <vmlinux.h>
> #include <bpf/bpf_helpers.h>
> -
> -extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
> -extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> -extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
> -extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
> -extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> -extern int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> -extern void bpf_kfunc_call_int_mem_release(int *p) __ksym;
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> struct syscall_test_args {
> __u8 data[16];
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_race.c b/tools/testing/selftests/bpf/progs/kfunc_call_race.c
> index 4e8fed75a4e0..a9558e434611 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_race.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_race.c
> @@ -1,8 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <vmlinux.h>
> #include <bpf/bpf_helpers.h>
> -
> -extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> SEC("tc")
> int kfunc_call_fail(struct __sk_buff *ctx)
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> index d91c58d06d38..d2fe17e2b433 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
> @@ -2,21 +2,7 @@
> /* Copyright (c) 2021 Facebook */
> #include <vmlinux.h>
> #include <bpf/bpf_helpers.h>
> -
> -extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
> -extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
> -extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
> - __u32 c, __u64 d) __ksym;
> -
> -extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
> -extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> -extern void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
> -extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
> -extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
> -extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
> -extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
> -extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
> -extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> SEC("tc")
> int kfunc_call_test4(struct __sk_buff *skb)
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> index c1fdecabeabf..f74c78bb5efd 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> @@ -4,10 +4,21 @@
> #include <bpf/bpf_helpers.h>
> #include "bpf_tcp_helpers.h"
>
> +/*
> + * We can't include vmlinux.h, because it conflicts with bpf_tcp_helpers.h,
> + * but we need refcount_t typedef for bpf_testmod_kfunc.h.
> + * Adding it directly.
> + */
> +typedef struct {
> + int counter;
> +} atomic_t;
> +typedef struct refcount_struct {
> + atomic_t refs;
> +} refcount_t;
Meh, this is kind of unfortunate, but OK, not the end of the world.
Don't really see an easy way to resolve these types of typedef / include
spaghetti issues in a general way.
As an alternative, it looks like this also works:
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
index f74c78bb5efd..7b3472ebc445 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
@@ -1,21 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2021 Facebook */
-#include <linux/bpf.h>
+#include <linux/types.h>
#include <bpf/bpf_helpers.h>
-#include "bpf_tcp_helpers.h"
-
-/*
- * We can't include vmlinux.h, because it conflicts with bpf_tcp_helpers.h,
- * but we need refcount_t typedef for bpf_testmod_kfunc.h.
- * Adding it directly.
- */
-typedef struct {
- int counter;
-} atomic_t;
-typedef struct refcount_struct {
- atomic_t refs;
-} refcount_t;
-
+#include <vmlinux.h>
#include "bpf_testmod/bpf_testmod_kfunc.h"
extern const int bpf_prog_active __ksym;
@@ -39,7 +26,7 @@ int __noinline f1(struct __sk_buff *skb)
if (active)
active_res = *active;
- sk_state_res = bpf_kfunc_call_test3((struct sock *)sk)->sk_state;
+ sk_state_res = bpf_kfunc_call_test3((struct sock *)sk)->__sk_common.skc_state;
return (__u32)bpf_kfunc_call_test1((struct sock *)sk, 1, 2, 3, 4);
}
Thoughts?
> +
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
> +
> extern const int bpf_prog_active __ksym;
> -extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
> - __u32 c, __u64 d) __ksym;
> -extern struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
> int active_res = -1;
> int sk_state_res = -1;
>
> diff --git a/tools/testing/selftests/bpf/progs/map_kptr.c b/tools/testing/selftests/bpf/progs/map_kptr.c
> index eb8217803493..d53474f5b05b 100644
> --- a/tools/testing/selftests/bpf/progs/map_kptr.c
> +++ b/tools/testing/selftests/bpf/progs/map_kptr.c
> @@ -2,6 +2,7 @@
> #include <vmlinux.h>
> #include <bpf/bpf_tracing.h>
> #include <bpf/bpf_helpers.h>
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> struct map_value {
> struct prog_test_ref_kfunc __kptr *unref_ptr;
> @@ -57,11 +58,6 @@ DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, hash_map, hash_of_hash_maps);
> DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, hash_malloc_map, hash_of_hash_malloc_maps);
> DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, lru_hash_map, hash_of_lru_hash_maps);
>
> -extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
> -extern struct prog_test_ref_kfunc *
> -bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
> -extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> -
> static void test_kptr_unref(struct map_value *v)
> {
> struct prog_test_ref_kfunc *p;
> diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> index 760e41e1a632..1358a7c9e399 100644
> --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> @@ -4,6 +4,7 @@
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_core_read.h>
> #include "bpf_misc.h"
> +#include "bpf_testmod/bpf_testmod_kfunc.h"
>
> struct map_value {
> char buf[8];
> @@ -19,10 +20,6 @@ struct array_map {
> __uint(max_entries, 1);
> } array_map SEC(".maps");
>
> -extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
> -extern struct prog_test_ref_kfunc *
> -bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
> -
> SEC("?tc")
> __failure __msg("kptr access size must be BPF_DW")
> int size_not_bpf_dw(struct __sk_buff *ctx)
> --
> 2.39.1
>
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object
2023-01-30 8:55 ` [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object Jiri Olsa
@ 2023-01-30 15:23 ` David Vernet
2023-01-30 23:16 ` Jiri Olsa
0 siblings, 1 reply; 14+ messages in thread
From: David Vernet @ 2023-01-30 15:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Kumar Kartikeya Dwivedi,
Artem Savkov
On Mon, Jan 30, 2023 at 09:55:35AM +0100, Jiri Olsa wrote:
> Moving test_progs helpers to testing_helpers object so they can be
> used from test_verifier in following changes.
>
> Also adding missing ifndef header guard to testing_helpers.h header.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/test_progs.c | 67 +------------------
> tools/testing/selftests/bpf/test_progs.h | 1 -
> tools/testing/selftests/bpf/testing_helpers.c | 63 +++++++++++++++++
> tools/testing/selftests/bpf/testing_helpers.h | 10 +++
> 4 files changed, 75 insertions(+), 66 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 6d5e3022c75f..a150c35516ef 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -11,7 +11,6 @@
> #include <signal.h>
> #include <string.h>
> #include <execinfo.h> /* backtrace */
> -#include <linux/membarrier.h>
> #include <sys/sysinfo.h> /* get_nprocs */
> #include <netinet/in.h>
> #include <sys/select.h>
> @@ -616,68 +615,6 @@ int extract_build_id(char *build_id, size_t size)
> return -1;
> }
>
> -static int finit_module(int fd, const char *param_values, int flags)
> -{
> - return syscall(__NR_finit_module, fd, param_values, flags);
> -}
> -
> -static int delete_module(const char *name, int flags)
> -{
> - return syscall(__NR_delete_module, name, flags);
> -}
> -
> -/*
> - * Trigger synchronize_rcu() in kernel.
> - */
> -int kern_sync_rcu(void)
> -{
> - return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0);
> -}
> -
> -static void unload_bpf_testmod(void)
> -{
> - if (kern_sync_rcu())
> - fprintf(env.stderr, "Failed to trigger kernel-side RCU sync!\n");
> - if (delete_module("bpf_testmod", 0)) {
> - if (errno == ENOENT) {
> - if (verbose())
> - fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
> - return;
> - }
> - fprintf(env.stderr, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
> - return;
> - }
> - if (verbose())
> - fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
> -}
> -
> -static int load_bpf_testmod(void)
> -{
> - int fd;
> -
> - /* ensure previous instance of the module is unloaded */
> - unload_bpf_testmod();
> -
> - if (verbose())
> - fprintf(stdout, "Loading bpf_testmod.ko...\n");
> -
> - fd = open("bpf_testmod.ko", O_RDONLY);
> - if (fd < 0) {
> - fprintf(env.stderr, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
> - return -ENOENT;
> - }
> - if (finit_module(fd, "", 0)) {
> - fprintf(env.stderr, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
> - close(fd);
> - return -EINVAL;
> - }
> - close(fd);
> -
> - if (verbose())
> - fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
> - return 0;
> -}
> -
> /* extern declarations for test funcs */
> #define DEFINE_TEST(name) \
> extern void test_##name(void) __weak; \
> @@ -1655,7 +1592,7 @@ int main(int argc, char **argv)
> env.stderr = stderr;
>
> env.has_testmod = true;
> - if (!env.list_test_names && load_bpf_testmod()) {
> + if (!env.list_test_names && load_bpf_testmod(env.stderr, verbose())) {
> fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
> env.has_testmod = false;
> }
> @@ -1754,7 +1691,7 @@ int main(int argc, char **argv)
> close(env.saved_netns_fd);
> out:
> if (!env.list_test_names && env.has_testmod)
> - unload_bpf_testmod();
> + unload_bpf_testmod(env.stderr, verbose());
>
> free_test_selector(&env.test_selector);
> free_test_selector(&env.subtest_selector);
> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> index d5d51ec97ec8..b9dac3c32712 100644
> --- a/tools/testing/selftests/bpf/test_progs.h
> +++ b/tools/testing/selftests/bpf/test_progs.h
> @@ -390,7 +390,6 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name);
> int compare_map_keys(int map1_fd, int map2_fd);
> int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len);
> int extract_build_id(char *build_id, size_t size);
> -int kern_sync_rcu(void);
> int trigger_module_test_read(int read_sz);
> int trigger_module_test_write(int write_sz);
> int write_sysctl(const char *sysctl, const char *value);
> diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> index 9695318e8132..c0eb54bf08b3 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.c
> +++ b/tools/testing/selftests/bpf/testing_helpers.c
> @@ -8,6 +8,7 @@
> #include <bpf/libbpf.h>
> #include "test_progs.h"
> #include "testing_helpers.h"
> +#include <linux/membarrier.h>
>
> int parse_num_list(const char *s, bool **num_set, int *num_set_len)
> {
> @@ -229,3 +230,65 @@ int bpf_test_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
>
> return bpf_prog_load(type, NULL, license, insns, insns_cnt, &opts);
> }
> +
> +static int finit_module(int fd, const char *param_values, int flags)
> +{
> + return syscall(__NR_finit_module, fd, param_values, flags);
> +}
> +
> +static int delete_module(const char *name, int flags)
> +{
> + return syscall(__NR_delete_module, name, flags);
> +}
> +
> +void unload_bpf_testmod(FILE *err, bool verbose)
Maybe you should pass a const struct test_env * here and in
load_bpf_testmod() instead? Technically it also has a FILE *stdout, so
to be consistent we should probably also pass that to the fprintf()
calls on the success path.
> +{
> + if (kern_sync_rcu())
> + fprintf(err, "Failed to trigger kernel-side RCU sync!\n");
> + if (delete_module("bpf_testmod", 0)) {
> + if (errno == ENOENT) {
> + if (verbose)
> + fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
> + return;
> + }
> + fprintf(err, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
> + return;
> + }
> + if (verbose)
> + fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
> +}
> +
> +int load_bpf_testmod(FILE *err, bool verbose)
> +{
> + int fd;
> +
> + /* ensure previous instance of the module is unloaded */
> + unload_bpf_testmod(err, verbose);
> +
> + if (verbose)
> + fprintf(stdout, "Loading bpf_testmod.ko...\n");
> +
> + fd = open("bpf_testmod.ko", O_RDONLY);
> + if (fd < 0) {
> + fprintf(err, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
> + return -ENOENT;
> + }
> + if (finit_module(fd, "", 0)) {
> + fprintf(err, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
> + close(fd);
> + return -EINVAL;
> + }
> + close(fd);
> +
> + if (verbose)
> + fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
> + return 0;
> +}
> +
> +/*
> + * Trigger synchronize_rcu() in kernel.
> + */
> +int kern_sync_rcu(void)
> +{
> + return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0);
> +}
> diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
> index 6ec00bf79cb5..2f80ca5b5f54 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.h
> +++ b/tools/testing/selftests/bpf/testing_helpers.h
> @@ -1,5 +1,9 @@
> /* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> /* Copyright (C) 2020 Facebook, Inc. */
> +
> +#ifndef __TRACING_HELPERS_H
> +#define __TRACING_HELPERS_H
s/__TRACING/__TESTING here and below
> +
> #include <stdbool.h>
> #include <bpf/bpf.h>
> #include <bpf/libbpf.h>
> @@ -20,3 +24,9 @@ struct test_filter_set;
> int parse_test_list(const char *s,
> struct test_filter_set *test_set,
> bool is_glob_pattern);
> +
> +int load_bpf_testmod(FILE *err, bool verbose);
> +void unload_bpf_testmod(FILE *err, bool verbose);
> +int kern_sync_rcu(void);
> +
> +#endif /* __TRACING_HELPERS_H */
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 bpf-next 3/7] selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod
2023-01-30 8:55 ` [PATCHv2 bpf-next 3/7] selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod Jiri Olsa
@ 2023-01-30 15:28 ` David Vernet
0 siblings, 0 replies; 14+ messages in thread
From: David Vernet @ 2023-01-30 15:28 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Kumar Kartikeya Dwivedi,
Artem Savkov
On Mon, Jan 30, 2023 at 09:55:36AM +0100, Jiri Olsa wrote:
> Do not unload bpf_testmod in load_bpf_testmod, instead call
> unload_bpf_testmod separatelly.
>
> This way we will be able use un/load_bpf_testmod functions
> in other tests that un/load bpf_testmod module.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/test_progs.c | 11 ++++++++---
> tools/testing/selftests/bpf/testing_helpers.c | 3 ---
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index a150c35516ef..9ca718c84890 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1592,9 +1592,14 @@ int main(int argc, char **argv)
> env.stderr = stderr;
>
> env.has_testmod = true;
> - if (!env.list_test_names && load_bpf_testmod(env.stderr, verbose())) {
> - fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
> - env.has_testmod = false;
> + if (!env.list_test_names) {
> + /* ensure previous instance of the module is unloaded */
> + unload_bpf_testmod(env.stderr, verbose());
> +
> + if (load_bpf_testmod(env.stderr, verbose())) {
> + fprintf(env.stderr, "WARNING! Selftests relying on bpf_testmod.ko will be skipped.\n");
> + env.has_testmod = false;
> + }
> }
>
> /* initializing tests */
> diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> index c0eb54bf08b3..ade6208b4a69 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.c
> +++ b/tools/testing/selftests/bpf/testing_helpers.c
> @@ -262,9 +262,6 @@ int load_bpf_testmod(FILE *err, bool verbose)
> {
> int fd;
>
> - /* ensure previous instance of the module is unloaded */
> - unload_bpf_testmod(err, verbose);
> -
> if (verbose)
> fprintf(stdout, "Loading bpf_testmod.ko...\n");
>
> --
> 2.39.1
>
Acked-by: David Vernet <void@manifault.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 bpf-next 4/7] selftests/bpf: Use un/load_bpf_testmod functions in tests
2023-01-30 8:55 ` [PATCHv2 bpf-next 4/7] selftests/bpf: Use un/load_bpf_testmod functions in tests Jiri Olsa
@ 2023-01-30 15:32 ` David Vernet
0 siblings, 0 replies; 14+ messages in thread
From: David Vernet @ 2023-01-30 15:32 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Kumar Kartikeya Dwivedi,
Artem Savkov
On Mon, Jan 30, 2023 at 09:55:37AM +0100, Jiri Olsa wrote:
> Now that we have un/load_bpf_testmod helpers in testing_helpers.h,
> we can use it in other tests and save some lines.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../selftests/bpf/prog_tests/bpf_mod_race.c | 34 +++----------------
> .../selftests/bpf/prog_tests/module_attach.c | 12 +++----
> tools/testing/selftests/bpf/testing_helpers.c | 7 ++--
> tools/testing/selftests/bpf/testing_helpers.h | 2 +-
> 4 files changed, 14 insertions(+), 41 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c b/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
> index a4d0cc9d3367..40c5b3b5ff78 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_mod_race.c
> @@ -11,6 +11,7 @@
> #include "ksym_race.skel.h"
> #include "bpf_mod_race.skel.h"
> #include "kfunc_call_race.skel.h"
> +#include "testing_helpers.h"
>
> /* This test crafts a race between btf_try_get_module and do_init_module, and
> * checks whether btf_try_get_module handles the invocation for a well-formed
> @@ -44,35 +45,10 @@ enum bpf_test_state {
>
> static _Atomic enum bpf_test_state state = _TS_INVALID;
>
> -static int sys_finit_module(int fd, const char *param_values, int flags)
> -{
> - return syscall(__NR_finit_module, fd, param_values, flags);
> -}
> -
> -static int sys_delete_module(const char *name, unsigned int flags)
> -{
> - return syscall(__NR_delete_module, name, flags);
> -}
> -
> -static int load_module(const char *mod)
> -{
> - int ret, fd;
> -
> - fd = open("bpf_testmod.ko", O_RDONLY);
> - if (fd < 0)
> - return fd;
> -
> - ret = sys_finit_module(fd, "", 0);
> - close(fd);
> - if (ret < 0)
> - return ret;
> - return 0;
> -}
> -
> static void *load_module_thread(void *p)
> {
>
> - if (!ASSERT_NEQ(load_module("bpf_testmod.ko"), 0, "load_module_thread must fail"))
> + if (!ASSERT_NEQ(load_bpf_testmod(stderr, false), 0, "load_module_thread must fail"))
> atomic_store(&state, TS_MODULE_LOAD);
> else
> atomic_store(&state, TS_MODULE_LOAD_FAIL);
> @@ -124,7 +100,7 @@ static void test_bpf_mod_race_config(const struct test_config *config)
> if (!ASSERT_NEQ(fault_addr, MAP_FAILED, "mmap for uffd registration"))
> return;
>
> - if (!ASSERT_OK(sys_delete_module("bpf_testmod", 0), "unload bpf_testmod"))
> + if (!ASSERT_OK(unload_bpf_testmod(stderr, false), "unload bpf_testmod"))
> goto end_mmap;
>
> skel = bpf_mod_race__open();
> @@ -202,8 +178,8 @@ static void test_bpf_mod_race_config(const struct test_config *config)
> bpf_mod_race__destroy(skel);
> ASSERT_OK(kern_sync_rcu(), "kern_sync_rcu");
> end_module:
> - sys_delete_module("bpf_testmod", 0);
> - ASSERT_OK(load_module("bpf_testmod.ko"), "restore bpf_testmod");
> + unload_bpf_testmod(stderr, false);
> + ASSERT_OK(load_bpf_testmod(stderr, false), "restore bpf_testmod");
> end_mmap:
> munmap(fault_addr, 4096);
> atomic_store(&state, _TS_INVALID);
> diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> index 7fc01ff490db..54dee902a30a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> @@ -4,6 +4,7 @@
> #include <test_progs.h>
> #include <stdbool.h>
> #include "test_module_attach.skel.h"
> +#include "testing_helpers.h"
>
> static int duration;
>
> @@ -32,11 +33,6 @@ static int trigger_module_test_writable(int *val)
> return 0;
> }
>
> -static int delete_module(const char *name, int flags)
> -{
> - return syscall(__NR_delete_module, name, flags);
> -}
> -
> void test_module_attach(void)
> {
> const int READ_SZ = 456;
> @@ -93,21 +89,21 @@ void test_module_attach(void)
> if (!ASSERT_OK_PTR(link, "attach_fentry"))
> goto cleanup;
>
> - ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> + ASSERT_ERR(unload_bpf_testmod(stderr, false), "unload_bpf_testmod");
> bpf_link__destroy(link);
>
> link = bpf_program__attach(skel->progs.handle_fexit);
> if (!ASSERT_OK_PTR(link, "attach_fexit"))
> goto cleanup;
>
> - ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> + ASSERT_ERR(unload_bpf_testmod(stderr, false), "unload_bpf_testmod");
> bpf_link__destroy(link);
>
> link = bpf_program__attach(skel->progs.kprobe_multi);
> if (!ASSERT_OK_PTR(link, "attach_kprobe_multi"))
> goto cleanup;
>
> - ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> + ASSERT_ERR(unload_bpf_testmod(stderr, false), "unload_bpf_testmod");
> bpf_link__destroy(link);
>
> cleanup:
> diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> index ade6208b4a69..c8326d2355da 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.c
> +++ b/tools/testing/selftests/bpf/testing_helpers.c
> @@ -241,7 +241,7 @@ static int delete_module(const char *name, int flags)
> return syscall(__NR_delete_module, name, flags);
> }
>
> -void unload_bpf_testmod(FILE *err, bool verbose)
> +int unload_bpf_testmod(FILE *err, bool verbose)
> {
> if (kern_sync_rcu())
> fprintf(err, "Failed to trigger kernel-side RCU sync!\n");
> @@ -249,13 +249,14 @@ void unload_bpf_testmod(FILE *err, bool verbose)
> if (errno == ENOENT) {
> if (verbose)
> fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
> - return;
> + return -1;
> }
> fprintf(err, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
> - return;
> + return -1;
> }
> if (verbose)
> fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
> + return 0;
> }
>
> int load_bpf_testmod(FILE *err, bool verbose)
> diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
> index 2f80ca5b5f54..dd725c02b31f 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.h
> +++ b/tools/testing/selftests/bpf/testing_helpers.h
> @@ -26,7 +26,7 @@ int parse_test_list(const char *s,
> bool is_glob_pattern);
>
> int load_bpf_testmod(FILE *err, bool verbose);
> -void unload_bpf_testmod(FILE *err, bool verbose);
> +int unload_bpf_testmod(FILE *err, bool verbose);
> int kern_sync_rcu(void);
>
> #endif /* __TRACING_HELPERS_H */
> --
> 2.39.1
>
LGTM, nice cleanup.
Acked-by: David Vernet <void@manifault.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h
2023-01-30 15:15 ` David Vernet
@ 2023-01-30 23:16 ` Jiri Olsa
0 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 23:16 UTC (permalink / raw)
To: David Vernet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Kumar Kartikeya Dwivedi,
Artem Savkov
On Mon, Jan 30, 2023 at 09:15:26AM -0600, David Vernet wrote:
> On Mon, Jan 30, 2023 at 09:55:34AM +0100, Jiri Olsa wrote:
> > Move all kfunc exports into separate header file and include it
> > in tests that need it.
> >
> > We will move all test kfuncs into bpf_testmod in following change,
> > so it's convenient to have declarations in single place.
>
> Nice, good call.
>
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > .../bpf/bpf_testmod/bpf_testmod_kfunc.h | 92 +++++++++++++++++++
> > tools/testing/selftests/bpf/progs/cb_refs.c | 4 +-
> > .../selftests/bpf/progs/jit_probe_mem.c | 4 +-
> > .../bpf/progs/kfunc_call_destructive.c | 3 +-
> > .../selftests/bpf/progs/kfunc_call_fail.c | 9 +-
> > .../selftests/bpf/progs/kfunc_call_race.c | 3 +-
> > .../selftests/bpf/progs/kfunc_call_test.c | 16 +---
> > .../bpf/progs/kfunc_call_test_subprog.c | 17 +++-
> > tools/testing/selftests/bpf/progs/map_kptr.c | 6 +-
> > .../selftests/bpf/progs/map_kptr_fail.c | 5 +-
> > 10 files changed, 114 insertions(+), 45 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > new file mode 100644
> > index 000000000000..164cbae2b0d7
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
>
> Should we update the selftests Makefile to rebuild progs when the testmod
> changes? Something like:
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index e79039726173..ed0fb32aa855 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -438,6 +438,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \
> $(TRUNNER_BPF_PROGS_DIR)/%.c \
> $(TRUNNER_BPF_PROGS_DIR)/*.h \
> $$(INCLUDE_DIR)/vmlinux.h \
> + $(OUTPUT)/bpf_testmod.ko \
> $(wildcard $(BPFDIR)/bpf_*.h) \
> $(wildcard $(BPFDIR)/*.bpf.h) \
> | $(TRUNNER_OUTPUT) $$(BPFOBJ)
ok, looks good will add it
>
> > @@ -0,0 +1,92 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef _BPF_TESTMOD_KFUNC_H
> > +#define _BPF_TESTMOD_KFUNC_H
> > +
> > +#ifndef __ksym
> > +#define __ksym __attribute__((section(".ksyms")))
> > +#endif
>
> What about doing something like this:
>
> #ifndef __KERNEL__
> #include <vmlinux.h>
> #include <bpf/bpf_helpers.h>
> #else
> #define __ksym
> #endif
>
> Thoughts?
that goes nicely along with the solution for extra typedef you suggested below
SNIP
> > +extern struct prog_test_ref_kfunc *
> > +bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
> > +extern struct prog_test_ref_kfunc *
> > +bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
> > +extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> > +
> > +extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
> > +extern int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size) __ksym;
> > +extern int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> > +extern int *bpf_kfunc_call_test_acq_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size) __ksym;
> > +extern void bpf_kfunc_call_int_mem_release(int *p) __ksym;
> > +
> > +extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
> > +
> > +extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
> > + __u32 c, __u64 d) __ksym;
> > +extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
> > +extern struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
> > +extern long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
> > +
> > +extern void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
> > +extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
> > +extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
> > +extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
> > +
> > +extern void bpf_kfunc_call_test_destructive(void) __ksym;
>
> nit: Can we remove extern from all of these function signatures? Doesn't
> really matter much to leave it there, but given that the keyword does
> nothing for functions it feels unnecessary / noisy.
np, I can remove it
SNIP
> > diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> > index c1fdecabeabf..f74c78bb5efd 100644
> > --- a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> > +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> > @@ -4,10 +4,21 @@
> > #include <bpf/bpf_helpers.h>
> > #include "bpf_tcp_helpers.h"
> >
> > +/*
> > + * We can't include vmlinux.h, because it conflicts with bpf_tcp_helpers.h,
> > + * but we need refcount_t typedef for bpf_testmod_kfunc.h.
> > + * Adding it directly.
> > + */
> > +typedef struct {
> > + int counter;
> > +} atomic_t;
> > +typedef struct refcount_struct {
> > + atomic_t refs;
> > +} refcount_t;
>
> Meh, this is kind of unfortunate, but OK, not the end of the world.
> Don't really see an easy way to resolve these types of typedef / include
> spaghetti issues in a general way.
>
> As an alternative, it looks like this also works:
>
> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> index f74c78bb5efd..7b3472ebc445 100644
> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test_subprog.c
> @@ -1,21 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright (c) 2021 Facebook */
> -#include <linux/bpf.h>
> +#include <linux/types.h>
> #include <bpf/bpf_helpers.h>
> -#include "bpf_tcp_helpers.h"
> -
> -/*
> - * We can't include vmlinux.h, because it conflicts with bpf_tcp_helpers.h,
> - * but we need refcount_t typedef for bpf_testmod_kfunc.h.
> - * Adding it directly.
> - */
> -typedef struct {
> - int counter;
> -} atomic_t;
> -typedef struct refcount_struct {
> - atomic_t refs;
> -} refcount_t;
> -
> +#include <vmlinux.h>
> #include "bpf_testmod/bpf_testmod_kfunc.h"
>
> extern const int bpf_prog_active __ksym;
> @@ -39,7 +26,7 @@ int __noinline f1(struct __sk_buff *skb)
> if (active)
> active_res = *active;
>
> - sk_state_res = bpf_kfunc_call_test3((struct sock *)sk)->sk_state;
> + sk_state_res = bpf_kfunc_call_test3((struct sock *)sk)->__sk_common.skc_state;
great, I was wondering why the sock type was different when I was trying similar fix,
that looks much better, will try that
thanks,
jirka
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object
2023-01-30 15:23 ` David Vernet
@ 2023-01-30 23:16 ` Jiri Olsa
0 siblings, 0 replies; 14+ messages in thread
From: Jiri Olsa @ 2023-01-30 23:16 UTC (permalink / raw)
To: David Vernet
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Kumar Kartikeya Dwivedi,
Artem Savkov
On Mon, Jan 30, 2023 at 09:23:16AM -0600, David Vernet wrote:
SNIP
> > diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> > index 9695318e8132..c0eb54bf08b3 100644
> > --- a/tools/testing/selftests/bpf/testing_helpers.c
> > +++ b/tools/testing/selftests/bpf/testing_helpers.c
> > @@ -8,6 +8,7 @@
> > #include <bpf/libbpf.h>
> > #include "test_progs.h"
> > #include "testing_helpers.h"
> > +#include <linux/membarrier.h>
> >
> > int parse_num_list(const char *s, bool **num_set, int *num_set_len)
> > {
> > @@ -229,3 +230,65 @@ int bpf_test_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
> >
> > return bpf_prog_load(type, NULL, license, insns, insns_cnt, &opts);
> > }
> > +
> > +static int finit_module(int fd, const char *param_values, int flags)
> > +{
> > + return syscall(__NR_finit_module, fd, param_values, flags);
> > +}
> > +
> > +static int delete_module(const char *name, int flags)
> > +{
> > + return syscall(__NR_delete_module, name, flags);
> > +}
> > +
> > +void unload_bpf_testmod(FILE *err, bool verbose)
>
> Maybe you should pass a const struct test_env * here and in
> load_bpf_testmod() instead? Technically it also has a FILE *stdout, so
> to be consistent we should probably also pass that to the fprintf()
> calls on the success path.
struct test_env is specific for test_progs and we want to call
un/load_bpf_testmod from test_verifier.. but yes, it looks weird
to pass just 'err' and verbose.. maybe we could pass both out/err
>
> > +{
> > + if (kern_sync_rcu())
> > + fprintf(err, "Failed to trigger kernel-side RCU sync!\n");
> > + if (delete_module("bpf_testmod", 0)) {
> > + if (errno == ENOENT) {
> > + if (verbose)
> > + fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
> > + return;
> > + }
> > + fprintf(err, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
> > + return;
> > + }
> > + if (verbose)
> > + fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
> > +}
> > +
SNIP
> > diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
> > index 6ec00bf79cb5..2f80ca5b5f54 100644
> > --- a/tools/testing/selftests/bpf/testing_helpers.h
> > +++ b/tools/testing/selftests/bpf/testing_helpers.h
> > @@ -1,5 +1,9 @@
> > /* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> > /* Copyright (C) 2020 Facebook, Inc. */
> > +
> > +#ifndef __TRACING_HELPERS_H
> > +#define __TRACING_HELPERS_H
>
> s/__TRACING/__TESTING here and below
right, thanks
jirka
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-01-30 23:17 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30 8:55 [PATCHv2 bpf-next 0/7] bpf: Move kernel test kfuncs into bpf_testmod Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 1/7] selftests/bpf: Move kfunc exports to bpf_testmod/bpf_testmod_kfunc.h Jiri Olsa
2023-01-30 15:15 ` David Vernet
2023-01-30 23:16 ` Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 2/7] selftests/bpf: Move test_progs helpers to testing_helpers object Jiri Olsa
2023-01-30 15:23 ` David Vernet
2023-01-30 23:16 ` Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 3/7] selftests/bpf: Do not unload bpf_testmod in load_bpf_testmod Jiri Olsa
2023-01-30 15:28 ` David Vernet
2023-01-30 8:55 ` [PATCHv2 bpf-next 4/7] selftests/bpf: Use un/load_bpf_testmod functions in tests Jiri Olsa
2023-01-30 15:32 ` David Vernet
2023-01-30 8:55 ` [PATCHv2 bpf-next 5/7] selftests/bpf: Load bpf_testmod for verifier test Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 6/7] selftests/bpf: Allow to use kfunc from testmod.ko in test_verifier Jiri Olsa
2023-01-30 8:55 ` [PATCHv2 bpf-next 7/7] bpf: Move kernel test kfuncs to bpf_testmod Jiri Olsa
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.