netdev - Re: [PATCH net-next v10 1/2] net: af_packet: remove last_kactive_blk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20250904023537.934715-1-jackzxcui1989@163.com>
Date: Thu,  4 Sep 2025 10:35:37 +0800
From: Xin Zhao <jackzxcui1989@....com>
To: kerneljasonxing@...il.com,
	willemdebruijn.kernel@...il.com,
	edumazet@...gle.com,
	ferenc@...es.dev
Cc: davem@...emloft.net,
	kuba@...nel.org,
	pabeni@...hat.com,
	horms@...nel.org,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v10 1/2] net: af_packet: remove last_kactive_blk_num field

On Thu, Sep 4, 2025 at 10:09 +0800 Jason Xing <kerneljasonxing@...il.com> wrote:

> > Consider the following case:
> > (before applying this patch)
> > cpu0                                  cpu1
> > tpacket_rcv
> >   ...
> >     prb_dispatch_next_block
> >       prb_freeze_queue (R = 1)
> >                                       prb_retire_rx_blk_timer_expired
> >                                         L != K
> >                                           _prb_refresh_rx_retire_blk_timer
> >                                             refresh timer
> >                                             set L = K
> 
> I do not think the above can happen because:
> 1) tpacket_rcv() owns the sk_receive_queue.lock and then calls
> packet_current_rx_frame()->__packet_lookup_frame_in_block()->prb_dispatch_next_block()
> 2) the timer prb_retire_rx_blk_timer_expired() also needs to acquire
> the same lock first.
> 
> > (after applying this patch)
> > cpu0                                  cpu1
> > tpacket_rcv
> >   ...
> >     prb_dispatch_next_block
> >       prb_freeze_queue (R = 1)
> >                                       prb_retire_rx_blk_timer_expired
> >                                         !forzen is 0
> >                                           check prb_curr_blk_in_use
> >                                             if true
> >                                               same as (before apply)
> >                                             if false
> >                                               prb_open_block
> > Before applying this patch, prb_retire_rx_blk_timer_expired will do nothing
> > but refresh timer and set L = K in the case above. After applying this
> > patch, it will check prb_curr_blk_in_use and call prb_open_block if
> > user-space caught up.
> 
> The major difference after this patch is that even if L != K we would
> call prb_open_block(). So I think the key point is that this patch
> provides another checkpoint to thaw the might-be-frozen block in any
> case. It doesn't have any effect because
> __packet_lookup_frame_in_block() has the same logic and does it again
> without this patch when detecting the ring is frozen. The patch only
> advances checking the status of the ring.


In the prb_dispatch_next_block function, after executing prb_freeze_queue, it
directly returns without executing prb_open_block. As a result, tpacket_rcv
completes and exits the lock, and then callback executes while (L != K).
Perhaps my diagram did not convey this clearly. I think it might be better to
use your description above to replace the flowchart representation.


Thanks
Xin Zhao