* [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program
@ 2026-07-07 16:15 Mattia Meleleo via B4 Relay
2026-07-07 17:19 ` Emil Tsalapatis
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mattia Meleleo via B4 Relay @ 2026-07-07 16:15 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Jiayuan Chen; +Cc: netdev, bpf, Mattia Meleleo
From: Mattia Meleleo <mattia.meleleo@coralogix.com>
tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only
counts bytes in ingress_msg. Without a stream/skb verdict program
nothing is diverted there: data stays in sk_receive_queue, so FIONREAD
returns 0 even though read() returns data.
Add tcp_inq() to the reported value when the psock has no verdict
program. The two queues are disjoint, so bytes redirected into
ingress_msg from other sockets stay correctly accounted through
msg_tot_len.
Add a selftest covering FIONREAD without a verdict program.
Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap")
Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
net/ipv4/tcp_bpf.c | 16 ++++++++-
.../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f3..a001b1fff 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
{
+ struct sk_psock *psock;
bool slow;
if (cmd != SIOCINQ)
@@ -344,7 +345,20 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
return -EINVAL;
slow = lock_sock_fast(sk);
- *karg = sk_psock_msg_inq(sk);
+ psock = sk_psock_get(sk);
+ if (unlikely(!psock)) {
+ unlock_sock_fast(sk, slow);
+ return tcp_ioctl(sk, cmd, karg);
+ }
+ *karg = sk_psock_get_msg_len_nolock(psock);
+ /* Without a verdict program, ingress data is never diverted to
+ * ingress_msg: it stays in sk_receive_queue and is read through
+ * the fallback to tcp_recvmsg(), so account for it like
+ * tcp_ioctl() does.
+ */
+ if (!psock->progs.stream_verdict && !psock->progs.skb_verdict)
+ *karg += tcp_inq(sk);
+ sk_psock_put(sk, psock);
unlock_sock_fast(sk, slow);
return 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index cb3229711..f0f368201 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype)
test_sockmap_pass_prog__destroy(skel);
}
+/* A socket in a sockmap without a verdict program keeps its ingress data
+ * in sk_receive_queue: FIONREAD must account for it.
+ */
+static void test_sockmap_no_verdict_fionread(void)
+{
+ int err, map, zero = 0, sent, avail;
+ int c0 = -1, c1 = -1, p0 = -1, p1 = -1;
+ struct test_sockmap_pass_prog *skel;
+ char buf[256] = "0123456789";
+
+ 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);
+
+ err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
+ if (!ASSERT_OK(err, "create_socket_pairs()"))
+ goto out;
+
+ err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
+ if (!ASSERT_OK(err, "bpf_map_update_elem(c1)"))
+ goto out_close;
+
+ sent = xsend(p1, &buf, sizeof(buf), 0);
+ ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+ avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC);
+ ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)");
+
+out_close:
+ close(c0);
+ close(p0);
+ close(c1);
+ close(p1);
+out:
+ test_sockmap_pass_prog__destroy(skel);
+}
+
void test_sockmap_basic(void)
{
if (test__start_subtest("sockmap create_update_free"))
@@ -1415,6 +1452,8 @@ void test_sockmap_basic(void)
test_sockmap_skb_verdict_shutdown();
if (test__start_subtest("sockmap skb_verdict fionread"))
test_sockmap_skb_verdict_fionread(true);
+ if (test__start_subtest("sockmap no_verdict fionread"))
+ test_sockmap_no_verdict_fionread();
if (test__start_subtest("sockmap skb_verdict fionread on drop"))
test_sockmap_skb_verdict_fionread(false);
if (test__start_subtest("sockmap skb_verdict change tail"))
---
base-commit: d2c9a99135da931377240942d44f3dea104cedb8
change-id: 20260707-fionread-no-verdict-a4f8697ac9f9
Best regards,
--
Mattia Meleleo <mattia.meleleo@coralogix.com>
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-07 16:15 [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program Mattia Meleleo via B4 Relay @ 2026-07-07 17:19 ` Emil Tsalapatis 2026-07-08 4:41 ` Kumar Kartikeya Dwivedi 2026-07-08 4:30 ` Jiayuan Chen 2026-07-08 4:43 ` Kumar Kartikeya Dwivedi 2 siblings, 1 reply; 6+ messages in thread From: Emil Tsalapatis @ 2026-07-07 17:19 UTC (permalink / raw) To: mattia.meleleo, John Fastabend, Jakub Sitnicki, Jiayuan Chen; +Cc: netdev, bpf On Tue Jul 7, 2026 at 12:15 PM EDT, Mattia Meleleo via B4 Relay wrote: > From: Mattia Meleleo <mattia.meleleo@coralogix.com> > > tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only > counts bytes in ingress_msg. Without a stream/skb verdict program > nothing is diverted there: data stays in sk_receive_queue, so FIONREAD > returns 0 even though read() returns data. > > Add tcp_inq() to the reported value when the psock has no verdict > program. The two queues are disjoint, so bytes redirected into > ingress_msg from other sockets stay correctly accounted through > msg_tot_len. > > Add a selftest covering FIONREAD without a verdict program. > > Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap") > Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> I don't think the Sashiko READ_ONCE() recommendation is that important, we're just checking for the pointers' existence and I don't see how the reads can be moved/optimized out/merged in a way that breaks this code. > --- > net/ipv4/tcp_bpf.c | 16 ++++++++- > .../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++ > 2 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > index cc0bd73f3..a001b1fff 100644 > --- a/net/ipv4/tcp_bpf.c > +++ b/net/ipv4/tcp_bpf.c > @@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk, > > static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) > { > + struct sk_psock *psock; > bool slow; > > if (cmd != SIOCINQ) > @@ -344,7 +345,20 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) > return -EINVAL; > > slow = lock_sock_fast(sk); > - *karg = sk_psock_msg_inq(sk); > + psock = sk_psock_get(sk); > + if (unlikely(!psock)) { > + unlock_sock_fast(sk, slow); > + return tcp_ioctl(sk, cmd, karg); > + } > + *karg = sk_psock_get_msg_len_nolock(psock); > + /* Without a verdict program, ingress data is never diverted to > + * ingress_msg: it stays in sk_receive_queue and is read through > + * the fallback to tcp_recvmsg(), so account for it like > + * tcp_ioctl() does. > + */ > + if (!psock->progs.stream_verdict && !psock->progs.skb_verdict) > + *karg += tcp_inq(sk); > + sk_psock_put(sk, psock); > unlock_sock_fast(sk, slow); > > return 0; > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > index cb3229711..f0f368201 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > @@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype) > test_sockmap_pass_prog__destroy(skel); > } > > +/* A socket in a sockmap without a verdict program keeps its ingress data > + * in sk_receive_queue: FIONREAD must account for it. > + */ > +static void test_sockmap_no_verdict_fionread(void) > +{ > + int err, map, zero = 0, sent, avail; > + int c0 = -1, c1 = -1, p0 = -1, p1 = -1; > + struct test_sockmap_pass_prog *skel; > + char buf[256] = "0123456789"; > + > + 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); > + > + err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1); > + if (!ASSERT_OK(err, "create_socket_pairs()")) > + goto out; > + > + err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST); > + if (!ASSERT_OK(err, "bpf_map_update_elem(c1)")) > + goto out_close; > + > + sent = xsend(p1, &buf, sizeof(buf), 0); > + ASSERT_EQ(sent, sizeof(buf), "xsend(p1)"); > + avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC); > + ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)"); > + > +out_close: > + close(c0); > + close(p0); > + close(c1); > + close(p1); > +out: > + test_sockmap_pass_prog__destroy(skel); > +} > + > void test_sockmap_basic(void) > { > if (test__start_subtest("sockmap create_update_free")) > @@ -1415,6 +1452,8 @@ void test_sockmap_basic(void) > test_sockmap_skb_verdict_shutdown(); > if (test__start_subtest("sockmap skb_verdict fionread")) > test_sockmap_skb_verdict_fionread(true); > + if (test__start_subtest("sockmap no_verdict fionread")) > + test_sockmap_no_verdict_fionread(); > if (test__start_subtest("sockmap skb_verdict fionread on drop")) > test_sockmap_skb_verdict_fionread(false); > if (test__start_subtest("sockmap skb_verdict change tail")) > > --- > base-commit: d2c9a99135da931377240942d44f3dea104cedb8 > change-id: 20260707-fionread-no-verdict-a4f8697ac9f9 > > Best regards, > -- > Mattia Meleleo <mattia.meleleo@coralogix.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-07 17:19 ` Emil Tsalapatis @ 2026-07-08 4:41 ` Kumar Kartikeya Dwivedi 2026-07-08 16:19 ` Emil Tsalapatis 0 siblings, 1 reply; 6+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-07-08 4:41 UTC (permalink / raw) To: Emil Tsalapatis, mattia.meleleo, John Fastabend, Jakub Sitnicki, Jiayuan Chen Cc: netdev, bpf On Tue Jul 7, 2026 at 7:19 PM CEST, Emil Tsalapatis wrote: > On Tue Jul 7, 2026 at 12:15 PM EDT, Mattia Meleleo via B4 Relay wrote: >> From: Mattia Meleleo <mattia.meleleo@coralogix.com> >> >> tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only >> counts bytes in ingress_msg. Without a stream/skb verdict program >> nothing is diverted there: data stays in sk_receive_queue, so FIONREAD >> returns 0 even though read() returns data. >> >> Add tcp_inq() to the reported value when the psock has no verdict >> program. The two queues are disjoint, so bytes redirected into >> ingress_msg from other sockets stay correctly accounted through >> msg_tot_len. >> >> Add a selftest covering FIONREAD without a verdict program. >> >> Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap") >> Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> > > Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> > > I don't think the Sashiko READ_ONCE() recommendation is that important, > we're just checking for the pointers' existence and I don't see how the > reads can be moved/optimized out/merged in a way that breaks this code. > It would be necessary if the xchg() can happen even when the lock is held, at the very least, to suppress potential KCSAN warnings, I think. Even for the theoretical load tearing that causes false positive, it would be benign due to wrong accounting. >> --- >> net/ipv4/tcp_bpf.c | 16 ++++++++- >> .../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++ >> 2 files changed, 54 insertions(+), 1 deletion(-) >> >> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c >> index cc0bd73f3..a001b1fff 100644 >> --- a/net/ipv4/tcp_bpf.c >> +++ b/net/ipv4/tcp_bpf.c >> @@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk, >> >> static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) >> { >> + struct sk_psock *psock; >> bool slow; >> >> if (cmd != SIOCINQ) >> @@ -344,7 +345,20 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) >> return -EINVAL; >> >> slow = lock_sock_fast(sk); >> - *karg = sk_psock_msg_inq(sk); >> + psock = sk_psock_get(sk); >> + if (unlikely(!psock)) { >> + unlock_sock_fast(sk, slow); >> + return tcp_ioctl(sk, cmd, karg); >> + } >> + *karg = sk_psock_get_msg_len_nolock(psock); >> + /* Without a verdict program, ingress data is never diverted to >> + * ingress_msg: it stays in sk_receive_queue and is read through >> + * the fallback to tcp_recvmsg(), so account for it like >> + * tcp_ioctl() does. >> + */ >> + if (!psock->progs.stream_verdict && !psock->progs.skb_verdict) >> + *karg += tcp_inq(sk); >> + sk_psock_put(sk, psock); >> unlock_sock_fast(sk, slow); >> >> return 0; >> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c >> index cb3229711..f0f368201 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c >> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c >> @@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype) >> test_sockmap_pass_prog__destroy(skel); >> } >> >> +/* A socket in a sockmap without a verdict program keeps its ingress data >> + * in sk_receive_queue: FIONREAD must account for it. >> + */ >> +static void test_sockmap_no_verdict_fionread(void) >> +{ >> + int err, map, zero = 0, sent, avail; >> + int c0 = -1, c1 = -1, p0 = -1, p1 = -1; >> + struct test_sockmap_pass_prog *skel; >> + char buf[256] = "0123456789"; >> + >> + 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); >> + >> + err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1); >> + if (!ASSERT_OK(err, "create_socket_pairs()")) >> + goto out; >> + >> + err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST); >> + if (!ASSERT_OK(err, "bpf_map_update_elem(c1)")) >> + goto out_close; >> + >> + sent = xsend(p1, &buf, sizeof(buf), 0); >> + ASSERT_EQ(sent, sizeof(buf), "xsend(p1)"); >> + avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC); >> + ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)"); >> + >> +out_close: >> + close(c0); >> + close(p0); >> + close(c1); >> + close(p1); >> +out: >> + test_sockmap_pass_prog__destroy(skel); >> +} >> + >> void test_sockmap_basic(void) >> { >> if (test__start_subtest("sockmap create_update_free")) >> @@ -1415,6 +1452,8 @@ void test_sockmap_basic(void) >> test_sockmap_skb_verdict_shutdown(); >> if (test__start_subtest("sockmap skb_verdict fionread")) >> test_sockmap_skb_verdict_fionread(true); >> + if (test__start_subtest("sockmap no_verdict fionread")) >> + test_sockmap_no_verdict_fionread(); >> if (test__start_subtest("sockmap skb_verdict fionread on drop")) >> test_sockmap_skb_verdict_fionread(false); >> if (test__start_subtest("sockmap skb_verdict change tail")) >> >> --- >> base-commit: d2c9a99135da931377240942d44f3dea104cedb8 >> change-id: 20260707-fionread-no-verdict-a4f8697ac9f9 >> >> Best regards, >> -- >> Mattia Meleleo <mattia.meleleo@coralogix.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-08 4:41 ` Kumar Kartikeya Dwivedi @ 2026-07-08 16:19 ` Emil Tsalapatis 0 siblings, 0 replies; 6+ messages in thread From: Emil Tsalapatis @ 2026-07-08 16:19 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi, Emil Tsalapatis, mattia.meleleo, John Fastabend, Jakub Sitnicki, Jiayuan Chen Cc: netdev, bpf On Wed Jul 8, 2026 at 12:41 AM EDT, Kumar Kartikeya Dwivedi wrote: > On Tue Jul 7, 2026 at 7:19 PM CEST, Emil Tsalapatis wrote: >> On Tue Jul 7, 2026 at 12:15 PM EDT, Mattia Meleleo via B4 Relay wrote: >>> From: Mattia Meleleo <mattia.meleleo@coralogix.com> >>> >>> tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only >>> counts bytes in ingress_msg. Without a stream/skb verdict program >>> nothing is diverted there: data stays in sk_receive_queue, so FIONREAD >>> returns 0 even though read() returns data. >>> >>> Add tcp_inq() to the reported value when the psock has no verdict >>> program. The two queues are disjoint, so bytes redirected into >>> ingress_msg from other sockets stay correctly accounted through >>> msg_tot_len. >>> >>> Add a selftest covering FIONREAD without a verdict program. >>> >>> Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap") >>> Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> >> >> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> >> >> I don't think the Sashiko READ_ONCE() recommendation is that important, >> we're just checking for the pointers' existence and I don't see how the >> reads can be moved/optimized out/merged in a way that breaks this code. >> > > It would be necessary if the xchg() can happen even when the lock is held, at > the very least, to suppress potential KCSAN warnings, I think. Even for the > theoretical load tearing that causes false positive, it would be benign due to > wrong accounting. > Fair enough, nothing wrong with adding the READ_ONCE since the set already needed respinning. >>> --- >>> net/ipv4/tcp_bpf.c | 16 ++++++++- >>> .../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++ >>> 2 files changed, 54 insertions(+), 1 deletion(-) >>> >>> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c >>> index cc0bd73f3..a001b1fff 100644 >>> --- a/net/ipv4/tcp_bpf.c >>> +++ b/net/ipv4/tcp_bpf.c >>> @@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk, >>> >>> static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) >>> { >>> + struct sk_psock *psock; >>> bool slow; >>> >>> if (cmd != SIOCINQ) >>> @@ -344,7 +345,20 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) >>> return -EINVAL; >>> >>> slow = lock_sock_fast(sk); >>> - *karg = sk_psock_msg_inq(sk); >>> + psock = sk_psock_get(sk); >>> + if (unlikely(!psock)) { >>> + unlock_sock_fast(sk, slow); >>> + return tcp_ioctl(sk, cmd, karg); >>> + } >>> + *karg = sk_psock_get_msg_len_nolock(psock); >>> + /* Without a verdict program, ingress data is never diverted to >>> + * ingress_msg: it stays in sk_receive_queue and is read through >>> + * the fallback to tcp_recvmsg(), so account for it like >>> + * tcp_ioctl() does. >>> + */ >>> + if (!psock->progs.stream_verdict && !psock->progs.skb_verdict) >>> + *karg += tcp_inq(sk); >>> + sk_psock_put(sk, psock); >>> unlock_sock_fast(sk, slow); >>> >>> return 0; >>> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c >>> index cb3229711..f0f368201 100644 >>> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c >>> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c >>> @@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype) >>> test_sockmap_pass_prog__destroy(skel); >>> } >>> >>> +/* A socket in a sockmap without a verdict program keeps its ingress data >>> + * in sk_receive_queue: FIONREAD must account for it. >>> + */ >>> +static void test_sockmap_no_verdict_fionread(void) >>> +{ >>> + int err, map, zero = 0, sent, avail; >>> + int c0 = -1, c1 = -1, p0 = -1, p1 = -1; >>> + struct test_sockmap_pass_prog *skel; >>> + char buf[256] = "0123456789"; >>> + >>> + 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); >>> + >>> + err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1); >>> + if (!ASSERT_OK(err, "create_socket_pairs()")) >>> + goto out; >>> + >>> + err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST); >>> + if (!ASSERT_OK(err, "bpf_map_update_elem(c1)")) >>> + goto out_close; >>> + >>> + sent = xsend(p1, &buf, sizeof(buf), 0); >>> + ASSERT_EQ(sent, sizeof(buf), "xsend(p1)"); >>> + avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC); >>> + ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)"); >>> + >>> +out_close: >>> + close(c0); >>> + close(p0); >>> + close(c1); >>> + close(p1); >>> +out: >>> + test_sockmap_pass_prog__destroy(skel); >>> +} >>> + >>> void test_sockmap_basic(void) >>> { >>> if (test__start_subtest("sockmap create_update_free")) >>> @@ -1415,6 +1452,8 @@ void test_sockmap_basic(void) >>> test_sockmap_skb_verdict_shutdown(); >>> if (test__start_subtest("sockmap skb_verdict fionread")) >>> test_sockmap_skb_verdict_fionread(true); >>> + if (test__start_subtest("sockmap no_verdict fionread")) >>> + test_sockmap_no_verdict_fionread(); >>> if (test__start_subtest("sockmap skb_verdict fionread on drop")) >>> test_sockmap_skb_verdict_fionread(false); >>> if (test__start_subtest("sockmap skb_verdict change tail")) >>> >>> --- >>> base-commit: d2c9a99135da931377240942d44f3dea104cedb8 >>> change-id: 20260707-fionread-no-verdict-a4f8697ac9f9 >>> >>> Best regards, >>> -- >>> Mattia Meleleo <mattia.meleleo@coralogix.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-07 16:15 [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program Mattia Meleleo via B4 Relay 2026-07-07 17:19 ` Emil Tsalapatis @ 2026-07-08 4:30 ` Jiayuan Chen 2026-07-08 4:43 ` Kumar Kartikeya Dwivedi 2 siblings, 0 replies; 6+ messages in thread From: Jiayuan Chen @ 2026-07-08 4:30 UTC (permalink / raw) To: mattia.meleleo, John Fastabend, Jakub Sitnicki; +Cc: netdev, bpf On 7/8/26 12:15 AM, Mattia Meleleo via B4 Relay wrote: > From: Mattia Meleleo <mattia.meleleo@coralogix.com> > > tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only > counts bytes in ingress_msg. Without a stream/skb verdict program > nothing is diverted there: data stays in sk_receive_queue, so FIONREAD > returns 0 even though read() returns data. > > Add tcp_inq() to the reported value when the psock has no verdict > program. The two queues are disjoint, so bytes redirected into > ingress_msg from other sockets stay correctly accounted through > msg_tot_len. > > Add a selftest covering FIONREAD without a verdict program. > > Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap") > Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev> I guess udp_bpf_ioctl has same issue. You can fix it if you want. > --- > net/ipv4/tcp_bpf.c | 16 ++++++++- > .../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++ > 2 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > index cc0bd73f3..a001b1fff 100644 > --- a/net/ipv4/tcp_bpf.c > +++ b/net/ipv4/tcp_bpf.c > @@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk, > > static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) > { > + struct sk_psock *psock; > bool slow; > > if (cmd != SIOCINQ) > @@ -344,7 +345,20 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg) > return -EINVAL; > > slow = lock_sock_fast(sk); > - *karg = sk_psock_msg_inq(sk); > + psock = sk_psock_get(sk); > + if (unlikely(!psock)) { > + unlock_sock_fast(sk, slow); > + return tcp_ioctl(sk, cmd, karg); > + } > + *karg = sk_psock_get_msg_len_nolock(psock); > + /* Without a verdict program, ingress data is never diverted to > + * ingress_msg: it stays in sk_receive_queue and is read through > + * the fallback to tcp_recvmsg(), so account for it like > + * tcp_ioctl() does. > + */ > + if (!psock->progs.stream_verdict && !psock->progs.skb_verdict) > + *karg += tcp_inq(sk); > + sk_psock_put(sk, psock); > unlock_sock_fast(sk, slow); > > return 0; > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > index cb3229711..f0f368201 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > @@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype) > test_sockmap_pass_prog__destroy(skel); > } > > +/* A socket in a sockmap without a verdict program keeps its ingress data > + * in sk_receive_queue: FIONREAD must account for it. > + */ > +static void test_sockmap_no_verdict_fionread(void) > +{ > + int err, map, zero = 0, sent, avail; > + int c0 = -1, c1 = -1, p0 = -1, p1 = -1; > + struct test_sockmap_pass_prog *skel; > + char buf[256] = "0123456789"; > + > + 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); > + > + err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1); > + if (!ASSERT_OK(err, "create_socket_pairs()")) > + goto out; > + > + err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST); > + if (!ASSERT_OK(err, "bpf_map_update_elem(c1)")) > + goto out_close; > + > + sent = xsend(p1, &buf, sizeof(buf), 0); > + ASSERT_EQ(sent, sizeof(buf), "xsend(p1)"); > + avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC); > + ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)"); > + > +out_close: > + close(c0); > + close(p0); > + close(c1); > + close(p1); > +out: > + test_sockmap_pass_prog__destroy(skel); > +} > + > void test_sockmap_basic(void) > { > if (test__start_subtest("sockmap create_update_free")) > @@ -1415,6 +1452,8 @@ void test_sockmap_basic(void) > test_sockmap_skb_verdict_shutdown(); > if (test__start_subtest("sockmap skb_verdict fionread")) > test_sockmap_skb_verdict_fionread(true); > + if (test__start_subtest("sockmap no_verdict fionread")) > + test_sockmap_no_verdict_fionread(); > if (test__start_subtest("sockmap skb_verdict fionread on drop")) > test_sockmap_skb_verdict_fionread(false); > if (test__start_subtest("sockmap skb_verdict change tail")) > > --- > base-commit: d2c9a99135da931377240942d44f3dea104cedb8 > change-id: 20260707-fionread-no-verdict-a4f8697ac9f9 > > Best regards, > -- > Mattia Meleleo <mattia.meleleo@coralogix.com> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-07 16:15 [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program Mattia Meleleo via B4 Relay 2026-07-07 17:19 ` Emil Tsalapatis 2026-07-08 4:30 ` Jiayuan Chen @ 2026-07-08 4:43 ` Kumar Kartikeya Dwivedi 2 siblings, 0 replies; 6+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-07-08 4:43 UTC (permalink / raw) To: mattia.meleleo, John Fastabend, Jakub Sitnicki, Jiayuan Chen; +Cc: netdev, bpf On Tue Jul 7, 2026 at 6:15 PM CEST, Mattia Meleleo via B4 Relay wrote: > From: Mattia Meleleo <mattia.meleleo@coralogix.com> > > tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only > counts bytes in ingress_msg. Without a stream/skb verdict program > nothing is diverted there: data stays in sk_receive_queue, so FIONREAD > returns 0 even though read() returns data. > > Add tcp_inq() to the reported value when the psock has no verdict > program. The two queues are disjoint, so bytes redirected into > ingress_msg from other sockets stay correctly accounted through > msg_tot_len. > > Add a selftest covering FIONREAD without a verdict program. > > Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap") > Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> > --- Please respin with the fix and selftest as separate patches, but still targeting bpf tree. You can add the Reviewed-by: tags to both. pw-bot: cr > [...] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-08 16:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-07 16:15 [PATCH bpf] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program Mattia Meleleo via B4 Relay 2026-07-07 17:19 ` Emil Tsalapatis 2026-07-08 4:41 ` Kumar Kartikeya Dwivedi 2026-07-08 16:19 ` Emil Tsalapatis 2026-07-08 4:30 ` Jiayuan Chen 2026-07-08 4:43 ` Kumar Kartikeya Dwivedi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox