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

From: Jakub Kicinski <[email-masked]>
Date: Fri, 24 Jan 2025 07:26:21 -0800

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.