Thread (4 messages) flat view 4 messages, 2 authors, 4d ago

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

From: Michael Chan <michael.chan@broadcom.com>
Date: 2026-09-21 18:11:03
Also in: lkml

On Thu, Sep 17, 2026 at 4:32 PM Joe Damato [off-list ref] 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.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help