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

[Joe Damato <joe@dama.to>]

On Tue, Nov 10, 2026 at 10:14:50AM -0800, Michael Chan wrote:
> On Tue, Nov 10, 2026 at 9:39 AM Joe Damato <joe@dama.to> wrote:
> >
> > On Mon, Nov 09, 2026 at 04:13:57PM -0800, Michael Chan wrote:
> > > On Mon, Nov 9, 2026 at 12:12 PM Joe Damato <joe@dama.to> wrote:
> > >
> > > >   - Notifying bnxt_re when DMA is stopped.
> > >
> > > I think this is probably the one item that should be added to the
> > > patchset.  When DMA is disabled, RoCE will not work at all so adding
> > > this will make the series more complete.  It may be as simple as
> > > calling bnxt_ulp_stop(), but I haven't looked into it in detail.
> >
> > Hm. bnxt_stop_dma runs with rtnl and the netdev instance lock held and
> > bnxt_ulp_stop -> bnxt_re_suspend takes both. So, I don't think calling
> > bnxt_ulp_stop is going to work.
> >
> > Maybe I need to make a new patch where bnxt_stop_dma queues a work item that
> > calls bnxt_ulp_stop without locks being held? Not sure.
> >
> > Also: would you be OK with bnxt_re treating BNXT_STATE_DMA_STOPPED as
> > detached? I'm probably getting this wrong but I think with DMA off
> > bnxt_re_suspend only takes the detach path when FW_FATAL_COND is set.
> >
> 
> 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.

Not sure if I am getting that right; maybe Selvin or Kalesh can comment.

If that is accurate, do you have a preference on how to handle this?