Re: [PATCH v6 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest

[Nnamdi Onyeyiri]

On Mon, Jul 20, 2026 at 05:47:09PM -0400, Emil Tsalapatis wrote:
> On Mon Jul 20, 2026 at 1:15 PM EDT, Nnamdi Onyeyiri wrote:
> > These selftests exercise the tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser()
> > functions, to ensure that they are properly handling spurious wakeups in
> > tcp_msg_wait_data().
> >
> > The expected behaviour is that recvfrom() does not return an EAGAIN
> > error.  If the spurious wakeups are incorrectly handled, this assertion
> > will fail.
> >
> > Signed-off-by: Nnamdi Onyeyiri <nnamdio@xxxxxxxxx>
> 
> The test looks fine, even if slightly flaky. Running with only patch 2/2
> still passes sometimes on my box. Can we handle this somehow, e.g., do
> more attempts?
> 
> There's also a couple magic numbers in the tests that may need some
> explanation (noted below).
>

Thanks for the review!  I'll attach an explaination to the numbers.
Essentially the larger the payload the fewer iterations seemed to be
required to reproduce (on my machine).

With the issue fixed though, larger payloads and more iterations make
the test run longer.  Is there is a rule of thumb I should follow for
tuning the runtime?

> > ---
> >  .../selftests/bpf/prog_tests/sockmap_basic.c  | 124 ++++++++++++++++++
> >  1 file changed, 124 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > index cb3229711f93..d18faf46fac0 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > @@ -1373,6 +1373,126 @@ static void test_sockmap_multi_channels(int sotype)
> >  	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);
> > +
> > +	close(fd);
> > +
> > +	return result;
> > +}
> > +
> > +static void test_sockmap_recvfrom_eagain(bool with_verdict)
> > +{
> > +	struct test_sockmap_pass_prog *skel = NULL;
> > +	struct bpf_program *prog = NULL;
> > +	size_t buflen = 1024 * 1024 * 25;
> 
> Here
> 
> > +	char *buf = NULL;
> > +	int map, err;
> > +
> > +	skel = test_sockmap_pass_prog__open_and_load();
> > +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> > +		return;
> > +
> > +	map = bpf_map__fd(skel->maps.sock_map_msg);
> > +
> > +	if (with_verdict) {
> > +		prog = skel->progs.prog_skb_verdict;
> > +		err = bpf_prog_attach(bpf_program__fd(prog), map, BPF_SK_SKB_STREAM_VERDICT, 0);
> > +		if (!ASSERT_OK(err, "bpf_prog_attach verdict"))
> > +			goto cleanup;
> > +	}
> > +
> > +	buf = malloc(buflen);
> > +	if (!ASSERT_OK_PTR(buf, "malloc buf"))
> > +		goto cleanup;
> > +	memset(buf, 0, buflen);
> > +	buf[buflen - 1] = 'e';
> > +
> > +	for (int i = 0; i < 200; ++i) {
> 
> Also here. Why 200 iterations specifically? Can we at least name the
> defaults to make it clearer that we've chosen those numbers because
> that's how we trigger the bug?
> 
> > +		ssize_t sent;
> > +		char ignored[128];
> > +		pthread_t thread;
> > +		bool thread_created = false;
> > +		size_t rem = buflen;
> > +		int c = -1, p = -1, zero = 0;
> > +		bool success = false;
> > +
> > +		err = create_pair(AF_INET, SOCK_STREAM, &c, &p);
> > +		if (!ASSERT_OK(err, "create_pair"))
> > +			goto end_attempt;
> > +
> > +		err = pthread_create(&thread, NULL, &test_sockmap_recvfrom_eagain_thread, &p);
> > +		if (!ASSERT_OK(err, "pthread_create"))
> > +			goto end_attempt;
> > +		thread_created = true;
> > +
> > +		err = bpf_map_update_elem(map, &zero, &c, BPF_ANY);
> > +		if (!ASSERT_OK(err, "bpf_map_update_elem"))
> > +			goto end_attempt;
> > +
> > +		while (rem) {
> > +			sent = xsend(c, buf + (buflen - rem), rem, 0);
> > +			if (sent == -1)
> > +				goto end_attempt;
> > +			rem -= sent;
> > +		}
> > +
> > +		/* we cannot use recv_timeout(), otherwise EAGAIN would be an expected errno. */
> > +		err = recvfrom(c, ignored, sizeof(ignored), 0, NULL, NULL);
> > +
> > +		/*
> > +		 * we are checking for the invalid return of EAGAIN, any other return is considered
> > +		 * successful for the purposes of this test.
> > +		 */
> > +		if (err < 0 && !ASSERT_NEQ(errno, EAGAIN, "recvfrom eagain"))
> > +			goto end_attempt;
> > +
> > +		success = true;
> > +
> > +end_attempt:
> > +		if (c >= 0)
> > +			close(c);
> > +
> > +		if (thread_created) {
> > +			void *retval = NULL;
> > +
> > +			pthread_join(thread, &retval);
> > +			if (!ASSERT_NULL(retval, "retval"))
> > +				success = false;
> > +		}
> > +
> > +		if (!thread_created && p >= 0)
> > +			close(p);
> > +		if (!success)
> > +			break;
> > +	}
> > +
> > +cleanup:
> > +	if (buf)
> > +		free(buf);
> > +
> > +	test_sockmap_pass_prog__destroy(skel);
> > +}
> > +
> >  void test_sockmap_basic(void)
> >  {
> >  	if (test__start_subtest("sockmap create_update_free"))
> > @@ -1451,4 +1571,8 @@ void test_sockmap_basic(void)
> >  		test_sockmap_multi_channels(SOCK_STREAM);
> >  	if (test__start_subtest("sockmap udp multi channels"))
> >  		test_sockmap_multi_channels(SOCK_DGRAM);
> > +	if (test__start_subtest("sockmap recvfrom eagain"))
> > +		test_sockmap_recvfrom_eagain(false);
> > +	if (test__start_subtest("sockmap recvfrom eagain with verdict"))
> > +		test_sockmap_recvfrom_eagain(true);
> >  }
>