Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] net: opt loopback into instance locking
@ 2026-09-18 21:04 Jakub Kicinski
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2026-09-18 21:04 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, andrew+netdev, horms, 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>
---
 drivers/net/loopback.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 1fb6ce6843ad..689ab853c2db 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -172,6 +172,7 @@ static void gen_lo_setup(struct net_device *dev,
 	dev->type		= ARPHRD_LOOPBACK;	/* 0x0001*/
 	dev->flags		= IFF_LOOPBACK;
 	dev->priv_flags		|= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE;
+	dev->request_ops_lock	= true;
 	dev->lltx		= true;
 	dev->netns_immutable	= true;
 	netif_keep_dst(dev);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net-next] net: opt loopback into instance locking
@ 2026-11-09 17:39 Jakub Kicinski
  2026-11-10 16:55 ` Stanislav Fomichev
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jakub Kicinski @ 2026-11-09 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, edumazet, pabeni, andrew+netdev, horms, sdf,
	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>
---
v3:
 - pre-req commit b6f74dff6d26 ("net: use two lockdep classes for
   the netdev instance lock") was merged
 - no change
v2: https://lore.kernel.org/20260921190452.1467853-1-kuba@kernel.org
 - 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 8a6cb34e6b5f..1e6d1b7d71ba 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -207,6 +207,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.56.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: opt loopback into instance locking
  2026-11-09 17:39 [PATCH net-next] net: opt loopback into instance locking Jakub Kicinski
@ 2026-11-10 16:55 ` Stanislav Fomichev
  2026-11-11  5:07 ` Wang Zhan
  2026-11-11 22:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Stanislav Fomichev @ 2026-11-10 16:55 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, sdf

On 11/09, 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>
> ---
> v3:
>  - pre-req commit b6f74dff6d26 ("net: use two lockdep classes for
>    the netdev instance lock") was merged
>  - no change

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: opt loopback into instance locking
  2026-11-09 17:39 [PATCH net-next] net: opt loopback into instance locking Jakub Kicinski
  2026-11-10 16:55 ` Stanislav Fomichev
@ 2026-11-11  5:07 ` Wang Zhan
  2026-11-11 22:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Wang Zhan @ 2026-11-11  5:07 UTC (permalink / raw)
  To: netdev, Jakub Kicinski
  Cc: Wang Zhan, davem, edumazet, pabeni, andrew+netdev, horms, sdf

On Mon, 09 Nov 2026 09:39:02 -0800 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.

nit: NAPI dump no longer takes RTNL since commit
062e789172226 ("netdev-genl: remove rtnl_lock protection from NAPI
ops"). "dumping queues/qstats" is enough, right?

> 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>

Tested on x86_64 QEMU TCG with lockdep:
- with RTNL held for 3 seconds, all-device queue and qstats dumps waited
  before this patch and completed in about 4 ms after it
- no lockdep warnings across lo up/down, MTU change, rename,
  promisc/allmulti, ethtool get_link/get_ts_info and netns teardown

Tested-by: Wang Zhan <wang.zhan@smartx.com>

Thanks

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: opt loopback into instance locking
  2026-11-09 17:39 [PATCH net-next] net: opt loopback into instance locking Jakub Kicinski
  2026-11-10 16:55 ` Stanislav Fomichev
  2026-11-11  5:07 ` Wang Zhan
@ 2026-11-11 22:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-11-11 22:40 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, sdf

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  9 Nov 2026 09:39:02 -0800 you 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.
> 
> [...]

Here is the summary with links:
  - [net-next] net: opt loopback into instance locking
    https://git.kernel.org/netdev/net-next/c/69716ab31a4b

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] 5+ messages in thread

end of thread, other threads:[~2026-11-11 22:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 17:39 [PATCH net-next] net: opt loopback into instance locking Jakub Kicinski
2026-11-10 16:55 ` Stanislav Fomichev
2026-11-11  5:07 ` Wang Zhan
2026-11-11 22:40 ` patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2026-09-18 21:04 Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox