Re: [RFC PATCH bpf-next 5/5] selftests/bpf: set SO_BUSY_POLL from the tcp_splice sockops prog

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The BPF global variable `busy_poll_us` is never initialized by the userspace test runner, rendering the newly added `SO_BUSY_POLL` test logic unreachable dead code.
- [Low] Multi-line comment does not follow BPF subsystem guidelines.
--

commit c9396935bece9a3118a940f3f318b60c9109d778
Author: Cong Wang <xiyou.wangcong@gmail.com>

selftests/bpf: set SO_BUSY_POLL from the tcp_splice sockops prog

Set SO_BUSY_POLL (busy_poll_us) on each paired socket via
bpf_setsockopt() so the splice receiver busy-polls the ring instead of
parking - without net.core.busy_read or an application setsockopt.

> diff --git a/tools/testing/selftests/bpf/progs/test_tcp_splice.c b/tools/testing/selftests/bpf/progs/test_tcp_splice.c
> index 09c7f0f9e3110..da43f00046c0f 100644
> --- a/tools/testing/selftests/bpf/progs/test_tcp_splice.c
> +++ b/tools/testing/selftests/bpf/progs/test_tcp_splice.c

[ ... ]

> @@ -71,6 +80,21 @@ int sockops_splice(struct bpf_sock_ops *skops)
>  	if (skops->family != 2 /* AF_INET */)
>  		return 0;
>  
> +	/* Enable busy-poll on this socket. Both endpoints run this callback,
> +	 * so each sets its own socket; this must happen here, before the

[Severity: Low]
This isn't a bug, but the BPF subsystem multi-line comment style requires
the opening /* to be on its own line. Should this be updated to match the
modern kernel comment style?

> +	 * peer-not-found early return below, because pairing is asymmetric -
> +	 * only the second side to establish finds a peer and calls
> +	 * bpf_sock_splice_pair. Setting it only on the pairing side would
> +	 * leave the other side without busy-poll. bpf_setsockopt acts on
> +	 * skops->sk only - there is no variant to set the peer - but the peer
> +	 * sets itself when its own ESTABLISHED callback fires.
> +	 */
> +	if (busy_poll_us) {
> +		int us = busy_poll_us;
> +
> +		bpf_setsockopt(skops, SOL_SOCKET, SO_BUSY_POLL, &us, sizeof(us));
> +	}

[Severity: Medium]
Is the global variable busy_poll_us ever initialized by the userspace test
runner?

Since BPF .bss variables are zero-initialized by default, it appears that
skel->bss->busy_poll_us is never assigned a value in the userspace setup
code (like prog_tests/tcp_splice.c). Does this mean the if (busy_poll_us)
condition will always evaluate to false, leaving the new SO_BUSY_POLL logic
unreachable and untested?

> +
>  	mk_key(skops, &self_key, 0);
>  	mk_key(skops, &peer_key, 1);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612011452.134466-1-xiyou.wangcong@gmail.com?part=5