linux-kernel - Re: [PATCH net-next v2] net: mana: Handle SKB if TX SGEs exceed hardware limit

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: <bb692420-25f9-4d6e-a68b-dd83c8f4be10@linux.microsoft.com>
Date: Thu, 6 Nov 2025 18:30:50 +0530
From: Aditya Garg <gargaditya@...ux.microsoft.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
 decui@...rosoft.com, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, pabeni@...hat.com, longli@...rosoft.com,
 kotaranov@...rosoft.com, horms@...nel.org, shradhagupta@...ux.microsoft.com,
 ssengar@...ux.microsoft.com, ernis@...ux.microsoft.com,
 dipayanroy@...ux.microsoft.com, shirazsaleem@...rosoft.com,
 linux-hyperv@...r.kernel.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-rdma@...r.kernel.org,
 gargaditya@...rosoft.com
Subject: Re: [PATCH net-next v2] net: mana: Handle SKB if TX SGEs exceed
 hardware limit

On 06-11-2025 05:47, Jakub Kicinski wrote:
> On Wed, 5 Nov 2025 22:10:23 +0530 Aditya Garg wrote:
>>>>    	if (err) {
>>>>    		(void)skb_dequeue_tail(&txq->pending_skbs);
>>>> +		mana_unmap_skb(skb, apc);
>>>>    		netdev_warn(ndev, "Failed to post TX OOB: %d\n", err);
>>>
>>> You have a print right here and in the callee. This condition must
>>> (almost) never happen in practice. It's likely fine to just drop
>>> the packet.
>>
>> The logs placed in callee doesn't covers all the failure scenarios,
>> hence I feel to have this log here with proper status. Maybe I can
>> remove the log in the callee?
> 
> I think my point was that since there are logs (per packet!) when the
> condition is hit -- if it did in fact hit with any noticeable frequency
> your users would have complained. So handling the condition gracefully
> and returning BUSY is likely just unnecessary complexity in practice.
> 

In this, we are returning tx_busy when the error reason is -ENOSPC, for 
all other errors, skb is dropped.
Is it okay requeue only for -ENOSPC cases or should we drop the skb?

> The logs themselves I don't care all that much about. Sure, having two
> lines for one error is a bit unclean.
>   
>>> Either way -- this should be a separate patch.
>>>    
>> Are you suggesting a separate patch altogether or two patch in the same
>> series?
> 
> The changes feel related enough to make them a series, but either way
> is fine.

Regards,
Aditya