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: <20251105161754.4b9a1363@kernel.org>
Date: Wed, 5 Nov 2025 16:17:54 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Aditya Garg <gargaditya@...ux.microsoft.com>
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 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.

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.