netdev - Re: [PATCH net-next v6] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAAywjhQZDd2rJiF35iyYqMd86zzgDbLVinfEcva0b1=6tne3Pg@mail.gmail.com>
Date: Tue, 29 Apr 2025 18:16:29 -0700
From: Samiullah Khawaja <skhawaja@...gle.com>
To: Joe Damato <jdamato@...tly.com>, Samiullah Khawaja <skhawaja@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, "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 v6] Add support to set napi threaded for
 individual napi

On Tue, Apr 29, 2025 at 4:57 PM Joe Damato <jdamato@...tly.com> wrote:
>
> On Tue, Apr 29, 2025 at 10:26:56PM +0000, Samiullah Khawaja wrote:
>
> > v6:
> >  - Set the threaded property at device level even if the currently set
> >    value is same. This is to override any per napi settings. Update
> >    selftest to verify this scenario.
> >  - Use u8 instead of uint in netdev_nl_napi_set_config implementation.
> >  - Extend the selftest to verify the existing behaviour that the PID
> >    stays valid once threaded napi is enabled. It stays valid even after
> >    disabling the threaded napi. Also verify that the same kthread(PID)
> >    is reused when threaded napi is enabled again. Will keep this
> >    behaviour as based on the discussion on v5.
>
> This doesn't address the feedback from Jakub in the v5 [1] [2]:
>
>  - Jakub said the netlink attributes need to make sense from day 1.
>    Threaded = 0 and pid = 1234 does not make sense, and
Jakub mentioned following in v5 and that is the existing behaviour:
```
That part I think needs to stay as is, the thread can be started and
stopped on napi_add / del, IMO.
```
Please see my reply to him in v5 also to confirm this. I also quoted
the original reason, when this was added, behind not doing
kthread_stop when unsetting napi threaded.
>
>  - The thread should be started and stopped.
>
> [1]: https://lore.kernel.org/netdev/20250425201220.58bf25d7@kernel.org/
> [2]: https://lore.kernel.org/netdev/20250428112306.62ff198b@kernel.org/