With the introduction of ndo_set_rx_mode_async (as discussed in [0])
we can call bnxt_cfg_rx_mode directly. Convert bnxt_cfg_rx_mode to
use uc/mc snapshots and move its call in bnxt_sp_task to the
section that resets BNXT_STATE_IN_SP_TASK. Switch to direct call in
bnxt_set_rx_mode.
0: https://lore.kernel.org/netdev/CACKFLi=5vj8hPqEUKDd8RTw3au5G+zRgQEqjF+6NZnyoNm90KA@mail.gmail.com/
Cc: Michael Chan <michael.chan@broadcom.com>
Cc: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 26 ++++++++++++++---------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 225217b32e4b..12265bd7fda4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11039,7 +11039,8 @@ static int bnxt_setup_nitroa0_vnic(struct bnxt *bp)
return rc;
}
-static int bnxt_cfg_rx_mode(struct bnxt *);
+static int bnxt_cfg_rx_mode(struct bnxt *, struct netdev_hw_addr_list *,
+ struct netdev_hw_addr_list *);
static bool bnxt_mc_list_updated(struct bnxt *, u32 *,
const struct netdev_hw_addr_list *);
@@ -11135,7 +11136,7 @@ static int bnxt_init_chip(struct bnxt *bp, bool irq_re_init)
vnic->rx_mask |= mask;
}
- rc = bnxt_cfg_rx_mode(bp);
+ rc = bnxt_cfg_rx_mode(bp, &bp->dev->uc, &bp->dev->mc);
if (rc)
goto err_out;
@@ -13610,11 +13611,12 @@ static void bnxt_set_rx_mode(struct net_device *dev,
if (mask != vnic->rx_mask || uc_update || mc_update) {
vnic->rx_mask = mask;
- bnxt_queue_sp_work(bp, BNXT_RX_MASK_SP_EVENT);
+ bnxt_cfg_rx_mode(bp, uc, mc);
}
}
-static int bnxt_cfg_rx_mode(struct bnxt *bp)
+static int bnxt_cfg_rx_mode(struct bnxt *bp, struct netdev_hw_addr_list *uc,
+ struct netdev_hw_addr_list *mc)
{
struct net_device *dev = bp->dev;
struct bnxt_vnic_info *vnic = &bp->vnic_info[BNXT_VNIC_DEFAULT];
@@ -13623,7 +13625,7 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp)
bool uc_update;
netif_addr_lock_bh(dev);
- uc_update = bnxt_uc_list_updated(bp, &dev->uc);
+ uc_update = bnxt_uc_list_updated(bp, uc);
netif_addr_unlock_bh(dev);
if (!uc_update)
@@ -13639,10 +13641,10 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp)
vnic->uc_filter_count = 1;
netif_addr_lock_bh(dev);
- if (netdev_uc_count(dev) > (BNXT_MAX_UC_ADDRS - 1)) {
+ if (netdev_hw_addr_list_count(uc) > (BNXT_MAX_UC_ADDRS - 1)) {
vnic->rx_mask |= CFA_L2_SET_RX_MASK_REQ_MASK_PROMISCUOUS;
} else {
- netdev_for_each_uc_addr(ha, dev) {
+ netdev_hw_addr_list_for_each(ha, uc) {
memcpy(vnic->uc_list + off, ha->addr, ETH_ALEN);
off += ETH_ALEN;
vnic->uc_filter_count++;
@@ -14600,6 +14602,7 @@ static void bnxt_ulp_restart(struct bnxt *bp)
static void bnxt_sp_task(struct work_struct *work)
{
struct bnxt *bp = container_of(work, struct bnxt, sp_task);
+ struct net_device *dev = bp->dev;
set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
smp_mb__after_atomic();
@@ -14613,9 +14616,6 @@ static void bnxt_sp_task(struct work_struct *work)
bnxt_reenable_sriov(bp);
}
- if (test_and_clear_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event))
- bnxt_cfg_rx_mode(bp);
-
if (test_and_clear_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event))
bnxt_cfg_ntp_filters(bp);
if (test_and_clear_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp->sp_event))
@@ -14680,6 +14680,12 @@ static void bnxt_sp_task(struct work_struct *work)
/* These functions below will clear BNXT_STATE_IN_SP_TASK. They
* must be the last functions to be called before exiting.
*/
+ if (test_and_clear_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event)) {
+ bnxt_lock_sp(bp);
+ bnxt_cfg_rx_mode(bp, &dev->uc, &dev->mc);
+ bnxt_unlock_sp(bp);
+ }
+
if (test_and_clear_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event))
bnxt_reset(bp, false);
--
2.53.0On Thu, Mar 19, 2026 at 6:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> With the introduction of ndo_set_rx_mode_async (as discussed in [0])
> we can call bnxt_cfg_rx_mode directly. Convert bnxt_cfg_rx_mode to
> use uc/mc snapshots and move its call in bnxt_sp_task to the
> section that resets BNXT_STATE_IN_SP_TASK. Switch to direct call in
> bnxt_set_rx_mode.
>
> 0: https://lore.kernel.org/netdev/CACKFLi=5vj8hPqEUKDd8RTw3au5G+zRgQEqjF+6NZnyoNm90KA@mail.gmail.com/
>
> Cc: Michael Chan <michael.chan@broadcom.com>
> Cc: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 26 ++++++++++++++---------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 225217b32e4b..12265bd7fda4 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -11039,7 +11039,8 @@ static int bnxt_setup_nitroa0_vnic(struct bnxt *bp)
> return rc;
> }
>
> -static int bnxt_cfg_rx_mode(struct bnxt *);
> +static int bnxt_cfg_rx_mode(struct bnxt *, struct netdev_hw_addr_list *,
> + struct netdev_hw_addr_list *);
> static bool bnxt_mc_list_updated(struct bnxt *, u32 *,
> const struct netdev_hw_addr_list *);
>
> @@ -11135,7 +11136,7 @@ static int bnxt_init_chip(struct bnxt *bp, bool irq_re_init)
> vnic->rx_mask |= mask;
> }
>
> - rc = bnxt_cfg_rx_mode(bp);
> + rc = bnxt_cfg_rx_mode(bp, &bp->dev->uc, &bp->dev->mc);
> if (rc)
> goto err_out;
>
> @@ -13610,11 +13611,12 @@ static void bnxt_set_rx_mode(struct net_device *dev,
> if (mask != vnic->rx_mask || uc_update || mc_update) {
> vnic->rx_mask = mask;
>
> - bnxt_queue_sp_work(bp, BNXT_RX_MASK_SP_EVENT);
> + bnxt_cfg_rx_mode(bp, uc, mc);
> }
> }
>
> -static int bnxt_cfg_rx_mode(struct bnxt *bp)
> +static int bnxt_cfg_rx_mode(struct bnxt *bp, struct netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc)
> {
> struct net_device *dev = bp->dev;
> struct bnxt_vnic_info *vnic = &bp->vnic_info[BNXT_VNIC_DEFAULT];
> @@ -13623,7 +13625,7 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp)
> bool uc_update;
>
> netif_addr_lock_bh(dev);
> - uc_update = bnxt_uc_list_updated(bp, &dev->uc);
> + uc_update = bnxt_uc_list_updated(bp, uc);
Will the uc list snapshot change between bnxt_set_rx_mode() and
bnxt_cfg_rx_mode() with the direct call now? In the original deferred
update implementation, the uc list can change and that's why we check
in both functions.
On 03/23, Michael Chan wrote:
> On Thu, Mar 19, 2026 at 6:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > With the introduction of ndo_set_rx_mode_async (as discussed in [0])
> > we can call bnxt_cfg_rx_mode directly. Convert bnxt_cfg_rx_mode to
> > use uc/mc snapshots and move its call in bnxt_sp_task to the
> > section that resets BNXT_STATE_IN_SP_TASK. Switch to direct call in
> > bnxt_set_rx_mode.
> >
> > 0: https://lore.kernel.org/netdev/CACKFLi=5vj8hPqEUKDd8RTw3au5G+zRgQEqjF+6NZnyoNm90KA@mail.gmail.com/
> >
> > Cc: Michael Chan <michael.chan@broadcom.com>
> > Cc: Pavan Chebbi <pavan.chebbi@broadcom.com>
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > ---
> > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 26 ++++++++++++++---------
> > 1 file changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index 225217b32e4b..12265bd7fda4 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > @@ -11039,7 +11039,8 @@ static int bnxt_setup_nitroa0_vnic(struct bnxt *bp)
> > return rc;
> > }
> >
> > -static int bnxt_cfg_rx_mode(struct bnxt *);
> > +static int bnxt_cfg_rx_mode(struct bnxt *, struct netdev_hw_addr_list *,
> > + struct netdev_hw_addr_list *);
> > static bool bnxt_mc_list_updated(struct bnxt *, u32 *,
> > const struct netdev_hw_addr_list *);
> >
> > @@ -11135,7 +11136,7 @@ static int bnxt_init_chip(struct bnxt *bp, bool irq_re_init)
> > vnic->rx_mask |= mask;
> > }
> >
> > - rc = bnxt_cfg_rx_mode(bp);
> > + rc = bnxt_cfg_rx_mode(bp, &bp->dev->uc, &bp->dev->mc);
> > if (rc)
> > goto err_out;
> >
> > @@ -13610,11 +13611,12 @@ static void bnxt_set_rx_mode(struct net_device *dev,
> > if (mask != vnic->rx_mask || uc_update || mc_update) {
> > vnic->rx_mask = mask;
> >
> > - bnxt_queue_sp_work(bp, BNXT_RX_MASK_SP_EVENT);
> > + bnxt_cfg_rx_mode(bp, uc, mc);
> > }
> > }
> >
> > -static int bnxt_cfg_rx_mode(struct bnxt *bp)
> > +static int bnxt_cfg_rx_mode(struct bnxt *bp, struct netdev_hw_addr_list *uc,
> > + struct netdev_hw_addr_list *mc)
> > {
> > struct net_device *dev = bp->dev;
> > struct bnxt_vnic_info *vnic = &bp->vnic_info[BNXT_VNIC_DEFAULT];
> > @@ -13623,7 +13625,7 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp)
> > bool uc_update;
> >
> > netif_addr_lock_bh(dev);
> > - uc_update = bnxt_uc_list_updated(bp, &dev->uc);
> > + uc_update = bnxt_uc_list_updated(bp, uc);
>
> Will the uc list snapshot change between bnxt_set_rx_mode() and
> bnxt_cfg_rx_mode() with the direct call now? In the original deferred
> update implementation, the uc list can change and that's why we check
> in both functions.
The snapshot is gonna be the same for bnxt_set_rx_mode->bnxt_cfg_rx_mode path.
So you're saying that it's ok to remove the one in bnxt_cfg_rx_mode
because it's called either from bnxt_set_rx_mode (with a new list) or,
explicitly, via the BNXT_RX_MASK_SP_EVENT retry mechanism (where we know
that we need to redo the updates anyway)?
This makes me wonder whether I need to push the retrying mechanism to
the core stack... Right now, if some of the allocations in wq handler
fail, we just give up, maybe I should handle it better. And I can plug
the signal from the driver (make ndo_set_rx_mode_async return int)
in the same retry mechanism.
> -----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 08/13] bnxt: use snapshot in
> bnxt_cfg_rx_mode
>
> With the introduction of ndo_set_rx_mode_async (as discussed in [0])
> we can call bnxt_cfg_rx_mode directly. Convert bnxt_cfg_rx_mode to use
> uc/mc snapshots and move its call in bnxt_sp_task to the section that
> resets BNXT_STATE_IN_SP_TASK. Switch to direct call in
> bnxt_set_rx_mode.
>
> 0:
> https://lore.kernel.org/netdev/CACKFLi=5vj8hPqEUKDd8RTw3au5G+zRgQEqjF+
> 6NZnyoNm90KA@mail.gmail.com/
>
> Cc: Michael Chan <michael.chan@broadcom.com>
> Cc: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 26 ++++++++++++++--------
> -
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 225217b32e4b..12265bd7fda4 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -11039,7 +11039,8 @@ static int bnxt_setup_nitroa0_vnic(struct bnxt
> *bp)
> return rc;
> }
>
> -static int bnxt_cfg_rx_mode(struct bnxt *);
> +static int bnxt_cfg_rx_mode(struct bnxt *, struct netdev_hw_addr_list
> *,
> + struct netdev_hw_addr_list *);
> static bool bnxt_mc_list_updated(struct bnxt *, u32 *,
> const struct netdev_hw_addr_list *);
>
> @@ -11135,7 +11136,7 @@ static int bnxt_init_chip(struct bnxt *bp,
> bool irq_re_init)
> vnic->rx_mask |= mask;
> }
>
> - rc = bnxt_cfg_rx_mode(bp);
> + rc = bnxt_cfg_rx_mode(bp, &bp->dev->uc, &bp->dev->mc);
> if (rc)
> goto err_out;
>
> @@ -13610,11 +13611,12 @@ static void bnxt_set_rx_mode(struct
> net_device *dev,
> if (mask != vnic->rx_mask || uc_update || mc_update) {
> vnic->rx_mask = mask;
>
> - bnxt_queue_sp_work(bp, BNXT_RX_MASK_SP_EVENT);
> + bnxt_cfg_rx_mode(bp, uc, mc);
> }
> }
>
> -static int bnxt_cfg_rx_mode(struct bnxt *bp)
> +static int bnxt_cfg_rx_mode(struct bnxt *bp, struct
> netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc)
> {
> struct net_device *dev = bp->dev;
> struct bnxt_vnic_info *vnic = &bp-
> >vnic_info[BNXT_VNIC_DEFAULT];
> @@ -13623,7 +13625,7 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp)
> bool uc_update;
>
> netif_addr_lock_bh(dev);
> - uc_update = bnxt_uc_list_updated(bp, &dev->uc);
> + uc_update = bnxt_uc_list_updated(bp, uc);
> netif_addr_unlock_bh(dev);
>
> if (!uc_update)
> @@ -13639,10 +13641,10 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp)
> vnic->uc_filter_count = 1;
>
> netif_addr_lock_bh(dev);
> - if (netdev_uc_count(dev) > (BNXT_MAX_UC_ADDRS - 1)) {
> + if (netdev_hw_addr_list_count(uc) > (BNXT_MAX_UC_ADDRS - 1)) {
> vnic->rx_mask |=
> CFA_L2_SET_RX_MASK_REQ_MASK_PROMISCUOUS;
> } else {
> - netdev_for_each_uc_addr(ha, dev) {
> + netdev_hw_addr_list_for_each(ha, uc) {
> memcpy(vnic->uc_list + off, ha->addr, ETH_ALEN);
> off += ETH_ALEN;
> vnic->uc_filter_count++;
> @@ -14600,6 +14602,7 @@ static void bnxt_ulp_restart(struct bnxt *bp)
> static void bnxt_sp_task(struct work_struct *work) {
> struct bnxt *bp = container_of(work, struct bnxt, sp_task);
> + struct net_device *dev = bp->dev;
>
> set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
> smp_mb__after_atomic();
> @@ -14613,9 +14616,6 @@ static void bnxt_sp_task(struct work_struct
> *work)
> bnxt_reenable_sriov(bp);
> }
>
> - if (test_and_clear_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event))
> - bnxt_cfg_rx_mode(bp);
> -
> if (test_and_clear_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp-
> >sp_event))
> bnxt_cfg_ntp_filters(bp);
> if (test_and_clear_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp-
> >sp_event)) @@ -14680,6 +14680,12 @@ static void bnxt_sp_task(struct
> work_struct *work)
> /* These functions below will clear BNXT_STATE_IN_SP_TASK.
> They
> * must be the last functions to be called before exiting.
> */
> + if (test_and_clear_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event)) {
> + bnxt_lock_sp(bp);
> + bnxt_cfg_rx_mode(bp, &dev->uc, &dev->mc);
> + bnxt_unlock_sp(bp);
> + }
> +
> if (test_and_clear_bit(BNXT_RESET_TASK_SP_EVENT, &bp-
> >sp_event))
> bnxt_reset(bp, false);
>
> --
> 2.53.0
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
© 2016 - 2026 Red Hat, Inc.