[RFC PATCH net v3 0/1] af_packet: avoid waiting for TPACKET_V3 copies at retirement

[RFC PATCH net v3 0/1] af_packet: avoid waiting for TPACKET_V3 copies at retirement

[Jingxuan Xie]

From: jingxuanxie <jingxuanxie@deeproute.ai>

This is v3 of the TPACKET_V3 block-retirement fix. It replaces both v2
postings; please review only this thread.

v2 tried to remove the wait by allowing capture into the next block while
copies were still in flight. That required per-block copy accounting,
ordered publication, and handling for a backlog of ready blocks. Willem
asked for a smaller fix first: make block retirement non-blocking and let
the existing timer retry.

v3 does exactly that, in one patch:

- prb_retire_current_block() uses write_trylock() instead of write_lock().
  If a copier still holds the read lock, the current block and its
  metadata are left untouched and false is returned.
- __packet_lookup_frame_in_block() stops instead of dispatching the next
  block when retirement fails; tpacket_rcv() drops this capture record
  and accounts it in tp_drops. The original packet is not affected.
- The retirement timer re-arms and retries if it cannot take the lock.
- A later packet can also retire the block once the copier has finished.

There is no spillover into the next block, no per-block state, no worker
and no publication budget. The existing lock order and rwlock completion
ordering are preserved.

The cost is a higher capture drop rate while a copier is preempted. This
is a deliberate trade-off to keep the receive path moving on PREEMPT_RT.

The v2 1/2 virtio-header prevalidation patch is not part of v3. It is an
independent bug fix; I will rework it separately with a concrete
description of the state left behind and a shared error path, as Willem
asked.

Testing so far:
- ARM64 PREEMPT_RT W=1 build of af_packet.o: pass
- QEMU ARM64 SMP PREEMPT_RT (4 CPUs, lockdep, debug spinlocks):
  psock_tpacket and psock_fanout pass; three consecutive candidate
  boots pass the delayed-copier tests. The TCG fanout queue-length
  warning is also present on the unpatched baseline and the program
  exits 0.
- Full-block path: with A delayed, B's sendto returned in 0.7 ms, only
  B's capture record was dropped (tp_drops=1), and no block was
  published. After release, A and the next packet were published, B was
  absent, and payload and length were verified.
- Timer-only path: 15 failed timer trylocks while A was delayed; the
  timer published A after release.
- Close/unmap while A was delayed: no hang or crash.
- Baseline contrast: without the patch, B blocked 505 ms until A was
  released and was captured.
- Control-flow harness covering rx-busy, timer-busy, recovery, full ring
  and shutdown: 6/6 pass. checkpatch on the diff: clean.

Not yet run: Orin hardware, KASAN/KCSAN, and capture drop-rate or
latency measurements. Timings above are QEMU TCG observations, not
hardware RT guarantees.

v2 thread (second posting):
https://lore.kernel.org/netdev/20261030081704.251652-1-jingxuanxie@deeproute.ai/

v2 review:
- 1/2: https://lore.kernel.org/netdev/willemdebruijn.kernel.17d3748debf8b@gmail.com/
- 2/2: https://lore.kernel.org/netdev/willemdebruijn.kernel.54d115047658@gmail.com/

jingxuanxie (1):
  af_packet: avoid waiting for TPACKET_V3 copies at retirement

 net/packet/af_packet.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

base-commit: 1f1f8e8c5176234fa31858cd25f2e9b88aac0a45

[RFC PATCH net v3 1/1] af_packet: avoid waiting for TPACKET_V3 copies at retirement

[Jingxuan Xie]

From: jingxuanxie <jingxuanxie@deeproute.ai>

TPACKET_V3 retires a full or expired block while holding
sk_receive_queue.lock. prb_retire_current_block() takes
blk_fill_in_prog_lock for write to wait for in-flight copy_bits()
before publishing the block.

On PREEMPT_RT a copier can be preempted while holding the read lock.
The packet path or the retirement timer then blocks on the write lock
while holding sk_receive_queue.lock, and the receive path resumes only
after the copier is scheduled again.

Try to take the write lock without blocking. If a copy is still in
progress, leave the current block and its metadata untouched:

- the retirement timer re-arms itself and retries after the configured
  interval;
- packet allocation returns NULL, so tpacket_rcv() drops this capture
  record and accounts it in tp_drops, while the original packet remains
  in the network stack;
- a later packet can retire the block once the copier has finished.

Do not dispatch the next block while the current block is still open.
The existing lock order and completion ordering are preserved, and no
new state or worker is introduced. The trade-off is a higher capture
drop rate while a copier is preempted; original traffic is unaffected.

Fixes: 632ca50f2cbd ("af_packet: TPACKET_V3: replace busy-wait loop")
Assisted-by: LLM
Signed-off-by: Jingxuan Xie <jingxuanxie@deeproute.ai>
---
 net/packet/af_packet.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index cf6429f09477..8fd6372ab59b 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -200,8 +200,8 @@ static void packet_increment_head(struct packet_ring_buffer *buff);
 static int prb_curr_blk_in_use(struct tpacket_block_desc *);
 static void *prb_dispatch_next_block(struct tpacket_kbdq_core *,
 			struct packet_sock *);
-static void prb_retire_current_block(struct tpacket_kbdq_core *,
-		struct packet_sock *, unsigned int status);
+static bool prb_retire_current_block(struct tpacket_kbdq_core *,
+				     struct packet_sock *, unsigned int status);
 static int prb_queue_frozen(struct tpacket_kbdq_core *);
 static void prb_open_block(struct tpacket_kbdq_core *,
 		struct tpacket_block_desc *);
@@ -711,8 +711,9 @@ static enum hrtimer_restart prb_retire_rx_blk_timer_expired(struct hrtimer *t)
 	 *
 	 */
 	if (BLOCK_NUM_PKTS(pbd)) {
-		/* Waiting for skb_copy_bits to finish... */
-		write_lock(&pkc->blk_fill_in_prog_lock);
+		/* Retry later if a packet copy is still in progress. */
+		if (!write_trylock(&pkc->blk_fill_in_prog_lock))
+			goto refresh_timer;
 		write_unlock(&pkc->blk_fill_in_prog_lock);
 	}
 
@@ -738,6 +739,7 @@ static enum hrtimer_restart prb_retire_rx_blk_timer_expired(struct hrtimer *t)
 		}
 	}
 
+refresh_timer:
 	hrtimer_forward_now(&pkc->retire_blk_timer, pkc->interval_ktime);
 	spin_unlock(&po->sk.sk_receive_queue.lock);
 	return HRTIMER_RESTART;
@@ -946,8 +948,8 @@ static void *prb_dispatch_next_block(struct tpacket_kbdq_core *pkc,
 	return (void *)pkc->nxt_offset;
 }
 
-static void prb_retire_current_block(struct tpacket_kbdq_core *pkc,
-		struct packet_sock *po, unsigned int status)
+static bool prb_retire_current_block(struct tpacket_kbdq_core *pkc,
+				     struct packet_sock *po, unsigned int status)
 {
 	struct tpacket_block_desc *pbd = GET_CURR_PBLOCK_DESC_FROM_CORE(pkc);
 
@@ -963,13 +965,14 @@ static void prb_retire_current_block(struct tpacket_kbdq_core *pkc,
 		 * the timer-handler already handled this case.
 		 */
 		if (!(status & TP_STATUS_BLK_TMO)) {
-			/* Waiting for skb_copy_bits to finish... */
-			write_lock(&pkc->blk_fill_in_prog_lock);
+			/* Do not wait for copies while holding the queue lock. */
+			if (!write_trylock(&pkc->blk_fill_in_prog_lock))
+				return false;
 			write_unlock(&pkc->blk_fill_in_prog_lock);
 		}
 		prb_close_block(pkc, pbd, po, status);
-		return;
 	}
+	return true;
 }
 
 static int prb_curr_blk_in_use(struct tpacket_block_desc *pbd)
@@ -1097,7 +1100,8 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
 	}
 
 	/* Ok, close the current block */
-	prb_retire_current_block(pkc, po, 0);
+	if (!prb_retire_current_block(pkc, po, 0))
+		return NULL;
 
 	/* Now, try to dispatch the next block */
 	curr = (char *)prb_dispatch_next_block(pkc, po);

Re: [RFC PATCH net v3 0/1] af_packet: avoid waiting for TPACKET_V3 copies at retirement

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.