Re: [PATCH bpf-next v2 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 modern BPF style
--

commit 8ad5e7ea23b9dcb55029206d5cb36453b231f9c8
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. It
uses inline assembly to construct precise call-site brackets, landing pads,
and cleanup records that mirror compiler output. It also tests shapes that
the kernel should refuse, ensuring the JIT correctly handles or rejects
different landing pad configurations.

> diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
> new file mode 100644
> index 0000000000000..d1e45b765af26
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/exceptions_cleanup.c
[ ... ]
> @@ -0,0 +1,79 @@
[ ... ]
> +	/* 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]
Does this comment block follow the proper multi-line comment formatting
for the BPF subsystem?

The BPF subsystem strict guidelines require multi-line comments to have
the opening /* on its own line:

/*
 * 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.
 */

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

> diff --git a/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c b/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c
> new file mode 100644
> index 0000000000000..ce2dac306a84c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/exceptions_cleanup_fail.c
[ ... ]
> @@ -0,0 +1,600 @@
[ ... ]
> +SEC("socket")
> +__failure __msg("reaches an indirect jump")
> +__naked void gotox_in_pad(void)
> +{
> +	asm volatile (
> +	".pushsection .jumptables,\"\",@progbits;"
> +"jt0_%=:"
> +	".quad l0_%= - socket;"
> +	".quad l1_%= - socket;"
> +	".size jt0_%=, 16;"
> +	".global jt0_%=;"
> +	".popsection;"
> +
> +"1:"	"call gotox_thrower;"		/* cleanup region */
> +"2:"
> +	"r0 = 0;"
> +	"exit;"
> +"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]
Does this multi-line comment format conform to the BPF style guide?

As with the previous comment in exceptions_cleanup.c, the BPF subsystem
mandates that the opening /* resides on its own line for multi-line
comments.

> +	".8byte 0x20d;"
> +"l0_%=:"
> +	"call bpf_unwind_resume;"
> +	"exit;"
> +"l1_%=:"
> +	"call bpf_unwind_resume;"
> +	"exit;"
> +	CLEANUP_REC("1b", "2b", "3b")
> +	::: __clobber_all);
> +}
> +
> +#endif /* x86 || arm64 */
> +
> +char _license[] SEC("license") = "GPL";

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