On Fri, Aug 29, 2025 at 09:54:26AM +0000, Hangbin Liu wrote:
> Some high level virtual drivers need to compute features from lower
> devices. But each has their own implementations and may lost some
> feature compute. Let's use one common function to compute features
> for kinds of these devices.
>
> The new helper uses the current bond implementation as the reference
> one, as the latter already handles all the relevant aspects: netdev
> features, TSO limits and dst retention.
>
> Suggested-by: Paolo Abeni <[email protected]>
> Signed-off-by: Hangbin Liu <[email protected]>
> ---
> include/linux/netdevice.h | 19 ++++++++++
> net/core/dev.c | 79 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index f3a3b761abfb..42742a47f2c6 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -5279,6 +5279,25 @@ int __netdev_update_features(struct net_device *dev);
> void netdev_update_features(struct net_device *dev);
> void netdev_change_features(struct net_device *dev);
>
> +/* netdevice features */
> +#define VIRTUAL_DEV_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
> + NETIF_F_FRAGLIST |
> NETIF_F_GSO_SOFTWARE | \
> + NETIF_F_GSO_ENCAP_ALL | \
> + NETIF_F_HIGHDMA | NETIF_F_LRO)
> +
> +#define VIRTUAL_DEV_ENC_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
> + NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE
> | \
> + NETIF_F_GSO_PARTIAL)
> +
> +#define VIRTUAL_DEV_MPLS_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
> + NETIF_F_GSO_SOFTWARE)
> +
> +#define VIRTUAL_DEV_XFRM_FEATURES (NETIF_F_HW_ESP |
> NETIF_F_HW_ESP_TX_CSUM | \
> + NETIF_F_GSO_ESP)
> +
> +#define VIRTUAL_DEV_GSO_PARTIAL_FEATURES (NETIF_F_GSO_ESP)
> +void netdev_compute_features_from_lowers(struct net_device *dev);
> +
> void netif_stacked_transfer_operstate(const struct net_device *rootdev,
> struct net_device *dev);
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1d1650d9ecff..fcad2a9f6b65 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -12577,6 +12577,85 @@ netdev_features_t
> netdev_increment_features(netdev_features_t all,
> }
> EXPORT_SYMBOL(netdev_increment_features);
>
> +/**
> + * netdev_compute_features_from_lowers - compute feature from lowers
> + * @dev: the upper device
> + *
> + * Recompute the upper device's feature based on all lower devices.
> + */
> +void netdev_compute_features_from_lowers(struct net_device *dev)
> +{
> + unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
> IFF_XMIT_DST_RELEASE_PERM;
> + netdev_features_t gso_partial_features =
> VIRTUAL_DEV_GSO_PARTIAL_FEATURES;
> +#ifdef CONFIG_XFRM_OFFLOAD
> + netdev_features_t xfrm_features = VIRTUAL_DEV_XFRM_FEATURES;
^ double space (in other places as well)
> +#endif
> + netdev_features_t mpls_features = VIRTUAL_DEV_MPLS_FEATURES;
> + netdev_features_t vlan_features = VIRTUAL_DEV_VLAN_FEATURES;
> + netdev_features_t enc_features = VIRTUAL_DEV_ENC_FEATURES;
> + unsigned short max_hard_header_len = ETH_HLEN;
hard_header_len is not really a feature, so does not sound like it
belongs here. I'm pretty sure it's not needed at all.
It was added to the bond driver in 2006 by commit 54ef31371407 ("[PATCH]
bonding: Handle large hard_header_len") citing panics with gianfar on
xmit. In 2009 commit 93c1285c5d92 ("gianfar: reallocate skb when
headroom is not enough for fcb") fixed the gianfar driver to stop
assuming that it has enough room to push its custom header. Further,
commit bee9e58c9e98 ("gianfar:don't add FCB length to hard_header_len")
from 2012 fixed this driver to use needed_headroom instead of
hard_header_len.
The team driver is also adjusting hard_header_len according to the lower
devices, but it most likely copied it from the bond driver. On the other
hand, the bridge driver does not mess with hard_header_len and no
problems were reported there (that I know of).
Might be a good idea to remove this hard_header_len logic from bond and
team and instead set their needed_headroom according to the lower device
with the highest needed_headroom. Paolo added similar logic in bridge
and ovs but the use case is a bit different there.
> + unsigned int tso_max_size = TSO_MAX_SIZE;
> + u16 tso_max_segs = TSO_MAX_SEGS;
> + struct net_device *lower_dev;
> + struct list_head *iter;
> +
> + mpls_features = netdev_base_features(mpls_features);
> + vlan_features = netdev_base_features(vlan_features);
> + enc_features = netdev_base_features(enc_features);
> +
> + netdev_for_each_lower_dev(dev, lower_dev, iter) {
> + gso_partial_features =
> netdev_increment_features(gso_partial_features,
> +
> lower_dev->gso_partial_features,
> +
> VIRTUAL_DEV_GSO_PARTIAL_FEATURES);
> +
> + vlan_features = netdev_increment_features(vlan_features,
> +
> lower_dev->vlan_features,
> +
> VIRTUAL_DEV_VLAN_FEATURES);
> +
> +#ifdef CONFIG_XFRM_OFFLOAD
> + xfrm_features = netdev_increment_features(xfrm_features,
> +
> lower_dev->hw_enc_features,
> +
> VIRTUAL_DEV_XFRM_FEATURES);
> +#endif
> +
> + enc_features = netdev_increment_features(enc_features,
> +
> lower_dev->hw_enc_features,
> +
> VIRTUAL_DEV_ENC_FEATURES);
> +
> + mpls_features = netdev_increment_features(mpls_features,
> +
> lower_dev->mpls_features,
> +
> VIRTUAL_DEV_MPLS_FEATURES);
> +
> + dst_release_flag &= lower_dev->priv_flags;
> + if (lower_dev->hard_header_len > max_hard_header_len)
> + max_hard_header_len = lower_dev->hard_header_len;
> +
> + tso_max_size = min(tso_max_size, lower_dev->tso_max_size);
> + tso_max_segs = min(tso_max_segs, lower_dev->tso_max_segs);
> + }
> +
> + dev->gso_partial_features = gso_partial_features;
> + dev->hard_header_len = max_hard_header_len;
> + dev->vlan_features = vlan_features;
> + dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
> + NETIF_F_HW_VLAN_CTAG_TX |
> + NETIF_F_HW_VLAN_STAG_TX;
> +#ifdef CONFIG_XFRM_OFFLOAD
> + dev->hw_enc_features |= xfrm_features;
> +#endif
> + dev->mpls_features = mpls_features;
> + netif_set_tso_max_segs(dev, tso_max_segs);
> + netif_set_tso_max_size(dev, tso_max_size);
> +
> + dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
> + if ((dev->priv_flags & IFF_XMIT_DST_RELEASE_PERM) &&
> + dst_release_flag == (IFF_XMIT_DST_RELEASE |
> IFF_XMIT_DST_RELEASE_PERM))
> + dev->priv_flags |= IFF_XMIT_DST_RELEASE;
> +
> + netdev_change_features(dev);
> +}
> +EXPORT_SYMBOL(netdev_compute_features_from_lowers);
> +
> static struct hlist_head * __net_init netdev_create_hash(void)
> {
> int i;
> --
> 2.50.1
>
