Thread (1 message) 1 message, 1 author, 2026-01-23

Re: [PATCH bpf-next v7 2/3] bpf, sockmap: Fix FIONREAD for sockmap

From: Jakub Sitnicki <jakub@cloudflare.com>
Date: 2026-01-23 14:59:37
Also in: bpf, linux-kselftest, lkml

On Thu, Jan 22, 2026 at 03:56 AM GMT, Jiayuan Chen wrote:
January 21, 2026 at 20:55, "Jiayuan Chen" <jiayuan.chen@linux.dev
mailto:jiayuan.chen@linux.dev?to=%22Jiayuan%20Chen%22%20%3Cjiayuan.chen%40linux.dev%3E
quoted
wrote:
January 21, 2026 at 17:36, "Jakub Sitnicki" <jakub@cloudflare.com
mailto:jakub@cloudflare.com?to=%22Jakub%20Sitnicki%22%20%3Cjakub%40cloudflare.com%3E
quoted
 I've been thinking about this some more and came to the conclusion that
 this udp_bpf_ioctl implementation is actually what we want, while
 tcp_bpf_ioctl *should not* be checking if the sk_receive_queue is
 non-empty.
 
 Why? Because the verdict prog might redirect or drop the skbs from
 sk_receive_queue once it actually runs. The messages might never appear
 on the msg_ingress queue.
 
 What I think we should be doing, in the end, is kicking the
 sk_receive_queue processing on bpf_map_update_elem, if there's data
 ready.
 
 The API semantics I'm proposing is:
 
 1. ioctl(FIONREAD) -> reports N bytes
 2. bpf_map_update_elem(sk) -> socket inserted into sockmap
 3. poll() for POLLIN -> wait for socket to be ready to read
 5. ioctl(FIONREAD) -> report N bytes if verdict prog didn't
 redirect or drop it
 
 We don't have to add the the queue kick on map update in this series.
 
 If you decide to leave it for later, can I ask that you open an issue at
 our GH project [1]?
 
 I don't want it to fall through the cracks. And I sometimes have people
 asking what they could help with in sockmap.
 
 Thanks,
 -jkbs
 
 [1] https://github.com/sockmap-project/sockmap-project/issues
Hi Jakub,

Thanks for taking the time to think through this carefully. I agree with your
analysis - reporting sk_receive_queue length is misleading since the verdict
prog might redirect or drop those skbs.

There's no rush to merge this patch.

Since the kick queue on bpf_map_update_elem addresses a closely related issue,
I think it makes sense to include it in this patchset for easier tracking rather
than splitting it out.

I'll spend more time looking into this and come back with an updated version.

Thanks,
Jiayuan

Hi Jakub,

  I've been thinking about this more, and I realize the problem is not as simple as it seems.

  Regarding kicking the sk_receive_queue on bpf_map_update_elem: the BPF
  program may not be fully initialized at that point. For example, with a
  redirect program, the destination fd might not yet be inserted into the
  map. If we kick the data through the BPF program immediately, the
  redirect lookup would fail, leading to unexpected behavior (data being
  dropped or passed to the wrong socket).
I reckon there is not much we can do about it because we have no control
over when inserts/removes sockets from sockmap. It can happen at any
time.

Also, a newly received segment can trigger sk_data_ready callback,
and that would also cause the skbs to get processed. We don't have
control of that either.

Does this change break any of our existing tests/benchmarks or some
other setup of yours?
  I also considered triggering the kick in poll/select via
  sk_msg_is_readable(). However, this approach doesn't work for TCP
  because tcp_poll() -> tcp_stream_is_readable() -> tcp_epollin_ready()
  will return early when sk_receive_queue has data, before ever calling
  sk_is_readable().

  In the next version, I'll address your other nits and remove the
  sk_receive_queue check from tcp_bpf_ioctl. I'll also open an issue on
  the GH project to track this problem so we can continue exploring
  better solutions.
Sounds like a plan. Thanks!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help