* [PATCH v7 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
@ 2026-07-21 22:38 Nnamdi Onyeyiri
2026-07-21 22:38 ` [PATCH v7 1/2] " Nnamdi Onyeyiri
2026-07-21 22:38 ` [PATCH v7 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest Nnamdi Onyeyiri
0 siblings, 2 replies; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-21 22:38 UTC (permalink / raw)
To: nnamdio
Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
linux-kernel, emil
Spurious wakeups in tcp_msg_wait_data() aren't being handled by
tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser(), leading to unexpected
EAGAIN errors returned by recvfrom()/recv(). Adding handling for the
wakeup and a selftest.
This issue was first discovered in an application that adds sockets to
a sockmap as a way to view the received data. No redirects or any
other operations are performed.
Sashiko has noted a few other pre-existing issues in the same area
(https://patch.msgid.link/20260714205118.17DB11F000E9@smtp.kernel.org)
that can lead to EAGAIN in tcp_bpf_recvmsg() and
tcp_bpf_recvmsg_parser(). A local run of Sashiko also identified a
potential issue in tcp_bpf_recvmsg() handling zero-data FIN packets when
the MSG_PEEK flag is set. To prevent this patchset from growing too
large, I intend to submit follow up patches to address these once this
one has been accepted.
Changes in v7:
- In tcp_bpf_recvmsg check for data in the TCP_CLOSE, !timeo and
signal_pending branches.
- Refactored changes to tcp_bpf_recvmsg to reduce duplication.
- Increase the number of iterations in the selftests.
- Documented selftests numeric constants.
- Link to v6: https://patch.msgid.link/20260720171535.67867-1-nnamdio@gmail.com
Changes in v6:
- Closing the file descriptor in the selftest worker thread.
- Ensuring the selftest loop breaks early for an error in the worker
thread.
- Added comments to selftest regarding focus on EAGAIN error.
- Updated selftest commit message to imperative mood.
- Link to v5: https://patch.msgid.link/20260717155348.54975-1-nnamdio@gmail.com
Changes in v5:
- Move selftest from net into bpf/prog_tests/sockmap_basic.c
- Link to v4: https://patch.msgid.link/20260715213538.37229-1-nnamdio@gmail.com
Changes in v4:
- Fix potential data loss in tcp_bpf_recvmsg() when a FIN or RST has
been received.
- Check the return code of pthread_create() in the selftest.
- Fix race caused by using EXPECT macros in a thread in selftest.
- Link to v3: https://patch.msgid.link/20260714203927.32289-1-nnamdio@gmail.com
Changes in v3:
- Added the sockmap_recvfrom selftest.
- Link to v2: https://patch.msgid.link/alFRK66z45eDNZA7@localhost.localdomain
Changes in v2:
- In tcp_bpf_recvmsg, handle signals and the socket closing in the loop.
- Fix spurious wakeups when SO_RCVTIMEO has been set on the socket.
- Link to v1: https://patch.msgid.link/ak_rR-Skd8Mvn4mH@localhost.localdomain
Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
---
Nnamdi Onyeyiri (2):
bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
selftests/bpf: add sockmap recvfrom EAGAIN selftest
net/ipv4/tcp_bpf.c | 72 ++++++++--
.../selftests/bpf/prog_tests/sockmap_basic.c | 136 ++++++++++++++++++
2 files changed, 199 insertions(+), 9 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
2026-07-21 22:38 [PATCH v7 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
@ 2026-07-21 22:38 ` Nnamdi Onyeyiri
2026-07-21 22:55 ` sashiko-bot
2026-07-22 11:01 ` Jakub Sitnicki
2026-07-21 22:38 ` [PATCH v7 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest Nnamdi Onyeyiri
1 sibling, 2 replies; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-21 22:38 UTC (permalink / raw)
To: nnamdio
Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
linux-kernel, emil
recvfrom()/recv() are documented as only returning EAGAIN for blocking
sockets when they have a receive timeout configured. However, adding a
blocking ipv4 tcp socket without a receive timeout to a sockmap will cause
EAGAIN errors sporadically. A socket with a receive timeout may return
EAGAIN before the timeout expires.
There are 2 code paths affected by this:
1. tcp_bpf_recvmsg() - Used when the socket has been added to a sockmap
that has no verdict program attached.
2. tcp_bpf_recvmsg_parser() - Used when the socket has been added to a
sockmap that has a verdict program. To reproduce this issue, it is
enough for the verdict program to do nothing but return SK_PASS.
In both cases this happens when tcp_msg_wait_data() wakes spuriously
(returning 0). To fix it, we now loop back to msg_bytes_ready instead
of returning -EAGAIN on spurious wakeup.
To ensure the looping does not cause sockets with a SO_RCVTIMEO set to
wait excessively long, tcp_msg_wait_data() now takes a pointer to timeo,
allowing sk_wait_event() to update it as appropriate.
The logic in tcp_bpf_recvmsg_parser() that allow it to handle signals,
socket errors and closuers in its loop was also added to tcp_bpf_recvmsg().
Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
---
net/ipv4/tcp_bpf.c | 72 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 63 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f36b6..1755fcf726fc 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -179,7 +179,7 @@ EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
#ifdef CONFIG_BPF_SYSCALL
static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
- long timeo)
+ long *timeo)
{
DEFINE_WAIT_FUNC(wait, woken_wake_function);
int ret = 0;
@@ -187,12 +187,12 @@ static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
if (sk->sk_shutdown & RCV_SHUTDOWN)
return 1;
- if (!timeo)
+ if (!*timeo)
return ret;
add_wait_queue(sk_sleep(sk), &wait);
sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
- ret = sk_wait_event(sk, &timeo,
+ ret = sk_wait_event(sk, timeo,
!list_empty(&psock->ingress_msg) ||
!skb_queue_empty_lockless(&sk->sk_receive_queue), &wait);
sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
@@ -229,6 +229,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
int copied_from_self = 0;
int copied = 0;
u32 seq;
+ long timeo;
if (unlikely(flags & MSG_ERRQUEUE))
return inet_recv_error(sk, msg, len);
@@ -262,6 +263,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
}
}
+ timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
msg_bytes_ready:
copied = __sk_msg_recvmsg(sk, psock, msg, len, flags, &copied_from_self);
/* The typical case for EFAULT is the socket was gracefully
@@ -280,7 +283,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
}
seq += copied_from_self;
if (!copied) {
- long timeo;
int data;
if (sock_flag(sk, SOCK_DONE))
@@ -299,7 +301,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
goto out;
}
- timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
if (!timeo) {
copied = -EAGAIN;
goto out;
@@ -310,13 +311,15 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
goto out;
}
- data = tcp_msg_wait_data(sk, psock, timeo);
+ data = tcp_msg_wait_data(sk, psock, &timeo);
if (data < 0) {
copied = data;
goto unlock;
}
if (data && !sk_psock_queue_empty(psock))
goto msg_bytes_ready;
+ if (!data && timeo > 0)
+ goto msg_bytes_ready;
copied = -EAGAIN;
}
out:
@@ -355,6 +358,7 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
{
struct sk_psock *psock;
int copied, ret;
+ long timeo;
if (unlikely(flags & MSG_ERRQUEUE))
return inet_recv_error(sk, msg, len);
@@ -371,14 +375,52 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
return tcp_recvmsg(sk, msg, len, flags);
}
lock_sock(sk);
+
+ timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
msg_bytes_ready:
copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
if (!copied) {
- long timeo;
int data;
- timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
- data = tcp_msg_wait_data(sk, psock, timeo);
+ if (sock_flag(sk, SOCK_DONE)) {
+ ret = 0;
+ goto check_queues;
+ }
+
+ if (sk->sk_err) {
+ if (!sk_psock_queue_empty(psock))
+ goto msg_bytes_ready;
+ if (!skb_queue_empty(&sk->sk_receive_queue)) {
+ release_sock(sk);
+ sk_psock_put(sk, psock);
+ return tcp_recvmsg(sk, msg, len, flags);
+ }
+ ret = sock_error(sk);
+ goto unlock;
+ }
+
+ if (sk->sk_shutdown & RCV_SHUTDOWN) {
+ ret = 0;
+ goto check_queues;
+ }
+
+ if (sk->sk_state == TCP_CLOSE) {
+ ret = -ENOTCONN;
+ goto check_queues;
+ }
+
+ if (!timeo) {
+ ret = -EAGAIN;
+ goto check_queues;
+ }
+
+ if (signal_pending(current)) {
+ ret = sock_intr_errno(timeo);
+ goto check_queues;
+ }
+
+ data = tcp_msg_wait_data(sk, psock, &timeo);
if (data < 0) {
ret = data;
goto unlock;
@@ -390,6 +432,8 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
sk_psock_put(sk, psock);
return tcp_recvmsg(sk, msg, len, flags);
}
+ if (!data && timeo > 0)
+ goto msg_bytes_ready;
copied = -EAGAIN;
}
ret = copied;
@@ -398,6 +442,16 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
release_sock(sk);
sk_psock_put(sk, psock);
return ret;
+
+check_queues:
+ if (!sk_psock_queue_empty(psock))
+ goto msg_bytes_ready;
+ if (!skb_queue_empty(&sk->sk_receive_queue)) {
+ release_sock(sk);
+ sk_psock_put(sk, psock);
+ return tcp_recvmsg(sk, msg, len, flags);
+ }
+ goto unlock;
}
static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v7 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest
2026-07-21 22:38 [PATCH v7 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
2026-07-21 22:38 ` [PATCH v7 1/2] " Nnamdi Onyeyiri
@ 2026-07-21 22:38 ` Nnamdi Onyeyiri
1 sibling, 0 replies; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-21 22:38 UTC (permalink / raw)
To: nnamdio
Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
linux-kernel, emil
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>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 136 ++++++++++++++++++
1 file changed, 136 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..afda4d90e573 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1373,6 +1373,138 @@ 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;
+ char *buf = NULL;
+ int map, err;
+
+ /*
+ * the size of the buffer to send. completion of the sending can trigger a spurious wake
+ * up. larger values make the issue more likely, but make successful test runs longer.
+ */
+ const size_t buflen = 1024 * 1024 * 25;
+
+ /*
+ * maximum number of attempts to reproduce EAGAIN on spurious wake up. more attempts
+ * increases the chances of triggering the issue, and the test ends as soon as we do.
+ * however, successful runs would take longer to complete.
+ */
+ const int max_attempts = 300;
+
+ 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_rx);
+
+ 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 < max_attempts; ++i) {
+ ssize_t len;
+ 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) {
+ len = xsend(c, buf + (buflen - rem), rem, 0);
+ if (len == -1)
+ goto end_attempt;
+ rem -= len;
+ }
+
+ /* we cannot use recv_timeout(), otherwise EAGAIN would be an expected errno. */
+ len = 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 (len < 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 +1583,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);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
2026-07-21 22:38 ` [PATCH v7 1/2] " Nnamdi Onyeyiri
@ 2026-07-21 22:55 ` sashiko-bot
2026-07-22 11:01 ` Jakub Sitnicki
1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-21 22:55 UTC (permalink / raw)
To: Nnamdi Onyeyiri; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] tcp_bpf_recvmsg_parser() fails to handle unparsed data in sk_receive_queue, returning premature EAGAIN on blocking sockets.
--
--- Patch [1]: [PATCH v7 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup ---
commit 7468e0f3ba324455b02b600d4c77965cd4caa92c
Author: Nnamdi Onyeyiri <nnamdio@gmail.com>
bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
This commit addresses spurious wakeups from tcp_msg_wait_data() by looping
back to msg_bytes_ready instead of returning -EAGAIN. It also passes a pointer
for timeo to allow sk_wait_event() to update the remaining timeout.
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 8e905b50deadb..87aef99a13792 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
[ ... ]
> @@ -310,13 +311,15 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> goto out;
> }
>
> - data = tcp_msg_wait_data(sk, psock, timeo);
> + data = tcp_msg_wait_data(sk, psock, &timeo);
> if (data < 0) {
> copied = data;
> goto unlock;
> }
> if (data && !sk_psock_queue_empty(psock))
> goto msg_bytes_ready;
> + if (!data && timeo > 0)
> + goto msg_bytes_ready;
[Severity: High]
This is a pre-existing issue, but could this return a premature -EAGAIN on a
blocking socket when data is waiting to be parsed?
If data arrives and is added to sk_receive_queue, but a stream parser requires
more bytes to complete a message (or the parser is deferred to a workqueue),
tcp_msg_wait_data() will return 1.
Because psock->ingress_msg is still empty, both the existing condition:
if (data && !sk_psock_queue_empty(psock))
and the new condition:
if (!data && timeo > 0)
will evaluate to false.
Does this mean tcp_bpf_recvmsg_parser() falls through and returns -EAGAIN
instead of continuing to wait for the remaining data to be parsed?
> copied = -EAGAIN;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721223807.75101-1-nnamdio@gmail.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
2026-07-21 22:38 ` [PATCH v7 1/2] " Nnamdi Onyeyiri
2026-07-21 22:55 ` sashiko-bot
@ 2026-07-22 11:01 ` Jakub Sitnicki
2026-07-22 11:22 ` Nnamdi Onyeyiri
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Sitnicki @ 2026-07-22 11:01 UTC (permalink / raw)
To: Nnamdi Onyeyiri
Cc: bpf, davem, edumazet, horms, jiayuan.chen, john.fastabend, kuba,
kuniyu, ncardwell, netdev, pabeni, sashiko-reviews, linux-kernel,
emil
On Tue, Jul 21, 2026 at 11:38 PM +01, Nnamdi Onyeyiri wrote:
> recvfrom()/recv() are documented as only returning EAGAIN for blocking
> sockets when they have a receive timeout configured. However, adding a
> blocking ipv4 tcp socket without a receive timeout to a sockmap will cause
> EAGAIN errors sporadically. A socket with a receive timeout may return
> EAGAIN before the timeout expires.
>
> There are 2 code paths affected by this:
>
> 1. tcp_bpf_recvmsg() - Used when the socket has been added to a sockmap
> that has no verdict program attached.
>
> 2. tcp_bpf_recvmsg_parser() - Used when the socket has been added to a
> sockmap that has a verdict program. To reproduce this issue, it is
> enough for the verdict program to do nothing but return SK_PASS.
>
> In both cases this happens when tcp_msg_wait_data() wakes spuriously
> (returning 0). To fix it, we now loop back to msg_bytes_ready instead
> of returning -EAGAIN on spurious wakeup.
>
> To ensure the looping does not cause sockets with a SO_RCVTIMEO set to
> wait excessively long, tcp_msg_wait_data() now takes a pointer to timeo,
> allowing sk_wait_event() to update it as appropriate.
>
> The logic in tcp_bpf_recvmsg_parser() that allow it to handle signals,
> socket errors and closuers in its loop was also added to tcp_bpf_recvmsg().
>
> Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
> ---
> net/ipv4/tcp_bpf.c | 72 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 63 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6..1755fcf726fc 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -179,7 +179,7 @@ EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
>
> #ifdef CONFIG_BPF_SYSCALL
> static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
> - long timeo)
> + long *timeo)
> {
> DEFINE_WAIT_FUNC(wait, woken_wake_function);
> int ret = 0;
> @@ -187,12 +187,12 @@ static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
> if (sk->sk_shutdown & RCV_SHUTDOWN)
> return 1;
>
> - if (!timeo)
> + if (!*timeo)
> return ret;
>
> add_wait_queue(sk_sleep(sk), &wait);
> sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
> - ret = sk_wait_event(sk, &timeo,
> + ret = sk_wait_event(sk, timeo,
> !list_empty(&psock->ingress_msg) ||
> !skb_queue_empty_lockless(&sk->sk_receive_queue), &wait);
> sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
> @@ -229,6 +229,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> int copied_from_self = 0;
> int copied = 0;
> u32 seq;
> + long timeo;
>
> if (unlikely(flags & MSG_ERRQUEUE))
> return inet_recv_error(sk, msg, len);
> @@ -262,6 +263,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> }
> }
>
> + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> +
> msg_bytes_ready:
> copied = __sk_msg_recvmsg(sk, psock, msg, len, flags, &copied_from_self);
> /* The typical case for EFAULT is the socket was gracefully
> @@ -280,7 +283,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> }
> seq += copied_from_self;
> if (!copied) {
> - long timeo;
> int data;
>
> if (sock_flag(sk, SOCK_DONE))
> @@ -299,7 +301,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> goto out;
> }
>
> - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> if (!timeo) {
> copied = -EAGAIN;
> goto out;
> @@ -310,13 +311,15 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> goto out;
> }
>
> - data = tcp_msg_wait_data(sk, psock, timeo);
> + data = tcp_msg_wait_data(sk, psock, &timeo);
> if (data < 0) {
> copied = data;
> goto unlock;
> }
> if (data && !sk_psock_queue_empty(psock))
> goto msg_bytes_ready;
> + if (!data && timeo > 0)
> + goto msg_bytes_ready;
> copied = -EAGAIN;
> }
> out:
> @@ -355,6 +358,7 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> {
> struct sk_psock *psock;
> int copied, ret;
> + long timeo;
>
> if (unlikely(flags & MSG_ERRQUEUE))
> return inet_recv_error(sk, msg, len);
> @@ -371,14 +375,52 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> return tcp_recvmsg(sk, msg, len, flags);
> }
> lock_sock(sk);
> +
> + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> +
> msg_bytes_ready:
> copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> if (!copied) {
> - long timeo;
> int data;
>
> - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> - data = tcp_msg_wait_data(sk, psock, timeo);
> + if (sock_flag(sk, SOCK_DONE)) {
> + ret = 0;
> + goto check_queues;
> + }
> +
> + if (sk->sk_err) {
> + if (!sk_psock_queue_empty(psock))
> + goto msg_bytes_ready;
> + if (!skb_queue_empty(&sk->sk_receive_queue)) {
> + release_sock(sk);
> + sk_psock_put(sk, psock);
> + return tcp_recvmsg(sk, msg, len, flags);
> + }
> + ret = sock_error(sk);
> + goto unlock;
> + }
Isn't that branch the same as:
if (sk->sk_err) {
ret = sock_error(sk);
goto check_queues;
}
?
> +
> + if (sk->sk_shutdown & RCV_SHUTDOWN) {
> + ret = 0;
> + goto check_queues;
> + }
> +
> + if (sk->sk_state == TCP_CLOSE) {
> + ret = -ENOTCONN;
> + goto check_queues;
> + }
> +
> + if (!timeo) {
> + ret = -EAGAIN;
> + goto check_queues;
> + }
> +
> + if (signal_pending(current)) {
> + ret = sock_intr_errno(timeo);
> + goto check_queues;
> + }
> +
> + data = tcp_msg_wait_data(sk, psock, &timeo);
> if (data < 0) {
> ret = data;
> goto unlock;
> @@ -390,6 +432,8 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> sk_psock_put(sk, psock);
> return tcp_recvmsg(sk, msg, len, flags);
> }
> + if (!data && timeo > 0)
> + goto msg_bytes_ready;
> copied = -EAGAIN;
> }
> ret = copied;
> @@ -398,6 +442,16 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> release_sock(sk);
> sk_psock_put(sk, psock);
> return ret;
> +
> +check_queues:
> + if (!sk_psock_queue_empty(psock))
> + goto msg_bytes_ready;
> + if (!skb_queue_empty(&sk->sk_receive_queue)) {
> + release_sock(sk);
> + sk_psock_put(sk, psock);
> + return tcp_recvmsg(sk, msg, len, flags);
> + }
> + goto unlock;
> }
>
> static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
2026-07-22 11:01 ` Jakub Sitnicki
@ 2026-07-22 11:22 ` Nnamdi Onyeyiri
0 siblings, 0 replies; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-22 11:22 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: bpf, davem, edumazet, horms, jiayuan.chen, john.fastabend, kuba,
kuniyu, ncardwell, netdev, pabeni, sashiko-reviews, linux-kernel,
emil
On Wed, Jul 22, 2026 at 01:01:09PM +0200, Jakub Sitnicki wrote:
> On Tue, Jul 21, 2026 at 11:38 PM +01, Nnamdi Onyeyiri wrote:
> > recvfrom()/recv() are documented as only returning EAGAIN for blocking
> > sockets when they have a receive timeout configured. However, adding a
> > blocking ipv4 tcp socket without a receive timeout to a sockmap will cause
> > EAGAIN errors sporadically. A socket with a receive timeout may return
> > EAGAIN before the timeout expires.
> >
> > There are 2 code paths affected by this:
> >
> > 1. tcp_bpf_recvmsg() - Used when the socket has been added to a sockmap
> > that has no verdict program attached.
> >
> > 2. tcp_bpf_recvmsg_parser() - Used when the socket has been added to a
> > sockmap that has a verdict program. To reproduce this issue, it is
> > enough for the verdict program to do nothing but return SK_PASS.
> >
> > In both cases this happens when tcp_msg_wait_data() wakes spuriously
> > (returning 0). To fix it, we now loop back to msg_bytes_ready instead
> > of returning -EAGAIN on spurious wakeup.
> >
> > To ensure the looping does not cause sockets with a SO_RCVTIMEO set to
> > wait excessively long, tcp_msg_wait_data() now takes a pointer to timeo,
> > allowing sk_wait_event() to update it as appropriate.
> >
> > The logic in tcp_bpf_recvmsg_parser() that allow it to handle signals,
> > socket errors and closuers in its loop was also added to tcp_bpf_recvmsg().
> >
> > Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
> > ---
> > net/ipv4/tcp_bpf.c | 72 ++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 63 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> > index cc0bd73f36b6..1755fcf726fc 100644
> > --- a/net/ipv4/tcp_bpf.c
> > +++ b/net/ipv4/tcp_bpf.c
> > @@ -179,7 +179,7 @@ EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
> >
> > #ifdef CONFIG_BPF_SYSCALL
> > static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
> > - long timeo)
> > + long *timeo)
> > {
> > DEFINE_WAIT_FUNC(wait, woken_wake_function);
> > int ret = 0;
> > @@ -187,12 +187,12 @@ static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
> > if (sk->sk_shutdown & RCV_SHUTDOWN)
> > return 1;
> >
> > - if (!timeo)
> > + if (!*timeo)
> > return ret;
> >
> > add_wait_queue(sk_sleep(sk), &wait);
> > sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
> > - ret = sk_wait_event(sk, &timeo,
> > + ret = sk_wait_event(sk, timeo,
> > !list_empty(&psock->ingress_msg) ||
> > !skb_queue_empty_lockless(&sk->sk_receive_queue), &wait);
> > sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
> > @@ -229,6 +229,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > int copied_from_self = 0;
> > int copied = 0;
> > u32 seq;
> > + long timeo;
> >
> > if (unlikely(flags & MSG_ERRQUEUE))
> > return inet_recv_error(sk, msg, len);
> > @@ -262,6 +263,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > }
> > }
> >
> > + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > +
> > msg_bytes_ready:
> > copied = __sk_msg_recvmsg(sk, psock, msg, len, flags, &copied_from_self);
> > /* The typical case for EFAULT is the socket was gracefully
> > @@ -280,7 +283,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > }
> > seq += copied_from_self;
> > if (!copied) {
> > - long timeo;
> > int data;
> >
> > if (sock_flag(sk, SOCK_DONE))
> > @@ -299,7 +301,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > goto out;
> > }
> >
> > - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > if (!timeo) {
> > copied = -EAGAIN;
> > goto out;
> > @@ -310,13 +311,15 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> > goto out;
> > }
> >
> > - data = tcp_msg_wait_data(sk, psock, timeo);
> > + data = tcp_msg_wait_data(sk, psock, &timeo);
> > if (data < 0) {
> > copied = data;
> > goto unlock;
> > }
> > if (data && !sk_psock_queue_empty(psock))
> > goto msg_bytes_ready;
> > + if (!data && timeo > 0)
> > + goto msg_bytes_ready;
> > copied = -EAGAIN;
> > }
> > out:
> > @@ -355,6 +358,7 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > {
> > struct sk_psock *psock;
> > int copied, ret;
> > + long timeo;
> >
> > if (unlikely(flags & MSG_ERRQUEUE))
> > return inet_recv_error(sk, msg, len);
> > @@ -371,14 +375,52 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > return tcp_recvmsg(sk, msg, len, flags);
> > }
> > lock_sock(sk);
> > +
> > + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > +
> > msg_bytes_ready:
> > copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> > if (!copied) {
> > - long timeo;
> > int data;
> >
> > - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > - data = tcp_msg_wait_data(sk, psock, timeo);
> > + if (sock_flag(sk, SOCK_DONE)) {
> > + ret = 0;
> > + goto check_queues;
> > + }
> > +
> > + if (sk->sk_err) {
> > + if (!sk_psock_queue_empty(psock))
> > + goto msg_bytes_ready;
> > + if (!skb_queue_empty(&sk->sk_receive_queue)) {
> > + release_sock(sk);
> > + sk_psock_put(sk, psock);
> > + return tcp_recvmsg(sk, msg, len, flags);
> > + }
> > + ret = sock_error(sk);
> > + goto unlock;
> > + }
>
> Isn't that branch the same as:
>
> if (sk->sk_err) {
> ret = sock_error(sk);
> goto check_queues;
> }
>
> ?
>
sock_error clears the error in sk->sk_err. using check_queues means
tcp_recvmsg would not see it. is that ok?
> > +
> > + if (sk->sk_shutdown & RCV_SHUTDOWN) {
> > + ret = 0;
> > + goto check_queues;
> > + }
> > +
> > + if (sk->sk_state == TCP_CLOSE) {
> > + ret = -ENOTCONN;
> > + goto check_queues;
> > + }
> > +
> > + if (!timeo) {
> > + ret = -EAGAIN;
> > + goto check_queues;
> > + }
> > +
> > + if (signal_pending(current)) {
> > + ret = sock_intr_errno(timeo);
> > + goto check_queues;
> > + }
> > +
> > + data = tcp_msg_wait_data(sk, psock, &timeo);
> > if (data < 0) {
> > ret = data;
> > goto unlock;
> > @@ -390,6 +432,8 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > sk_psock_put(sk, psock);
> > return tcp_recvmsg(sk, msg, len, flags);
> > }
> > + if (!data && timeo > 0)
> > + goto msg_bytes_ready;
> > copied = -EAGAIN;
> > }
> > ret = copied;
> > @@ -398,6 +442,16 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> > release_sock(sk);
> > sk_psock_put(sk, psock);
> > return ret;
> > +
> > +check_queues:
> > + if (!sk_psock_queue_empty(psock))
> > + goto msg_bytes_ready;
> > + if (!skb_queue_empty(&sk->sk_receive_queue)) {
> > + release_sock(sk);
> > + sk_psock_put(sk, psock);
> > + return tcp_recvmsg(sk, msg, len, flags);
> > + }
> > + goto unlock;
> > }
> >
> > static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-22 11:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 22:38 [PATCH v7 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
2026-07-21 22:38 ` [PATCH v7 1/2] " Nnamdi Onyeyiri
2026-07-21 22:55 ` sashiko-bot
2026-07-22 11:01 ` Jakub Sitnicki
2026-07-22 11:22 ` Nnamdi Onyeyiri
2026-07-21 22:38 ` [PATCH v7 2/2] selftests/bpf: add sockmap recvfrom EAGAIN selftest Nnamdi Onyeyiri
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.