Re: [PATCH net v5 0/4] bnxt_en: Make RING FREE more robust

[Joe Damato]

On Fri, Nov 06, 2026 at 11:14:16AM -0800, Joe Damato wrote:
> Greetings:
> 
> This is a follow up to the previous RFC (linked below).
> 
> On two production systems, I saw the following dmesg pattern:
> 
>   NETDEV WATCHDOG: transmit queue 0 timed out 6073 ms
>   Resp cmpl intr err msg: 0x51                  x20
>   hwrm_ring_free type 1 failed                  x12
>   hwrm_ring_free type 2 failed                  x8
>   AMD-Vi: IO_PAGE_FAULT  x3
> 
> This suggests that, for some currently unknown reason, TX completions stall
> and the netdev watchdog fires. The driver asks FW to free the rings, this
> times out, but the driver ignores the possible failure and frees ring memory.
> Since the FW didn't respond to the ring free command, it is possible that the
> FW is still DMAing to the memory which was freed.

OK so Sashiko found a lot of random stuff and I thought it might be good to
summarize what I plan on addressing in the next revision in one place.

For v6:

Patch 1:
  - Fix commit message
  - Return the errno from hwrm_req_send().

Patch 2:
  - Only check the response for HWRM_RING_FREE. HWRM_SELFTEST_IRQ and
    HWRM_RING_RESET keep the existing behavior.
  - Read the valid byte with READ_ONCE(), then dma_rmb() before reading
    the rest of the response, including error_code.
  - Respect BNXT_HWRM_CTX_SILENT for the new warning.

Patch 3:
  - Count a ring as stuck only when no response arrived, not when the
    firmware replied with an error.
  - Don't stop DMA when BNXT_STATE_IN_FW_RESET or BNXT_STATE_FW_FATAL_COND is
    set.

Patch 4:
  - Make BNXT_NO_FW_ACCESS() also test BNXT_STATE_DMA_STOPPED.

Update commit messages as needed.

The Sashiko feedback that I think is real, but that I haven't seen in prod or
I think feels more like "new features" instead of bug fix and should be a
series for net-next:

  - Making the per queue paths more robust.
  - Completion and NQ failures are not counted as stuck.
  - The polling path in __hwrm_send() probably needs a dma_rmb between reading
    the valid byte and reading the rest of the message.
  - bnxt_resume, bnxt_fw_reset_task, and bnxt_io_slot_reset all call
    pci_set_master without checking the DMA bit.
  - Notifying bnxt_re when DMA is stopped.

If Broadcom or any of the maintainers disagrees and thinks some or all of this
should be part of this series please let me know.