Netdev List
 help / color / mirror / Atom feed
From: "Emil Tsalapatis" <emil@etsalapatis.com>
To: "Nnamdi Onyeyiri" <nnamdio@gmail.com>
Cc: <bpf@vger.kernel.org>, <davem@davemloft.net>,
	<edumazet@google.com>, <horms@kernel.org>, <jakub@cloudflare.com>,
	<jiayuan.chen@linux.dev>, <john.fastabend@gmail.com>,
	<kuba@kernel.org>, <kuniyu@google.com>, <ncardwell@google.com>,
	<netdev@vger.kernel.org>, <pabeni@redhat.com>,
	<sashiko-reviews@lists.linux.dev>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest
Date: Mon, 20 Jul 2026 17:47:09 -0400	[thread overview]
Message-ID: <DK3Q8PFVGNAV.GZNZC89346A9@etsalapatis.com> (raw)
In-Reply-To: <20260720171535.67867-3-nnamdio@gmail.com>

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@gmail.com>

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).

> ---
>  .../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);
>  }


  reply	other threads:[~2026-07-20 21:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 17:15 [PATCH v6 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
2026-07-20 17:15 ` [PATCH v6 1/2] " Nnamdi Onyeyiri
2026-07-20 21:16   ` Emil Tsalapatis
2026-07-20 22:53     ` Nnamdi Onyeyiri
2026-07-20 23:58       ` Emil Tsalapatis
2026-07-20 17:15 ` [PATCH v6 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest Nnamdi Onyeyiri
2026-07-20 21:47   ` Emil Tsalapatis [this message]
2026-07-20 22:07     ` Nnamdi Onyeyiri
2026-07-20 22:17       ` Emil Tsalapatis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DK3Q8PFVGNAV.GZNZC89346A9@etsalapatis.com \
    --to=emil@etsalapatis.com \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jakub@cloudflare.com \
    --cc=jiayuan.chen@linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=nnamdio@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox