| Message ID | 20220204005519.60361-3-mcroce@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 976a38e05a49f401cf9ae3ae20273db6d69783cf |
| Delegated to: | BPF |
| Headers | show |
| Series | limit bpf_core_types_are_compat recursion | expand |
On Thu, Feb 3, 2022 at 4:55 PM Matteo Croce <mcroce@linux.microsoft.com> wrote: > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > @@ -13,6 +13,11 @@ > #define CREATE_TRACE_POINTS > #include "bpf_testmod-events.h" > > +typedef int (*func_proto_typedef___match)(long); > +typedef int (*func_proto_typedef___overflow)(func_proto_typedef___match); There is no need for "___flavor" on the kernel side of type definition. It makes the test confusing to read. > +func_proto_typedef___match funcp = NULL; > +func_proto_typedef___overflow funcp_of = NULL; We have BTF_TYPE_EMIT() macro to avoid unnecessary declaration. > +typedef int (*func_proto_typedef___match)(long); > +typedef int (*func_proto_typedef___overflow)(func_proto_typedef___match); With <=1 in the previous patch such single depth of func_proto was reaching the recursion limit. Hence the fix <=0 was necessary. I've also changed this test to: +typedef int (*func_proto_typedef)(long); +typedef int (*func_proto_typedef_nested1)(func_proto_typedef); +typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1); in bpf_testmod.c and in progs/core_kern_overflow.c and bpf_core_type_exists(func_proto_typedef_nested2); to go above the limit. Also added bpf_core_type_exists(func_proto_typedef_nested1) to progs/core_kern.c to stay at the limit. Please see the result in bpf-next.
On Fri, Feb 4, 2022 at 8:38 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, Feb 3, 2022 at 4:55 PM Matteo Croce <mcroce@linux.microsoft.com> wrote: > > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > > @@ -13,6 +13,11 @@ > > #define CREATE_TRACE_POINTS > > #include "bpf_testmod-events.h" > > > > +typedef int (*func_proto_typedef___match)(long); > > +typedef int (*func_proto_typedef___overflow)(func_proto_typedef___match); > > There is no need for "___flavor" on the kernel side of type definition. > It makes the test confusing to read. > > > +func_proto_typedef___match funcp = NULL; > > +func_proto_typedef___overflow funcp_of = NULL; > > We have BTF_TYPE_EMIT() macro to avoid unnecessary declaration. > > > +typedef int (*func_proto_typedef___match)(long); > > +typedef int (*func_proto_typedef___overflow)(func_proto_typedef___match); > > With <=1 in the previous patch such single depth of func_proto > was reaching the recursion limit. > Hence the fix <=0 was necessary. > I've also changed this test to: > > +typedef int (*func_proto_typedef)(long); > +typedef int (*func_proto_typedef_nested1)(func_proto_typedef); > +typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1); > > in bpf_testmod.c and in progs/core_kern_overflow.c > and > bpf_core_type_exists(func_proto_typedef_nested2); > to go above the limit. > > Also added bpf_core_type_exists(func_proto_typedef_nested1) > to progs/core_kern.c to stay at the limit. > > Please see the result in bpf-next. Awesome. I've seen both patches in the repo, LGTM.
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 945f92d71db3..91ea729990da 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -330,7 +330,7 @@ LINKED_SKELS := test_static_linked.skel.h linked_funcs.skel.h \ LSKELS := kfunc_call_test.c fentry_test.c fexit_test.c fexit_sleep.c \ test_ringbuf.c atomics.c trace_printk.c trace_vprintk.c \ - map_ptr_kern.c core_kern.c + map_ptr_kern.c core_kern.c core_kern_overflow.c # Generate both light skeleton and libbpf skeleton for these LSKELS_EXTRA := test_ksyms_module.c test_ksyms_weak.c kfunc_call_test_subprog.c SKEL_BLACKLIST += $$(LSKELS) diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c index 595d32ab285a..e5ba8d8a17da 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c @@ -13,6 +13,11 @@ #define CREATE_TRACE_POINTS #include "bpf_testmod-events.h" +typedef int (*func_proto_typedef___match)(long); +typedef int (*func_proto_typedef___overflow)(func_proto_typedef___match); +func_proto_typedef___match funcp = NULL; +func_proto_typedef___overflow funcp_of = NULL; + DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123; noinline void diff --git a/tools/testing/selftests/bpf/prog_tests/core_kern.c b/tools/testing/selftests/bpf/prog_tests/core_kern.c index 561c5185d886..91493f5836ff 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_kern.c +++ b/tools/testing/selftests/bpf/prog_tests/core_kern.c @@ -7,8 +7,21 @@ void test_core_kern_lskel(void) { struct core_kern_lskel *skel; + int link_fd; skel = core_kern_lskel__open_and_load(); - ASSERT_OK_PTR(skel, "open_and_load"); + if (!ASSERT_OK_PTR(skel, "open_and_load")) + return; + + link_fd = core_kern_lskel__core_relo_proto__attach(skel); + if (!ASSERT_GT(link_fd, 0, "attach(core_relo_proto)")) + goto cleanup; + + /* trigger tracepoints */ + usleep(1); + ASSERT_TRUE(skel->bss->proto_out[0], "bpf_core_type_exists"); + ASSERT_FALSE(skel->bss->proto_out[1], "!bpf_core_type_exists"); + +cleanup: core_kern_lskel__destroy(skel); } diff --git a/tools/testing/selftests/bpf/prog_tests/core_kern_overflow.c b/tools/testing/selftests/bpf/prog_tests/core_kern_overflow.c new file mode 100644 index 000000000000..04cc145bc26a --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/core_kern_overflow.c @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "test_progs.h" +#include "core_kern_overflow.lskel.h" + +void test_core_kern_overflow_lskel(void) +{ + struct core_kern_overflow_lskel *skel; + + skel = core_kern_overflow_lskel__open_and_load(); + if (!ASSERT_NULL(skel, "open_and_load")) + core_kern_overflow_lskel__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/core_kern.c b/tools/testing/selftests/bpf/progs/core_kern.c index 13499cc15c7d..acabe4cb0480 100644 --- a/tools/testing/selftests/bpf/progs/core_kern.c +++ b/tools/testing/selftests/bpf/progs/core_kern.c @@ -101,4 +101,18 @@ int balancer_ingress(struct __sk_buff *ctx) return 0; } +typedef int (*func_proto_typedef___match)(long); +typedef void (*func_proto_typedef___doesnt_match)(char*); + +int proto_out[2]; + +SEC("raw_tracepoint/sys_enter") +int core_relo_proto(void *ctx) +{ + proto_out[0] = bpf_core_type_exists(func_proto_typedef___match); + proto_out[1] = bpf_core_type_exists(func_proto_typedef___doesnt_match); + + return 0; +} + char LICENSE[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/core_kern_overflow.c b/tools/testing/selftests/bpf/progs/core_kern_overflow.c new file mode 100644 index 000000000000..70417413af55 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/core_kern_overflow.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "vmlinux.h" + +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <bpf/bpf_core_read.h> + +typedef int (*func_proto_typedef___match)(long); +typedef int (*func_proto_typedef___overflow)(func_proto_typedef___match); + +int proto_out; + +SEC("raw_tracepoint/sys_enter") +int core_relo_proto(void *ctx) +{ + proto_out = bpf_core_type_exists(func_proto_typedef___overflow); + + return 0; +} + +char LICENSE[] SEC("license") = "GPL";