[PATCH net-next] net: run netdev work under the ops-compat lock

[PATCH net-next] net: run netdev work under the ops-compat lock

[Jakub Kicinski]

netdev_work_proc() took rtnl_lock() for the whole batch, forcing
even ops-locked drivers onto rtnl just to run their rx_mode / ndo_work
callbacks. Take the per-device ops-compat lock instead: the instance
lock for ops-locked drivers, rtnl_lock for the rest. That's what the
callbacks already assert (e.g. netif_rx_mode_run() asserts
netdev_assert_locked_ops_compat()), and non-ops-locked work
such as vlan ndo_work keeps rtnl, since its compat lock resolves
to rtnl anyway.

Without this adding more uses of ndo_work, for cases which don't
need rtnl_lock feels like a step back.

We can drop the comment about the extra ref now, with the relocking
it's a necessity.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: corbet@lwn.net
CC: skhan@linuxfoundation.org
CC: rdunlap@infradead.org
CC: sdf@fomichev.me
CC: kuniyu@google.com
CC: linux-doc@vger.kernel.org
---
 Documentation/networking/netdevices.rst | 14 ++++++++++----
 include/linux/netdevice.h               |  5 ++++-
 net/core/netdev_work.c                  | 15 ++-------------
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst
index db71d4283032..aac33497813f 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -292,15 +292,21 @@ struct net_device synchronization rules
 	in process context.
 
 ndo_set_rx_mode_async:
-	Synchronization: rtnl_lock() semaphore. In addition, netdev instance
-	lock if the driver implements queue management or shaper API.
+	Synchronization: netdev instance lock for "ops locked" drivers,
+	rtnl_lock() semaphore for all other drivers.
 	Context: process (from a work queue)
 	Notes: Async version of ndo_set_rx_mode which runs in process
 	context. Receives snapshots of the unicast and multicast address lists.
 
 ndo_change_rx_flags:
-	Synchronization: rtnl_lock() semaphore. In addition, netdev instance
-	lock if the driver implements queue management or shaper API.
+	Synchronization: netdev instance lock for "ops locked" drivers,
+	rtnl_lock() semaphore for all other drivers.
+
+ndo_work:
+	Synchronization: netdev instance lock for "ops locked" drivers,
+	rtnl_lock() semaphore for all other drivers.
+	Context: process (from a work queue)
+	Notes: Runs deferred work scheduled with netdev_work_sched().
 
 ndo_setup_tc:
 	Locking depends on ``tc_setup_type``. For most types the callback
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8454646d6a45..17b8fb8e94fe 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1135,13 +1135,16 @@ struct netdev_net_notifier {
  *				struct netdev_hw_addr_list *uc,
  *				struct netdev_hw_addr_list *mc);
  *	Async version of ndo_set_rx_mode which runs in process context
- *	with rtnl_lock and netdev_lock_ops(dev) held. The uc/mc parameters
+ *	under the netdev instance lock for "ops locked" drivers, or
+ *	rtnl_lock for all other drivers. The uc/mc parameters
  *	are snapshots of the address lists - iterate with
  *	netdev_hw_addr_list_for_each(ha, uc). Return 0 on success or a
  *	negative errno to request a retry via the core backoff.
  *
  * void (*ndo_work)(struct net_device *dev, unsigned long events);
  *	Run deferred work scheduled with netdev_work_sched(@events).
+ *	Runs in process context under the netdev instance lock for "ops
+ *	locked" drivers, or rtnl_lock for all other drivers.
  *
  * int (*ndo_set_mac_address)(struct net_device *dev, void *addr);
  *	This function  is called when the Media Access Control address
diff --git a/net/core/netdev_work.c b/net/core/netdev_work.c
index e721a06d58df..c99967907e18 100644
--- a/net/core/netdev_work.c
+++ b/net/core/netdev_work.c
@@ -3,7 +3,6 @@
 #include <linux/export.h>
 #include <linux/list.h>
 #include <linux/netdevice.h>
-#include <linux/rtnetlink.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <net/netdev_lock.h>
@@ -129,8 +128,6 @@ static void netdev_work_run(struct net_device *dev, unsigned long events,
 
 static void netdev_work_proc(struct work_struct *work)
 {
-	rtnl_lock();
-
 	while (true) {
 		unsigned long events = 0, core = 0;
 		netdevice_tracker tracker;
@@ -143,16 +140,10 @@ static void netdev_work_proc(struct work_struct *work)
 		}
 		dev = list_first_entry(&netdev_work_list, struct net_device,
 				       work_node);
-		/* Take a temporary reference so @dev can't be freed while we
-		 * drop the lock to grab its ops lock; the work reference is
-		 * only released once we claim the work below.
-		 * The re-locking dance is to ensure that ops lock is enough
-		 * to ensure canceling work is not racy with dequeue.
-		 */
 		netdev_hold(dev, &tracker, GFP_ATOMIC);
 		spin_unlock_bh(&netdev_work_lock);
 
-		netdev_lock_ops(dev);
+		netdev_lock_ops_compat(dev);
 		spin_lock_bh(&netdev_work_lock);
 		if (!list_empty(&dev->work_node)) {
 			list_del_init(&dev->work_node);
@@ -169,10 +160,8 @@ static void netdev_work_proc(struct work_struct *work)
 		spin_unlock_bh(&netdev_work_lock);
 
 		netdev_work_run(dev, events, core);
-		netdev_unlock_ops(dev);
+		netdev_unlock_ops_compat(dev);
 
 		netdev_put(dev, &tracker);
 	}
-
-	rtnl_unlock();
 }
-- 
2.55.0

Re: [PATCH net-next] net: run netdev work under the ops-compat lock

[Paolo Abeni]

On 9/4/26 8:00 PM, Jakub Kicinski wrote:
> netdev_work_proc() took rtnl_lock() for the whole batch, forcing
> even ops-locked drivers onto rtnl just to run their rx_mode / ndo_work
> callbacks. Take the per-device ops-compat lock instead: the instance
> lock for ops-locked drivers, rtnl_lock for the rest. That's what the
> callbacks already assert (e.g. netif_rx_mode_run() asserts
> netdev_assert_locked_ops_compat()), and non-ops-locked work
> such as vlan ndo_work keeps rtnl, since its compat lock resolves
> to rtnl anyway.
> 
> Without this adding more uses of ndo_work, for cases which don't
> need rtnl_lock feels like a step back.
> 
> We can drop the comment about the extra ref now, with the relocking
> it's a necessity.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Dump question: can the `netdev_work_list` work list grow significantly
under non pathological conditions? netdev_work_proc is now going to
acquire and release the rtnl lock n-times for non ops-enabled drivers.

Would it be overkill trying to acquire the rtnl lock at most once (i.e.
keeping the netdev_need_ops_lock() and legacy drivers on separate list
and process them accordingly?)

/P

Re: [PATCH net-next] net: run netdev work under the ops-compat lock

[Jakub Kicinski]

Oops, looks like this has been sitting in my outbox for some reason

On Tue, 8 Sep 2026 13:26:52 +0200 Paolo Abeni wrote:
> On 9/4/26 8:00 PM, Jakub Kicinski wrote:
> > netdev_work_proc() took rtnl_lock() for the whole batch, forcing
> > even ops-locked drivers onto rtnl just to run their rx_mode / ndo_work
> > callbacks. Take the per-device ops-compat lock instead: the instance
> > lock for ops-locked drivers, rtnl_lock for the rest. That's what the
> > callbacks already assert (e.g. netif_rx_mode_run() asserts
> > netdev_assert_locked_ops_compat()), and non-ops-locked work
> > such as vlan ndo_work keeps rtnl, since its compat lock resolves
> > to rtnl anyway.
> > 
> > Without this adding more uses of ndo_work, for cases which don't
> > need rtnl_lock feels like a step back.
> > 
> > We can drop the comment about the extra ref now, with the relocking
> > it's a necessity.
> > 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>  
> Dump question: can the `netdev_work_list` work list grow significantly
> under non pathological conditions? netdev_work_proc is now going to
> acquire and release the rtnl lock n-times for non ops-enabled drivers.
> 
> Would it be overkill trying to acquire the rtnl lock at most once (i.e.
> keeping the netdev_need_ops_lock() and legacy drivers on separate list
> and process them accordingly?)

I haven't seen any evidence of such relocking causing issues so I didn't
want to complicate the code. We have a similar situation in a number of
netdev genetlink dump handlers.

Re: [PATCH net-next] net: run netdev work under the ops-compat lock

[Paolo Abeni]

On 9/10/26 3:22 AM, Jakub Kicinski wrote:
> Oops, looks like this has been sitting in my outbox for some reason
> 
> On Tue, 8 Sep 2026 13:26:52 +0200 Paolo Abeni wrote:
>> On 9/4/26 8:00 PM, Jakub Kicinski wrote:
>>> netdev_work_proc() took rtnl_lock() for the whole batch, forcing
>>> even ops-locked drivers onto rtnl just to run their rx_mode / ndo_work
>>> callbacks. Take the per-device ops-compat lock instead: the instance
>>> lock for ops-locked drivers, rtnl_lock for the rest. That's what the
>>> callbacks already assert (e.g. netif_rx_mode_run() asserts
>>> netdev_assert_locked_ops_compat()), and non-ops-locked work
>>> such as vlan ndo_work keeps rtnl, since its compat lock resolves
>>> to rtnl anyway.
>>>
>>> Without this adding more uses of ndo_work, for cases which don't
>>> need rtnl_lock feels like a step back.
>>>
>>> We can drop the comment about the extra ref now, with the relocking
>>> it's a necessity.
>>>
>>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>  
>> Dump question: can the `netdev_work_list` work list grow significantly
>> under non pathological conditions? netdev_work_proc is now going to
>> acquire and release the rtnl lock n-times for non ops-enabled drivers.
>>
>> Would it be overkill trying to acquire the rtnl lock at most once (i.e.
>> keeping the netdev_need_ops_lock() and legacy drivers on separate list
>> and process them accordingly?)
> 
> I haven't seen any evidence of such relocking causing issues so I didn't
> want to complicate the code. We have a similar situation in a number of
> netdev genetlink dump handlers.

I see. I suspect containers orchestration/control-plane heavy
application could notice if they depends on the relevant handlers (I
think at least the one I'm less unfamiliar with doesn't) when the
relevant change will reach downstream.

I think we can improve incrementally as need (at least here should be
quite straight-forward, for gennetlink dump handlers it looks more complex).

/P

Re: [PATCH net-next] net: run netdev work under the ops-compat lock

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri,  4 Sep 2026 11:00:58 -0700 you wrote:
> netdev_work_proc() took rtnl_lock() for the whole batch, forcing
> even ops-locked drivers onto rtnl just to run their rx_mode / ndo_work
> callbacks. Take the per-device ops-compat lock instead: the instance
> lock for ops-locked drivers, rtnl_lock for the rest. That's what the
> callbacks already assert (e.g. netif_rx_mode_run() asserts
> netdev_assert_locked_ops_compat()), and non-ops-locked work
> such as vlan ndo_work keeps rtnl, since its compat lock resolves
> to rtnl anyway.
> 
> [...]

Here is the summary with links:
  - [net-next] net: run netdev work under the ops-compat lock
    https://git.kernel.org/netdev/net-next/c/5a81c72ef9ad

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html