From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 17184400DF0 for ; Mon, 20 Jul 2026 22:07:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784585252; cv=none; b=VS0IWufUKYcYcs2wc6ELkyySYOBIdbz1RNSxfXRuzsiYsjo6Ch5gf7tNSBEB8Hc2g3TEtL51gJxT0HyGqRJZ1AWky1Wa3VDfQp6KQv0rpDCByUOk8qsfJBh3U/Av9bwOIgcnGCe3yh//oVheOlU9hmSEsyL0qkDffJ+daLot6L0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784585252; c=relaxed/simple; bh=7Ufq8Gqdt0Yq2rwd1UOICRPhbWWW4yi1uYk/x/Ivq1c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=EOoNBLjuV/mKJ8bcRfgDk72cfqDJveAKRhZsFye8DrhSz7IrFtLnKwFJoHbZTOk6k6Q5OGSmTPGkD4zU6EsO3YbYW3CZ5JVb/6Ep4iemLJxtIYp5GjO1bbSTEvRQxqq3yWaYLvYqfU6plG0SziRhPo0yag1XvkKCWv7AT8bQGas= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=nKolN0IO; arc=none smtp.client-ip=209.85.221.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="nKolN0IO" Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-472326ca506so8214236f8f.2 for ; Mon, 20 Jul 2026 15:07:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1784585248; x=1785190048; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=mhIAwX66RMzzqWQQBA+7hZ0C/T1kQn62KHsmHZuPTAU=; b=nKolN0IOac9b2Eh97wPdyGzj3PvPw4qDxfpItQWWkX1zr01dnKXrloHfvFVWb2X4NY 1k+Fifj1w9WHLMHAiD9aThaJxgnzyT1SZkxgnQthMJ0IZH3V1v1WfDPCoN7+f161smKI OKCdpYGXi5eIDKtArBR/NLktsaHtaeQgV5eYXzmmTDsrzJWaI80r9GW2GrnjDCglUNkI MuB0RvmI5+1p45YpjABgSpkUyhN//w4WzQ5iRLOOnxiLr/DPNUhHt1WZKzfQ7enaK6L/ +d7XRfwJGTVeWKG4KFQvKdhuhmUsISwYHjPh95JaMA3Gx0CDjP0CgUCW0iNsrwwrT9aB 5rHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784585248; x=1785190048; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=mhIAwX66RMzzqWQQBA+7hZ0C/T1kQn62KHsmHZuPTAU=; b=sBAcgBIsNsZ581reR2eo9HmChrfskPP6FUMq4LMy+o0Aaq2G6YIzGNNtNFzOifzY7M Ptn89wujwMRwJuBsdn+LvlmRjqwr/9jLhBOhmNrx/rMJbPBtGVEtxEo1IQSF/JmQrPZo 1VMJwT18RkUA3HJT+uLx3M7yn0aF+kyVyZ7OiR4wwhtUtuJ8dG9vQXr8do/MohstvseY bFwYSM71kfBHatjb3vWaUXe2n+oS5GyyGz5rfZrzvZvZlYLAUg8OzRTrKh1RVQ6Pchud df0W+5cZIne8AnqPK3QSOW1NiueHOD9hm79+Ij+g1j2tr4MZo09hcAgF0W2vrnj9XPUc NYIw== X-Forwarded-Encrypted: i=1; AHgh+RrItVe7lcins3UTF+K6Bf1I9CsZ7pnRvo+aHnziROkLK+Gc7iIebA1yRyoHHHwMil9F2ztiqUfzfGZJbVs=@vger.kernel.org X-Gm-Message-State: AOJu0Yzyi1OA3RwWS+F3FTj6aQoiUZgQfJEje3J4jZ5NCtg5j6tFt61y WlnDoHi8bAZ93cDK2HGiUa5qkKqAMS+I7RUEoUA4534adEucGFgMJuZu X-Gm-Gg: AR+sD10cRwAOXkQ6XwjfYEBvqNjt4JzSQZuprhkHVQNTE820HNaqosHOT3/OrU30jl3 4BnRLU/baHUqh71jyujzn5xEEWUbPMAfFaJXXVg9po9kndJhU1Ax95IeKGlMNzGkR9qdyQXeTHh +xxvnp35e+7+7UdSC9dIaNwdmjB9fFtNYpsBsMZ4oewhod+Er+2+vQUxbVmVf3kcACSF57D698W eQM4X/2TiICEr8NS6rAeailKuqB+LxJcel/YiCqtz39raTyeXXly0WJqADBPIIbCU053PDUeZ7x SJN+mXsYNnhPSaw5PHaSZnOzm5jzuocDJUSNGjAQaA/DtigVDYvdaIgxsqB3ASw0xC0o8MsJlQK r9Ob34XJGTx0TatNosFegy4kpWVZacLTfgyVycS/6X7qiaq6ybW2H4U1ckW7Ugkxw64trxM3Ov0 CN5t/u8tkaJy/Z8SQMkvfWQgXaFuEajA/NfWGwoq7WVhmcXkmvTSq8/YF9kvfZSHUekA== X-Received: by 2002:a5d:5d85:0:b0:47f:6d55:a063 with SMTP id ffacd0b85a97d-47f6d55a3f6mr13332571f8f.32.1784585248103; Mon, 20 Jul 2026 15:07:28 -0700 (PDT) Received: from localhost.localdomain (host86-153-201-25.range86-153.btcentralplus.com. [86.153.201.25]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f63ec7c69sm35467457f8f.17.2026.07.20.15.07.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Jul 2026 15:07:27 -0700 (PDT) Date: Mon, 20 Jul 2026 23:07:25 +0100 From: Nnamdi Onyeyiri To: Emil Tsalapatis 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 Message-ID: References: <20260720171535.67867-1-nnamdio@gmail.com> <20260720171535.67867-3-nnamdio@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 > > 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); > > } >