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

[Michael Chan]

On Tue, Nov 10, 2026 at 2:13 PM Joe Damato <joe@xxxxxxx> wrote:
>
> On Tue, Nov 10, 2026 at 10:14:50AM -0800, Michael Chan wrote:
> > I'm copying our RoCE developers Selvin and Kalesh.  I think that if
> > you call bnxt_re_suspend() before you disable DMA, nothing special
> > needs to be done because the chip is still functioning.  It will go
> > through the normal removal process.
>
> I'm probably missing something but I think that the ring frees can only fail
> in __bnxt_close_nic, where rtnl and the netdev instance lock are held. DMA has
> to be stopped before bnxt_free_mem. bnxt_re_suspend() takes both locks, so it
> can't be called.
>
> I think there's two options:
>
> 1. Stop DMA as now, but queue a work item to run bnxt_ulp_stop without the
> locks. bnxt_re_suspend would need to treat BNXT_STATE_DMA_STOPPED like
> FW_FATAL_COND and take the detach path.
>
> or
>
> 2. Call bnxt_ulp_stop() in bnxt_reset() before taking the locks, the way
> bnxt_fw_reset() does, and bnxt_ulp_start() after a successful reopen. That
> covers the TX timeout reset, which is where all the ring free failves I've
> seen come from and it doesn't need any bnxt_re change, but it restarts RoCE on
> every TX timeout and it wouldn't cover a case where the user triggers
> close/open.
>

I have a slight preference for option 2 because TX timeout ->
bnxt_reset() should be rare and stopping RoCE briefly even if all the
HWRM_RING_FREE calls succeed should be ok.  We don't want to stop RoCE
unnecessarily during a user-initiated close/open though (failure
should be very rare).  So we may still need option 1 to cover the
other rare failure cases.

Let's wait for Selvin's and Kalesh's feedback.

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