diff options
| author | Jakub Kicinski <kuba@kernel.org> | 2026-09-29 12:15:43 -0700 |
|---|---|---|
| committer | Paolo Abeni <pabeni@redhat.com> | 2026-10-01 12:43:46 +0200 |
| commit | ddef4c2d6d07e54816a073c7b5cad7f337fa1251 (patch) | |
| tree | e58585aea68cb9b56f1b59a4b2b6465a33bac5c9 /scripts/objdump-func | |
| parent | 69d9c07dc867ceb507a0f624fa2a56b5eaa385b4 (diff) | |
net: only give queue leasing devices a separate instance lock class
Commit b6f74dff6d26 ("net: use two lockdep classes for the netdev
instance lock") put every device without a parent in the virtual class.
It also restricted the locking order for the virtual class, because
queue leasing has hard requirements on the exact order.
This bites us back on bond, which is "virtual" and needs to be taken
before taking the locks of the lowers. NIPA hit the following on the
new test I recently posted for XDP+bond:
WARNING: possible circular locking dependency detected
------------------------------------------------------
python3/18635 is trying to acquire lock:
ff11000120b8ce30 (&dev->lock){+.+.}-{4:4}, at:
netdev_put_lock+0x2d/0x1a0
but task is already holding lock:
ff110001ef77ae30 (&netdev_virt_instance_lock_key){+.+.}-{4:4}, at:
netdev_put_lock+0x2d/0x1a0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&netdev_virt_instance_lock_key){+.+.}-{4:4}:
__mutex_lock+0x1ae/0x1f10
xdp_set_features_flag+0x2b/0x50
bond_xdp_set_features+0x1eb/0x360
bond_netdev_event+0x13f/0x300
notifier_call_chain+0xae/0x300
call_netdevice_notifiers+0x70/0xa0
bnxt_xdp_set+0x2f6/0x620
netif_xdp_propagate+0x503/0xc60
dev_xdp_propagate+0xa1/0x230
bond_xdp_set+0x234/0x700
dev_xdp_install+0x592/0xd70
dev_xdp_attach+0x355/0xf50
dev_change_xdp_fd+0x176/0x210
do_setlink.isra.0+0x220d/0x2b20
rtnl_newlink+0x9f1/0x11b0
-> #0 (&dev->lock){+.+.}-{4:4}:
__mutex_lock+0x1ae/0x1f10
netdev_put_lock+0x2d/0x1a0
netdev_nl_queue_create_doit+0x801/0x1a70
genl_family_rcv_msg_doit+0x206/0x300
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&netdev_virt_instance_lock_key);
lock(&dev->lock);
lock(&netdev_virt_instance_lock_key);
lock(&dev->lock);
Let's narrow down the "virtual" class to only the devices which
can actually create a queue. More LoC and complexity, but that
is what we actually care about here. The rest needs to nest
under rtnl_lock, which bond does (famous last words?)
Take the instance locks in two passes, first the netkits then
the rest (matching the queue leasing order).
An alternative would be to make sure the close list is sorted
correctly from the start (queue head/tail appropriately in
unregister_netdevice_queue()). I think it works but feels
a little more fragile. Happy to change, tho.
netdev_can_create_queue() will now be used on paths where we
genuinely handle non-netkit, so we can't always set the extack.
Unfortunately, the (recently) added tracepoint in extack fires
even when extack is NULL.
Fixes: b6f74dff6d26 ("net: use two lockdep classes for the netdev instance lock")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://patch.msgid.link/20260929191543.3295633-1-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Diffstat (limited to 'scripts/objdump-func')
0 files changed, 0 insertions, 0 deletions
