[PATCH net] net: xilinx: axienet: Increment Rx skb ring head pointer after BD is successfully allocated in dmaengine flow

[Pandey, Radhey Shyam]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Jakub Kicinski <kuba at kernel.org>
> Sent: Monday, August 11, 2025 9:08 PM
> To: Gupta, Suraj <Suraj.Gupta2 at amd.com>
> Cc: andrew+netdev at lunn.ch; davem at davemloft.net; edumazet at google.com;
> pabeni at redhat.com; Simek, Michal <michal.simek at amd.com>;
> sean.anderson at linux.dev; Pandey, Radhey Shyam
> <radhey.shyam.pandey at amd.com>; horms at kernel.org; netdev at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; Katakam, Harini
> <harini.katakam at amd.com>
> Subject: Re: [PATCH net] net: xilinx: axienet: Increment Rx skb ring head pointer
> after BD is successfully allocated in dmaengine flow
>
> On Sat, 9 Aug 2025 20:31:40 +0000 Gupta, Suraj wrote:
> > > The fix itself seems incomplete. Even if we correctly skip the
> > > increment we will never try to catch up with the allocations, the
> > > ring will have fewer outstanding Rx skbs until reset, right? Worst
> > > case we drop all the skbs and the ring will be empty, no Rx will happen until
> reset.
> > > The shutdown path seems to be checking for skb = NULL so I guess
> > > it's correct but good to double check..
> >
> > I agree that Rx ring will have fewer outstanding skbs. But I think
> > that difference won't exceed one anytime as descriptors submission
> > will fail only once due to insufficient space in AXIDMA BD ring. Rest
> > of the time we already will have an extra entry in AXIDMA BD ring.
> > Also, invoking callback (where Rx skb ring hp is filled in axienet)and
> > freeing AXIDMA BD are part of same tasklet in AXIDMA driver so next
> > callback will only be called after freeing a BD. I tested running
> > stress tests (Both UPD and TCP netperf). Please let me know your
> > thoughts if I'm missing something.
>
> That wasn't my reading, maybe I misinterpreted the code.
>
> From what I could tell the driver tries to give one new buffer for each buffer
> completed. So it never tries to "catch up" on previously missed allocations. IOW say
> we have a queue with 16 indexes, after 16 failures (which may be spread out over
> time) the ring will be empty.

Yes, IIRC there is 1:1 mapping for RX DMA callback and
axienet_rx_submit_desc(). In case there are failure in
axienet_rx_submit_desc() it is not able to reattempt
in current implementation. Theoretically there could
be other error in rx_submit_desc() (like dma_mapping/netdev
allocation)

One thought is to have some flag/index to tell that it should
be reattempted in subsequent axienet_rx_submit_desc() ?

Thanks,
Radhey