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
