* [PATCH net-next] net: only give queue leasing devices a separate instance lock class
@ 2026-09-29 19:15 Jakub Kicinski
2026-09-29 23:56 ` Stanislav Fomichev
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-09-29 19:15 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
daniel, hawk, john.fastabend, sdf, razor, dw
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>
---
CC: daniel@iogearbox.net
CC: hawk@kernel.org
CC: john.fastabend@gmail.com
CC: sdf@fomichev.me
CC: razor@blackwall.org
CC: dw@davidwei.uk
---
net/core/dev.c | 48 ++++++++++++++++++++++------------------
net/core/netdev_queues.c | 31 ++++++++++++++------------
2 files changed, 44 insertions(+), 35 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index a8eb382f40ca..f225906f7b6f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -575,7 +575,7 @@ static int netdev_lock_cmp_fn(const struct lockdep_map *a,
if (a == b)
return 0;
- /* @a and @b must be of same class - both virtual or physical.
+ /* @a and @b are of same lock class.
* cmp_fn won't be called for devices of different classes.
*
* For the same class only allow nesting under the protection
@@ -589,12 +589,13 @@ static int netdev_lock_cmp_fn(const struct lockdep_map *a,
* 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.
+ * Other virtual devices stay in the default class.
*/
void netdev_set_instance_lock_class(struct net_device *dev)
{
static struct lock_class_key netdev_virt_instance_lock_key;
- if (dev->dev.parent)
+ if (!netdev_can_create_queue(dev, NULL))
return;
lockdep_set_class(&dev->lock, &netdev_virt_instance_lock_key);
@@ -12465,19 +12466,31 @@ static void netif_close_many_and_unlock(struct list_head *close_head)
}
}
-static void netif_close_many_and_unlock_cond(struct list_head *close_head)
+/* Handle one class of ops-locked devices. Since close requires the lock
+ * we need to be careful about which classes we allow to nest.
+ */
+static void netdev_lock_ops_close_many(struct list_head *head,
+ struct list_head *close_head,
+ bool leasing)
{
-#ifdef CONFIG_LOCKDEP
- /* We can only track up to MAX_LOCK_DEPTH locks per task.
- *
- * Reserve half the available slots for additional locks possibly
- * taken by notifiers and (soft)irqs.
- */
- unsigned int limit = MAX_LOCK_DEPTH / 2;
+ struct net_device *dev;
- if (lockdep_depth(current) > limit)
- netif_close_many_and_unlock(close_head);
+ list_for_each_entry(dev, head, unreg_list) {
+ if (!(dev->flags & IFF_UP) || !netdev_need_ops_lock(dev) ||
+ netdev_can_create_queue(dev, NULL) != leasing)
+ continue;
+ list_add_tail(&dev->close_list, close_head);
+ netdev_lock(dev);
+
+#ifdef CONFIG_LOCKDEP
+ /* We can only track up to MAX_LOCK_DEPTH locks per task.
+ * Reserve half the available slots for additional locks
+ * possibly taken by notifiers and (soft)irqs.
+ */
+ if (lockdep_depth(current) > MAX_LOCK_DEPTH / 2)
+ netif_close_many_and_unlock(close_head);
#endif
+ }
}
bool unregister_netdevice_queued(const struct net_device *dev)
@@ -12517,15 +12530,8 @@ void unregister_netdevice_many_notify(struct list_head *head,
}
/* If device is running, close it first. Start with ops locked... */
- list_for_each_entry(dev, head, unreg_list) {
- if (!(dev->flags & IFF_UP))
- continue;
- if (netdev_need_ops_lock(dev)) {
- list_add_tail(&dev->close_list, &close_head);
- netdev_lock(dev);
- }
- netif_close_many_and_unlock_cond(&close_head);
- }
+ netdev_lock_ops_close_many(head, &close_head, true); /* queue leasing */
+ netdev_lock_ops_close_many(head, &close_head, false); /* the rest */
netif_close_many_and_unlock(&close_head);
/* ... now go over the rest. */
list_for_each_entry(dev, head, unreg_list) {
diff --git a/net/core/netdev_queues.c b/net/core/netdev_queues.c
index f5558b12877c..27cb6397bdd4 100644
--- a/net/core/netdev_queues.c
+++ b/net/core/netdev_queues.c
@@ -61,21 +61,24 @@ struct device *netdev_queue_get_dma_dev(struct net_device *dev,
bool netdev_can_create_queue(const struct net_device *dev,
struct netlink_ext_ack *extack)
{
- if (dev->dev.parent) {
- NL_SET_ERR_MSG(extack, "Device is not a virtual device");
- return false;
- }
- if (!dev->queue_mgmt_ops ||
- !dev->queue_mgmt_ops->ndo_queue_create) {
- NL_SET_ERR_MSG(extack, "Device does not support queue creation");
- return false;
- }
+ const char *msg = NULL;
+
if (dev->real_num_rx_queues < 1 ||
- dev->real_num_tx_queues < 1) {
- NL_SET_ERR_MSG(extack, "Device must have at least one real queue");
- return false;
- }
- return true;
+ dev->real_num_tx_queues < 1)
+ msg = "Device must have at least one real queue";
+ if (!dev->queue_mgmt_ops ||
+ !dev->queue_mgmt_ops->ndo_queue_create)
+ msg = "Device does not support queue creation";
+ if (dev->dev.parent)
+ msg = "Device is not a virtual device";
+
+ /* callers with extack=NULL are not from uAPI and don't want to trigger
+ * the extack tracepoint.
+ */
+ if (msg && extack)
+ NL_SET_ERR_MSG_FMT(extack, "%s", msg);
+
+ return !msg;
}
bool netdev_can_lease_queue(const struct net_device *dev,
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: only give queue leasing devices a separate instance lock class
2026-09-29 19:15 [PATCH net-next] net: only give queue leasing devices a separate instance lock class Jakub Kicinski
@ 2026-09-29 23:56 ` Stanislav Fomichev
2026-09-30 8:14 ` Nikolay Aleksandrov
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2026-09-29 23:56 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, daniel,
hawk, john.fastabend, sdf, razor, dw
On 09/29, Jakub Kicinski wrote:
> 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>
> ---
> CC: daniel@iogearbox.net
> CC: hawk@kernel.org
> CC: john.fastabend@gmail.com
> CC: sdf@fomichev.me
> CC: razor@blackwall.org
> CC: dw@davidwei.uk
> ---
> net/core/dev.c | 48 ++++++++++++++++++++++------------------
> net/core/netdev_queues.c | 31 ++++++++++++++------------
> 2 files changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a8eb382f40ca..f225906f7b6f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -575,7 +575,7 @@ static int netdev_lock_cmp_fn(const struct lockdep_map *a,
> if (a == b)
> return 0;
>
> - /* @a and @b must be of same class - both virtual or physical.
> + /* @a and @b are of same lock class.
> * cmp_fn won't be called for devices of different classes.
> *
> * For the same class only allow nesting under the protection
> @@ -589,12 +589,13 @@ static int netdev_lock_cmp_fn(const struct lockdep_map *a,
> * 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.
> + * Other virtual devices stay in the default class.
> */
> void netdev_set_instance_lock_class(struct net_device *dev)
> {
> static struct lock_class_key netdev_virt_instance_lock_key;
>
> - if (dev->dev.parent)
> + if (!netdev_can_create_queue(dev, NULL))
> return;
>
> lockdep_set_class(&dev->lock, &netdev_virt_instance_lock_key);
> @@ -12465,19 +12466,31 @@ static void netif_close_many_and_unlock(struct list_head *close_head)
> }
> }
>
> -static void netif_close_many_and_unlock_cond(struct list_head *close_head)
> +/* Handle one class of ops-locked devices. Since close requires the lock
> + * we need to be careful about which classes we allow to nest.
> + */
> +static void netdev_lock_ops_close_many(struct list_head *head,
> + struct list_head *close_head,
> + bool leasing)
> {
> -#ifdef CONFIG_LOCKDEP
> - /* We can only track up to MAX_LOCK_DEPTH locks per task.
> - *
> - * Reserve half the available slots for additional locks possibly
> - * taken by notifiers and (soft)irqs.
> - */
> - unsigned int limit = MAX_LOCK_DEPTH / 2;
> + struct net_device *dev;
>
> - if (lockdep_depth(current) > limit)
> - netif_close_many_and_unlock(close_head);
> + list_for_each_entry(dev, head, unreg_list) {
> + if (!(dev->flags & IFF_UP) || !netdev_need_ops_lock(dev) ||
> + netdev_can_create_queue(dev, NULL) != leasing)
> + continue;
> + list_add_tail(&dev->close_list, close_head);
> + netdev_lock(dev);
> +
> +#ifdef CONFIG_LOCKDEP
> + /* We can only track up to MAX_LOCK_DEPTH locks per task.
> + * Reserve half the available slots for additional locks
> + * possibly taken by notifiers and (soft)irqs.
> + */
> + if (lockdep_depth(current) > MAX_LOCK_DEPTH / 2)
> + netif_close_many_and_unlock(close_head);
> #endif
> + }
> }
>
> bool unregister_netdevice_queued(const struct net_device *dev)
> @@ -12517,15 +12530,8 @@ void unregister_netdevice_many_notify(struct list_head *head,
> }
>
> /* If device is running, close it first. Start with ops locked... */
> - list_for_each_entry(dev, head, unreg_list) {
> - if (!(dev->flags & IFF_UP))
> - continue;
> - if (netdev_need_ops_lock(dev)) {
> - list_add_tail(&dev->close_list, &close_head);
> - netdev_lock(dev);
> - }
> - netif_close_many_and_unlock_cond(&close_head);
> - }
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Don't know if it's gonna help anyone, but if you happen to respin,
maybe add a comment here along the lines of
/* see netdev_set_instance_lock_class kdoc on why queue leasing first */
Although the hole is so deep now, not sure it's worth it :-D
> + netdev_lock_ops_close_many(head, &close_head, true); /* queue leasing */
> + netdev_lock_ops_close_many(head, &close_head, false); /* the rest */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: only give queue leasing devices a separate instance lock class
2026-09-29 19:15 [PATCH net-next] net: only give queue leasing devices a separate instance lock class Jakub Kicinski
2026-09-29 23:56 ` Stanislav Fomichev
@ 2026-09-30 8:14 ` Nikolay Aleksandrov
2026-09-30 11:42 ` Daniel Borkmann
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2026-09-30 8:14 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, daniel, hawk,
john.fastabend, sdf, dw
On 29/09/2026 22:15, Jakub Kicinski wrote:
> 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>
> ---
> CC: daniel@iogearbox.net
> CC: hawk@kernel.org
> CC: john.fastabend@gmail.com
> CC: sdf@fomichev.me
> CC: razor@blackwall.org
> CC: dw@davidwei.uk
> ---
> net/core/dev.c | 48 ++++++++++++++++++++++------------------
> net/core/netdev_queues.c | 31 ++++++++++++++------------
> 2 files changed, 44 insertions(+), 35 deletions(-)
>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: only give queue leasing devices a separate instance lock class
2026-09-29 19:15 [PATCH net-next] net: only give queue leasing devices a separate instance lock class Jakub Kicinski
2026-09-29 23:56 ` Stanislav Fomichev
2026-09-30 8:14 ` Nikolay Aleksandrov
@ 2026-09-30 11:42 ` Daniel Borkmann
2026-10-01 10:43 ` Paolo Abeni
2026-10-01 10:50 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2026-09-30 11:42 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, hawk,
john.fastabend, sdf, razor, dw
On 9/29/26 9:15 PM, Jakub Kicinski wrote:
> 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: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: only give queue leasing devices a separate instance lock class
2026-09-29 19:15 [PATCH net-next] net: only give queue leasing devices a separate instance lock class Jakub Kicinski
` (2 preceding siblings ...)
2026-09-30 11:42 ` Daniel Borkmann
@ 2026-10-01 10:43 ` Paolo Abeni
2026-10-01 10:50 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2026-10-01 10:43 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, andrew+netdev, horms, daniel, hawk,
john.fastabend, sdf, razor, dw
On 9/29/26 21:15, Jakub Kicinski wrote:
> diff --git a/net/core/netdev_queues.c b/net/core/netdev_queues.c
> index f5558b12877c..27cb6397bdd4 100644
> --- a/net/core/netdev_queues.c
> +++ b/net/core/netdev_queues.c
> @@ -61,21 +61,24 @@ struct device *netdev_queue_get_dma_dev(struct net_device *dev,
> bool netdev_can_create_queue(const struct net_device *dev,
> struct netlink_ext_ack *extack)
> {
> - if (dev->dev.parent) {
> - NL_SET_ERR_MSG(extack, "Device is not a virtual device");
> - return false;
> - }
> - if (!dev->queue_mgmt_ops ||
> - !dev->queue_mgmt_ops->ndo_queue_create) {
> - NL_SET_ERR_MSG(extack, "Device does not support queue creation");
> - return false;
> - }
> + const char *msg = NULL;
> +
> if (dev->real_num_rx_queues < 1 ||
Clashiko flags a possible data race while reading `real_num_rx_queues`
but that should be IMHO addressed separately, if ever addressed.
/P
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: only give queue leasing devices a separate instance lock class
2026-09-29 19:15 [PATCH net-next] net: only give queue leasing devices a separate instance lock class Jakub Kicinski
` (3 preceding siblings ...)
2026-10-01 10:43 ` Paolo Abeni
@ 2026-10-01 10:50 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-01 10:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, daniel,
hawk, john.fastabend, sdf, razor, dw
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 29 Sep 2026 12:15:43 -0700 you wrote:
> 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:
>
> [...]
Here is the summary with links:
- [net-next] net: only give queue leasing devices a separate instance lock class
https://git.kernel.org/netdev/net-next/c/ddef4c2d6d07
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-01 10:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-29 19:15 [PATCH net-next] net: only give queue leasing devices a separate instance lock class Jakub Kicinski
2026-09-29 23:56 ` Stanislav Fomichev
2026-09-30 8:14 ` Nikolay Aleksandrov
2026-09-30 11:42 ` Daniel Borkmann
2026-10-01 10:43 ` Paolo Abeni
2026-10-01 10:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).