Re: [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test silently ignores non-EAGAIN errors from recvfrom(), incorrectly treating socket failures or premature EOFs as a test success.
- [Medium] Test suite hangs indefinitely if the receiver thread encounters an error during recvfrom() and exits early.
--

--- Patch [2]: [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain ---
commit 5b1a8047d6f2573ba61a579d78502e3442c615cc
Author: Nnamdi Onyeyiri <nnamdio@gmail.com>

selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain

This commit adds a selftest to ensure that tcp_bpf_recvmsg() and
tcp_bpf_recvmsg_parser() correctly handle spurious wakeups in
tcp_msg_wait_data(). It verifies that recvfrom() does not incorrectly return
EAGAIN.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index e5fc038d747b6..c38d1514e2533 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c

[ ... ]

> @@ -1410,6 +1410,117 @@ static void test_sockmap_no_verdict_fionread(void)
>  	test_sockmap_pass_prog__destroy(skel);
>  }
>  
> +static void *test_sockmap_recvfrom_eagain_thread(void *arg)
> +{
> +	int fd = *(int *)arg;
> +	char buf[1024];
> +	void *result = NULL;
> +
> +	while (true) {
> +		ssize_t len = recvfrom(fd, buf, sizeof(buf), 0, NULL, NULL);
> +
> +		if (len == -1) {
> +			if (errno == EINTR)
> +				continue;
> +			result = (void *)1;
> +			break;
> +		}
> +
> +		if (!len || buf[len - 1] == 'e')
> +			break;
> +	}
> +
> +	send(fd, "test", 4, MSG_NOSIGNAL);
> +
> +	return result;

[Severity: Medium]
If the receiver thread encounters an unexpected error (e.g., -1 from recvfrom),
it breaks out of the loop and exits without closing the file descriptor fd.

Meanwhile, the main thread attempts to send 25MB of data via a blocking xsend().
Since fd remains open but is no longer being read, the TCP receive window fills
up, and the main thread will block indefinitely in xsend().

Should the socket fd be closed before returning to prevent the test suite from
hanging?

> +}

[ ... ]

> +		while (rem) {
> +			sent = xsend(c, buf + (buflen - rem), rem, 0);
> +			if (sent == -1)
> +				goto end_attempt;
> +			rem -= sent;
> +		}
> +
> +		err = recvfrom(c, ignored, sizeof(ignored), 0, NULL, NULL);
> +		if (err < 0 && !ASSERT_NEQ(errno, EAGAIN, "recvfrom eagain"))
> +			goto end_attempt;
> +
> +		success = true;

[Severity: Medium]
Does this logic silently ignore errors from recvfrom() when errno is not EAGAIN?

If recvfrom() fails with an error other than EAGAIN, ASSERT_NEQ(errno, EAGAIN)
evaluates to true. The logical NOT operator negates this to false, causing the
if condition to fail.

The code then falls through to success = true, which treats the failure as a
successful test pass.

> +
> +end_attempt:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717155348.54975-1-nnamdio@gmail.com?part=2