BPF List
 help / color / mirror / Atom feed
* [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

* [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

* 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

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