| Message ID | 20210605111034.1810858-17-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | x86/ftrace/bpf: Add batch support for direct/tracing attach | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 6/5/21 4:10 AM, Jiri Olsa wrote: > Adding selftest for fentry multi func test that attaches > to bpf_fentry_test* functions and checks argument values > based on the processed function. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ Should we put this file under selftests/bpf/progs directory? It is included only by bpf programs. > .../bpf/prog_tests/fentry_multi_test.c | 43 +++++++++++++++ > .../selftests/bpf/progs/fentry_multi_test.c | 18 +++++++ > 3 files changed, 113 insertions(+) > create mode 100644 tools/testing/selftests/bpf/multi_check.h > create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > create mode 100644 tools/testing/selftests/bpf/progs/fentry_multi_test.c > > diff --git a/tools/testing/selftests/bpf/multi_check.h b/tools/testing/selftests/bpf/multi_check.h > new file mode 100644 > index 000000000000..36c2a93f9be3 > --- /dev/null > +++ b/tools/testing/selftests/bpf/multi_check.h > @@ -0,0 +1,52 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef __MULTI_CHECK_H > +#define __MULTI_CHECK_H > + > +extern unsigned long long bpf_fentry_test[8]; > + > +static __attribute__((unused)) inline > +void multi_arg_check(unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f, __u64 *test_result) > +{ > + if (ip == bpf_fentry_test[0]) { > + *test_result += (int) a == 1; > + } else if (ip == bpf_fentry_test[1]) { > + *test_result += (int) a == 2 && (__u64) b == 3; > + } else if (ip == bpf_fentry_test[2]) { > + *test_result += (char) a == 4 && (int) b == 5 && (__u64) c == 6; > + } else if (ip == bpf_fentry_test[3]) { > + *test_result += (void *) a == (void *) 7 && (char) b == 8 && (int) c == 9 && (__u64) d == 10; > + } else if (ip == bpf_fentry_test[4]) { > + *test_result += (__u64) a == 11 && (void *) b == (void *) 12 && (short) c == 13 && (int) d == 14 && (__u64) e == 15; > + } else if (ip == bpf_fentry_test[5]) { > + *test_result += (__u64) a == 16 && (void *) b == (void *) 17 && (short) c == 18 && (int) d == 19 && (void *) e == (void *) 20 && (__u64) f == 21; > + } else if (ip == bpf_fentry_test[6]) { > + *test_result += 1; > + } else if (ip == bpf_fentry_test[7]) { > + *test_result += 1; > + } > +} > + > +static __attribute__((unused)) inline > +void multi_ret_check(unsigned long ip, int ret, __u64 *test_result) > +{ > + if (ip == bpf_fentry_test[0]) { > + *test_result += ret == 2; > + } else if (ip == bpf_fentry_test[1]) { > + *test_result += ret == 5; > + } else if (ip == bpf_fentry_test[2]) { > + *test_result += ret == 15; > + } else if (ip == bpf_fentry_test[3]) { > + *test_result += ret == 34; > + } else if (ip == bpf_fentry_test[4]) { > + *test_result += ret == 65; > + } else if (ip == bpf_fentry_test[5]) { > + *test_result += ret == 111; > + } else if (ip == bpf_fentry_test[6]) { > + *test_result += ret == 0; > + } else if (ip == bpf_fentry_test[7]) { > + *test_result += ret == 0; > + } > +} > + > +#endif /* __MULTI_CHECK_H */ > diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > new file mode 100644 > index 000000000000..e4a8089533d6 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <test_progs.h> > +#include "fentry_multi_test.skel.h" > +#include "trace_helpers.h" > + > +void test_fentry_multi_test(void) > +{ > + struct fentry_multi_test *skel = NULL; > + unsigned long long *bpf_fentry_test; > + __u32 duration = 0, retval; > + int err, prog_fd; > + > + skel = fentry_multi_test__open_and_load(); > + if (!ASSERT_OK_PTR(skel, "fentry_multi_skel_load")) > + goto cleanup; > + > + bpf_fentry_test = &skel->bss->bpf_fentry_test[0]; > + ASSERT_OK(kallsyms_find("bpf_fentry_test1", &bpf_fentry_test[0]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test2", &bpf_fentry_test[1]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test3", &bpf_fentry_test[2]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test4", &bpf_fentry_test[3]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test5", &bpf_fentry_test[4]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test6", &bpf_fentry_test[5]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test7", &bpf_fentry_test[6]), "kallsyms_find"); > + ASSERT_OK(kallsyms_find("bpf_fentry_test8", &bpf_fentry_test[7]), "kallsyms_find"); > + > + err = fentry_multi_test__attach(skel); > + if (!ASSERT_OK(err, "fentry_attach")) > + goto cleanup; > + > + prog_fd = bpf_program__fd(skel->progs.test); > + err = bpf_prog_test_run(prog_fd, 1, NULL, 0, > + NULL, NULL, &retval, &duration); > + ASSERT_OK(err, "test_run"); > + ASSERT_EQ(retval, 0, "test_run"); > + > + ASSERT_EQ(skel->bss->test_result, 8, "test_result"); > + > + fentry_multi_test__detach(skel); > + > +cleanup: > + fentry_multi_test__destroy(skel); > +} > diff --git a/tools/testing/selftests/bpf/progs/fentry_multi_test.c b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > new file mode 100644 > index 000000000000..a443fc958e5a > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include "multi_check.h" > + > +char _license[] SEC("license") = "GPL"; > + > +unsigned long long bpf_fentry_test[8]; > + > +__u64 test_result = 0; > + > +SEC("fentry.multi/bpf_fentry_test*") > +int BPF_PROG(test, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f) > +{ > + multi_arg_check(ip, a, b, c, d, e, f, &test_result); > + return 0; > +} >
On Sun, Jun 06, 2021 at 11:06:14PM -0700, Yonghong Song wrote: > > > On 6/5/21 4:10 AM, Jiri Olsa wrote: > > Adding selftest for fentry multi func test that attaches > > to bpf_fentry_test* functions and checks argument values > > based on the processed function. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ > > Should we put this file under selftests/bpf/progs directory? > It is included only by bpf programs. ok jirka
On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote: > > Adding selftest for fentry multi func test that attaches > to bpf_fentry_test* functions and checks argument values > based on the processed function. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ > .../bpf/prog_tests/fentry_multi_test.c | 43 +++++++++++++++ > .../selftests/bpf/progs/fentry_multi_test.c | 18 +++++++ > 3 files changed, 113 insertions(+) > create mode 100644 tools/testing/selftests/bpf/multi_check.h > create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > create mode 100644 tools/testing/selftests/bpf/progs/fentry_multi_test.c > > diff --git a/tools/testing/selftests/bpf/multi_check.h b/tools/testing/selftests/bpf/multi_check.h > new file mode 100644 > index 000000000000..36c2a93f9be3 > --- /dev/null > +++ b/tools/testing/selftests/bpf/multi_check.h we have a proper static linking now, we don't have to use header inclusion hacks, let's do this properly? > @@ -0,0 +1,52 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef __MULTI_CHECK_H > +#define __MULTI_CHECK_H > + > +extern unsigned long long bpf_fentry_test[8]; > + > +static __attribute__((unused)) inline > +void multi_arg_check(unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f, __u64 *test_result) > +{ > + if (ip == bpf_fentry_test[0]) { > + *test_result += (int) a == 1; > + } else if (ip == bpf_fentry_test[1]) { > + *test_result += (int) a == 2 && (__u64) b == 3; > + } else if (ip == bpf_fentry_test[2]) { > + *test_result += (char) a == 4 && (int) b == 5 && (__u64) c == 6; > + } else if (ip == bpf_fentry_test[3]) { > + *test_result += (void *) a == (void *) 7 && (char) b == 8 && (int) c == 9 && (__u64) d == 10; > + } else if (ip == bpf_fentry_test[4]) { > + *test_result += (__u64) a == 11 && (void *) b == (void *) 12 && (short) c == 13 && (int) d == 14 && (__u64) e == 15; > + } else if (ip == bpf_fentry_test[5]) { > + *test_result += (__u64) a == 16 && (void *) b == (void *) 17 && (short) c == 18 && (int) d == 19 && (void *) e == (void *) 20 && (__u64) f == 21; > + } else if (ip == bpf_fentry_test[6]) { > + *test_result += 1; > + } else if (ip == bpf_fentry_test[7]) { > + *test_result += 1; > + } why not use switch? and why the casting? > +} > + [...] > diff --git a/tools/testing/selftests/bpf/progs/fentry_multi_test.c b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > new file mode 100644 > index 000000000000..a443fc958e5a > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include "multi_check.h" > + > +char _license[] SEC("license") = "GPL"; > + > +unsigned long long bpf_fentry_test[8]; > + > +__u64 test_result = 0; > + > +SEC("fentry.multi/bpf_fentry_test*") wait, that's a regexp syntax that libc supports?.. Not .*? We should definitely not provide btf__find_by_pattern_kind() API, I'd like to avoid explaining what flavors of regexps libbpf supports. > +int BPF_PROG(test, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f) > +{ > + multi_arg_check(ip, a, b, c, d, e, f, &test_result); > + return 0; > +} > -- > 2.31.1 >
On Tue, Jun 08, 2021 at 10:40:24PM -0700, Andrii Nakryiko wrote: > On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Adding selftest for fentry multi func test that attaches > > to bpf_fentry_test* functions and checks argument values > > based on the processed function. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ > > .../bpf/prog_tests/fentry_multi_test.c | 43 +++++++++++++++ > > .../selftests/bpf/progs/fentry_multi_test.c | 18 +++++++ > > 3 files changed, 113 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/multi_check.h > > create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > > create mode 100644 tools/testing/selftests/bpf/progs/fentry_multi_test.c > > > > diff --git a/tools/testing/selftests/bpf/multi_check.h b/tools/testing/selftests/bpf/multi_check.h > > new file mode 100644 > > index 000000000000..36c2a93f9be3 > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/multi_check.h > > we have a proper static linking now, we don't have to use header > inclusion hacks, let's do this properly? ok, will change > > > @@ -0,0 +1,52 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +#ifndef __MULTI_CHECK_H > > +#define __MULTI_CHECK_H > > + > > +extern unsigned long long bpf_fentry_test[8]; > > + > > +static __attribute__((unused)) inline > > +void multi_arg_check(unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f, __u64 *test_result) > > +{ > > + if (ip == bpf_fentry_test[0]) { > > + *test_result += (int) a == 1; > > + } else if (ip == bpf_fentry_test[1]) { > > + *test_result += (int) a == 2 && (__u64) b == 3; > > + } else if (ip == bpf_fentry_test[2]) { > > + *test_result += (char) a == 4 && (int) b == 5 && (__u64) c == 6; > > + } else if (ip == bpf_fentry_test[3]) { > > + *test_result += (void *) a == (void *) 7 && (char) b == 8 && (int) c == 9 && (__u64) d == 10; > > + } else if (ip == bpf_fentry_test[4]) { > > + *test_result += (__u64) a == 11 && (void *) b == (void *) 12 && (short) c == 13 && (int) d == 14 && (__u64) e == 15; > > + } else if (ip == bpf_fentry_test[5]) { > > + *test_result += (__u64) a == 16 && (void *) b == (void *) 17 && (short) c == 18 && (int) d == 19 && (void *) e == (void *) 20 && (__u64) f == 21; > > + } else if (ip == bpf_fentry_test[6]) { > > + *test_result += 1; > > + } else if (ip == bpf_fentry_test[7]) { > > + *test_result += 1; > > + } > > why not use switch? and why the casting? hum, for switch I'd need constants right? casting is extra ;-) wanted to check the actual argument types, but probably makes no sense will check > > > +} > > + > > [...] > > > diff --git a/tools/testing/selftests/bpf/progs/fentry_multi_test.c b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > > new file mode 100644 > > index 000000000000..a443fc958e5a > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > > @@ -0,0 +1,18 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include <linux/bpf.h> > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > +#include "multi_check.h" > > + > > +char _license[] SEC("license") = "GPL"; > > + > > +unsigned long long bpf_fentry_test[8]; > > + > > +__u64 test_result = 0; > > + > > +SEC("fentry.multi/bpf_fentry_test*") > > wait, that's a regexp syntax that libc supports?.. Not .*? We should > definitely not provide btf__find_by_pattern_kind() API, I'd like to > avoid explaining what flavors of regexps libbpf supports. ok thanks, jirka
On Wed, Jun 9, 2021 at 7:29 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Tue, Jun 08, 2021 at 10:40:24PM -0700, Andrii Nakryiko wrote: > > On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > Adding selftest for fentry multi func test that attaches > > > to bpf_fentry_test* functions and checks argument values > > > based on the processed function. > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ > > > .../bpf/prog_tests/fentry_multi_test.c | 43 +++++++++++++++ > > > .../selftests/bpf/progs/fentry_multi_test.c | 18 +++++++ > > > 3 files changed, 113 insertions(+) > > > create mode 100644 tools/testing/selftests/bpf/multi_check.h > > > create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > > > create mode 100644 tools/testing/selftests/bpf/progs/fentry_multi_test.c > > > > > > diff --git a/tools/testing/selftests/bpf/multi_check.h b/tools/testing/selftests/bpf/multi_check.h > > > new file mode 100644 > > > index 000000000000..36c2a93f9be3 > > > --- /dev/null > > > +++ b/tools/testing/selftests/bpf/multi_check.h > > > > we have a proper static linking now, we don't have to use header > > inclusion hacks, let's do this properly? > > ok, will change > > > > > > @@ -0,0 +1,52 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > + > > > +#ifndef __MULTI_CHECK_H > > > +#define __MULTI_CHECK_H > > > + > > > +extern unsigned long long bpf_fentry_test[8]; > > > + > > > +static __attribute__((unused)) inline > > > +void multi_arg_check(unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f, __u64 *test_result) > > > +{ > > > + if (ip == bpf_fentry_test[0]) { > > > + *test_result += (int) a == 1; > > > + } else if (ip == bpf_fentry_test[1]) { > > > + *test_result += (int) a == 2 && (__u64) b == 3; > > > + } else if (ip == bpf_fentry_test[2]) { > > > + *test_result += (char) a == 4 && (int) b == 5 && (__u64) c == 6; > > > + } else if (ip == bpf_fentry_test[3]) { > > > + *test_result += (void *) a == (void *) 7 && (char) b == 8 && (int) c == 9 && (__u64) d == 10; > > > + } else if (ip == bpf_fentry_test[4]) { > > > + *test_result += (__u64) a == 11 && (void *) b == (void *) 12 && (short) c == 13 && (int) d == 14 && (__u64) e == 15; > > > + } else if (ip == bpf_fentry_test[5]) { > > > + *test_result += (__u64) a == 16 && (void *) b == (void *) 17 && (short) c == 18 && (int) d == 19 && (void *) e == (void *) 20 && (__u64) f == 21; > > > + } else if (ip == bpf_fentry_test[6]) { > > > + *test_result += 1; > > > + } else if (ip == bpf_fentry_test[7]) { > > > + *test_result += 1; > > > + } > > > > why not use switch? and why the casting? > > hum, for switch I'd need constants right? doh, of course :) but! you don't need to fill out bpf_fentry_test[] array from user-space, just use extern const void variables to get addresses of those functions: extern const void bpf_fentry_test1 __ksym; extern const void bpf_fentry_test2 __ksym; ... > > casting is extra ;-) wanted to check the actual argument types, > but probably makes no sense probably doesn't given you already declared it u64 and use integer values for comparison > > will check > > > > > > +} > > > + > > > > [...] > > > > > diff --git a/tools/testing/selftests/bpf/progs/fentry_multi_test.c b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > > > new file mode 100644 > > > index 000000000000..a443fc958e5a > > > --- /dev/null > > > +++ b/tools/testing/selftests/bpf/progs/fentry_multi_test.c > > > @@ -0,0 +1,18 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +#include <linux/bpf.h> > > > +#include <bpf/bpf_helpers.h> > > > +#include <bpf/bpf_tracing.h> > > > +#include "multi_check.h" > > > + > > > +char _license[] SEC("license") = "GPL"; > > > + > > > +unsigned long long bpf_fentry_test[8]; > > > + > > > +__u64 test_result = 0; > > > + > > > +SEC("fentry.multi/bpf_fentry_test*") > > > > wait, that's a regexp syntax that libc supports?.. Not .*? We should > > definitely not provide btf__find_by_pattern_kind() API, I'd like to > > avoid explaining what flavors of regexps libbpf supports. > > ok > > thanks, > jirka >
On Thu, Jun 10, 2021 at 10:00:34AM -0700, Andrii Nakryiko wrote: > On Wed, Jun 9, 2021 at 7:29 AM Jiri Olsa <jolsa@redhat.com> wrote: > > > > On Tue, Jun 08, 2021 at 10:40:24PM -0700, Andrii Nakryiko wrote: > > > On Sat, Jun 5, 2021 at 4:12 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > Adding selftest for fentry multi func test that attaches > > > > to bpf_fentry_test* functions and checks argument values > > > > based on the processed function. > > > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ > > > > .../bpf/prog_tests/fentry_multi_test.c | 43 +++++++++++++++ > > > > .../selftests/bpf/progs/fentry_multi_test.c | 18 +++++++ > > > > 3 files changed, 113 insertions(+) > > > > create mode 100644 tools/testing/selftests/bpf/multi_check.h > > > > create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c > > > > create mode 100644 tools/testing/selftests/bpf/progs/fentry_multi_test.c > > > > > > > > diff --git a/tools/testing/selftests/bpf/multi_check.h b/tools/testing/selftests/bpf/multi_check.h > > > > new file mode 100644 > > > > index 000000000000..36c2a93f9be3 > > > > --- /dev/null > > > > +++ b/tools/testing/selftests/bpf/multi_check.h > > > > > > we have a proper static linking now, we don't have to use header > > > inclusion hacks, let's do this properly? > > > > ok, will change > > > > > > > > > @@ -0,0 +1,52 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > + > > > > +#ifndef __MULTI_CHECK_H > > > > +#define __MULTI_CHECK_H > > > > + > > > > +extern unsigned long long bpf_fentry_test[8]; > > > > + > > > > +static __attribute__((unused)) inline > > > > +void multi_arg_check(unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f, __u64 *test_result) > > > > +{ > > > > + if (ip == bpf_fentry_test[0]) { > > > > + *test_result += (int) a == 1; > > > > + } else if (ip == bpf_fentry_test[1]) { > > > > + *test_result += (int) a == 2 && (__u64) b == 3; > > > > + } else if (ip == bpf_fentry_test[2]) { > > > > + *test_result += (char) a == 4 && (int) b == 5 && (__u64) c == 6; > > > > + } else if (ip == bpf_fentry_test[3]) { > > > > + *test_result += (void *) a == (void *) 7 && (char) b == 8 && (int) c == 9 && (__u64) d == 10; > > > > + } else if (ip == bpf_fentry_test[4]) { > > > > + *test_result += (__u64) a == 11 && (void *) b == (void *) 12 && (short) c == 13 && (int) d == 14 && (__u64) e == 15; > > > > + } else if (ip == bpf_fentry_test[5]) { > > > > + *test_result += (__u64) a == 16 && (void *) b == (void *) 17 && (short) c == 18 && (int) d == 19 && (void *) e == (void *) 20 && (__u64) f == 21; > > > > + } else if (ip == bpf_fentry_test[6]) { > > > > + *test_result += 1; > > > > + } else if (ip == bpf_fentry_test[7]) { > > > > + *test_result += 1; > > > > + } > > > > > > why not use switch? and why the casting? > > > > hum, for switch I'd need constants right? > > doh, of course :) > > but! you don't need to fill out bpf_fentry_test[] array from > user-space, just use extern const void variables to get addresses of > those functions: > > extern const void bpf_fentry_test1 __ksym; > extern const void bpf_fentry_test2 __ksym; > ... nice, will use that jirka
diff --git a/tools/testing/selftests/bpf/multi_check.h b/tools/testing/selftests/bpf/multi_check.h new file mode 100644 index 000000000000..36c2a93f9be3 --- /dev/null +++ b/tools/testing/selftests/bpf/multi_check.h @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __MULTI_CHECK_H +#define __MULTI_CHECK_H + +extern unsigned long long bpf_fentry_test[8]; + +static __attribute__((unused)) inline +void multi_arg_check(unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f, __u64 *test_result) +{ + if (ip == bpf_fentry_test[0]) { + *test_result += (int) a == 1; + } else if (ip == bpf_fentry_test[1]) { + *test_result += (int) a == 2 && (__u64) b == 3; + } else if (ip == bpf_fentry_test[2]) { + *test_result += (char) a == 4 && (int) b == 5 && (__u64) c == 6; + } else if (ip == bpf_fentry_test[3]) { + *test_result += (void *) a == (void *) 7 && (char) b == 8 && (int) c == 9 && (__u64) d == 10; + } else if (ip == bpf_fentry_test[4]) { + *test_result += (__u64) a == 11 && (void *) b == (void *) 12 && (short) c == 13 && (int) d == 14 && (__u64) e == 15; + } else if (ip == bpf_fentry_test[5]) { + *test_result += (__u64) a == 16 && (void *) b == (void *) 17 && (short) c == 18 && (int) d == 19 && (void *) e == (void *) 20 && (__u64) f == 21; + } else if (ip == bpf_fentry_test[6]) { + *test_result += 1; + } else if (ip == bpf_fentry_test[7]) { + *test_result += 1; + } +} + +static __attribute__((unused)) inline +void multi_ret_check(unsigned long ip, int ret, __u64 *test_result) +{ + if (ip == bpf_fentry_test[0]) { + *test_result += ret == 2; + } else if (ip == bpf_fentry_test[1]) { + *test_result += ret == 5; + } else if (ip == bpf_fentry_test[2]) { + *test_result += ret == 15; + } else if (ip == bpf_fentry_test[3]) { + *test_result += ret == 34; + } else if (ip == bpf_fentry_test[4]) { + *test_result += ret == 65; + } else if (ip == bpf_fentry_test[5]) { + *test_result += ret == 111; + } else if (ip == bpf_fentry_test[6]) { + *test_result += ret == 0; + } else if (ip == bpf_fentry_test[7]) { + *test_result += ret == 0; + } +} + +#endif /* __MULTI_CHECK_H */ diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c new file mode 100644 index 000000000000..e4a8089533d6 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> +#include "fentry_multi_test.skel.h" +#include "trace_helpers.h" + +void test_fentry_multi_test(void) +{ + struct fentry_multi_test *skel = NULL; + unsigned long long *bpf_fentry_test; + __u32 duration = 0, retval; + int err, prog_fd; + + skel = fentry_multi_test__open_and_load(); + if (!ASSERT_OK_PTR(skel, "fentry_multi_skel_load")) + goto cleanup; + + bpf_fentry_test = &skel->bss->bpf_fentry_test[0]; + ASSERT_OK(kallsyms_find("bpf_fentry_test1", &bpf_fentry_test[0]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test2", &bpf_fentry_test[1]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test3", &bpf_fentry_test[2]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test4", &bpf_fentry_test[3]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test5", &bpf_fentry_test[4]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test6", &bpf_fentry_test[5]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test7", &bpf_fentry_test[6]), "kallsyms_find"); + ASSERT_OK(kallsyms_find("bpf_fentry_test8", &bpf_fentry_test[7]), "kallsyms_find"); + + err = fentry_multi_test__attach(skel); + if (!ASSERT_OK(err, "fentry_attach")) + goto cleanup; + + prog_fd = bpf_program__fd(skel->progs.test); + err = bpf_prog_test_run(prog_fd, 1, NULL, 0, + NULL, NULL, &retval, &duration); + ASSERT_OK(err, "test_run"); + ASSERT_EQ(retval, 0, "test_run"); + + ASSERT_EQ(skel->bss->test_result, 8, "test_result"); + + fentry_multi_test__detach(skel); + +cleanup: + fentry_multi_test__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/fentry_multi_test.c b/tools/testing/selftests/bpf/progs/fentry_multi_test.c new file mode 100644 index 000000000000..a443fc958e5a --- /dev/null +++ b/tools/testing/selftests/bpf/progs/fentry_multi_test.c @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include "multi_check.h" + +char _license[] SEC("license") = "GPL"; + +unsigned long long bpf_fentry_test[8]; + +__u64 test_result = 0; + +SEC("fentry.multi/bpf_fentry_test*") +int BPF_PROG(test, unsigned long ip, __u64 a, __u64 b, __u64 c, __u64 d, __u64 e, __u64 f) +{ + multi_arg_check(ip, a, b, c, d, e, f, &test_result); + return 0; +}
Adding selftest for fentry multi func test that attaches to bpf_fentry_test* functions and checks argument values based on the processed function. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/testing/selftests/bpf/multi_check.h | 52 +++++++++++++++++++ .../bpf/prog_tests/fentry_multi_test.c | 43 +++++++++++++++ .../selftests/bpf/progs/fentry_multi_test.c | 18 +++++++ 3 files changed, 113 insertions(+) create mode 100644 tools/testing/selftests/bpf/multi_check.h create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_multi_test.c create mode 100644 tools/testing/selftests/bpf/progs/fentry_multi_test.c