Commit f8cb38c3 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-depend-on-instance-lock-for-queue-related-netlink-ops' 

Jakub Kicinski says:

====================
net: depend on instance lock for queue related netlink ops

netdev-genl used to be protected by rtnl_lock. In previous release
we already switched the queue management ops (for Rx zero-copy) to
the instance lock. This series converts other ops to depend on the
instance lock when possible.

Unfortunately queue related state is hard to lock (unlike NAPI)
as the process of switching the number of queues usually involves
a large reconfiguration of the driver. The reconfig process has
historically been under rtnl_lock, but for drivers which opt into
ops locking it is also under the instance lock. Leverage that
and conditionally take rtnl_lock or instance lock depending
on the device capabilities.

v1: https://lore.kernel.org/20250407190117.16528-1-kuba@kernel.org
====================

Link: https://patch.msgid.link/20250408195956.412733-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 420aabef ce7b1494

Documentation/networking/netdevices.rst

+49 −12
Original line number Diff line number Diff line
@@ -314,13 +314,8 @@ napi->poll: 
		 softirq

		 will be called with interrupts disabled by netconsole.



struct netdev_queue_mgmt_ops synchronization rules

==================================================



All queue management ndo callbacks are holding netdev instance lock.



RTNL and netdev instance lock

=============================

netdev instance lock

====================



Historically, all networking control operations were protected by a single

global lock known as ``rtnl_lock``. There is an ongoing effort to replace this
@@ -328,10 +323,13 @@ global lock with separate locks for each network namespace. Additionally, 
properties of individual netdev are increasingly protected by per-netdev locks.



For device drivers that implement shaping or queue management APIs, all control

operations will be performed under the netdev instance lock. Currently, this

instance lock is acquired within the context of ``rtnl_lock``. The drivers

can also explicitly request instance lock to be acquired via

``request_ops_lock``. In the future, there will be an option for individual

operations will be performed under the netdev instance lock.

Drivers can also explicitly request instance lock to be held during ops

by setting ``request_ops_lock`` to true. Code comments and docs refer

to drivers which have ops called under the instance lock as "ops locked".

See also the documentation of the ``lock`` member of struct net_device.



In the future, there will be an option for individual

drivers to opt out of using ``rtnl_lock`` and instead perform their control

operations directly under the netdev instance lock.
@@ -343,8 +341,46 @@ there are two sets of interfaces: ``dev_xxx`` and ``netif_xxx`` (e.g., 
acquiring the instance lock themselves, while the ``netif_xxx`` functions

assume that the driver has already acquired the instance lock.



struct net_device_ops

---------------------



``ndos`` are called without holding the instance lock for most drivers.



"Ops locked" drivers will have most of the ``ndos`` invoked under

the instance lock.



struct ethtool_ops

------------------



Similarly to ``ndos`` the instance lock is only held for select drivers.

For "ops locked" drivers all ethtool ops without exceptions should

be called under the instance lock.



struct netdev_stat_ops

----------------------



"qstat" ops are invoked under the instance lock for "ops locked" drivers,

and under rtnl_lock for all other drivers.



struct net_shaper_ops

---------------------



All net shaper callbacks are invoked while holding the netdev instance

lock. ``rtnl_lock`` may or may not be held.



Note that supporting net shapers automatically enables "ops locking".



struct netdev_queue_mgmt_ops

----------------------------



All queue management callbacks are invoked while holding the netdev instance

lock. ``rtnl_lock`` may or may not be held.



Note that supporting struct netdev_queue_mgmt_ops automatically enables

"ops locking".



Notifiers and netdev instance lock

==================================

----------------------------------



For device drivers that implement shaping or queue management APIs,

some of the notifiers (``enum netdev_cmd``) are running under the netdev
@@ -354,6 +390,7 @@ For devices with locked ops, currently only the following notifiers are 
running under the lock:

* ``NETDEV_REGISTER``

* ``NETDEV_UP``

* ``NETDEV_XDP_FEAT_CHANGE``



The following notifiers are running without the lock:

* ``NETDEV_UNREGISTER``

drivers/net/ethernet/google/gve/gve_main.c

+1 −1
Original line number Diff line number Diff line
@@ -2185,7 +2185,7 @@ static void gve_set_netdev_xdp_features(struct gve_priv *priv) 
		xdp_features = 0;

	}



	xdp_set_features_flag(priv->dev, xdp_features);

	xdp_set_features_flag_locked(priv->dev, xdp_features);

}



static int gve_init_priv(struct gve_priv *priv, bool skip_describe_device)

include/linux/netdevice.h

+6 −1
Original line number Diff line number Diff line
@@ -688,6 +688,7 @@ struct netdev_queue { 
	/* Subordinate device that the queue has been assigned to */

	struct net_device	*sb_dev;

#ifdef CONFIG_XDP_SOCKETS

	/* "ops protected", see comment about net_device::lock */

	struct xsk_buff_pool    *pool;

#endif
@@ -1952,6 +1953,7 @@ enum netdev_reg_state { 
 *	@priv_destructor:	Called from unregister

 *	@npinfo:		XXX: need comments on this one

 * 	@nd_net:		Network namespace this network device is inside

 *				protected by @lock

 *

 * 	@ml_priv:	Mid-layer private

 *	@ml_priv_type:  Mid-layer private type
@@ -2359,6 +2361,9 @@ struct net_device { 


	bool dismantle;



	/** @moving_ns: device is changing netns, protected by @lock */

	bool moving_ns;



	enum {

		RTNL_LINK_INITIALIZED,

		RTNL_LINK_INITIALIZING,
@@ -2521,7 +2526,7 @@ struct net_device { 
	 *	@net_shaper_hierarchy, @reg_state, @threaded

	 *

	 * Double protects:

	 *	@up

	 *	@up, @moving_ns, @nd_net, @xdp_flags

	 *

	 * Double ops protects:

	 *	@real_num_rx_queues, @real_num_tx_queues

include/net/netdev_lock.h

+16 −0
Original line number Diff line number Diff line
@@ -64,6 +64,22 @@ netdev_ops_assert_locked_or_invisible(const struct net_device *dev) 
		netdev_ops_assert_locked(dev);

}



static inline void netdev_lock_ops_compat(struct net_device *dev)

{

	if (netdev_need_ops_lock(dev))

		netdev_lock(dev);

	else

		rtnl_lock();

}



static inline void netdev_unlock_ops_compat(struct net_device *dev)

{

	if (netdev_need_ops_lock(dev))

		netdev_unlock(dev);

	else

		rtnl_unlock();

}



static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,

				     const struct lockdep_map *b)

{

include/net/netdev_queues.h

+3 −1
Original line number Diff line number Diff line
@@ -85,9 +85,11 @@ struct netdev_queue_stats_tx { 
 * for some of the events is not maintained, and reliable "total" cannot

 * be provided).

 *

 * Ops are called under the instance lock if netdev_need_ops_lock()

 * returns true, otherwise under rtnl_lock.

 * Device drivers can assume that when collecting total device stats,

 * the @get_base_stats and subsequent per-queue calls are performed

 * "atomically" (without releasing the rtnl_lock).

 * "atomically" (without releasing the relevant lock).

 *

 * Device drivers are encouraged to reset the per-queue statistics when

 * number of queues change. This is because the primary use case for