netdev - Re: [PATCH net-next 2/5] ixgbe: xsk: resolve the underflow of budget in ixgbe_xmit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL+tcoCCM2Yxy=rQHcLJmi9=Vm=4whCJbVH=EqU8hazL5XXA-A@mail.gmail.com>
Date: Sat, 26 Jul 2025 08:22:21 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: Larysa Zaremba <larysa.zaremba@...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 Sat, Jul 26, 2025 at 12:54 AM Tony Nguyen <anthony.l.nguyen@...el.com> wrote:
>
>
>
> On 7/25/2025 3:57 AM, Larysa Zaremba wrote:
> > 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.
>
> I agree. Normally in these situations, send the fix first and after that
> one is
> applied, the other patches can be sent.
> This patch would've fit in nice with the other series, however, as that
> one is already in process and this one can standalone. I would send this
> fix by itself.

Got it. I will leave those two fixes as they are and send this one
targetting the right branch as soon as possible today.

Thanks,
Jason