Message-ID: <aINjHQU7Uwz_ZThs@soc-5CG4396X81.clients.intel.com>
Date: Fri, 25 Jul 2025 12:57:33 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Jason Xing <kerneljasonxing@...il.com>
CC: Tony Nguyen <anthony.l.nguyen@...el.com>, <przemyslaw.kitszel@...el.com>,
	<andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <bjorn@...nel.org>,
	<magnus.karlsson@...el.com>, <maciej.fijalkowski@...el.com>,
	<jonathan.lemon@...il.com>, <sdf@...ichev.me>, <ast@...nel.org>,
	<daniel@...earbox.net>, <hawk@...nel.org>, <john.fastabend@...il.com>,
	<bpf@...r.kernel.org>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget
 in ixgbe_xmit_zc

On Fri, Jul 25, 2025 at 07:18:11AM +0800, Jason Xing wrote:
> Hi Tony,
> 
> On Fri, Jul 25, 2025 at 4:21 AM Tony Nguyen <anthony.l.nguyen@...el.com> wrote:
> >
> >
> >
> > On 7/20/2025 2:11 AM, Jason Xing wrote:
> > > From: Jason Xing <kernelxing@...cent.com>
> > >
> > > Resolve the budget underflow which leads to returning true in ixgbe_xmit_zc
> > > even when the budget of descs are thoroughly consumed.
> > >
> > > Before this patch, when the budget is decreased to zero and finishes
> > > sending the last allowed desc in ixgbe_xmit_zc, it will always turn back
> > > and enter into the while() statement to see if it should keep processing
> > > packets, but in the meantime it unexpectedly decreases the value again to
> > > 'unsigned int (0--)', namely, UINT_MAX. Finally, the ixgbe_xmit_zc returns
> > > true, showing 'we complete cleaning the budget'. That also means
> > > 'clean_complete = true' in ixgbe_poll.
> > >
> > > The true theory behind this is if that budget number of descs are consumed,
> > > it implies that we might have more descs to be done. So we should return
> > > false in ixgbe_xmit_zc to tell napi poll to find another chance to start
> > > polling to handle the rest of descs. On the contrary, returning true here
> > > means job done and we know we finish all the possible descs this time and
> > > we don't intend to start a new napi poll.
> > >
> > > It is apparently against our expectations. Please also see how
> > > ixgbe_clean_tx_irq() handles the problem: it uses do..while() statement
> > > to make sure the budget can be decreased to zero at most and the underflow
> > > never happens.
> > >
> > > Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
> >
> > Hi Jason,
> >
> > Seems like this one should be split off and go to iwl-net/net like the
> > other similar ones [1]? Also, some of the updates made to the other
> > series apply here as well?
> 
> The other three patches are built on top of this one. If without the
> patch, the whole series will be warned because of build failure. I was
> thinking we could backport this patch that will be backported to the
> net branch after the whole series goes into the net-next branch.
> 
> Or you expect me to cook four patches without this one first so that
> you could easily cherry pick this one then without building conflict?
> 
> >
> > Thanks,
> > Tony
> >
> > [1]
> > https://lore.kernel.org/netdev/20250723142327.85187-1-kerneljasonxing@gmail.com/
> 
> Regarding this one, should I send a v4 version with the current patch
> included? And target [iwl-net/net] explicitly as well?
> 
> I'm not sure if I follow you. Could you instruct me on how to proceed
> next in detail?
>

What I usually do is send the fix as soon as I have it. While I prepare and test 
the series, the fix usually manages to get into the tree. Advise you do the 
same, given you have things to change in v2, but the fix can be resent almost 
as it is now (just change the tree).

Tony can have a different opinion though.
 
> Thanks,
> Jason
> 

Powered by blists - more mailing lists