netdev - Re: [PATCH net-next v4 4/4] net: wangxun: support to use adaptive RX/TX coalescing

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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250820084513.587f560b@kernel.org>
Date: Wed, 20 Aug 2025 08:45:13 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: <netdev@...r.kernel.org>, "'Andrew Lunn'" <andrew+netdev@...n.ch>,
 "'David S. Miller'" <davem@...emloft.net>, "'Eric Dumazet'"
 <edumazet@...gle.com>, "'Paolo Abeni'" <pabeni@...hat.com>, "'Simon
 Horman'" <horms@...nel.org>, "'Jacob Keller'" <jacob.e.keller@...el.com>,
 "'Mengyuan Lou'" <mengyuanlou@...-swift.com>
Subject: Re: [PATCH net-next v4 4/4] net: wangxun: support to use adaptive
 RX/TX coalescing

On Wed, 20 Aug 2025 09:57:43 +0800 Jiawen Wu wrote:
> On Sat, Aug 16, 2025 2:19 AM, Jakub Kicinski wrote:
> > On Tue, 12 Aug 2025 09:50:23 +0800 Jiawen Wu wrote:  
> > > @@ -878,6 +909,8 @@ static int wx_poll(struct napi_struct *napi, int budget)
> > >
> > >  	/* all work done, exit the polling mode */
> > >  	if (likely(napi_complete_done(napi, work_done))) {
> > > +		if (wx->adaptive_itr)
> > > +			wx_update_dim_sample(q_vector);  
> > 
> > this is racy, napi is considered released after napi_complete_done()
> > returns. So napi_disable() can succeed right after that point...
> >   
> > > @@ -1611,6 +1708,8 @@ void wx_napi_disable_all(struct wx *wx)
> > >  	for (q_idx = 0; q_idx < wx->num_q_vectors; q_idx++) {
> > >  		q_vector = wx->q_vector[q_idx];
> > >  		napi_disable(&q_vector->napi);
> > > +		cancel_work_sync(&q_vector->rx.dim.work);
> > > +		cancel_work_sync(&q_vector->tx.dim.work);  
> > 
> > so you may end up with the DIM work scheduled after the device is
> > stopped.  
> 
> But the DIM work doesn't seem to be concerned about the status of napi.
> And even if the device is stopped, setting itr would not cause any errors.
> 
> I can't fully grasp this point...
> Should I move cancel_work_sync() in front of napi_disable()?

My point is that this is possible today:

     CPU 0                     CPU 1

  napi_complete_done()
                            napi_disable()
                            cancel_work()
  wx_update_dim_sample()
    schedule_work()

You can probably use disable_work_sync() and enable_work..
to fix it.