Re: [PATCH net v2 1/9] bnxt_en: Clear bp->total_irqs in bnxt_init_int_mode() during error

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The bnxt_io_resume() hunk (`if (!err && !bp->irq_tbl) err =
  bnxt_init_int_mode(bp);`) fixes a defect introduced by c2d20a3814d1…

--

bnxt_en: Clear bp->total_irqs in bnxt_init_int_mode() during error

This patch clears bp->total_irqs when bnxt_init_int_mode() fails, so that
bnxt_reserve_rings() and bnxt_check_rings() do not call
bnxt_change_msix() with a NULL bp->irq_tbl. It adds bp->irq_tbl_size to
bound dynamic MSI-X growth. It also skips the second bnxt_init_int_mode()
call in bnxt_io_resume() when bnxt_reserve_rings() has already
initialized MSI-X.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index d7728d0c5b6e6..51557ee6c9ade 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

[ ... ]

> @@ -17692,7 +17702,7 @@ static void bnxt_io_resume(struct pci_dev *pdev)
>  			err = bnxt_open(netdev);
>  		} else {
>  			err = bnxt_reserve_rings(bp, true);
> -			if (!err)
> +			if (!err && !bp->irq_tbl)
>  				err = bnxt_init_int_mode(bp);
>  		}
>  	}

[Severity: Low]
This isn't a bug, but should the patch also carry a Fixes tag for
c2d20a3814d1 ("bnxt_en: delay pci_alloc_irq_vectors() in the AER path")?

The patch only has:

    Fixes: e68256c8a73c ("bnxt_en: Support dynamic MSIX")

Commit c2d20a3814d1 added the unconditional bnxt_init_int_mode() call
after bnxt_reserve_rings() in bnxt_io_resume(), and it is newer than
e68256c8a73c.

The double init can happen without dynamic MSI-X:

bnxt_io_slot_reset()
    bnxt_clear_int_mode()

bnxt_io_resume()
    bnxt_reserve_rings(bp, true)
        /* BNXT_NEW_RM(bp) && irqs_required != bp->total_irqs */
        bnxt_clear_int_mode()
        bnxt_init_int_mode()      <-- MSI-X enabled here
    bnxt_init_int_mode()
        pci_alloc_irq_vectors()
            __pci_enable_msix_range()
                WARN_ON_ONCE(dev->msix_enabled) -> -EINVAL

The second bnxt_init_int_mode() then goes to its error path. It frees
bp->irq_tbl and the vectors and returns -ENODEV, so netif_device_attach()
is never called and the device stays detached after AER recovery.

Because e68256c8a73c is older, stable selection probably already covers
every tree that has c2d20a3814d1. Trees that have e68256c8a73c but not
c2d20a3814d1 would not need this hunk, though.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928041712.3467803-1-michael.chan%40broadcom.com