* [PATCH wireless v2] wifi: wext: Limit notifier event flushes to the device's netns
@ 2026-10-23 8:10 Cosmin Ratiu
2026-10-27 23:27 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Cosmin Ratiu @ 2026-10-23 8:10 UTC (permalink / raw)
To: linux-wireless
Cc: Johannes Berg, Tariq Toukan, Carolina Jubran, Dragos Tatulea,
Cosmin Ratiu, Jakub Kicinski, netdev
With mlx5 netdev instance locking enabled, registering an IPoIB netdev
calls the WEXT notifier while holding the instance mutex. Its call to
wireless_nlevent_flush() takes net_rwsem to walk every namespace,
completing the following lockdep dependency cycle:
netdev instance mutex -> net_rwsem (read) -> GID table mutex
-> event_handler_rwsem (read) -> netdev instance mutex
RoCE GID enumeration takes the GID table mutex under net_rwsem. GID
updates dispatch events under the table mutex, taking event_handler_rwsem.
The IPoIB event handler takes the netdev instance mutex in
ipoib_queue_work() to traverse child interfaces.
The all-namespace walk originated in the single global worker servicing
per-namespace WEXT queues. The ordering fixes later reused that routine
in netdevice notifiers to prevent delayed WEXT RTM_NEWLINK messages from
following RTM_DELLINK or reporting stale interface flags. A notifier only
needs to drain the affected device's namespace; there is no reason for it
to walk and flush all namespaces.
Pass dev_net(dev) from both notifier callers and make the flush helper
operate on that namespace. Keep the all-namespace walk and net_rwsem in
the existing worker. This removes the instance-mutex -> net_rwsem
dependency while retaining synchronous flushing from the notifiers.
It may seem strange that the fix for a lock cycle across multiple
subsystems lies in a completely unrelated part of the code, but this
seemed the most justified lock cycle edge to break.
Fixes: 8f7b00307bf1 ("net/mlx5e: Convert mlx5 netdevs to instance locking")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
---
v1->v2:
- Removed unnecessary git tags from commit description.
include/net/iw_handler.h | 8 +++++---
net/wireless/core.c | 2 +-
net/wireless/wext-core.c | 33 +++++++++++++++++----------------
3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/include/net/iw_handler.h b/include/net/iw_handler.h
index b80e474cb0aa..632618d9ff93 100644
--- a/include/net/iw_handler.h
+++ b/include/net/iw_handler.h
@@ -201,6 +201,8 @@
#include <linux/wireless.h> /* IOCTL user space API */
#include <linux/if_ether.h>
+struct net;
+
/***************************** VERSION *****************************/
/*
* This constant is used to know which version of the driver API is
@@ -411,10 +413,10 @@ struct iw_spy_data {
void wireless_send_event(struct net_device *dev, unsigned int cmd,
union iwreq_data *wrqu, const char *extra);
#ifdef CONFIG_WEXT_CORE
-/* flush all previous wext events - if work is done from netdev notifiers */
-void wireless_nlevent_flush(void);
+/* Flush previous wext events in this netns from netdev notifiers. */
+void wireless_nlevent_flush(struct net *net);
#else
-static inline void wireless_nlevent_flush(void) {}
+static inline void wireless_nlevent_flush(struct net *net) {}
#endif
/* We may need a function to send a stream of events to user space.
diff --git a/net/wireless/core.c b/net/wireless/core.c
index cde3ca85494d..2f49ed14b26e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1878,7 +1878,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
return NOTIFY_DONE;
}
- wireless_nlevent_flush();
+ wireless_nlevent_flush(dev_net(dev));
return NOTIFY_OK;
}
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index db77912b3994..f09c589ff94c 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -342,32 +342,28 @@ static const int compat_event_type_size[] = {
/* IW event code */
-void wireless_nlevent_flush(void)
+void wireless_nlevent_flush(struct net *net)
{
struct sk_buff *skb;
- struct net *net;
- down_read(&net_rwsem);
- for_each_net(net) {
- while ((skb = skb_dequeue(&net->wext_nlevents)))
- rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL,
- GFP_KERNEL);
- }
- up_read(&net_rwsem);
+ while ((skb = skb_dequeue(&net->wext_nlevents)))
+ rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
}
EXPORT_SYMBOL_GPL(wireless_nlevent_flush);
static int wext_netdev_notifier_call(struct notifier_block *nb,
unsigned long state, void *ptr)
{
+ struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+
/*
- * When a netdev changes state in any way, flush all pending messages
- * to avoid them going out in a strange order, e.g. RTM_NEWLINK after
- * RTM_DELLINK, or with IFF_UP after without IFF_UP during dev_close()
- * or similar - all of which could otherwise happen due to delays from
- * schedule_work().
+ * When a netdev changes state in any way, flush all pending messages in
+ * its network namespace to avoid them going out in a strange order,
+ * e.g. RTM_NEWLINK after RTM_DELLINK, or with IFF_UP after without
+ * IFF_UP during dev_close() or similar - all of which could otherwise
+ * happen due to delays from schedule_work().
*/
- wireless_nlevent_flush();
+ wireless_nlevent_flush(dev_net(dev));
return NOTIFY_OK;
}
@@ -410,7 +406,12 @@ subsys_initcall(wireless_nlevent_init);
/* Process events generated by the wireless layer or the driver. */
static void wireless_nlevent_process(struct work_struct *work)
{
- wireless_nlevent_flush();
+ struct net *net;
+
+ down_read(&net_rwsem);
+ for_each_net(net)
+ wireless_nlevent_flush(net);
+ up_read(&net_rwsem);
}
static DECLARE_WORK(wireless_nlevent_work, wireless_nlevent_process);
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH wireless v2] wifi: wext: Limit notifier event flushes to the device's netns
2026-10-23 8:10 [PATCH wireless v2] wifi: wext: Limit notifier event flushes to the device's netns Cosmin Ratiu
@ 2026-10-27 23:27 ` Jakub Kicinski
2026-10-29 7:01 ` Johannes Berg
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2026-10-27 23:27 UTC (permalink / raw)
To: Cosmin Ratiu
Cc: linux-wireless, Johannes Berg, Tariq Toukan, Carolina Jubran,
Dragos Tatulea, netdev
On Fri, 23 Oct 2026 11:10:44 +0300 Cosmin Ratiu wrote:
> With mlx5 netdev instance locking enabled, registering an IPoIB netdev
> calls the WEXT notifier while holding the instance mutex. Its call to
> wireless_nlevent_flush() takes net_rwsem to walk every namespace,
> completing the following lockdep dependency cycle:
>
> netdev instance mutex -> net_rwsem (read) -> GID table mutex
> -> event_handler_rwsem (read) -> netdev instance mutex
>
> RoCE GID enumeration takes the GID table mutex under net_rwsem. GID
> updates dispatch events under the table mutex, taking event_handler_rwsem.
> The IPoIB event handler takes the netdev instance mutex in
> ipoib_queue_work() to traverse child interfaces.
>
> The all-namespace walk originated in the single global worker servicing
> per-namespace WEXT queues. The ordering fixes later reused that routine
> in netdevice notifiers to prevent delayed WEXT RTM_NEWLINK messages from
> following RTM_DELLINK or reporting stale interface flags. A notifier only
> needs to drain the affected device's namespace; there is no reason for it
> to walk and flush all namespaces.
>
> Pass dev_net(dev) from both notifier callers and make the flush helper
> operate on that namespace. Keep the all-namespace walk and net_rwsem in
> the existing worker. This removes the instance-mutex -> net_rwsem
> dependency while retaining synchronous flushing from the notifiers.
>
> It may seem strange that the fix for a lock cycle across multiple
> subsystems lies in a completely unrelated part of the code, but this
> seemed the most justified lock cycle edge to break.
If Johannes thinks this scoping is good for other reasons - SGTM.
But my knee jerk fix would be to skip all the wifi processing
for ops-locked devices since no wifi device is ops locked.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH wireless v2] wifi: wext: Limit notifier event flushes to the device's netns
2026-10-27 23:27 ` Jakub Kicinski
@ 2026-10-29 7:01 ` Johannes Berg
0 siblings, 0 replies; 3+ messages in thread
From: Johannes Berg @ 2026-10-29 7:01 UTC (permalink / raw)
To: Jakub Kicinski, Cosmin Ratiu
Cc: linux-wireless, Tariq Toukan, Carolina Jubran, Dragos Tatulea,
netdev
On Tue, 2026-10-27 at 16:27 -0700, Jakub Kicinski wrote:
> On Fri, 23 Oct 2026 11:10:44 +0300 Cosmin Ratiu wrote:
> > With mlx5 netdev instance locking enabled, registering an IPoIB netdev
> > calls the WEXT notifier while holding the instance mutex. Its call to
> > wireless_nlevent_flush() takes net_rwsem to walk every namespace,
> > completing the following lockdep dependency cycle:
> >
> > netdev instance mutex -> net_rwsem (read) -> GID table mutex
> > -> event_handler_rwsem (read) -> netdev instance mutex
> >
> > RoCE GID enumeration takes the GID table mutex under net_rwsem. GID
> > updates dispatch events under the table mutex, taking event_handler_rwsem.
> > The IPoIB event handler takes the netdev instance mutex in
> > ipoib_queue_work() to traverse child interfaces.
> >
> > The all-namespace walk originated in the single global worker servicing
> > per-namespace WEXT queues. The ordering fixes later reused that routine
> > in netdevice notifiers to prevent delayed WEXT RTM_NEWLINK messages from
> > following RTM_DELLINK or reporting stale interface flags. A notifier only
> > needs to drain the affected device's namespace; there is no reason for it
> > to walk and flush all namespaces.
> >
> > Pass dev_net(dev) from both notifier callers and make the flush helper
> > operate on that namespace. Keep the all-namespace walk and net_rwsem in
> > the existing worker. This removes the instance-mutex -> net_rwsem
> > dependency while retaining synchronous flushing from the notifiers.
> >
> > It may seem strange that the fix for a lock cycle across multiple
> > subsystems lies in a completely unrelated part of the code, but this
> > seemed the most justified lock cycle edge to break.
>
> If Johannes thinks this scoping is good for other reasons - SGTM.
It seems reasonable, in truth roughly nobody [1] ever really runs WiFi
in network namespaces.
[1] except security LLM-babysitters trying to prove certain bugs are bad
> But my knee jerk fix would be to skip all the wifi processing
> for ops-locked devices since no wifi device is ops locked.
In wext_netdev_notifier_call()? I suppose these days we could also just
skip that for non-wifi devices, somehow.
But it's much easier for me now to just take this patch (in fact I
already did), rather than try to break it in other ways and have Cosmin
go back to test it. Maybe we'll just do both, the latter as an
optimisation, we can likely do better than "ops-locked" as a
discriminant too.
johannes
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-29 7:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23 8:10 [PATCH wireless v2] wifi: wext: Limit notifier event flushes to the device's netns Cosmin Ratiu
2026-10-27 23:27 ` Jakub Kicinski
2026-10-29 7:01 ` Johannes Berg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.