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] [thread-next>] [day] [month] [year] [list]

Message-ID: <cae34d33-3e32-4c89-a775-df764d964c4d@intel.com>
Date: Fri, 25 Jul 2025 09:54:19 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Larysa Zaremba <larysa.zaremba@...el.com>, Jason Xing
	<kerneljasonxing@...il.com>
CC: <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 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.

Thanks,
Tony