[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 1/1] af_packet: avoid waiting for TPACKET_V3 copies at retirement

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] af_packet: premature packet drops in prb_retire_current_block() on SMP
--

commit ed9927a1021a4ff736d3e13140014c9d82d1ba49
Author: jingxuanxie <jingxuanxie@deeproute.ai>

af_packet: avoid waiting for TPACKET_V3 copies at retirement

This commit replaces blocking write_lock() calls on blk_fill_in_prog_lock
with write_trylock() in the TPACKET_V3 block retirement paths so that
callers holding sk_receive_queue.lock do not block waiting for in-progress
packet copies. When write_trylock() fails during frame lookup,
prb_retire_current_block() returns false and the packet capture record is
dropped.

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index cf6429f094772..8fd6372ab59bd 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
[ ... ]
> @@ -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;

[Severity: Medium]
Does using write_trylock() here cause premature packet drops and delayed
block retirement during normal multi-core traffic, even on non-PREEMPT_RT
kernels and when free blocks are available in the ring?

In tpacket_rcv(), a CPU reserves a slot in the current block via:

  tpacket_rcv()
    packet_current_rx_frame()
      __packet_lookup_frame_in_block()
        prb_fill_curr_block()
          read_lock(&pkc->blk_fill_in_prog_lock);

and then drops sk->sk_receive_queue.lock before running skb_copy_bits(),
tpacket_get_timestamp(), and dev_parse_header() concurrently with other
CPUs.

While CPU 0 is mid-copy on the last packet of a block, CPU 1 can arrive in
tpacket_rcv() with a packet that does not fit in the current block and call
__packet_lookup_frame_in_block() -> prb_retire_current_block(). Even though
CPU 0 has not been preempted (and on non-PREEMPT_RT kernels read_lock()
disables preemption), write_trylock() on CPU 1 will fail immediately,
causing tpacket_rcv() to drop the packet, increment tp_drops (setting
TP_STATUS_LOSING), and skip dispatching the next free block.

Furthermore, when CPU 0 finishes its copy and calls
prb_clear_blk_fill_status(), it only releases the read lock without closing
the block. If the dropped packet on CPU 1 was the end of a burst, won't the
full block remain unretired in TP_STATUS_KERNEL until
prb_retire_rx_blk_timer_expired() fires (and could be delayed by another
full interval_ktime if write_trylock() in the timer handler also coincides
with an active copy)?

Finally, when __packet_lookup_frame_in_block() returns NULL here,
tpacket_rcv() jumps to drop_n_account:

  drop_n_account:
        spin_unlock(&sk->sk_receive_queue.lock);
        atomic_inc(&po->tp_drops);
        drop_reason = SKB_DROP_REASON_PACKET_SOCK_ERROR;

        sk->sk_data_ready(sk);

which wakes user-space waiters even though no block was retired to
TP_STATUS_USER. On PREEMPT_RT, could waking a higher-priority user-space
reader thread on every dropped packet preempt the copier thread holding
blk_fill_in_prog_lock and prolong the state where write_trylock() keeps
failing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103092541.273847-1-jingxuanxie@deeproute.ai?part=1

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.

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

[Jingxuan Xie]

The issue was found from a field observation in a PREEMPT_RT
deployment: a receive path blocked for hundreds of milliseconds in
prb_retire_current_block() waiting for blk_fill_in_prog_lock, which was
held for read by a preempted packet copy. Manual code inspection found
the same unconditional write_lock() in both the full-block and timer
retirement paths.

I reproduced the ordering on QEMU ARM64 SMP PREEMPT_RT by delaying a
copier after frame reservation. Without the patch, the second send
blocks until the copier is released; with the patch, only that capture
record is dropped, and the block is published after release.

I will include this in the commit message if a v4 is needed for other
reasons.

Thanks,
Jingxuan