Re: [PATCH net-next v2 3/3] bnxt: convert to core rx_mode retry mechanism

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 06/04, Michael Chan wrote:
> On Wed, Jun 3, 2026 at 3:35 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >
> > Remove the driver-specific BNXT_STATE_L2_FILTER_RETRY + timer + sp_task
> > retry mechanism and rely on the core stack's ndo_set_rx_mode_async retry
> > instead.
> >
> > bnxt_cfg_rx_mode() now returns errors instead of swallowing them. The
> > PF-unavailable case (-ENODEV from HWRM on a VF) is normalized to
> > -EAGAIN at the boundary so callers can match on a single "retry me"
> > errno without re-implementing the VF/-ENODEV check. Other errors
> > propagate unchanged.
> >
> > This removes:
> > - BNXT_STATE_L2_FILTER_RETRY state bit
> > - BNXT_RX_MASK_SP_EVENT sp_event bit
> > - Retry trigger from bnxt_timer()
> > - BNXT_RX_MASK_SP_EVENT handling from bnxt_sp_task()
> >
> > bnxt_init_chip() still calls bnxt_cfg_rx_mode() directly during open.
> > On a fresh open dev->uc is empty and the call effectively cannot fail
> > on the unicast path. But on FW reset reopen (bnxt_fw_reset_task ->
> > bnxt_open) a VF may have a populated dev->uc and the PF may be
> > transiently unavailable; since that path doesn't go through
> > __dev_open(), the follow-up rx_mode call that would otherwise drive
> > the core retry doesn't fire. On -EAGAIN, swallow the error and call
> > netif_rx_mode_schedule_retry() explicitly. The unicast filter loop
> > truncates vnic->uc_filter_count on failure, so the retry's delta check
> > sees pending work and reinstalls.
> >
> > Cc: Michael Chan <michael.chan@broadcom.com>
> > Cc: Pavan Chebbi <pavan.chebbi@broadcom.com>
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> 
> > @@ -13758,11 +13762,8 @@ static int bnxt_cfg_rx_mode(struct bnxt *bp, struct netdev_hw_addr_list *uc,
> >                 rc = bnxt_hwrm_set_vnic_filter(bp, 0, i, vnic->uc_list + off);
> >                 if (rc) {
> >                         if (BNXT_VF(bp) && rc == -ENODEV) {
> > -                               if (!test_and_set_bit(BNXT_STATE_L2_FILTER_RETRY, &bp->state))
> > -                                       netdev_warn(bp->dev, "Cannot configure L2 filters while PF is unavailable, will retry\n");
> > -                               else
> > -                                       netdev_dbg(bp->dev, "PF still unavailable while configuring L2 filters.\n");
> > -                               rc = 0;
> > +                               netdev_warn(bp->dev, "Cannot configure L2 filters while PF is unavailable, will retry\n");
> > +                               rc = -EAGAIN;
> 
> Note that bnxt_hwrm_set_vnic_filter() can also directly return -EAGAIN
> if the FW is busy and retrying is correct.  It will drop to the else
> statement below.  Adding an else if statement will provide a more
> proper warning:
> 
> else if (rc == -EAGAIN)
>         netdev_warn(bp->dev, "FW busy while setting vnic filter, will retry\n");

SG, will add, thanks again for the review!