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);
> }
next prev parent 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