Re: [PATCH bpf 2/2] selftests/bpf: Test bpf_sock_destroy() on a TIME_WAIT sock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test creates a simultaneous close race condition that will cause it to randomly fail by observing the socket in TCP_CLOSING state instead of TCP_TIME_WAIT.
- [Low] The multi-line comment uses the incorrect style for the BPF subsystem.
--

commit c91152edb079a3ce44e0f45e8d080efb432c5870
Author: Jiayuan Chen <jiayuan.chen@linux.dev>
Date:   Thu Sep 3 12:52:49 2026 +0000

selftests/bpf: Test bpf_sock_destroy() on a TIME_WAIT sock

This commit adds a tcp_timewait subtest to verify that bpf_sock_destroy()
correctly handles TIME_WAIT sockets. A tcp iterator finds the timewait
socket by its inherited cookie and destroys it.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
> index 9c11938fe597d..6ccc7cda410bf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
[ ... ]
> @@ -110,6 +110,65 @@ static void test_tcp_server(struct sock_destroy_prog *skel)
[ ... ]
> +	/* Active close from the client, then close the server side. Once
> +	 * recv() sees EOF the server FIN has been processed and the client
> +	 * sock is in TIME_WAIT. Block without timeout so a loaded CI box
> +	 * can't race us.
> +	 */

[Severity: Low]
Does this multi-line comment follow the BPF subsystem coding style? The
guidelines prefer placing the opening /* on its own line before starting
the comment text.

> +	if (!ASSERT_OK(setsockopt(clien, SOL_SOCKET, SO_RCVTIMEO, &tv,
> +				  sizeof(tv)), "clear rcvtimeo"))
> +		goto cleanup;
> +	if (!ASSERT_OK(shutdown(clien, SHUT_WR), "client shutdown"))
> +		goto cleanup;
> +
> +	close(accept_serv);
> +	accept_serv = -1;
> +
> +	/* block until return EOF */
> +	n = recv(clien, buf, sizeof(buf), 0);

[Severity: Medium]
Could this sequence create a simultaneous close race condition that leads to
random test failures?

If shutdown(clien, SHUT_WR) sends a FIN, and the test immediately calls
close(accept_serv) on the server without waiting for the client's FIN to be
processed, the server might send its FIN before receiving the client's FIN.

If these FINs cross (simultaneous close), the client will receive the
server's FIN, enter the TCP_CLOSING state, and wake up the blocking recv()
call. The subsequent BPF iterator would then run while the socket is still
in TCP_CLOSING rather than TCP_TIME_WAIT.

Because the socket is in TCP_CLOSING, bpf_skc_to_tcp_timewait_sock() will
return NULL, causing the iterator to miss the socket and the assertion
ASSERT_EQ(skel->bss->tw_found, 1) to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903125306.299943-1-jiayuan.chen@linux.dev?part=2