[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<MN0PR12MB5953EF5C6557457C1C893668B728A@MN0PR12MB5953.namprd12.prod.outlook.com>
Date: Mon, 11 Aug 2025 15:55:02 +0000
From: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
To: Jakub Kicinski <kuba@...nel.org>, "Gupta, Suraj" <Suraj.Gupta2@....com>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"pabeni@...hat.com" <pabeni@...hat.com>, "Simek, Michal"
<michal.simek@....com>, "sean.anderson@...ux.dev" <sean.anderson@...ux.dev>,
"horms@...nel.org" <horms@...nel.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "Katakam, Harini" <harini.katakam@....com>
Subject: 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@...nel.org>
> Sent: Monday, August 11, 2025 9:08 PM
> To: Gupta, Suraj <Suraj.Gupta2@....com>
> Cc: andrew+netdev@...n.ch; davem@...emloft.net; edumazet@...gle.com;
> pabeni@...hat.com; Simek, Michal <michal.simek@....com>;
> sean.anderson@...ux.dev; Pandey, Radhey Shyam
> <radhey.shyam.pandey@....com>; horms@...nel.org; netdev@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; Katakam, Harini
> <harini.katakam@....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
Powered by blists - more mailing lists