* [PATCH bpf v2 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program
@ 2026-07-08 13:40 Mattia Meleleo via B4 Relay
2026-07-08 13:40 ` [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD " Mattia Meleleo via B4 Relay
2026-07-08 13:40 ` [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket " Mattia Meleleo via B4 Relay
0 siblings, 2 replies; 6+ messages in thread
From: Mattia Meleleo via B4 Relay @ 2026-07-08 13:40 UTC (permalink / raw)
To: bpf
Cc: netdev, John Fastabend, Jakub Sitnicki, Jiayuan Chen,
Kumar Kartikeya Dwivedi, Emil Tsalapatis, Mattia Meleleo
Sockets added to a sockmap/sockhash with no stream/skb verdict program
attached answer FIONREAD with 0 even when unread data is pending in
sk_receive_queue. Fix tcp_bpf_ioctl() to account for the receive queue
in that case, and add a selftest.
Changes in v2:
- Split the fix and the selftest into separate patches
- Use READ_ONCE() to read the verdict program pointers
- Link to v1: https://patch.msgid.link/20260707-fionread-no-verdict-v1-1-ce94a72357ec@coralogix.com
Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
Mattia Meleleo (2):
bpf, sockmap: Account for receive queue in FIONREAD without a verdict program
selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program
net/ipv4/tcp_bpf.c | 17 +++++++++-
.../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 1 deletion(-)
---
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* [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-08 13:40 [PATCH bpf v2 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program Mattia Meleleo via B4 Relay @ 2026-07-08 13:40 ` Mattia Meleleo via B4 Relay 2026-07-08 13:56 ` sashiko-bot 2026-07-08 13:40 ` [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket " Mattia Meleleo via B4 Relay 1 sibling, 1 reply; 6+ messages in thread From: Mattia Meleleo via B4 Relay @ 2026-07-08 13:40 UTC (permalink / raw) To: bpf Cc: netdev, John Fastabend, Jakub Sitnicki, Jiayuan Chen, Kumar Kartikeya Dwivedi, Emil Tsalapatis, 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. Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap") Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> --- net/ipv4/tcp_bpf.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c index cc0bd73f3..8e905b50d 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,21 @@ 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 (!READ_ONCE(psock->progs.stream_verdict) && + !READ_ONCE(psock->progs.skb_verdict)) + *karg += tcp_inq(sk); + sk_psock_put(sk, psock); unlock_sock_fast(sk, slow); return 0; -- Git-155) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-08 13:40 ` [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD " Mattia Meleleo via B4 Relay @ 2026-07-08 13:56 ` sashiko-bot 2026-07-08 16:17 ` Emil Tsalapatis 0 siblings, 1 reply; 6+ messages in thread From: sashiko-bot @ 2026-07-08 13:56 UTC (permalink / raw) To: Mattia Meleleo; +Cc: bpf Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The function sk_psock_msg_inq() is now dead code and should be removed. -- --- Patch [1]: [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program --- commit c5a219566b06af42fd341fd941447a9a448fa4f0 Author: Mattia Meleleo <mattia.meleleo@coralogix.com> bpf, sockmap: Account for receive queue in FIONREAD without a verdict program This commit fixes SIOCINQ reporting for tcp_bpf_ioctl() when no verdict program is attached. It accounts for bytes remaining in the sk_receive_queue by adding tcp_inq() to the reported value. > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > index cc0bd73f36b6d..8e905b50deadb 100644 > --- a/net/ipv4/tcp_bpf.c > +++ b/net/ipv4/tcp_bpf.c > @@ -344,7 +345,21 @@ 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); [Severity: Low] Does replacing this call to sk_psock_msg_inq() leave the function as dead code? Looking at include/linux/skmsg.h, it appears this patch removes the only caller of sk_psock_msg_inq() in the tree. Should the function definition be removed alongside this change to clean up the unused code? > + psock = sk_psock_get(sk); > + if (unlikely(!psock)) { [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260708-fionread-no-verdict-v2-0-29dd293621c7@coralogix.com?part=1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program 2026-07-08 13:56 ` sashiko-bot @ 2026-07-08 16:17 ` Emil Tsalapatis 0 siblings, 0 replies; 6+ messages in thread From: Emil Tsalapatis @ 2026-07-08 16:17 UTC (permalink / raw) To: sashiko-reviews, Mattia Meleleo; +Cc: bpf On Wed Jul 8, 2026 at 9:56 AM EDT, sashiko-bot wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Low] The function sk_psock_msg_inq() is now dead code and should be removed. > -- > > --- Patch [1]: [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program --- > commit c5a219566b06af42fd341fd941447a9a448fa4f0 > Author: Mattia Meleleo <mattia.meleleo@coralogix.com> > > bpf, sockmap: Account for receive queue in FIONREAD without a verdict program > > This commit fixes SIOCINQ reporting for tcp_bpf_ioctl() when no verdict > program is attached. It accounts for bytes remaining in the sk_receive_queue > by adding tcp_inq() to the reported value. > >> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c >> index cc0bd73f36b6d..8e905b50deadb 100644 >> --- a/net/ipv4/tcp_bpf.c >> +++ b/net/ipv4/tcp_bpf.c >> @@ -344,7 +345,21 @@ 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); > > [Severity: Low] > Does replacing this call to sk_psock_msg_inq() leave the function as > dead code? > > Looking at include/linux/skmsg.h, it appears this patch removes the only > caller of sk_psock_msg_inq() in the tree. Should the function definition > be removed alongside this change to clean up the unused code? This is a nice suggestion, if we can remove the function let's do so. > >> + psock = sk_psock_get(sk); >> + if (unlikely(!psock)) { > [ ... ] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program 2026-07-08 13:40 [PATCH bpf v2 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program Mattia Meleleo via B4 Relay 2026-07-08 13:40 ` [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD " Mattia Meleleo via B4 Relay @ 2026-07-08 13:40 ` Mattia Meleleo via B4 Relay 2026-07-08 13:51 ` sashiko-bot 1 sibling, 1 reply; 6+ messages in thread From: Mattia Meleleo via B4 Relay @ 2026-07-08 13:40 UTC (permalink / raw) To: bpf Cc: netdev, John Fastabend, Jakub Sitnicki, Jiayuan Chen, Kumar Kartikeya Dwivedi, Emil Tsalapatis, Mattia Meleleo From: Mattia Meleleo <mattia.meleleo@coralogix.com> Add a test validating that FIONREAD on a TCP socket in a sockmap without a verdict program reports data pending in sk_receive_queue. Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com> --- .../selftests/bpf/prog_tests/sockmap_basic.c | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+) 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")) -- Git-155) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program 2026-07-08 13:40 ` [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket " Mattia Meleleo via B4 Relay @ 2026-07-08 13:51 ` sashiko-bot 0 siblings, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-07-08 13:51 UTC (permalink / raw) To: Mattia Meleleo; +Cc: bpf Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] The test allocates two socket pairs but only uses one. - [Low] Multi-line comment violates BPF subsystem styling rules. -- --- Patch [2]: [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program --- commit b610a0eeb2389887cf749b47e6bba8ebcb416f08 Author: Mattia Meleleo <mattia.meleleo@coralogix.com> selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program This commit adds a test to validate that FIONREAD on a TCP socket in a sockmap without a verdict program correctly reports data pending in sk_receive_queue. > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c > index cb3229711f93a..f0f368201ddad 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. > + */ [Severity: Low] This isn't a bug, but does this multi-line comment follow the BPF subsystem coding style? The preferred style requires the opening /* to be on its own line, rather than sharing a line with the comment text. > +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); [Severity: Low] Is it necessary to allocate two socket pairs here? The test allocates c0 and p0 alongside c1 and p1, but c0 and p0 appear to be entirely unused except for being closed at the end of the test. Could this use create_pair() to allocate just a single socket pair instead? > + if (!ASSERT_OK(err, "create_socket_pairs()")) > + goto out; > + > + err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST); [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260708-fionread-no-verdict-v2-0-29dd293621c7@coralogix.com?part=2 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-08 16:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-08 13:40 [PATCH bpf v2 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program Mattia Meleleo via B4 Relay 2026-07-08 13:40 ` [PATCH bpf v2 1/2] bpf, sockmap: Account for receive queue in FIONREAD " Mattia Meleleo via B4 Relay 2026-07-08 13:56 ` sashiko-bot 2026-07-08 16:17 ` Emil Tsalapatis 2026-07-08 13:40 ` [PATCH bpf v2 2/2] selftests/bpf: Test FIONREAD on a sockmap socket " Mattia Meleleo via B4 Relay 2026-07-08 13:51 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox