[PATCH net-next v2] net: opt loopback into instance locking

[PATCH net-next v2] net: opt loopback into instance locking

[Jakub Kicinski]

netdev netlink iterates over devices when dumping queues/NAPIs/qstats
etc. and takes rtnl_lock or instance lock depending on the underlying
device. Currently even if all the "real" devices in the system are ops
locked we still have to take rtnl_lock for lo (only to find out
that it doesn't even have queues or NAPIs to report).
Opt loopback into having control path under the ops lock.

This is a pretty obvious thing to do, I've held back this
patch because we used to only apply ops locking on physical devices.
netkit queue leasing made a precedent for (far more complex)
SW devices enabling ops locking. Now adding it to lo should
not create much new bug surface.

I considered an alternative of adding a "predicate" to the iteration
primitive so that we can skip the devices which obviously don't
support given API (eg qstat) without any locking. But it's more
LoC and real_num_.x_queues is not currently WRITE_ONCE()ed so
it doesn't work too well for queues.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - move setting the request flag from gen_lo_setup() to loopback_setup()
   to avoid splats on the blackhole dev which we don't care about
v1: https://lore.kernel.org/20260918210419.4088201-1-kuba@kernel.org
---
 drivers/net/loopback.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 1fb6ce6843ad..6187fcca82d7 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -201,6 +201,8 @@ static void loopback_setup(struct net_device *dev)
 {
 	gen_lo_setup(dev, (64 * 1024), &loopback_ethtool_ops, &eth_header_ops,
 		     &loopback_ops, loopback_dev_free);
+
+	dev->request_ops_lock	= true;
 }
 
 /* Setup and register the loopback device. */
-- 
2.55.0

Re: [PATCH net-next v2] net: opt loopback into instance locking

[Stanislav Fomichev]

On 09/21, Jakub Kicinski wrote:
> netdev netlink iterates over devices when dumping queues/NAPIs/qstats
> etc. and takes rtnl_lock or instance lock depending on the underlying
> device. Currently even if all the "real" devices in the system are ops
> locked we still have to take rtnl_lock for lo (only to find out
> that it doesn't even have queues or NAPIs to report).
> Opt loopback into having control path under the ops lock.
> 
> This is a pretty obvious thing to do, I've held back this
> patch because we used to only apply ops locking on physical devices.
> netkit queue leasing made a precedent for (far more complex)
> SW devices enabling ops locking. Now adding it to lo should
> not create much new bug surface.
> 
> I considered an alternative of adding a "predicate" to the iteration
> primitive so that we can skip the devices which obviously don't
> support given API (eg qstat) without any locking. But it's more
> LoC and real_num_.x_queues is not currently WRITE_ONCE()ed so
> it doesn't work too well for queues.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - move setting the request flag from gen_lo_setup() to loopback_setup()
>    to avoid splats on the blackhole dev which we don't care about

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

(mostly because dummy is also ops locked, don't think loopback should
bring any problems)

Re: [PATCH net-next v2] net: opt loopback into instance locking

[Jakub Kicinski]

On Mon, 21 Sep 2026 14:14:19 -0700 Stanislav Fomichev wrote:
> On 09/21, Jakub Kicinski wrote:
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> > v2:
> >  - move setting the request flag from gen_lo_setup() to loopback_setup()
> >    to avoid splats on the blackhole dev which we don't care about  
> 
> Acked-by: Stanislav Fomichev <sdf@fomichev.me>
> 
> (mostly because dummy is also ops locked, don't think loopback should
> bring any problems)

Stan, the issue Matt reported in the BPF CI is a generic sw device +
netdev lock issue. We are triggering it on lo because.. probability,
but it reproduces with dummy + netkit already today.

Do you recall why we have separate classes for each device type?

Classes make the lock_set_cmp_fn() ineffective across different device
types. Quick test removing the classes and sticking to just the cmp fn
seems make lockdep happy, I'm running the ksft suite now. But maybe you
can tell me if it's the right idea before it finishes..?

Re: [PATCH net-next v2] net: opt loopback into instance locking

[Stanislav Fomichev]

On 09/22, Jakub Kicinski wrote:
> On Mon, 21 Sep 2026 14:14:19 -0700 Stanislav Fomichev wrote:
> > On 09/21, Jakub Kicinski wrote:
> > > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > > ---
> > > v2:
> > >  - move setting the request flag from gen_lo_setup() to loopback_setup()
> > >    to avoid splats on the blackhole dev which we don't care about  
> > 
> > Acked-by: Stanislav Fomichev <sdf@fomichev.me>
> > 
> > (mostly because dummy is also ops locked, don't think loopback should
> > bring any problems)
> 
> Stan, the issue Matt reported in the BPF CI is a generic sw device +
> netdev lock issue. We are triggering it on lo because.. probability,
> but it reproduces with dummy + netkit already today.
> 
> Do you recall why we have separate classes for each device type?
> 
> Classes make the lock_set_cmp_fn() ineffective across different device
> types. Quick test removing the classes and sticking to just the cmp fn
> seems make lockdep happy, I'm running the ksft suite now. But maybe you
> can tell me if it's the right idea before it finishes..?

No, I don't think there is a reason we added both. It might have been
that we started with a class and then added cmp_fn on top. This whole
function is a 10+ years of whack-a-mole with lockdep :-( Agreed that
removing class (which prevents cmp_fn from working properly across
all instance locks) sounds sensible.

Re: [PATCH net-next v2] net: opt loopback into instance locking

[Matthieu Baerts]

Hi Jakub,

On 21/09/2026 21:04, Jakub Kicinski wrote:
> netdev netlink iterates over devices when dumping queues/NAPIs/qstats
> etc. and takes rtnl_lock or instance lock depending on the underlying
> device. Currently even if all the "real" devices in the system are ops
> locked we still have to take rtnl_lock for lo (only to find out
> that it doesn't even have queues or NAPIs to report).
> Opt loopback into having control path under the ops lock.
> 
> This is a pretty obvious thing to do, I've held back this
> patch because we used to only apply ops locking on physical devices.
> netkit queue leasing made a precedent for (far more complex)
> SW devices enabling ops locking. Now adding it to lo should
> not create much new bug surface.
> 
> I considered an alternative of adding a "predicate" to the iteration
> primitive so that we can skip the devices which obviously don't
> support given API (eg qstat) without any locking. But it's more
> LoC and real_num_.x_queues is not currently WRITE_ONCE()ed so
> it doesn't work too well for queues.

Thank you for this! But apparently, it looks like it is still causing
some issues with the BPF selftests for some arch, e.g.

https://github.com/kernel-patches/bpf/actions/runs/35693264483/job/106636983010

Cheers,
Matt
---
(I don't know if I have the rights to ask for ↓)
pw-bot: cr

[syzbot ci] Re: net: opt loopback into instance locking

[syzbot ci]

syzbot ci has tested the following series

[v2] net: opt loopback into instance locking
https://lore.kernel.org/all/20260921190452.1467853-1-kuba@kernel.org
* [PATCH net-next v2] net: opt loopback into instance locking

and found the following issue:
possible deadlock in default_device_exit_batch

Full report is available here:
https://ci.syzbot.org/series/23f51d46-5d1a-4839-a0a8-35d88361ffdb

***

possible deadlock in default_device_exit_batch

tree:      net-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base:      944ae66642b726bd6b25ae71b1e9ff88a0e0bdb0
arch:      amd64
compiler:  Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/014063b0-7001-43f1-aa04-57649607febc/config

batman_adv: batadv0: Removing interface: batadv_slave_0
batman_adv: batadv0: Removing interface: batadv_slave_1
======================================================
WARNING: possible circular locking dependency detected
syzkaller #0 Not tainted
------------------------------------------------------
kworker/u8:3/5652 is trying to acquire lock:
ffff88811671ae70 (&dev_instance_lock_key#3){+.+.}-{4:4}, at: unregister_netdevice_many_notify+0x5bb/0x2140

but task is already holding lock:
ffff88816d8dae70 (&dev_instance_lock_key){+.+.}-{4:4}, at: unregister_netdevice_many_notify+0x5bb/0x2140

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&dev_instance_lock_key){+.+.}-{4:4}:
       __mutex_lock+0x19d/0x1550
       unregister_netdevice_many_notify+0x5bb/0x2140
       default_device_exit_batch+0x6d4/0x760
       ops_undo_list+0x4b4/0x8d0
       cleanup_net+0x572/0x810
       process_scheduled_works+0xc3d/0x1630
       worker_thread+0xa47/0xfb0
       kthread+0x38b/0x480
       ret_from_fork+0x514/0xb70
       ret_from_fork_asm+0x1a/0x30

-> #0 (&dev_instance_lock_key#3){+.+.}-{4:4}:
       __lock_acquire+0x164c/0x2de0
       lock_acquire+0x115/0x350
       __mutex_lock+0x19d/0x1550
       unregister_netdevice_many_notify+0x5bb/0x2140
       default_device_exit_batch+0x6d4/0x760
       ops_undo_list+0x4b4/0x8d0
       cleanup_net+0x572/0x810
       process_scheduled_works+0xc3d/0x1630
       worker_thread+0xa47/0xfb0
       kthread+0x38b/0x480
       ret_from_fork+0x514/0xb70
       ret_from_fork_asm+0x1a/0x30

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&dev_instance_lock_key);
                               lock(&dev_instance_lock_key#3);
                               lock(&dev_instance_lock_key);
  lock(&dev_instance_lock_key#3);

 *** DEADLOCK ***

locks held by kworker/u8:3/5652: 6, last CPU#1:
 #0: ffff8881012d5940 ((wq_completion)netns){+.+.}-{0:0}, at: process_scheduled_works+0x97a/0x1630
 #1: ffffc9000481fc40 (net_cleanup_work){+.+.}-{0:0}, at: process_scheduled_works+0x97a/0x1630
 #2: ffffffff90244f08 (pernet_ops_rwsem){++++}-{4:4}, at: cleanup_net+0xf5/0x810
 #3: ffffffff90253760 (rtnl_mutex){+.+.}-{4:4}, at: default_device_exit_batch+0xd0/0x760
 #4: ffff888120c40e70 (&dev_instance_lock_key#3){+.+.}-{4:4}, at: unregister_netdevice_many_notify+0x5bb/0x2140
 #5: ffff88816d8dae70 (&dev_instance_lock_key){+.+.}-{4:4}, at: unregister_netdevice_many_notify+0x5bb/0x2140

stack backtrace:
CPU: 1 UID: 0 PID: 5652 Comm: kworker/u8:3 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Workqueue: netns cleanup_net
Call Trace:
 <TASK>
 dump_stack_lvl+0xe8/0x150
 print_circular_bug+0x2e2/0x300
 check_noncircular+0x12f/0x150
 __lock_acquire+0x164c/0x2de0
 lock_acquire+0x115/0x350
 __mutex_lock+0x19d/0x1550
 unregister_netdevice_many_notify+0x5bb/0x2140
 default_device_exit_batch+0x6d4/0x760
 ops_undo_list+0x4b4/0x8d0
 cleanup_net+0x572/0x810
 process_scheduled_works+0xc3d/0x1630
 worker_thread+0xa47/0xfb0
 kthread+0x38b/0x480
 ret_from_fork+0x514/0xb70
 ret_from_fork_asm+0x1a/0x30
 </TASK>


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.

Notes:
- The patch will be applied on top of the tested series (as an
  incremental fix).
- To test a new version of the whole series, please send it directly
  to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.