Re: fentry/fexit attach to EXT type XDP program does not work
From: Jiri Olsa <hidden>
Date: 2020-07-26 12:25:17
Also in:
bpf
Subsystem:
bpf [general] (safe dynamic programs and tools), bpf [selftests] (test runners & infrastructure), kernel selftest framework, the rest · Maintainers:
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Shuah Khan, Shuah Khan, Linus Torvalds
On Tue, Jun 09, 2020 at 10:52:34AM +0200, Eelco Chaudron wrote: SNIP
quoted
quoted
libbpf: failed to load object 'test_xdp_bpf2bpf' libbpf: failed to load BPF skeleton 'test_xdp_bpf2bpf': -4007 test_xdp_fentry_ext:FAIL:__load ftrace skeleton failed #91 xdp_fentry_ext:FAIL Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED Any idea what could be the case here? The same fentry/fexit attach code works fine in the xdp_bpf2bpf.c tests case.<SNIP>quoted
I think this is not supported now. That is, you cannot attach a fentry trace to the EXT program. The current implementation for fentry program simply trying to find and match the signature of freplace program which by default is a pointer to void. It is doable in that in kernel we could recognize to-be-attached program is a freplace and further trace down to find the real signature. The related kernel function is btf_get_prog_ctx_type(). You can try to implement by yourself or I can have a patch for this once bpf-next opens.I’m not familiar with this area of the code, so if you could prepare a patch that would nice. You can also send it to me before bpf-next opens and I can verify it, and clean up the self-test so it can be included as well.
hi,
it seems that you cannot exten fentry/fexit programs,
but it's possible to attach fentry/fexit to ext program.
/* Program extensions can extend all program types
* except fentry/fexit. The reason is the following.
* The fentry/fexit programs are used for performance
* analysis, stats and can be attached to any program
* type except themselves. When extension program is
* replacing XDP function it is necessary to allow
* performance analysis of all functions. Both original
* XDP program and its program extension. Hence
* attaching fentry/fexit to BPF_PROG_TYPE_EXT is
* allowed. If extending of fentry/fexit was allowed it
* would be possible to create long call chain
* fentry->extension->fentry->extension beyond
* reasonable stack size. Hence extending fentry is not
* allowed.
*/
I changed fexit_bpf2bpf.c test just to do a quick check
and it seems to work:
# echo > /sys/kernel/debug/tracing/trace
# ./test_progs -t fexit_bpf2bpf
#25 fexit_bpf2bpf:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
# cat /sys/kernel/debug/tracing/trace | tail -2
<...>-75365 [012] d... 313932.416780: bpf_trace_printk: ENTRY val 123
<...>-75365 [012] d... 313932.416784: bpf_trace_printk: EXIT val 123, ret 1
jirka
---diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
index a895bfed55db..4b6c26ac2362 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c@@ -17,6 +17,7 @@ static void test_fexit_bpf2bpf_common(const char *obj_file, struct bpf_map *data_map; const int zero = 0; u64 *result = NULL; + int ext_fd; err = bpf_prog_load(target_obj_file, BPF_PROG_TYPE_UNSPEC, &pkt_obj, &pkt_fd);
@@ -51,11 +52,50 @@ static void test_fexit_bpf2bpf_common(const char *obj_file, link[i] = bpf_program__attach_trace(prog[i]); if (CHECK(IS_ERR(link[i]), "attach_trace", "failed to link\n")) goto close_prog; + + if (!strcmp(prog_name[i], "freplace/get_constant")) + ext_fd = bpf_program__fd(prog[i]); } if (!run_prog) goto close_prog; +{ + struct bpf_object *obj_trace = NULL; + struct bpf_program *prog_trace = NULL; + struct bpf_link *link_trace = NULL; + + DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts_trace, + .attach_prog_fd = ext_fd, + ); + + obj_trace = bpf_object__open_file("trace_ext.o", &opts_trace); + if (CHECK(IS_ERR_OR_NULL(obj_trace), "obj_open", + "failed to open %s: %ld\n", obj_file, + PTR_ERR(obj_trace))) + goto close_prog; + + err = bpf_object__load(obj_trace); + if (CHECK(err, "obj_load", "err %d\n", err)) + goto close_prog; + + prog_trace = bpf_object__find_program_by_title(obj_trace, "fexit/new_get_constant"); + if (CHECK(!prog_trace, "find_prog", "prog %s not found\n", "fexit/new_get_constant")) + goto close_prog; + + link_trace = bpf_program__attach_trace(prog_trace); + if (CHECK(IS_ERR(link_trace), "attach_trace", "failed to link\n")) + goto close_prog; + + prog_trace = bpf_object__find_program_by_title(obj_trace, "fentry/new_get_constant"); + if (CHECK(!prog_trace, "find_prog", "prog %s not found\n", "fentry/new_get_constant")) + goto close_prog; + + link_trace = bpf_program__attach_trace(prog_trace); + if (CHECK(IS_ERR(link_trace), "attach_trace", "failed to link\n")) + goto close_prog; +} + data_map = bpf_object__find_map_by_name(obj, "fexit_bp.bss"); if (CHECK(!data_map, "find_data_map", "data map not found\n")) goto close_prog;
@@ -88,6 +128,7 @@ static void test_fexit_bpf2bpf_common(const char *obj_file, free(result); } +#if 0 static void test_target_no_callees(void) { const char *prog_name[] = {
@@ -112,6 +153,7 @@ static void test_target_yes_callees(void) ARRAY_SIZE(prog_name), prog_name, true); } +#endif static void test_func_replace(void) {
@@ -130,6 +172,7 @@ static void test_func_replace(void) prog_name, true); } +#if 0 static void test_func_replace_verify(void) { const char *prog_name[] = {
@@ -140,11 +183,9 @@ static void test_func_replace_verify(void) ARRAY_SIZE(prog_name), prog_name, false); } +#endif void test_fexit_bpf2bpf(void) { - test_target_no_callees(); - test_target_yes_callees(); test_func_replace(); - test_func_replace_verify(); }
diff --git a/tools/testing/selftests/bpf/progs/trace_ext.c b/tools/testing/selftests/bpf/progs/trace_ext.c
new file mode 100644
index 000000000000..c7eaf30bf5ba
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/trace_ext.c@@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <stddef.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_endian.h> +#include <bpf/bpf_tracing.h> + + +SEC("fentry/new_get_constant") +int BPF_PROG(test_fentry_new_get_constant, long val) +{ + bpf_printk("ENTRY val %ld", val); + return 0; +} + +SEC("fexit/new_get_constant") +int BPF_PROG(test_fexit_new_get_constant, long val, int ret) +{ + bpf_printk("EXIT val %ld, ret %d", val, ret); + return 0; +} + +char _license[] SEC("license") = "GPL";