netdev - Re: [net-next 10/11] net/mlx5e: Implement queue mgmt ops and single channel swap

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

Message-ID: <20250124072621.4ef8c763@kernel.org>
Date: Fri, 24 Jan 2025 07:26:21 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Saeed Mahameed <saeed@...nel.org>
Cc: Stanislav Fomichev <stfomichev@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, Eric Dumazet
 <edumazet@...gle.com>, Saeed Mahameed <saeedm@...dia.com>,
 netdev@...r.kernel.org, Tariq Toukan <tariqt@...dia.com>, Gal Pressman
 <gal@...dia.com>, Leon Romanovsky <leonro@...dia.com>, Dragos Tatulea
 <dtatulea@...dia.com>
Subject: Re: [net-next 10/11] net/mlx5e: Implement queue mgmt ops and single
 channel swap

On Thu, 23 Jan 2025 19:11:23 -0800 Saeed Mahameed wrote:
> On 23 Jan 16:55, Jakub Kicinski wrote:
> >> IIUC, we want queue API to move away from rtnl and use only (new) netdev
> >> lock. Otherwise, removing this dependency in the future might be
> >> complicated.  
> >
> >Correct. We only have one driver now which reportedly works (gve).
> >Let's pull queues under optional netdev_lock protection.
> >Then we can use queue mgmt op support as a carrot for drivers
> >to convert / test the netdev_lock protection... "compliance".
> >
> >I added netdev_lock protection for NAPI before the merge window.
> >Queues are configured in much more ad-hoc fashion, so I think
> >the best way to make queue changes netdev_lock safe would be to
> >wrap all driver ops which are currently under rtnl_lock with
> >netdev_lock.  
> 
> Are you expecting drivers to hold netdev_lock internally? 
> I was thinking something more scalable, queue_mgmt API to take
> netdev_lock,  and any other place in the stack that can access 
> "netdev queue config" e.g ethtool/netlink/netdev_ops should grab 
> netdev_lock as well, this is better for the future when we want to 
> reduce rtnl usage in the stack to protect single netdev ops where
> netdev_lock will be sufficient, otherwise you will have to wait for ALL
> drivers to properly use netdev_lock internally to even start thinking of
> getting rid of rtnl from some parts of the core stack.

Agreed, expecting drivers to get the locking right internally is easier
short term but messy long term. I'm thinking opt-in for drivers to have
netdev_lock taken by the core. Probably around all ops which today hold
rtnl_lock, to keep the expectations simple.

net_shaper and queue_mgmt ops can require that drivers that support
them opt-in and these ops can hold just the netdev_lock, no rtnl_lock.