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