[PATCH bpf-next 0/4] bpf: Allow get_func_[arg|arg_cnt] helpers in raw tracepoint programs
hi,
We can use get_func_[arg|arg_cnt] helpers in fentry/fexit/fmod_ret programs currently[1]. But they can't be used in raw_tp/tp_btf programs.
Adding support to use get_func_[arg|arg_cnt] helpers in raw_tp/tp_btf programs. Adding BPF_PROG_TEST_RUN for tp_btf. Add selftests to check them.
Thanks, KaFai
[1] https://lore.kernel.org/bpf/20211208193245.172141-1-jolsa@kernel.org/ --- KaFai Wan (4): bpf: Allow get_func_[arg|arg_cnt] helpers in raw tracepoint programs bpf: Enable BPF_PROG_TEST_RUN for tp_btf selftests/bpf: Add raw_tp_test_run for tp_btf selftests/bpf: Add tests for get_func_[arg|arg_cnt] helpers in raw tracepoint programs
kernel/trace/bpf_trace.c | 17 +++++-- net/bpf/test_run.c | 16 +++---- .../bpf/prog_tests/raw_tp_get_func_args.c | 48 +++++++++++++++++++ .../bpf/prog_tests/raw_tp_test_run.c | 18 ++++++- .../bpf/progs/test_raw_tp_get_func_args.c | 47 ++++++++++++++++++ .../bpf/progs/test_raw_tp_test_run.c | 16 +++++-- 6 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_get_func_args.c create mode 100644 tools/testing/selftests/bpf/progs/test_raw_tp_get_func_args.c
Adding support to use get_func_[arg|arg_cnt] helpers in raw_tp/tp_btf programs.
We can use get_func_[arg|ret|arg_cnt] helpers in fentry/fexit/fmod_ret programs currently. If we try to use get_func_[arg|arg_cnt] helpers in raw_tp/tp_btf programs, verifier will fail to load the program with:
; __u64 cnt = bpf_get_func_arg_cnt(ctx); 3: (85) call bpf_get_func_arg_cnt#185 unknown func bpf_get_func_arg_cnt#185
Adding get_func_[arg|arg_cnt] helpers in raw_tp_prog_func_proto and tracing_prog_func_proto for raw tracepoint.
Adding 1 arg on ctx of raw tracepoint program and make it stores number of arguments on ctx-8, so it's easy to verify argument index and find argument's position.
Signed-off-by: KaFai Wan mannkafai@gmail.com --- kernel/trace/bpf_trace.c | 17 ++++++++++++++--- net/bpf/test_run.c | 13 +++++-------- 2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 52c432a44aeb..eb4c56013493 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1892,6 +1892,10 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return &bpf_get_stackid_proto_raw_tp; case BPF_FUNC_get_stack: return &bpf_get_stack_proto_raw_tp; + case BPF_FUNC_get_func_arg: + return &bpf_get_func_arg_proto; + case BPF_FUNC_get_func_arg_cnt: + return &bpf_get_func_arg_cnt_proto; case BPF_FUNC_get_attach_cookie: return &bpf_get_attach_cookie_proto_tracing; default: @@ -1950,10 +1954,16 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) case BPF_FUNC_d_path: return &bpf_d_path_proto; case BPF_FUNC_get_func_arg: + if (prog->type == BPF_PROG_TYPE_TRACING && + prog->expected_attach_type == BPF_TRACE_RAW_TP) + return &bpf_get_func_arg_proto; return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL; case BPF_FUNC_get_func_ret: return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL; case BPF_FUNC_get_func_arg_cnt: + if (prog->type == BPF_PROG_TYPE_TRACING && + prog->expected_attach_type == BPF_TRACE_RAW_TP) + return &bpf_get_func_arg_cnt_proto; return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL; case BPF_FUNC_get_attach_cookie: if (prog->type == BPF_PROG_TYPE_TRACING && @@ -2312,7 +2322,7 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64 *args) #define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
#define SARG(X) u64 arg##X -#define COPY(X) args[X] = arg##X +#define COPY(X) args[X + 1] = arg##X
#define __DL_COM (,) #define __DL_SEM (;) @@ -2323,9 +2333,10 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64 *args) void bpf_trace_run##x(struct bpf_raw_tp_link *link, \ REPEAT(x, SARG, __DL_COM, __SEQ_0_11)) \ { \ - u64 args[x]; \ + u64 args[x + 1]; \ + args[0] = x; \ REPEAT(x, COPY, __DL_SEM, __SEQ_0_11); \ - __bpf_trace_run(link, args); \ + __bpf_trace_run(link, args + 1); \ } \ EXPORT_SYMBOL_GPL(bpf_trace_run##x) BPF_TRACE_DEFN_x(1); diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index aaf13a7d58ed..8cb285187270 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -760,6 +760,7 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog, void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in); __u32 ctx_size_in = kattr->test.ctx_size_in; struct bpf_raw_tp_test_run_info info; + u64 args[MAX_BPF_FUNC_ARGS + 1] = {}; int cpu = kattr->test.cpu, err = 0; int current_cpu;
@@ -776,14 +777,11 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog, if ((kattr->test.flags & BPF_F_TEST_RUN_ON_CPU) == 0 && cpu != 0) return -EINVAL;
- if (ctx_size_in) { - info.ctx = memdup_user(ctx_in, ctx_size_in); - if (IS_ERR(info.ctx)) - return PTR_ERR(info.ctx); - } else { - info.ctx = NULL; - } + if (ctx_size_in && copy_from_user(args + 1, ctx_in, ctx_size_in)) + return -EFAULT;
+ args[0] = ctx_size_in / sizeof(u64); + info.ctx = args + 1; info.prog = prog;
current_cpu = get_cpu(); @@ -807,7 +805,6 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog, copy_to_user(&uattr->test.retval, &info.retval, sizeof(u32))) err = -EFAULT;
- kfree(info.ctx); return err; }
Add .test_run for tp_btf. Use the .test_run for raw_tp.
Signed-off-by: KaFai Wan mannkafai@gmail.com --- net/bpf/test_run.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 8cb285187270..8c901ec92341 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -690,6 +690,9 @@ int bpf_prog_test_run_tracing(struct bpf_prog *prog, int b = 2, err = -EFAULT; u32 retval = 0;
+ if (prog->expected_attach_type == BPF_TRACE_RAW_TP) + return bpf_prog_test_run_raw_tp(prog, kattr, uattr); + if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size) return -EINVAL;
Add test runs test_run for tp_btf base on raw_tp.
Changing test_raw_tp_test_run to test both raw_tp and tp_btf.
Signed-off-by: KaFai Wan mannkafai@gmail.com --- .../selftests/bpf/prog_tests/raw_tp_test_run.c | 18 ++++++++++++++++-- .../selftests/bpf/progs/test_raw_tp_test_run.c | 16 +++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c index fe5b8fae2c36..e2968a1e73bf 100644 --- a/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c +++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_test_run.c @@ -5,7 +5,7 @@ #include "bpf/libbpf_internal.h" #include "test_raw_tp_test_run.skel.h"
-void test_raw_tp_test_run(void) +static void test_raw_tp(bool is_tp_btf) { int comm_fd = -1, err, nr_online, i, prog_fd; __u64 args[2] = {0x1234ULL, 0x5678ULL}; @@ -28,6 +28,9 @@ void test_raw_tp_test_run(void) if (!ASSERT_OK_PTR(skel, "skel_open")) goto cleanup;
+ bpf_program__set_autoattach(skel->progs.rename_tp_btf, is_tp_btf); + bpf_program__set_autoattach(skel->progs.rename_raw_tp, !is_tp_btf); + err = test_raw_tp_test_run__attach(skel); if (!ASSERT_OK(err, "skel_attach")) goto cleanup; @@ -42,7 +45,10 @@ void test_raw_tp_test_run(void) ASSERT_NEQ(skel->bss->count, 0, "check_count"); ASSERT_EQ(skel->data->on_cpu, 0xffffffff, "check_on_cpu");
- prog_fd = bpf_program__fd(skel->progs.rename); + if (is_tp_btf) + prog_fd = bpf_program__fd(skel->progs.rename_tp_btf); + else + prog_fd = bpf_program__fd(skel->progs.rename_raw_tp); opts.ctx_in = args; opts.ctx_size_in = sizeof(__u64);
@@ -84,3 +90,11 @@ void test_raw_tp_test_run(void) test_raw_tp_test_run__destroy(skel); free(online); } + +void test_raw_tp_test_run(void) +{ + if (test__start_subtest("raw_tp")) + test_raw_tp(false); + if (test__start_subtest("tp_btf")) + test_raw_tp(true); +} diff --git a/tools/testing/selftests/bpf/progs/test_raw_tp_test_run.c b/tools/testing/selftests/bpf/progs/test_raw_tp_test_run.c index 4c63cc87b9d0..ddc22e6cfdd9 100644 --- a/tools/testing/selftests/bpf/progs/test_raw_tp_test_run.c +++ b/tools/testing/selftests/bpf/progs/test_raw_tp_test_run.c @@ -8,10 +8,8 @@ __u32 count = 0; __u32 on_cpu = 0xffffffff;
-SEC("raw_tp/task_rename") -int BPF_PROG(rename, struct task_struct *task, char *comm) +static __always_inline int check_test_run(struct task_struct *task, char *comm) { - count++; if ((__u64) task == 0x1234ULL && (__u64) comm == 0x5678ULL) { on_cpu = bpf_get_smp_processor_id(); @@ -21,4 +19,16 @@ int BPF_PROG(rename, struct task_struct *task, char *comm) return 0; }
+SEC("raw_tp/task_rename") +int BPF_PROG(rename_raw_tp, struct task_struct *task, char *comm) +{ + return check_test_run(task, comm); +} + +SEC("tp_btf/task_rename") +int BPF_PROG(rename_tp_btf, struct task_struct *task, char *comm) +{ + return check_test_run(task, comm); +} + char _license[] SEC("license") = "GPL";
Adding tests for get_func_[arg|arg_cnt] helpers in raw tracepoint programs. Using these helpers in raw_tp/tp_btf programs.
Signed-off-by: KaFai Wan mannkafai@gmail.com --- .../bpf/prog_tests/raw_tp_get_func_args.c | 48 +++++++++++++++++++ .../bpf/progs/test_raw_tp_get_func_args.c | 47 ++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_get_func_args.c create mode 100644 tools/testing/selftests/bpf/progs/test_raw_tp_get_func_args.c
diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_get_func_args.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_get_func_args.c new file mode 100644 index 000000000000..cbe9b441d8d9 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_get_func_args.c @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> +#include <linux/bpf.h> +#include "bpf/libbpf_internal.h" +#include "test_raw_tp_get_func_args.skel.h" + +static void test_raw_tp_args(bool is_tp_btf) +{ + __u64 args[2] = {0x1234ULL, 0x5678ULL}; + int expected_retval = 0x1234 + 0x5678; + struct test_raw_tp_get_func_args *skel; + LIBBPF_OPTS(bpf_test_run_opts, opts, + .ctx_in = args, + .ctx_size_in = sizeof(args), + ); + int err, prog_fd; + + skel = test_raw_tp_get_func_args__open_and_load(); + if (!ASSERT_OK_PTR(skel, "skel_open")) + goto cleanup; + + bpf_program__set_autoattach(skel->progs.tp_btf_test, is_tp_btf); + bpf_program__set_autoattach(skel->progs.raw_tp_test, !is_tp_btf); + + err = test_raw_tp_get_func_args__attach(skel); + if (!ASSERT_OK(err, "skel_attach")) + goto cleanup; + + if (is_tp_btf) + prog_fd = bpf_program__fd(skel->progs.tp_btf_test); + else + prog_fd = bpf_program__fd(skel->progs.raw_tp_test); + err = bpf_prog_test_run_opts(prog_fd, &opts); + ASSERT_OK(err, "test_run"); + ASSERT_EQ(opts.retval, expected_retval, "check_retval"); + ASSERT_EQ(skel->bss->test_result, 1, "test_result"); + +cleanup: + test_raw_tp_get_func_args__destroy(skel); +} + +void test_raw_tp_get_func_args(void) +{ + if (test__start_subtest("raw_tp")) + test_raw_tp_args(false); + if (test__start_subtest("tp_btf")) + test_raw_tp_args(true); +} diff --git a/tools/testing/selftests/bpf/progs/test_raw_tp_get_func_args.c b/tools/testing/selftests/bpf/progs/test_raw_tp_get_func_args.c new file mode 100644 index 000000000000..5069bbd15283 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_raw_tp_get_func_args.c @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <errno.h> + +__u64 test_result = 0; + +static __always_inline int check_args(void *ctx, struct task_struct *task, + char *comm) +{ + __u64 cnt = bpf_get_func_arg_cnt(ctx); + __u64 a = 0, b = 0, z = 0; + __s64 err; + + if ((__u64)task != 0x1234ULL || (__u64)comm != 0x5678ULL) + return 0; + + test_result = cnt == 2; + + /* valid arguments */ + err = bpf_get_func_arg(ctx, 0, &a); + test_result &= err == 0 && a == 0x1234ULL; + + err = bpf_get_func_arg(ctx, 1, &b); + test_result &= err == 0 && b == 0x5678ULL; + + /* not valid argument */ + err = bpf_get_func_arg(ctx, 2, &z); + test_result &= err == -EINVAL; + + return a + b; +} + +SEC("raw_tp/task_rename") +int BPF_PROG(raw_tp_test, struct task_struct *task, char *comm) +{ + return check_args(ctx, task, comm); +} + +SEC("tp_btf/task_rename") +int BPF_PROG(tp_btf_test, struct task_struct *task, char *comm) +{ + return check_args(ctx, task, comm); +} + +char _license[] SEC("license") = "GPL";
linux-kselftest-mirror@lists.linaro.org
-
KaFai Wan