Re: [PATCH bpf-next 18/20] selftests/bpf: Add an end-to-end .bpf_cleanup exception test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: multi-line comments do not follow subsystem style
--

commit 5918cbd599a9ba7808857475f6ca49093862b54a
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Add an end-to-end .bpf_cleanup exception test

This commit adds an end-to-end test for .bpf_cleanup exception handling, using
inline assembly to construct landing pads and cleanup records. It validates
both successful exception propagation scenarios and various edge cases that
the kernel is expected to refuse.

> diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
> index d1e45b765af26..0000000000000 100644
> --- a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
> +++ b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c

[ ... ]

> +	/* foo3 throws; every pad runs and the cookie is delivered at entry. */
> +	if (test__start_subtest("throw_from_foo3"))
> +		run(skel, 101, THROW_COOKIE, PADS_FOO3_THREW);
> +
> +	/* foo3 returns 2 ^ 1 == 3, so foo2 throws from its own second region;
> +	 * foo3's frame is long gone, so its pad must not run.
> +	 */

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the BPF subsystem
style? The guidelines prefer placing the opening /* on its own line for
multi-line comments.

> +	if (test__start_subtest("throw_from_foo2"))
> +		run(skel, 2, THROW_COOKIE, PADS_FOO2_THREW);
> +
> +	exceptions_cleanup__destroy(skel);

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c b/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c
> index ce2dac306a84c..0000000000000 100644
> --- a/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c
> +++ b/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c

[ ... ]

> +"3:"					/* landing pad */
> +	"r1 = jt0_%= ll;"
> +	"r1 += 8;"
> +	"r2 = *(u64 *)(r1 + 0);"
> +	/* gotox r2. Spelled as a raw insn on purpose: the "gotox" mnemonic
> +	 * only reached the LLVM assembler in llvm 22, and BPF_RAW_INSN()
> +	 * needs <linux/bpf.h>, which this file cannot have -- vmlinux.h
> +	 * already defines the uapi enums.
> +	 */

[Severity: Low]
This is also not a bug, but does this comment formatting comply with the
subsystem guidelines? Like the previous instance, the preferred BPF style
places the opening /* on a separate line.

> +	".8byte 0x20d;"
> +"l0_%=:"
> +	"call bpf_unwind_resume;"
> +	"exit;"

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917055645.3926444-1-yonghong.song@linux.dev?part=18