Re: [PATCH net v2] bnxt_en: Don't keep MSI-X vectors added before FW reserves them
flat view
From: Michael Chan <michael.chan@broadcom.com>
Date: 2026-10-30 23:10:29
Also in:
lkml, regressions
On Wed, Oct 28, 2026 at 4:49 AM Vlad Poenaru [off-list ref] 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.
...
quoted hunk ↗ jump to hunk
@@ -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@broadcom.com> Attachments
- smime.p7s [application/pkcs7-signature] 5469 bytes