Re: [Intel-wired-lan] [PATCH net-next v3 04/13] net: move promiscuity handling into dev_rx_mode_work

Loktionov, Aleksandr
2026-03-20T08:02:06Z

> -----Original Message-----
> From: Stanislav Fomichev <sdf@fomichev.me>
> Sent: Friday, March 20, 2026 2:25 AM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; 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 <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; 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 <jacob.e.keller@intel.com>; willemb@google.com;
> skhawaja@google.com; bestswngs@gmail.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; 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 <aleksandr.loktionov@intel.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> 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?

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

> +
> + if (ops->ndo_set_rx_mode_async) {
> ops->ndo_set_rx_mode_async(dev, &uc_snap, &mc_snap);
>
> netif_addr_lock_bh(dev);
> @@ -9722,6 +9751,10 @@ static void dev_rx_mode_work(struct work_struct
> *work)
> __hw_addr_list_reconcile(&dev->mc, &mc_snap,
> &mc_ref, dev->addr_len);
> netif_addr_unlock_bh(dev);
> + } else if (ops->ndo_set_rx_mode) {
> + netif_addr_lock_bh(dev);
> + ops->ndo_set_rx_mode(dev);
> + netif_addr_unlock_bh(dev);
> }

...

> --
> 2.53.0