Re: [PATCH net v2] bnxt_en: Don't keep MSI-X vectors added before FW reserves them

[Michael Chan]

On Wed, Oct 28, 2026 at 4:49 AM Vlad Poenaru <vlad.wing@xxxxxxxxx> wrote:
>
> When dynamic MSI-X is supported, bnxt_check_rings() grows the MSI-X
> vectors with bnxt_change_msix() once FW says the new rings would fit,
> and keeps them.  FW only reserves the extra IRQs later, when
> bnxt_reserve_rings() calls __bnxt_reserve_rings().  The MSI-X table
> entries beyond the function's reserved IRQs are not backed until then:
> the MSI message written when the new vectors are activated is lost, and
> the entries read back as zero.  After FW reserves them they are live but
> still zero, so the NQ rings bound to them signal address 0 and every
> completion on those rings is lost.  The next ring change (for example
> ethtool -G) then times out freeing each of those rings and the TX
> watchdog fires:
>
>   bnxt_en 0002:01:00.0 eth0: Resp cmpl intr err msg: 0x51
>   bnxt_en 0002:01:00.0 eth0: hwrm_ring_free type 1 failed. rc:fffffff0 err:0
>   ...
>   bnxt_en 0002:01:00.0 eth0: NETDEV WATCHDOG: CPU: 17: transmit queue 18 timed out 5716 ms
>

I've discussed this issue with our FW team and they have confirmed it.
Writing to an MSI-X entry that has not been reserved by the FW will be
silently ignored by the HW.

...

> @@ -11701,8 +11702,23 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
>         int irqs_required;
>         int rc;
>
> -       if (!bnxt_need_reserve_rings(bp))
> +       if (!bnxt_need_reserve_rings(bp)) {
> +               /* FW may already hold the reservation from an earlier
> +                * reservation that was not followed by growing the dynamic
> +                * MSI-X vectors, e.g. because the allocation failed.
> +                */
> +               if (irq_re_init && BNXT_NEW_RM(bp) && bp->irq_tbl &&
> +                   pci_msix_can_alloc_dyn(bp->pdev)) {
> +                       irqs_required = bnxt_get_num_msix(bp);
> +                       if (irqs_required > bp->total_irqs &&
> +                           bnxt_change_msix(bp, irqs_required) != irqs_required) {
> +                               netdev_err(bp->dev, "Unable to allocate %d MSIX vectors, maximum available %d\n",
> +                                          irqs_required, bp->total_irqs);
> +                               return -ENOSPC;

Once I rebase my net-next series, this will likely not be needed since
there will be infrastructure to scale the rings down.  I will make
sure to handle the case where dynamic MSI-X falls short.

The patch looks good to me.  Thanks.
Reviewed-by: Michael Chan <michael.chan@xxxxxxxxxxxx>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature