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

[AMD Official Use Only - AMD Internal Distribution Only]

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Monday, August 11, 2025 9:08 PM
To: Gupta, Suraj <redacted>
Cc: andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
pabeni@redhat.com; Simek, Michal [off-list ref];
sean.anderson@linux.dev; Pandey, Radhey Shyam
[off-list ref]; horms@kernel.org; netdev@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Katakam, Harini
[off-list ref]
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:

quoted

quoted

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.

quoted

quoted

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