On Tue, Feb 7, 2023 at 6:28 AM David Vernet <void@xxxxxxxxxxxxx> wrote: > > On Fri, Feb 03, 2023 at 05:23:28PM +0100, Jiri Olsa wrote: > > Move all kfunc exports into separate bpf_testmod_kfunc.h 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. > > > > The bpf_testmod_kfunc.h is included by both bpf_testmod and bpf > > programs that use test kfuncs. > > > > As suggested by David, the bpf_testmod_kfunc.h includes vmlinux.h > > and bpf/bpf_helpers.h for bpf programs build, so the declarations > > have proper __ksym attribute and we can resolve all the structs. > > > > Note in kfunc_call_test_subprog.c we can no longer use the sk_state > > define from bpf_tcp_helpers.h (because it clashed with vmlinux.h) > > and we need to address __sk_common.skc_state field directly. > > > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx> > > --- > > .../bpf/bpf_testmod/bpf_testmod_kfunc.h | 41 +++++++++++++++++++ > > 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 | 17 +------- > > .../bpf/progs/kfunc_call_test_subprog.c | 9 +--- > > tools/testing/selftests/bpf/progs/map_kptr.c | 6 +-- > > .../selftests/bpf/progs/map_kptr_fail.c | 5 +-- > > 10 files changed, 51 insertions(+), 50 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..86d94257716a > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h > > @@ -0,0 +1,41 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +#ifndef _BPF_TESTMOD_KFUNC_H > > +#define _BPF_TESTMOD_KFUNC_H > > + > > +#ifndef __KERNEL__ > > +#include <vmlinux.h> > > +#include <bpf/bpf_helpers.h> > > +#else > > +#define __ksym > > +#endif > > + > > +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 u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused) __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" > > Feel free to ignore if you disagree, but here and elsewhere, should we > do this: > > #include <bpf_testmod/bpf_testmod_kfunc.h> > > rather than using #include "bpf_testmod/bpf_testmod_kfunc.h". Doesn't > matter much, but IMO it's just slightly more readable to use the <> to > show that we're relying on -I rather than expecting > bpf_testmod/bpf_testmod_kfunc.h to be found at a path relative to the > progs. #include "bpf_misc.h" makes more sense because it really is > located in the progs/ directory. We do <> for headers that are expected to be installed in the system (even if we cheat with -I sometimes). But in this case it's a local header, so using "" makes more sense to me. But shouldn't it be "../bpf_testmod/bpf_testmod_kfunc.h"? > > Either way: > > Acked-by: David Vernet <void@xxxxxxxxxxxxx> > > > [...]