netdev - Re: [PATCH net-next v5] Add support to set napi threaded for individual napi

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250428112306.62ff198b@kernel.org>
Date: Mon, 28 Apr 2025 11:23:06 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Samiullah Khawaja <skhawaja@...gle.com>
Cc: Joe Damato <jdamato@...tly.com>, "David S . Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
 <pabeni@...hat.com>, almasrymina@...gle.com, willemb@...gle.com,
 mkarsten@...terloo.ca, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5] Add support to set napi threaded for
 individual napi

On Fri, 25 Apr 2025 20:53:14 -0700 Samiullah Khawaja wrote:
> > > We should check the discussions we had when threaded NAPI was added.
> > > I feel nothing was exposed in terms of observability so leaving the
> > > thread running didn't seem all that bad back then. Stopping the NAPI
> > > polling safely is not entirely trivial, we'd need to somehow grab
> > > the SCHED bit like busy polling does, and then re-schedule.
> > > Or have the thread figure out that it's done and exit.  
> >
> > Actually, we ended up adding the explicit ownership bits so it may not
> > be all that hard any more.. Worth trying.  
> Agreed. NAPI kthread lets go of the ownership by unsetting the
> SCHED_THREADED flag at napi_complete_done. This makes sure that the
> next SCHED is scheduled when new IRQ arrives and no packets are
> missed. We just have to make sure that it does that if it sees the
> kthread_should_stop. Do you think we should handle this maybe as a
> separate series/patch orthogonal to this?

We need to handle the case Joe pointed out. The new Netlink attributes
must make sense from day 1. I think it will be cleanest to work on
killing the thread first, but it can be a separate series.

> Also some clarification, we can remove the kthread when disabling napi
> threaded state using device level or napi level setting using netlink.
> But do you think we should also stop the thread when disabling a NAPI?
> That would mean the NAPI would lose any configurations done on this
> kthread by the user and those configurations won't be restored when
> this NAPI is enabled again. Some drivers use enable/disable as a
> mechanism to do soft reset, so a simple softreset to change queue
> length etc might revert these configurations.

That part I think needs to stay as is, the thread can be started and
stopped on napi_add / del, IMO.