[PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[Jakub Kicinski]

netdev_lockdep_set_classes() puts dev->lock in a separate lockdep class,
by type (netkit vs dummy etc), with the intent of keeping the instance
locks of individual devices as independent from each other as possible.
In practice it does the opposite. lockdep only calls the cmp_fn for locks
of the same class, so netdev_lock_cmp_fn() never gets a say when devices
from different classes are nested. Instead lockdep records a dependency
between the classes, and reports a circular locking problem as soon as
the nesting happens the other way round, e.g. when devices are unregistered
in a batch.

  ======================================================
  WARNING: possible circular locking dependency detected
  7.3.0-rc3+ #26 Not tainted
  ------------------------------------------------------
  kworker/u256:1/326 is trying to acquire lock:
  ff110000104fce28 (&dev_instance_lock_key#6){+.+.}-{4:4}, at:
  unregister_netdevice_many_notify+0x1141/0x1c30

  but task is already holding lock:
  ff110000127f2e28 (&dev_instance_lock_key#7){+.+.}-{4:4}, at:
  unregister_netdevice_many_notify+0x1141/0x1c30

  -> #1 (&dev_instance_lock_key#7){+.+.}-{4:4}:
         __lock_acquire+0x767/0xd60
         lock_acquire.part.0+0xd0/0x260
         __mutex_lock+0x17d/0x1f20
         unregister_netdevice_many_notify+0x1141/0x1c30
         default_device_exit_batch+0x3ee/0x520
         ops_undo_list+0x2cc/0x8a0
         cleanup_net+0x442/0x9c0
         process_one_work+0x951/0x1ab0
         worker_thread+0x5a6/0xd10
         kthread+0x339/0x430
         ret_from_fork+0x4a4/0x6f0
         ret_from_fork_asm+0x1a/0x30

  -> #0 (&dev_instance_lock_key#6){+.+.}-{4:4}:
         check_prev_add+0xeb/0xe60
         validate_chain+0x598/0x900
         __lock_acquire+0x767/0xd60
         lock_acquire.part.0+0xd0/0x260
         __mutex_lock+0x17d/0x1f20
         unregister_netdevice_many_notify+0x1141/0x1c30
         default_device_exit_batch+0x3ee/0x520
         ops_undo_list+0x2cc/0x8a0
         cleanup_net+0x442/0x9c0
         process_one_work+0x951/0x1ab0
         worker_thread+0x5a6/0xd10
         kthread+0x339/0x430
         ret_from_fork+0x4a4/0x6f0
         ret_from_fork_asm+0x1a/0x30

   Possible unsafe locking scenario:

         CPU0                    CPU1
         ----                    ----
    lock(&dev_instance_lock_key#7);
                                 lock(&dev_instance_lock_key#6);
                                 lock(&dev_instance_lock_key#7);
    lock(&dev_instance_lock_key#6);

   *** DEADLOCK ***

  locks held by kworker/u256:1/326: 6, last CPU#6:
   #0: ff11000001c2b540 ((wq_completion)netns){+.+.}-{0:0}, at:
  process_one_work+0x117c/0x1ab0
   #1: ffa0000001a3fd18 (net_cleanup_work){+.+.}-{0:0}, at:
  process_one_work+0x8ce/0x1ab0
   #2: ffffffff98f53288 (pernet_ops_rwsem){++++}-{4:4}, at:
  cleanup_net+0xc1/0x9c0
   #3: ffffffff98f6ede0 (rtnl_mutex){+.+.}-{4:4}, at:
  default_device_exit_batch+0x92/0x520
   #4: ff1100001321ae28 (&dev_instance_lock_key#7){+.+.}-{4:4}, at:
  unregister_netdevice_many_notify+0x1141/0x1c30

We can't put all devices in the same class, that'd be too permissive.
netdev_nl_queue_create_doit() and netdev_nl_bind_tx_doit() lock
a virtual device (netkit) before a physical device, without rtnl_lock.
We can never allow locking in the opposite order even under rtnl_lock.
cmp_fn cannot enforce this sort of rule: lockdep keys its chain cache on
the sequence of lock classes, so with a single class both orders hash
to the same chain and only whichever happens first is validated.

netdev_lock_cmp_fn() itself has another source of false-negatives.
lockdep calls it from within __lock_acquire(), with the recursion counter
already raised, so lock_is_held_type() always returns LOCK_STATE_UNKNOWN,
which means lockdep_rtnl_is_held() always returns true / held.
Use rtnl_is_locked(), which looks at the mutex directly and does
work from that context. We may still miss a bad case if some other
process is holding the lock, not us, but that's better than the
100% false negative rate of lockdep_rtnl_is_held().

One last thing, lockdep compares the address of the cmp function,
so it can't be a static inline - that would work similarly to having
separate classes, again. Move it to a source file.

Tested by nesting two instance locks from a module, one scenario per boot
since the first splat turns debug_locks off:

  first       second      rtnl      reported
  -----------------------------------------------------------
  virt->virt              no        recursive locking
  virt->virt              yes       -
  virt->phys              no        -
  virt->phys              yes       -
  phys->virt              no        - (records the edge)
  phys->virt              yes       - (records the edge)
  phys->phys              no        recursive locking
  phys->phys              yes       -
  virt->phys  phys->virt  no, no    circular dependency
  virt->phys  phys->virt  no, yes   circular dependency
  virt->phys  phys->virt  yes, no   circular dependency
  virt->phys  phys->virt  yes, yes  circular dependency
  phys->virt  virt->phys  no, no    circular dependency
  phys->virt  virt->phys  yes, yes  circular dependency
  virt->virt  virt->virt  yes, no   recursive locking
  phys->phys  phys->phys  yes, no   recursive locking

(netkit and dummy stood in for the virtual devices, virtio_net and
netdevsim for the physical ones.)

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v1: https://lore.kernel.org/20260923223545.3815583-1-kuba@kernel.org
---
 include/net/netdev_lock.h | 20 +++-----------------
 net/core/dev.c            | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h
index 9fb3e93857c3..edd8cc2e2b36 100644
--- a/include/net/netdev_lock.h
+++ b/include/net/netdev_lock.h
@@ -110,34 +110,20 @@ static inline int netdev_is_locked_ops_compat(const struct net_device *dev)
 	return lockdep_rtnl_is_held();
 }
 
-static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,
-				     const struct lockdep_map *b)
-{
-	if (a == b)
-		return 0;
-
-	/* Allow locking multiple devices only under rtnl_lock,
-	 * the exact order doesn't matter.
-	 * Note that upper devices don't lock their ops, so nesting
-	 * mostly happens in batched device removal for now.
-	 */
-	return lockdep_rtnl_is_held() ? -1 : 1;
-}
+void netdev_set_instance_lock_class(struct net_device *dev);
 
 #define netdev_lockdep_set_classes(dev)				\
 {								\
 	static struct lock_class_key qdisc_tx_busylock_key;	\
 	static struct lock_class_key qdisc_xmit_lock_key;	\
 	static struct lock_class_key dev_addr_list_lock_key;	\
-	static struct lock_class_key dev_instance_lock_key;	\
 	unsigned int i;						\
 								\
 	(dev)->qdisc_tx_busylock = &qdisc_tx_busylock_key;	\
 	lockdep_set_class(&(dev)->addr_list_lock,		\
 			  &dev_addr_list_lock_key);		\
-	lockdep_set_class(&(dev)->lock,				\
-			  &dev_instance_lock_key);		\
-	lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);	\
+	if (IS_ENABLED(CONFIG_PROVE_LOCKING))			\
+		netdev_set_instance_lock_class(dev);		\
 	for (i = 0; i < (dev)->num_tx_queues; i++)		\
 		lockdep_set_class(&(dev)->_tx[i]._xmit_lock,	\
 				  &qdisc_xmit_lock_key);	\
diff --git a/net/core/dev.c b/net/core/dev.c
index f660fccfc0db..a8eb382f40ca 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -568,6 +568,41 @@ static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
 }
 #endif
 
+#ifdef CONFIG_PROVE_LOCKING
+static int netdev_lock_cmp_fn(const struct lockdep_map *a,
+			      const struct lockdep_map *b)
+{
+	if (a == b)
+		return 0;
+
+	/* @a and @b must be of same class - both virtual or physical.
+	 * cmp_fn won't be called for devices of different classes.
+	 *
+	 * For the same class only allow nesting under the protection
+	 * of rtnl_lock. Note that we can't use lockdep_rtnl_is_held()
+	 * here, it always answers UNKNOWN from within lockdep.
+	 */
+	return rtnl_is_locked() ? -1 : 1;
+}
+
+/* A virtual device can be locked before the physical device it leases
+ * queues from, see netdev_nl_queue_create_doit(). Keep the two kinds
+ * in separate classes so the dependency graph enforces the order;
+ * netdev_lock_cmp_fn() then only has to rule on same-class nesting.
+ */
+void netdev_set_instance_lock_class(struct net_device *dev)
+{
+	static struct lock_class_key netdev_virt_instance_lock_key;
+
+	if (dev->dev.parent)
+		return;
+
+	lockdep_set_class(&dev->lock, &netdev_virt_instance_lock_key);
+	lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);
+}
+EXPORT_SYMBOL_GPL(netdev_set_instance_lock_class);
+#endif
+
 /*******************************************************************************
  *
  *		Protocol management and registration routines
@@ -12183,6 +12218,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 #endif
 
 	mutex_init(&dev->lock);
+	/* see also netdev_set_instance_lock_class() */
+	lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);
 	netif_rx_mode_init(dev);
 
 	dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
-- 
2.55.0

[PATCH net-next v2 2/2] selftests: net: add test for netdev instance lock ordering on unregister

[Jakub Kicinski]

Dismantle two netns holding a dummy and a netkit pair registered in
opposite order. unregister_netdevice_many_notify() takes the instance
locks of all the devices at once, nesting the two kinds of devices one
way for the first netns and the other way for the second. With lockdep
enabled this used to trigger a circular locking report.

Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/net/Makefile       |  1 +
 tools/testing/selftests/net/netdev_lock.py | 42 ++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100755 tools/testing/selftests/net/netdev_lock.py

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 3ee3378f8b26..d4ca82fec0b4 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -67,6 +67,7 @@ TEST_PROGS := \
 	nat6to4.sh \
 	ndisc_unsolicited_na_test.sh \
 	netdev-l2addr.sh \
+	netdev_lock.py \
 	netdevice.sh \
 	netns-name.sh \
 	netns-sysctl.sh \
diff --git a/tools/testing/selftests/net/netdev_lock.py b/tools/testing/selftests/net/netdev_lock.py
new file mode 100755
index 000000000000..0015b987eb9d
--- /dev/null
+++ b/tools/testing/selftests/net/netdev_lock.py
@@ -0,0 +1,42 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""Tests for the netdev instance lock."""
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ip
+from lib.py import NetNS
+
+
+def test_unreg_order() -> None:
+    """Dismantle two netns holding the same two kinds of ops locked device,
+    registered in opposite order.
+
+    unregister_netdevice_many_notify() takes the instance lock of every
+    device in the batch and holds them all at once, so the two kinds end up
+    nested one way round for the first netns and the other way round for
+    the second.
+    """
+    with NetNS() as ns1, NetNS() as ns2:
+        net1, net2 = str(ns1), str(ns2)
+
+        ip("link add du0 type dummy", ns=net1)
+        ip("link add nk0 type netkit peer name nk1", ns=net1)
+
+        ip("link add nk0 type netkit peer name nk1", ns=net2)
+        ip("link add du0 type dummy", ns=net2)
+
+        # only devices which are up get locked during unregister
+        for net in (net1, net2):
+            for dev in ("du0", "nk0", "nk1"):
+                ip(f"link set {dev} up", ns=net)
+
+
+def main() -> None:
+    """Ksft boilerplate main."""
+    ksft_run([test_unreg_order])
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.55.0

Re: [PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[Eric Dumazet]

On Sat, Sep 26, 2026 at 6:18 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> netdev_lockdep_set_classes() puts dev->lock in a separate lockdep class,
> by type (netkit vs dummy etc), with the intent of keeping the instance
> locks of individual devices as independent from each other as possible.
> In practice it does the opposite. lockdep only calls the cmp_fn for locks
> of the same class, so netdev_lock_cmp_fn() never gets a say when devices
> from different classes are nested. Instead lockdep records a dependency
> between the classes, and reports a circular locking problem as soon as
> the nesting happens the other way round, e.g. when devices are unregistered
> in a batch.
>


> We can't put all devices in the same class, that'd be too permissive.
> netdev_nl_queue_create_doit() and netdev_nl_bind_tx_doit() lock
> a virtual device (netkit) before a physical device, without rtnl_lock.
> We can never allow locking in the opposite order even under rtnl_lock.
> cmp_fn cannot enforce this sort of rule: lockdep keys its chain cache on
> the sequence of lock classes, so with a single class both orders hash
> to the same chain and only whichever happens first is validated.
>
> netdev_lock_cmp_fn() itself has another source of false-negatives.
> lockdep calls it from within __lock_acquire(), with the recursion counter
> already raised, so lock_is_held_type() always returns LOCK_STATE_UNKNOWN,
> which means lockdep_rtnl_is_held() always returns true / held.
> Use rtnl_is_locked(), which looks at the mutex directly and does
> work from that context. We may still miss a bad case if some other
> process is holding the lock, not us, but that's better than the
> 100% false negative rate of lockdep_rtnl_is_held().
>
> One last thing, lockdep compares the address of the cmp function,
> so it can't be a static inline - that would work similarly to having
> separate classes, again. Move it to a source file.
>
> Tested by nesting two instance locks from a module, one scenario per boot
> since the first splat turns debug_locks off:
>
>   first       second      rtnl      reported
>   -----------------------------------------------------------
>   virt->virt              no        recursive locking
>   virt->virt              yes       -
>   virt->phys              no        -
>   virt->phys              yes       -
>   phys->virt              no        - (records the edge)
>   phys->virt              yes       - (records the edge)
>   phys->phys              no        recursive locking
>   phys->phys              yes       -
>   virt->phys  phys->virt  no, no    circular dependency
>   virt->phys  phys->virt  no, yes   circular dependency
>   virt->phys  phys->virt  yes, no   circular dependency
>   virt->phys  phys->virt  yes, yes  circular dependency
>   phys->virt  virt->phys  no, no    circular dependency
>   phys->virt  virt->phys  yes, yes  circular dependency
>   virt->virt  virt->virt  yes, no   recursive locking
>   phys->phys  phys->phys  yes, no   recursive locking
>
> (netkit and dummy stood in for the virtual devices, virtio_net and
> netdevsim for the physical ones.)
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v1: https://lore.kernel.org/20260923223545.3815583-1-kuba@kernel.org
> ---
>  include/net/netdev_lock.h | 20 +++-----------------
>  net/core/dev.c            | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h
> index 9fb3e93857c3..edd8cc2e2b36 100644
> --- a/include/net/netdev_lock.h
> +++ b/include/net/netdev_lock.h
> @@ -110,34 +110,20 @@ static inline int netdev_is_locked_ops_compat(const struct net_device *dev)
>         return lockdep_rtnl_is_held();
>  }
>
> -static inline int netdev_lock_cmp_fn(const struct lockdep_map *a,
> -                                    const struct lockdep_map *b)
> -{
> -       if (a == b)
> -               return 0;
> -
> -       /* Allow locking multiple devices only under rtnl_lock,
> -        * the exact order doesn't matter.
> -        * Note that upper devices don't lock their ops, so nesting
> -        * mostly happens in batched device removal for now.
> -        */
> -       return lockdep_rtnl_is_held() ? -1 : 1;
> -}
> +void netdev_set_instance_lock_class(struct net_device *dev);
>
>  #define netdev_lockdep_set_classes(dev)                                \
>  {                                                              \
>         static struct lock_class_key qdisc_tx_busylock_key;     \
>         static struct lock_class_key qdisc_xmit_lock_key;       \
>         static struct lock_class_key dev_addr_list_lock_key;    \
> -       static struct lock_class_key dev_instance_lock_key;     \
>         unsigned int i;                                         \
>                                                                 \
>         (dev)->qdisc_tx_busylock = &qdisc_tx_busylock_key;      \
>         lockdep_set_class(&(dev)->addr_list_lock,               \
>                           &dev_addr_list_lock_key);             \
> -       lockdep_set_class(&(dev)->lock,                         \
> -                         &dev_instance_lock_key);              \
> -       lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);  \
> +       if (IS_ENABLED(CONFIG_PROVE_LOCKING))                   \
> +               netdev_set_instance_lock_class(dev);            \
>         for (i = 0; i < (dev)->num_tx_queues; i++)              \
>                 lockdep_set_class(&(dev)->_tx[i]._xmit_lock,    \
>                                   &qdisc_xmit_lock_key);        \
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0db..a8eb382f40ca 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -568,6 +568,41 @@ static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
>  }
>  #endif
>
> +#ifdef CONFIG_PROVE_LOCKING
> +static int netdev_lock_cmp_fn(const struct lockdep_map *a,
> +                             const struct lockdep_map *b)
> +{
> +       if (a == b)
> +               return 0;
> +
> +       /* @a and @b must be of same class - both virtual or physical.
> +        * cmp_fn won't be called for devices of different classes.
> +        *
> +        * For the same class only allow nesting under the protection
> +        * of rtnl_lock. Note that we can't use lockdep_rtnl_is_held()
> +        * here, it always answers UNKNOWN from within lockdep.
> +        */
> +       return rtnl_is_locked() ? -1 : 1;
> +}
> +
> +/* A virtual device can be locked before the physical device it leases
> + * queues from, see netdev_nl_queue_create_doit(). Keep the two kinds
> + * in separate classes so the dependency graph enforces the order;
> + * netdev_lock_cmp_fn() then only has to rule on same-class nesting.
> + */
> +void netdev_set_instance_lock_class(struct net_device *dev)
> +{
> +       static struct lock_class_key netdev_virt_instance_lock_key;
> +

Right, they already get their class and cmp_fn in alloc_netdev_mqs().

> +       if (dev->dev.parent)
> +               return;
> +
> +       lockdep_set_class(&dev->lock, &netdev_virt_instance_lock_key);
> +       lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);
> +}
> +EXPORT_SYMBOL_GPL(netdev_set_instance_lock_class);
> +#endif
> +
>  /*******************************************************************************
>   *
>   *             Protocol management and registration routines
> @@ -12183,6 +12218,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>  #endif
>
>         mutex_init(&dev->lock);
> +       /* see also netdev_set_instance_lock_class() */
> +       lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);
>         netif_rx_mode_init(dev);
>
>         dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;

Some virttual drivers (like  tun and wireguard) do not use yet
netdev_lockdep_set_classes(),
I'm not sure if they eventually will.

We might ease their future request_ops_lock = true conversion by calling
netdev_set_instance_lock_class(dev) directly inside register_netdevice()

-> No EXPORT_SYMBOL_GPL(netdev_set_instance_lock_class) would be needed.
In fact netdev_set_instance_lock_class couldd be static in net/core/dev.c

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks.

Re: [PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[Stanislav Fomichev]

On 09/25, Jakub Kicinski wrote:
> netdev_lockdep_set_classes() puts dev->lock in a separate lockdep class,
> by type (netkit vs dummy etc), with the intent of keeping the instance
> locks of individual devices as independent from each other as possible.
> In practice it does the opposite. lockdep only calls the cmp_fn for locks
> of the same class, so netdev_lock_cmp_fn() never gets a say when devices
> from different classes are nested. Instead lockdep records a dependency
> between the classes, and reports a circular locking problem as soon as
> the nesting happens the other way round, e.g. when devices are unregistered
> in a batch.
> 
>   ======================================================
>   WARNING: possible circular locking dependency detected
>   7.3.0-rc3+ #26 Not tainted
>   ------------------------------------------------------
>   kworker/u256:1/326 is trying to acquire lock:
>   ff110000104fce28 (&dev_instance_lock_key#6){+.+.}-{4:4}, at:
>   unregister_netdevice_many_notify+0x1141/0x1c30
> 
>   but task is already holding lock:
>   ff110000127f2e28 (&dev_instance_lock_key#7){+.+.}-{4:4}, at:
>   unregister_netdevice_many_notify+0x1141/0x1c30
> 
>   -> #1 (&dev_instance_lock_key#7){+.+.}-{4:4}:
>          __lock_acquire+0x767/0xd60
>          lock_acquire.part.0+0xd0/0x260
>          __mutex_lock+0x17d/0x1f20
>          unregister_netdevice_many_notify+0x1141/0x1c30
>          default_device_exit_batch+0x3ee/0x520
>          ops_undo_list+0x2cc/0x8a0
>          cleanup_net+0x442/0x9c0
>          process_one_work+0x951/0x1ab0
>          worker_thread+0x5a6/0xd10
>          kthread+0x339/0x430
>          ret_from_fork+0x4a4/0x6f0
>          ret_from_fork_asm+0x1a/0x30
> 
>   -> #0 (&dev_instance_lock_key#6){+.+.}-{4:4}:
>          check_prev_add+0xeb/0xe60
>          validate_chain+0x598/0x900
>          __lock_acquire+0x767/0xd60
>          lock_acquire.part.0+0xd0/0x260
>          __mutex_lock+0x17d/0x1f20
>          unregister_netdevice_many_notify+0x1141/0x1c30
>          default_device_exit_batch+0x3ee/0x520
>          ops_undo_list+0x2cc/0x8a0
>          cleanup_net+0x442/0x9c0
>          process_one_work+0x951/0x1ab0
>          worker_thread+0x5a6/0xd10
>          kthread+0x339/0x430
>          ret_from_fork+0x4a4/0x6f0
>          ret_from_fork_asm+0x1a/0x30
> 
>    Possible unsafe locking scenario:
> 
>          CPU0                    CPU1
>          ----                    ----
>     lock(&dev_instance_lock_key#7);
>                                  lock(&dev_instance_lock_key#6);
>                                  lock(&dev_instance_lock_key#7);
>     lock(&dev_instance_lock_key#6);
> 
>    *** DEADLOCK ***
> 
>   locks held by kworker/u256:1/326: 6, last CPU#6:
>    #0: ff11000001c2b540 ((wq_completion)netns){+.+.}-{0:0}, at:
>   process_one_work+0x117c/0x1ab0
>    #1: ffa0000001a3fd18 (net_cleanup_work){+.+.}-{0:0}, at:
>   process_one_work+0x8ce/0x1ab0
>    #2: ffffffff98f53288 (pernet_ops_rwsem){++++}-{4:4}, at:
>   cleanup_net+0xc1/0x9c0
>    #3: ffffffff98f6ede0 (rtnl_mutex){+.+.}-{4:4}, at:
>   default_device_exit_batch+0x92/0x520
>    #4: ff1100001321ae28 (&dev_instance_lock_key#7){+.+.}-{4:4}, at:
>   unregister_netdevice_many_notify+0x1141/0x1c30
> 
> We can't put all devices in the same class, that'd be too permissive.
> netdev_nl_queue_create_doit() and netdev_nl_bind_tx_doit() lock
> a virtual device (netkit) before a physical device, without rtnl_lock.
> We can never allow locking in the opposite order even under rtnl_lock.
> cmp_fn cannot enforce this sort of rule: lockdep keys its chain cache on
> the sequence of lock classes, so with a single class both orders hash
> to the same chain and only whichever happens first is validated.
> 
> netdev_lock_cmp_fn() itself has another source of false-negatives.
> lockdep calls it from within __lock_acquire(), with the recursion counter
> already raised, so lock_is_held_type() always returns LOCK_STATE_UNKNOWN,
> which means lockdep_rtnl_is_held() always returns true / held.
> Use rtnl_is_locked(), which looks at the mutex directly and does
> work from that context. We may still miss a bad case if some other
> process is holding the lock, not us, but that's better than the
> 100% false negative rate of lockdep_rtnl_is_held().
> 
> One last thing, lockdep compares the address of the cmp function,
> so it can't be a static inline - that would work similarly to having
> separate classes, again. Move it to a source file.
> 
> Tested by nesting two instance locks from a module, one scenario per boot
> since the first splat turns debug_locks off:
> 
>   first       second      rtnl      reported
>   -----------------------------------------------------------
>   virt->virt              no        recursive locking
>   virt->virt              yes       -
>   virt->phys              no        -
>   virt->phys              yes       -
>   phys->virt              no        - (records the edge)
>   phys->virt              yes       - (records the edge)
>   phys->phys              no        recursive locking
>   phys->phys              yes       -
>   virt->phys  phys->virt  no, no    circular dependency
>   virt->phys  phys->virt  no, yes   circular dependency
>   virt->phys  phys->virt  yes, no   circular dependency
>   virt->phys  phys->virt  yes, yes  circular dependency
>   phys->virt  virt->phys  no, no    circular dependency
>   phys->virt  virt->phys  yes, yes  circular dependency
>   virt->virt  virt->virt  yes, no   recursive locking
>   phys->phys  phys->phys  yes, no   recursive locking
> 
> (netkit and dummy stood in for the virtual devices, virtio_net and
> netdevsim for the physical ones.)
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[Jakub Kicinski]

On Sat, 26 Sep 2026 09:08:16 +0200 Eric Dumazet wrote:
> >         mutex_init(&dev->lock);
> > +       /* see also netdev_set_instance_lock_class() */
> > +       lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);
> >         netif_rx_mode_init(dev);
> >
> >         dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;  
> 
> Some virttual drivers (like  tun and wireguard) do not use yet
> netdev_lockdep_set_classes(),
> I'm not sure if they eventually will.
> 
> We might ease their future request_ops_lock = true conversion by calling
> netdev_set_instance_lock_class(dev) directly inside register_netdevice()
> 
> -> No EXPORT_SYMBOL_GPL(netdev_set_instance_lock_class) would be needed.  
> In fact netdev_set_instance_lock_class couldd be static in net/core/dev.c

Ack, maybe I lazied out on this one a little. I wasn't sure what the
consequences are of not setting the class before the first lock use.
I _think_ I've seen a splat at some point when I did that.

I will investigate further if I need to respin

Re: [PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[Paolo Abeni]

On 9/26/26 06:18, Jakub Kicinski wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0db..a8eb382f40ca 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -568,6 +568,41 @@ static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
>   }
>   #endif
>   
> +#ifdef CONFIG_PROVE_LOCKING
> +static int netdev_lock_cmp_fn(const struct lockdep_map *a,
> +			      const struct lockdep_map *b)
> +{
> +	if (a == b)
> +		return 0;
> +
> +	/* @a and @b must be of same class - both virtual or physical.
> +	 * cmp_fn won't be called for devices of different classes.
> +	 *
> +	 * For the same class only allow nesting under the protection
> +	 * of rtnl_lock. Note that we can't use lockdep_rtnl_is_held()
> +	 * here, it always answers UNKNOWN from within lockdep.
> +	 */
> +	return rtnl_is_locked() ? -1 : 1;
> +}
> +
> +/* A virtual device can be locked before the physical device it leases
> + * queues from, see netdev_nl_queue_create_doit(). Keep the two kinds
> + * in separate classes so the dependency graph enforces the order;
> + * netdev_lock_cmp_fn() then only has to rule on same-class nesting.
> + */
> +void netdev_set_instance_lock_class(struct net_device *dev)
> +{
> +	static struct lock_class_key netdev_virt_instance_lock_key;
> +
> +	if (dev->dev.parent)
> +		return;
> +
> +	lockdep_set_class(&dev->lock, &netdev_virt_instance_lock_key);
> +	lock_set_cmp_fn(&dev->lock, netdev_lock_cmp_fn, NULL);
> +}

Sashiko noted that mlx5 IB should enter !dev->dev.parent code path and
could possibly need a follow-up.

/P

Re: [PATCH net-next v2 1/2] net: use two lockdep classes for the netdev instance lock

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 25 Sep 2026 21:18:31 -0700 you wrote:
> netdev_lockdep_set_classes() puts dev->lock in a separate lockdep class,
> by type (netkit vs dummy etc), with the intent of keeping the instance
> locks of individual devices as independent from each other as possible.
> In practice it does the opposite. lockdep only calls the cmp_fn for locks
> of the same class, so netdev_lock_cmp_fn() never gets a say when devices
> from different classes are nested. Instead lockdep records a dependency
> between the classes, and reports a circular locking problem as soon as
> the nesting happens the other way round, e.g. when devices are unregistered
> in a batch.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/2] net: use two lockdep classes for the netdev instance lock
    https://git.kernel.org/netdev/net-next/c/b6f74dff6d26
  - [net-next,v2,2/2] selftests: net: add test for netdev instance lock ordering on unregister
    https://git.kernel.org/netdev/net-next/c/e542bad8d52e

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