# Re: \[PATCH net-next v3 04/13\] net: move promiscuity handling into dev\_rx\_mode\_work - From: Stanislav Fomichev \ - Date: 2026-03-20 15:41:12 - List: netdev - Page: https://ratatoskr.run/netdev/2026/03/3471430 - Thread: https://ratatoskr.run/netdev/2026/03/3470147/t.md ```text On 03/20, Loktionov, Aleksandr wrote: > > > > -----Original Message----- > > From: Stanislav Fomichev > > Sent: Friday, March 20, 2026 2:25 AM > > To: > > Cc: davem@davemloft.net; ; kuba@kernel.org; > > pabeni@redhat.com; horms@kernel.org; corbet@lwn.net; > > skhan@linuxfoundation.org; andrew+netdev@lunn.ch; > > michael.chan@broadcom.com; pavan.chebbi@broadcom.com; Nguyen, Anthony > > L [ref]; Kitszel, Przemyslaw > > [ref]; saeedm@nvidia.com; tariqt@nvidia.com; > > mbloch@nvidia.com; alexanderduyck@fb.com; kernel-team@meta.com; > > johannes@sipsolutions.net; sd@queasysnail.net; jianbol@nvidia.com; > > dtatulea@nvidia.com; sdf@fomichev.me; mohsin.bashr@gmail.com; Keller, > > Jacob E [ref]; willemb@google.com; > > skhawaja@google.com; bestswngs@gmail.com; Loktionov, Aleksandr > > [ref]; kees@kernel.org; linux- > > doc@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired- > > lan@lists.osuosl.org; linux-rdma@vger.kernel.org; linux- > > wireless@vger.kernel.org; linux-kselftest@vger.kernel.org; > > leon@kernel.org > > Subject: [PATCH net-next v3 04/13] net: move promiscuity handling into > > dev_rx_mode_work > > > > Move unicast promiscuity tracking into dev_rx_mode_work so it runs > > under netdev_ops_lock instead of under the addr_lock spinlock. This is > > required because __dev_set_promiscuity calls dev_change_rx_flags and > > __dev_notify_flags, both of which may need to sleep. > > > > Change ASSERT_RTNL() to netdev_ops_assert_locked() in > > __dev_set_promiscuity, netif_set_allmulti and __dev_change_flags since > > these are now called from the work queue under the ops lock. > > > > Reviewed-by: Aleksandr Loktionov > > Signed-off-by: Stanislav Fomichev > > --- > > Documentation/networking/netdevices.rst | 4 ++ > > net/core/dev.c | 79 +++++++++++++++++------- > > - > > 2 files changed, 57 insertions(+), 26 deletions(-) > > > > diff --git a/Documentation/networking/netdevices.rst > > b/Documentation/networking/netdevices.rst > > index dc83d78d3b27..5cdaa1a3dcc8 100644 > > --- a/Documentation/networking/netdevices.rst > > +++ b/Documentation/networking/netdevices.rst > > @@ -298,6 +298,10 @@ struct net_device synchronization rules > > Notes: Sleepable version of ndo_set_rx_mode. Receives snapshots > > of the unicast and multicast address lists. > > > > +ndo_change_rx_flags: > > + Synchronization: rtnl_lock() semaphore. In addition, netdev > > instance > > + lock if the driver implements queue management or shaper API. > > + > > ndo_setup_tc: > > ``TC_SETUP_BLOCK`` and ``TC_SETUP_FT`` are running under NFT > > locks > > (i.e. no ``rtnl_lock`` and no device instance lock). The rest > > of diff --git a/net/core/dev.c b/net/core/dev.c index > > fedc423306fc..fc5c9b14faa0 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -9574,7 +9574,7 @@ static int __dev_set_promiscuity(struct > > net_device *dev, int inc, bool notify) > > kuid_t uid; > > kgid_t gid; > > > > - ASSERT_RTNL(); > > + netdev_ops_assert_locked(dev); > Can you explain why do you add new hard precondition of ops lock must be held? The context is that in f792709e0baa ("selftests: net: validate team flags propagation") I had to add locking around NETDEV_CHANGE notifiers and add that ugly `if (notify) netdev_ops_assert_locked` check. After this patch I believe we are consistently calling __dev_set_promiscuity with the ops lock (for ops locked netdev), so we can cleanup this enforcement part. > > promiscuity = dev->promiscuity + inc; > > if (promiscuity == 0) { > > @@ -9610,16 +9610,8 @@ static int __dev_set_promiscuity(struct > > net_device *dev, int inc, bool notify) > > > > dev_change_rx_flags(dev, IFF_PROMISC); > > } > > ... > > > __hw_addr_init(&uc_snap); > > @@ -9704,16 +9720,29 @@ static void dev_rx_mode_work(struct > > work_struct *work) > > if (!err) > > err = __hw_addr_list_snapshot(&mc_ref, &dev->mc, > > dev->addr_len); > > - netif_addr_unlock_bh(dev); > > > > if (err) { > > netdev_WARN(dev, "failed to sync uc/mc > > addresses\n"); > > __hw_addr_flush(&uc_snap); > > __hw_addr_flush(&uc_ref); > > __hw_addr_flush(&mc_snap); > > + netif_addr_unlock_bh(dev); > > goto out; > > } > > > > + promisc_inc = dev_uc_promisc_update(dev); > > + > > + netif_addr_unlock_bh(dev); > > + } else { > > + netif_addr_lock_bh(dev); > > + promisc_inc = dev_uc_promisc_update(dev); > > + netif_addr_unlock_bh(dev); > > + } > > + > > + if (promisc_inc) > > + __dev_set_promiscuity(dev, promisc_inc, false); > But it's being called here without any netdev_lock_ops(dev) ? We have the following at the start of dev_rx_mode_work: rtnl_lock(); netdev_lock_ops(dev); Or am I looking at something else? ```