Re: [PATCH bpf-next v3 5/9] selftests/bpf: Add kfunc __arena and __arena_nullable argument tests
[email protected] Mon, 03 Aug 2026 13:35:32 +0000
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Medium] The `arena_arg_stack` test incorrectly wraps its failure-inducin= g logic in an `#if defined(__BPF_FEATURE_ADDR_SPACE_CAST)` block, which cau= ses the test to falsely fail on compilers without this feature. -- --- Patch [5]: [PATCH bpf-next v3 5/9] selftests/bpf: Add kfunc __arena and= __arena_nullable argument tests --- commit b62ba0a2046a4e44e0da5d972133d046f475c161 Author: Tejun Heo <[email protected]> selftests/bpf: Add kfunc __arena and __arena_nullable argument tests This commit adds arena-argument kfuncs to bpf_testmod and a corresponding suite of BPF programs to test various argument forms and rebase semantics. The tests are intended to run on x86-64 where JIT support exists. > diff --git a/tools/testing/selftests/bpf/progs/arena_kfunc.c b/tools/test= ing/selftests/bpf/progs/arena_kfunc.c > new file mode 100644 > index 0000000000000..e7250c5197ab4 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/arena_kfunc.c [ ... ] > +SEC("syscall") > +__arch_x86_64 > +__failure __msg("arena pointer cannot be a stack argument") > +int arena_arg_stack(void *ctx) > +{ > +#if defined(__BPF_FEATURE_ADDR_SPACE_CAST) > + bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0); > + bpf_kfunc_arena_stack_arg_test(1, 2, 3, 4, 5, (u64 *)1); > +#endif [Severity: Medium] Will this test incorrectly fail on toolchains without __BPF_FEATURE_ADDR_SPACE_CAST? When the feature is missing, the #if block evaluates to false, leaving an empty program that simply returns 0. The BPF verifier will accept this empty program, but because the test is annotated with __failure __msg(...), the test harness expects the verifier to reject it. Since verification succeeds in this case, does this cause the test harness = to flag it as a spurious test failure during CI or user builds? > + return 0; > +} > + > +char _license[] SEC("license") =3D "GPL"; --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803125115.2264= [email protected]?part=3D5