netdev - Re: [PATCH net-next 1/5] net: add a common function to compute features from lowers devices

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: <aMNyhuC1DJsajuqs@fedora>
Date: Fri, 12 Sep 2025 01:08:22 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Ido Schimmel <idosch@...dia.com>
Cc: Sabrina Dubroca <sd@...asysnail.net>, netdev@...r.kernel.org,
	Jay Vosburgh <jv@...sburgh.net>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jiri Pirko <jiri@...nulli.us>, Simon Horman <horms@...nel.org>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Shuah Khan <shuah@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
	Kuniyuki Iwashima <kuniyu@...gle.com>,
	Ahmed Zaki <ahmed.zaki@...el.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	bridge@...ts.linux.dev, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next 1/5] net: add a common function to compute
 features from lowers devices

On Thu, Sep 11, 2025 at 03:59:59PM +0300, Ido Schimmel wrote:
> > > It is not clear to me why we are setting hard_header_len to the largest
> > > of all lowers and not needed_headroom. While bond/team allow
> > > non-Ethernet lowers (unlike bridge, which is also adjusted to use this
> > > helper), they do verify that all the lower devices are of the same type.
> > > Shouldn't devices of the same type have the same hardware header length?
> > 
> > At least not with VLANs. Both basic ethernet and vlan devices are
> > ARPHRD_ETHER, but the hard_header_len of the vlan device will be
> > larger if we're not offloading:
> > 
> >     dev->hard_header_len = real_dev->hard_header_len + VLAN_HLEN;
> 
> This looks like a remanent from the time before needed_headroom was
> introduced, aimed at making sure that the kernel has enough room to push
> the VLAN tag when the hardware is unable to. I believe it should be
> converted to adjust needed_headroom instead. Otherwise, looking at
> __is_skb_forwardable(), an skb might be forwarded to a VLAN device when
> its real device does not support Tx VLAN acceleration and dropped
> otherwise (due to a smaller hard_header_len).
> 
> Anyway, I'm OK with keeping hard_header_len for now, but ultimately I
> think netdev_compute_features_from_lowers() should be adjusting
> needed_headroom and not hard_header_len.

Thanks, I will add the needed_headroom update on my todo list.

Hangbin