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

[Hangbin Liu <liuhangbin@gmail.com>]

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