Re: [RFC net-next 0/2] bnxt_en: Recover from failed TX RING_FREE

[Michael Chan]

On Thu, Sep 17, 2026 at 4:32 PM Joe Damato <joe@xxxxxxx> wrote:

> But... if the completion ring is dead, the completion never arrives.
>
> This is reachable on production systems today:
>
>   - TX completions stop
>   - netdev watchdog fires
>   - reset closes the device
>   - every RING_FREE routed through that dead completion ring times out
>   - ring memory freed by the driver but still in use by the FW
>
> The result on an IOMMU host is IO_PAGE_FAULT or DMAR fault against freed
> pages.

If the FW did not receive the HWRM_RING_FREE command from the driver,
there is no guarantee that DMA will stop.  I agree that the driver is
not very robust in handling this in the close path.  We just continue
after the timeout hoping that the FW actually received it but couldn't
respond.

In contrast, the error recovery path for fatal errors is a little more
robust.  For example, in bnxt_fw_fatal_close(), we call
pci_disable_device() to stop DMA from the device.

We probably should do something similar in the close path when
HWRM_RING_FREE is not responding at all.  Maybe even FLR.

>
> I tried to test the code in patch 2 on a BCM57504 with FW 235.1.208.0/pkg
> 235.1.208.0.
>
> I hacked something together to inject a failure to test the reset paths on my
> device. It seems like HWRM_RING_RESET ring_type=TX is accepted by thte FW and
> the polled RING_FREE also succeeds, but in my testing the TX ring was idle. I
> never tested a reset against a ring with descriptors in flight. Which leads me
> to my questions.....
>
> 1. Does HWRM_RING_RESET with ring_type=TX cause the FW to abandon work already
> outstanding on that ring and stop DMA ? If not .... then this code is wrong :(
> and maybe see question (3) below.

HWRM_RING_RESET is not supported on BCM5750X and BCM5760X.  Even if
the FW accepts it, it probably does nothing internally.

>
> 3. Maybe the TX reset isn't necessary at all? Maybe instead the code should
> retry the RING_FREE over polled transport and that's good enough? This depends
> on the answer to question (1) above, but I guess it would simplify the code if
> a polled RING_FREE is enough?
>

If interrupt is not working and the HWRM_RING_FREE doesn't complete
using the interrupt path, it will timeout using the same FW polling
timeout.  In other words, if FW received it but just couldn't post the
completion on the completion ring and interrupt, the wait should be
long enough for it to complete.   One additional thing we can do that
we're currently not doing is to check for a valid response length and
check the valid bit anyway on interrupt timeout.  If these look valid,
the FW has accepted the HWRM_RING_FREE but just couldn't interrupt.
If these are not valid, sending a second HWRM_RING_FREE should yield
the same results.

Thanks for looking into this.

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