Some high-level virtual drivers need to compute features from their lower devices, but each currently has its own implementation and may miss some feature computations. This patch set introduces a common function to compute features for such devices.
Currently, bonding, team, and bridge have been updated to use the new helper.
Hangbin Liu (5): net: add a common function to compute features from lowers devices bonding: use common function to compute the features team: use common function to compute the features net: bridge: use common function to compute the features selftests/net: add offload checking test for virtual interface
drivers/net/bonding/bond_main.c | 99 +---------- drivers/net/team/team_core.c | 73 +------- include/linux/netdevice.h | 19 +++ net/bridge/br_if.c | 22 +-- net/core/dev.c | 79 +++++++++ tools/testing/selftests/net/config | 2 + tools/testing/selftests/net/vdev_offload.sh | 174 ++++++++++++++++++++ 7 files changed, 285 insertions(+), 183 deletions(-) create mode 100755 tools/testing/selftests/net/vdev_offload.sh
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 pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- 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; +#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; + 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;
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 pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
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
On Sun, Aug 31, 2025 at 06:35:49PM +0300, Ido Schimmel wrote:
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.
Wow, thanks a lot for the long history changes..
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.
OK, I will get the highest needed_headroom for each slave and set the minimal one to upper device.
Regards Hangbin
2025-08-31, 18:35:49 +0300, Ido Schimmel wrote:
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 pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
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;
Going back to this discussion about hard_header_len:
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.
I'm not convinced removing adapting hard_header_len on bond/team is correct, even with old and broken drivers getting fixed years ago. hard_header_len will be used on the TX path (for some devices like bridge/macvlan via dev_forward_skb() and similar helpers, for IP tunnels setting their MTU, and via LL_RESERVED_SPACE).
So I think we should keep setting hard_header_len to the largest of all lowers.
On Wed, Sep 10, 2025 at 04:29:35PM +0200, Sabrina Dubroca wrote:
2025-08-31, 18:35:49 +0300, Ido Schimmel wrote:
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 pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
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;
Going back to this discussion about hard_header_len:
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.
I'm not convinced removing adapting hard_header_len on bond/team is correct, even with old and broken drivers getting fixed years ago. hard_header_len will be used on the TX path (for some devices like bridge/macvlan via dev_forward_skb() and similar helpers, for IP tunnels setting their MTU, and via LL_RESERVED_SPACE).
So I think we should keep setting hard_header_len to the largest of all lowers.
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? On the other hand, needed_headroom can and does vary between devices of the same type.
2025-09-10, 20:08:03 +0300, Ido Schimmel wrote:
On Wed, Sep 10, 2025 at 04:29:35PM +0200, Sabrina Dubroca wrote:
2025-08-31, 18:35:49 +0300, Ido Schimmel wrote:
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 pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
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;
Going back to this discussion about hard_header_len:
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.
I'm not convinced removing adapting hard_header_len on bond/team is correct, even with old and broken drivers getting fixed years ago. hard_header_len will be used on the TX path (for some devices like bridge/macvlan via dev_forward_skb() and similar helpers, for IP tunnels setting their MTU, and via LL_RESERVED_SPACE).
So I think we should keep setting hard_header_len to the largest of all lowers.
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;
On the other hand, needed_headroom can and does vary between devices of the same type.
I'm not saying anything about needed_headroom. It sounds like it should be updated as well.
On Wed, Sep 10, 2025 at 07:41:47PM +0200, Sabrina Dubroca wrote:
2025-09-10, 20:08:03 +0300, Ido Schimmel wrote:
On Wed, Sep 10, 2025 at 04:29:35PM +0200, Sabrina Dubroca wrote:
2025-08-31, 18:35:49 +0300, Ido Schimmel wrote:
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 pabeni@redhat.com Signed-off-by: Hangbin Liu liuhangbin@gmail.com
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;
Going back to this discussion about hard_header_len:
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.
I'm not convinced removing adapting hard_header_len on bond/team is correct, even with old and broken drivers getting fixed years ago. hard_header_len will be used on the TX path (for some devices like bridge/macvlan via dev_forward_skb() and similar helpers, for IP tunnels setting their MTU, and via LL_RESERVED_SPACE).
So I think we should keep setting hard_header_len to the largest of all lowers.
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.
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
Use the new functon netdev_compute_features_from_lowers() to compute the bonding features.
Note that bond_compute_features() currently uses bond_for_each_slave() to traverse the lower devices list, and that is just a macro wrapper of netdev_for_each_lower_private(). We use similar helper netdev_for_each_lower_dev() in netdev_compute_features_from_lowers() to iterate the slave device, as there is not need to get the private data.
No functional change intended.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- drivers/net/bonding/bond_main.c | 99 ++------------------------------- 1 file changed, 4 insertions(+), 95 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 257333c88710..ec098a52ef75 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1540,97 +1540,6 @@ static netdev_features_t bond_fix_features(struct net_device *dev, return features; }
-#define BOND_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 BOND_ENC_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \ - NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE | \ - NETIF_F_GSO_PARTIAL) - -#define BOND_MPLS_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \ - NETIF_F_GSO_SOFTWARE) - -#define BOND_GSO_PARTIAL_FEATURES (NETIF_F_GSO_ESP) - - -static void bond_compute_features(struct bonding *bond) -{ - netdev_features_t gso_partial_features = BOND_GSO_PARTIAL_FEATURES; - unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE | - IFF_XMIT_DST_RELEASE_PERM; - netdev_features_t vlan_features = BOND_VLAN_FEATURES; - netdev_features_t enc_features = BOND_ENC_FEATURES; -#ifdef CONFIG_XFRM_OFFLOAD - netdev_features_t xfrm_features = BOND_XFRM_FEATURES; -#endif /* CONFIG_XFRM_OFFLOAD */ - netdev_features_t mpls_features = BOND_MPLS_FEATURES; - struct net_device *bond_dev = bond->dev; - struct list_head *iter; - struct slave *slave; - unsigned short max_hard_header_len = ETH_HLEN; - unsigned int tso_max_size = TSO_MAX_SIZE; - u16 tso_max_segs = TSO_MAX_SEGS; - - if (!bond_has_slaves(bond)) - goto done; - - vlan_features = netdev_base_features(vlan_features); - mpls_features = netdev_base_features(mpls_features); - - bond_for_each_slave(bond, slave, iter) { - vlan_features = netdev_increment_features(vlan_features, - slave->dev->vlan_features, BOND_VLAN_FEATURES); - - enc_features = netdev_increment_features(enc_features, - slave->dev->hw_enc_features, - BOND_ENC_FEATURES); - -#ifdef CONFIG_XFRM_OFFLOAD - xfrm_features = netdev_increment_features(xfrm_features, - slave->dev->hw_enc_features, - BOND_XFRM_FEATURES); -#endif /* CONFIG_XFRM_OFFLOAD */ - - gso_partial_features = netdev_increment_features(gso_partial_features, - slave->dev->gso_partial_features, - BOND_GSO_PARTIAL_FEATURES); - - mpls_features = netdev_increment_features(mpls_features, - slave->dev->mpls_features, - BOND_MPLS_FEATURES); - - dst_release_flag &= slave->dev->priv_flags; - if (slave->dev->hard_header_len > max_hard_header_len) - max_hard_header_len = slave->dev->hard_header_len; - - tso_max_size = min(tso_max_size, slave->dev->tso_max_size); - tso_max_segs = min(tso_max_segs, slave->dev->tso_max_segs); - } - bond_dev->hard_header_len = max_hard_header_len; - -done: - bond_dev->gso_partial_features = gso_partial_features; - bond_dev->vlan_features = vlan_features; - bond_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 - bond_dev->hw_enc_features |= xfrm_features; -#endif /* CONFIG_XFRM_OFFLOAD */ - bond_dev->mpls_features = mpls_features; - netif_set_tso_max_segs(bond_dev, tso_max_segs); - netif_set_tso_max_size(bond_dev, tso_max_size); - - bond_dev->priv_flags &= ~IFF_XMIT_DST_RELEASE; - if ((bond_dev->priv_flags & IFF_XMIT_DST_RELEASE_PERM) && - dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM)) - bond_dev->priv_flags |= IFF_XMIT_DST_RELEASE; - - netdev_change_features(bond_dev); -} - static void bond_setup_by_slave(struct net_device *bond_dev, struct net_device *slave_dev) { @@ -2370,7 +2279,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, }
bond->slave_cnt++; - bond_compute_features(bond); + netdev_compute_features_from_lowers(bond->dev); bond_set_carrier(bond);
/* Needs to be called before bond_select_active_slave(), which will @@ -2622,7 +2531,7 @@ static int __bond_release_one(struct net_device *bond_dev, call_netdevice_notifiers(NETDEV_RELEASE, bond->dev); }
- bond_compute_features(bond); + netdev_compute_features_from_lowers(bond->dev); if (!(bond_dev->features & NETIF_F_VLAN_CHALLENGED) && (old_features & NETIF_F_VLAN_CHALLENGED)) slave_info(bond_dev, slave_dev, "last VLAN challenged slave left bond - VLAN blocking is removed\n"); @@ -4126,7 +4035,7 @@ static int bond_slave_netdev_event(unsigned long event, case NETDEV_FEAT_CHANGE: if (!bond->notifier_ctx) { bond->notifier_ctx = true; - bond_compute_features(bond); + netdev_compute_features_from_lowers(bond->dev); bond->notifier_ctx = false; } break; @@ -6109,7 +6018,7 @@ void bond_setup(struct net_device *bond_dev) * capable */
- bond_dev->hw_features = BOND_VLAN_FEATURES | + bond_dev->hw_features = VIRTUAL_DEV_VLAN_FEATURES | NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_RX |
Use the new helper netdev_compute_features_from_lowers() to compute the team device features. This helper performs both the feature computation and the netdev_change_features() call.
Note that such change replace the lower layer traversing currently done using team->port_list with netdev_for_each_lower_dev(). Such change is safe as `port_list` contains exactly the same elements as `team->dev->adj_list.lower` and the helper is always invoked under the RTNL lock.
With this change, the explicit netdev_change_features() in team_add_slave() can be safely removed, as team_port_add() already takes care of the notification via netdev_compute_features_from_lowers(), and same thing for team_del_slave()
This also fixes missing computations for MPLS, XFRM, and TSO/GSO partial features.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- drivers/net/team/team_core.c | 73 ++---------------------------------- 1 file changed, 4 insertions(+), 69 deletions(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c index 17f07eb0ee52..018d876e140a 100644 --- a/drivers/net/team/team_core.c +++ b/drivers/net/team/team_core.c @@ -982,63 +982,6 @@ static void team_port_disable(struct team *team, team_lower_state_changed(port); }
-#define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \ - NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \ - NETIF_F_HIGHDMA | NETIF_F_LRO | \ - NETIF_F_GSO_ENCAP_ALL) - -#define TEAM_ENC_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \ - NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE) - -static void __team_compute_features(struct team *team) -{ - struct team_port *port; - netdev_features_t vlan_features = TEAM_VLAN_FEATURES; - netdev_features_t enc_features = TEAM_ENC_FEATURES; - unsigned short max_hard_header_len = ETH_HLEN; - unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE | - IFF_XMIT_DST_RELEASE_PERM; - - rcu_read_lock(); - if (list_empty(&team->port_list)) - goto done; - - vlan_features = netdev_base_features(vlan_features); - enc_features = netdev_base_features(enc_features); - - list_for_each_entry_rcu(port, &team->port_list, list) { - vlan_features = netdev_increment_features(vlan_features, - port->dev->vlan_features, - TEAM_VLAN_FEATURES); - enc_features = - netdev_increment_features(enc_features, - port->dev->hw_enc_features, - TEAM_ENC_FEATURES); - - dst_release_flag &= port->dev->priv_flags; - if (port->dev->hard_header_len > max_hard_header_len) - max_hard_header_len = port->dev->hard_header_len; - } -done: - rcu_read_unlock(); - - team->dev->vlan_features = vlan_features; - team->dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL | - NETIF_F_HW_VLAN_CTAG_TX | - NETIF_F_HW_VLAN_STAG_TX; - team->dev->hard_header_len = max_hard_header_len; - - team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE; - if (dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM)) - team->dev->priv_flags |= IFF_XMIT_DST_RELEASE; -} - -static void team_compute_features(struct team *team) -{ - __team_compute_features(team); - netdev_change_features(team->dev); -} - static int team_port_enter(struct team *team, struct team_port *port) { int err = 0; @@ -1300,7 +1243,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev, port->index = -1; list_add_tail_rcu(&port->list, &team->port_list); team_port_enable(team, port); - __team_compute_features(team); + netdev_compute_features_from_lowers(team->dev); __team_port_change_port_added(port, !!netif_oper_up(port_dev)); __team_options_change_check(team);
@@ -1382,7 +1325,7 @@ static int team_port_del(struct team *team, struct net_device *port_dev) dev_set_mtu(port_dev, port->orig.mtu); kfree_rcu(port, rcu); netdev_info(dev, "Port device %s removed\n", portname); - __team_compute_features(team); + netdev_compute_features_from_lowers(team->dev);
return 0; } @@ -1976,9 +1919,6 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
err = team_port_add(team, port_dev, extack);
- if (!err) - netdev_change_features(dev); - return err; }
@@ -1991,11 +1931,6 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
err = team_port_del(team, port_dev);
- if (err) - return err; - - netdev_change_features(dev); - return err; }
@@ -2190,7 +2125,7 @@ static void team_setup(struct net_device *dev)
dev->features |= NETIF_F_GRO;
- dev->hw_features = TEAM_VLAN_FEATURES | + dev->hw_features = VIRTUAL_DEV_VLAN_FEATURES | NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_RX | @@ -2994,7 +2929,7 @@ static int team_device_event(struct notifier_block *unused, case NETDEV_FEAT_CHANGE: if (!port->team->notifier_ctx) { port->team->notifier_ctx = true; - team_compute_features(port->team); + netdev_compute_features_from_lowers(port->team->dev); port->team->notifier_ctx = false; } break;
On 08/29, Hangbin Liu wrote:
Use the new helper netdev_compute_features_from_lowers() to compute the team device features. This helper performs both the feature computation and the netdev_change_features() call.
Note that such change replace the lower layer traversing currently done using team->port_list with netdev_for_each_lower_dev(). Such change is safe as `port_list` contains exactly the same elements as `team->dev->adj_list.lower` and the helper is always invoked under the RTNL lock.
With this change, the explicit netdev_change_features() in team_add_slave() can be safely removed, as team_port_add() already takes care of the notification via netdev_compute_features_from_lowers(), and same thing for team_del_slave()
This also fixes missing computations for MPLS, XFRM, and TSO/GSO partial features.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
drivers/net/team/team_core.c | 73 ++---------------------------------- 1 file changed, 4 insertions(+), 69 deletions(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c index 17f07eb0ee52..018d876e140a 100644 --- a/drivers/net/team/team_core.c +++ b/drivers/net/team/team_core.c @@ -982,63 +982,6 @@ static void team_port_disable(struct team *team, team_lower_state_changed(port); } -#define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \NETIF_F_HIGHDMA | NETIF_F_LRO | \NETIF_F_GSO_ENCAP_ALL)-#define TEAM_ENC_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)-static void __team_compute_features(struct team *team) -{
- struct team_port *port;
- netdev_features_t vlan_features = TEAM_VLAN_FEATURES;
- netdev_features_t enc_features = TEAM_ENC_FEATURES;
- unsigned short max_hard_header_len = ETH_HLEN;
- unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
IFF_XMIT_DST_RELEASE_PERM;- rcu_read_lock();
- if (list_empty(&team->port_list))
goto done;- vlan_features = netdev_base_features(vlan_features);
- enc_features = netdev_base_features(enc_features);
- list_for_each_entry_rcu(port, &team->port_list, list) {
vlan_features = netdev_increment_features(vlan_features,port->dev->vlan_features,TEAM_VLAN_FEATURES);enc_features =netdev_increment_features(enc_features,port->dev->hw_enc_features,TEAM_ENC_FEATURES);dst_release_flag &= port->dev->priv_flags;if (port->dev->hard_header_len > max_hard_header_len)max_hard_header_len = port->dev->hard_header_len;- }
-done:
- rcu_read_unlock();
- team->dev->vlan_features = vlan_features;
- team->dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
NETIF_F_HW_VLAN_CTAG_TX |NETIF_F_HW_VLAN_STAG_TX;- team->dev->hard_header_len = max_hard_header_len;
- team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
- if (dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM))
team->dev->priv_flags |= IFF_XMIT_DST_RELEASE;-}
-static void team_compute_features(struct team *team) -{
- __team_compute_features(team);
- netdev_change_features(team->dev);
-}
static int team_port_enter(struct team *team, struct team_port *port) { int err = 0; @@ -1300,7 +1243,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev, port->index = -1; list_add_tail_rcu(&port->list, &team->port_list); team_port_enable(team, port);
- __team_compute_features(team);
- netdev_compute_features_from_lowers(team->dev); __team_port_change_port_added(port, !!netif_oper_up(port_dev)); __team_options_change_check(team);
@@ -1382,7 +1325,7 @@ static int team_port_del(struct team *team, struct net_device *port_dev) dev_set_mtu(port_dev, port->orig.mtu); kfree_rcu(port, rcu); netdev_info(dev, "Port device %s removed\n", portname);
- __team_compute_features(team);
- netdev_compute_features_from_lowers(team->dev);
return 0; }
[..]
@@ -1976,9 +1919,6 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev, err = team_port_add(team, port_dev, extack);
- if (!err)
netdev_change_features(dev);- return err;
} @@ -1991,11 +1931,6 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev) err = team_port_del(team, port_dev);
- if (err)
return err;- netdev_change_features(dev);
- return err;
}
nit: change these to 'return team_port_xxx()' ? Can drop the 'err' as well, don't think it's needed anymore?
Previously, bridge ignored all features propagation and DST retention, only handling explicitly the GSO limits.
By switching to the new helper netdev_compute_features_from_lowers(), the bridge now expose additional features, depending on the lowers capabilities.
Since br_set_gso_limits() is already covered by the helper, it can be removed safely.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- net/bridge/br_if.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 98c5b9c3145f..8fe44e8c008c 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -525,20 +525,6 @@ void br_mtu_auto_adjust(struct net_bridge *br) br_opt_toggle(br, BROPT_MTU_SET_BY_USER, false); }
-static void br_set_gso_limits(struct net_bridge *br) -{ - unsigned int tso_max_size = TSO_MAX_SIZE; - const struct net_bridge_port *p; - u16 tso_max_segs = TSO_MAX_SEGS; - - list_for_each_entry(p, &br->port_list, list) { - tso_max_size = min(tso_max_size, p->dev->tso_max_size); - tso_max_segs = min(tso_max_segs, p->dev->tso_max_segs); - } - netif_set_tso_max_size(br->dev, tso_max_size); - netif_set_tso_max_segs(br->dev, tso_max_segs); -} - /* * Recomputes features using slave's features */ @@ -652,8 +638,6 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, netdev_err(dev, "failed to sync bridge static fdb addresses to this port\n"); }
- netdev_update_features(br->dev); - br_hr = br->dev->needed_headroom; dev_hr = netdev_get_fwd_headroom(dev); if (br_hr < dev_hr) @@ -694,7 +678,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
br_mtu_auto_adjust(br); - br_set_gso_limits(br); + + netdev_compute_features_from_lowers(br->dev);
kobject_uevent(&p->kobj, KOBJ_ADD);
@@ -740,7 +725,6 @@ int br_del_if(struct net_bridge *br, struct net_device *dev) del_nbp(p);
br_mtu_auto_adjust(br); - br_set_gso_limits(br);
spin_lock_bh(&br->lock); changed_addr = br_stp_recalculate_bridge_id(br); @@ -749,7 +733,7 @@ int br_del_if(struct net_bridge *br, struct net_device *dev) if (changed_addr) call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
- netdev_update_features(br->dev); + netdev_compute_features_from_lowers(br->dev);
return 0; }
make sure the virtual interface offload setting is correct after changing lower devices.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- tools/testing/selftests/net/config | 2 + tools/testing/selftests/net/vdev_offload.sh | 174 ++++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100755 tools/testing/selftests/net/vdev_offload.sh
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config index d548611e2698..0f3a64a86474 100644 --- a/tools/testing/selftests/net/config +++ b/tools/testing/selftests/net/config @@ -117,6 +117,7 @@ CONFIG_IP_SCTP=m CONFIG_NETFILTER_XT_MATCH_POLICY=m CONFIG_CRYPTO_ARIA=y CONFIG_XFRM_INTERFACE=m +CONFIG_XFRM_OFFLOAD=y CONFIG_XFRM_USER=m CONFIG_IP_NF_MATCH_RPFILTER=m CONFIG_IP6_NF_MATCH_RPFILTER=m @@ -128,3 +129,4 @@ CONFIG_NETKIT=y CONFIG_NET_PKTGEN=m CONFIG_IPV6_ILA=m CONFIG_IPV6_RPL_LWTUNNEL=y +CONFIG_NET_TEAM=m diff --git a/tools/testing/selftests/net/vdev_offload.sh b/tools/testing/selftests/net/vdev_offload.sh new file mode 100755 index 000000000000..4926774aef19 --- /dev/null +++ b/tools/testing/selftests/net/vdev_offload.sh @@ -0,0 +1,174 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# shellcheck disable=SC1091 +source lib.sh + +# Set related offload on lower deivces and check if upper devices re-compute +# Some fatures are fixed on veth interface. Just list here in case we have a +# better way to test in future. +set_offload() +{ + local dev="$1" + local state="$2" + + # VLAN features + # NETIF_F_FRAGLIST: tx-scatter-gather-fraglist + # shellcheck disable=SC2154 + ip netns exec "$ns" ethtool -K "$dev" tx-scatter-gather-fraglist "$state" + + # ENC features + # NETIF_F_RXCSUM: rx-checksum (bond/team/bridge fixed) + + # XFRM features (veth fixed, netdevsim supports) + # NETIF_F_HW_ESP: esp-hw-offload + # NETIF_F_GSO_ESP: tx-esp-segmentation + + # GSO partial features + # NETIF_F_GSO_PARTIAL: tx-gso-partial (veth/bond fixed) + + # Common features + # NETIF_F_SG: tx-scatter-gather + ip netns exec "$ns" ethtool -K "$dev" tx-scatter-gather "$state" &> /dev/null + # NETIF_F_GSO_SOFTWARE: NETIF_F_GSO_ACCECN: tx-tcp-accecn-segmentation + ip netns exec "$ns" ethtool -K "$dev" tx-tcp-accecn-segmentation "$state" + # NETIF_F_GSO_SOFTWARE: NETIF_F_GSO_SCTP: tx-sctp-segmentation + ip netns exec "$ns" ethtool -K "$dev" tx-sctp-segmentation "$state" + # NETIF_F_GSO_SOFTWARE: NETIF_F_GSO_FRAGLIST: tx-gso-list + ip netns exec "$ns" ethtool -K "$dev" tx-gso-list "$state" +} + +__check_offload() +{ + local dev=$1 + local opt=$2 + local expect=$3 + + ip netns exec "$ns" ethtool --json -k "$dev" | \ + jq -r -e ".[]."$opt".active == ${expect}" >/dev/null +} + +check_offload() +{ + local dev=$1 + local state=$2 + RET=0 + + __check_offload "$dev" "tx-scatter-gather-fraglist" "$state" || RET=1 + __check_offload "$dev" "tx-scatter-gather" "$state" || RET=1 + __check_offload "$dev" "tx-tcp-accecn-segmentation" "$state" || RET=1 + __check_offload "$dev" "tx-sctp-segmentation" "$state" || RET=1 + __check_offload "$dev" "tx-gso-list" "$state" || RET=1 +} + +setup_veth() +{ + # Set up test netns + setup_ns ns switch + + # shellcheck disable=SC2154 + ip -n "$ns" link add veth0 type veth peer name veth0 netns "$switch" + ip -n "$ns" link add veth1 type veth peer name veth1 netns "$switch" + ip -n "$switch" link set veth0 up + ip -n "$switch" link set veth1 up + + link_0=veth0 + link_1=veth1 +} + +setup_netdevsim() +{ + setup_ns ns + # The create_netdevsim() function will set the interface up. Later, + # when it is added to bonded, we need to set it down first. And when + # set down, it will have no carrier. So we need to add netdevsim ourselves. + modprobe netdevsim + udevadm settle + echo "0 2" | ip netns exec "$ns" tee /sys/bus/netdevsim/new_device >/dev/null + link_0=$(ip netns exec "$ns" ls /sys/bus/netdevsim/devices/netdevsim0/net | head -n 1) + link_1=$(ip netns exec "$ns" ls /sys/bus/netdevsim/devices/netdevsim0/net | tail -n 1) +} + +cleanup() +{ + cleanup_netdevsim 0 + cleanup_all_ns +} + +setup_bond() +{ + ip -n "$ns" link set "$link_0" nomaster + ip -n "$ns" link set "$link_1" nomaster + ip -n "$ns" link add bond0 type bond mode active-backup miimon 100 + ip -n "$ns" link set "$link_0" master bond0 + ip -n "$ns" link set "$link_1" master bond0 + ip -n "$ns" link set bond0 up +} + +setup_team() +{ + ip -n "$ns" link set "$link_0" nomaster + ip -n "$ns" link set "$link_1" nomaster + ip -n "$ns" link add team0 type team + ip -n "$ns" link set "$link_0" master team0 + ip -n "$ns" link set "$link_1" master team0 + ip -n "$ns" link set team0 up +} + +setup_bridge() +{ + ip -n "$ns" link set "$link_0" nomaster + ip -n "$ns" link set "$link_1" nomaster + ip -n "$ns" link add br0 type bridge + ip -n "$ns" link set "$link_0" master br0 + ip -n "$ns" link set "$link_1" master br0 + ip -n "$ns" link set br0 up +} + +check_xfrm() +{ + local dev=$1 + local src=192.0.2.1 + local dst=192.0.2.2 + local key="0x3132333435363738393031323334353664636261" + + RET=0 + + ip -n "$ns" xfrm state flush + ip -n "$ns" xfrm state add proto esp src "$src" dst "$dst" spi 9 \ + mode transport reqid 42 aead "rfc4106(gcm(aes))" "$key" 128 \ + sel src "$src"/24 dst "$dst"/24 offload dev "$dev" dir out + + # shellcheck disable=SC2034 + ip -n "$ns" xfrm state list | grep -q "crypto offload parameters: dev $dev dir" || RET=1 + log_test "$dev" "xfrm offload" +} + +do_test() +{ + local dev=$1 + set_offload veth0 "on" + set_offload veth1 "on" + check_offload "$dev" "true" + log_test "$dev" "enable offload" + + set_offload veth0 "off" + set_offload veth1 "off" + check_offload "$dev" "false" + log_test "$dev" "disable offload" +} + +trap cleanup EXIT +setup_veth +setup_bond +do_test bond0 +setup_team +do_test team0 +setup_bridge +do_test br0 + +# Check NETIF_F_HW_ESP +# Only test bond as team and bridge haven't implemented xfrm offload +setup_netdevsim +setup_bond +check_xfrm bond0
On Fri, Aug 29, 2025 at 09:54:30AM +0000, Hangbin Liu wrote:
make sure the virtual interface offload setting is correct after changing lower devices.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
tools/testing/selftests/net/config | 2 + tools/testing/selftests/net/vdev_offload.sh | 174 ++++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100755 tools/testing/selftests/net/vdev_offload.sh
Need to add to the Makefile
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config index d548611e2698..0f3a64a86474 100644 --- a/tools/testing/selftests/net/config +++ b/tools/testing/selftests/net/config @@ -117,6 +117,7 @@ CONFIG_IP_SCTP=m CONFIG_NETFILTER_XT_MATCH_POLICY=m CONFIG_CRYPTO_ARIA=y CONFIG_XFRM_INTERFACE=m +CONFIG_XFRM_OFFLOAD=y CONFIG_XFRM_USER=m CONFIG_IP_NF_MATCH_RPFILTER=m CONFIG_IP6_NF_MATCH_RPFILTER=m @@ -128,3 +129,4 @@ CONFIG_NETKIT=y CONFIG_NET_PKTGEN=m CONFIG_IPV6_ILA=m CONFIG_IPV6_RPL_LWTUNNEL=y +CONFIG_NET_TEAM=m diff --git a/tools/testing/selftests/net/vdev_offload.sh b/tools/testing/selftests/net/vdev_offload.sh new file mode 100755 index 000000000000..4926774aef19 --- /dev/null +++ b/tools/testing/selftests/net/vdev_offload.sh @@ -0,0 +1,174 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0
+# shellcheck disable=SC1091 +source lib.sh
+# Set related offload on lower deivces and check if upper devices re-compute +# Some fatures are fixed on veth interface. Just list here in case we have a
s/fatures/features/ (:set spell)
+# better way to test in future. +set_offload() +{
- local dev="$1"
- local state="$2"
- # VLAN features
- # NETIF_F_FRAGLIST: tx-scatter-gather-fraglist
- # shellcheck disable=SC2154
- ip netns exec "$ns" ethtool -K "$dev" tx-scatter-gather-fraglist "$state"
- # ENC features
- # NETIF_F_RXCSUM: rx-checksum (bond/team/bridge fixed)
- # XFRM features (veth fixed, netdevsim supports)
- # NETIF_F_HW_ESP: esp-hw-offload
- # NETIF_F_GSO_ESP: tx-esp-segmentation
- # GSO partial features
- # NETIF_F_GSO_PARTIAL: tx-gso-partial (veth/bond fixed)
- # Common features
- # NETIF_F_SG: tx-scatter-gather
- ip netns exec "$ns" ethtool -K "$dev" tx-scatter-gather "$state" &> /dev/null
Why the redirection here? I don't see it in other places
- # NETIF_F_GSO_SOFTWARE: NETIF_F_GSO_ACCECN: tx-tcp-accecn-segmentation
- ip netns exec "$ns" ethtool -K "$dev" tx-tcp-accecn-segmentation "$state"
- # NETIF_F_GSO_SOFTWARE: NETIF_F_GSO_SCTP: tx-sctp-segmentation
- ip netns exec "$ns" ethtool -K "$dev" tx-sctp-segmentation "$state"
- # NETIF_F_GSO_SOFTWARE: NETIF_F_GSO_FRAGLIST: tx-gso-list
- ip netns exec "$ns" ethtool -K "$dev" tx-gso-list "$state"
+}
+__check_offload() +{
- local dev=$1
- local opt=$2
- local expect=$3
- ip netns exec "$ns" ethtool --json -k "$dev" | \
jq -r -e ".[].\"$opt\".active == ${expect}" >/dev/null+}
+check_offload() +{
- local dev=$1
- local state=$2
- RET=0
- __check_offload "$dev" "tx-scatter-gather-fraglist" "$state" || RET=1
- __check_offload "$dev" "tx-scatter-gather" "$state" || RET=1
- __check_offload "$dev" "tx-tcp-accecn-segmentation" "$state" || RET=1
- __check_offload "$dev" "tx-sctp-segmentation" "$state" || RET=1
- __check_offload "$dev" "tx-gso-list" "$state" || RET=1
+}
+setup_veth() +{
- # Set up test netns
- setup_ns ns switch
- # shellcheck disable=SC2154
- ip -n "$ns" link add veth0 type veth peer name veth0 netns "$switch"
- ip -n "$ns" link add veth1 type veth peer name veth1 netns "$switch"
- ip -n "$switch" link set veth0 up
- ip -n "$switch" link set veth1 up
- link_0=veth0
- link_1=veth1
+}
+setup_netdevsim() +{
- setup_ns ns
- # The create_netdevsim() function will set the interface up. Later,
- # when it is added to bonded, we need to set it down first. And when
- # set down, it will have no carrier. So we need to add netdevsim ourselves.
- modprobe netdevsim
- udevadm settle
- echo "0 2" | ip netns exec "$ns" tee /sys/bus/netdevsim/new_device >/dev/null
- link_0=$(ip netns exec "$ns" ls /sys/bus/netdevsim/devices/netdevsim0/net | head -n 1)
- link_1=$(ip netns exec "$ns" ls /sys/bus/netdevsim/devices/netdevsim0/net | tail -n 1)
+}
+cleanup() +{
- cleanup_netdevsim 0
- cleanup_all_ns
+}
+setup_bond() +{
- ip -n "$ns" link set "$link_0" nomaster
- ip -n "$ns" link set "$link_1" nomaster
- ip -n "$ns" link add bond0 type bond mode active-backup miimon 100
- ip -n "$ns" link set "$link_0" master bond0
- ip -n "$ns" link set "$link_1" master bond0
- ip -n "$ns" link set bond0 up
+}
+setup_team() +{
- ip -n "$ns" link set "$link_0" nomaster
- ip -n "$ns" link set "$link_1" nomaster
- ip -n "$ns" link add team0 type team
- ip -n "$ns" link set "$link_0" master team0
- ip -n "$ns" link set "$link_1" master team0
- ip -n "$ns" link set team0 up
+}
+setup_bridge() +{
- ip -n "$ns" link set "$link_0" nomaster
- ip -n "$ns" link set "$link_1" nomaster
- ip -n "$ns" link add br0 type bridge
- ip -n "$ns" link set "$link_0" master br0
- ip -n "$ns" link set "$link_1" master br0
- ip -n "$ns" link set br0 up
+}
+check_xfrm() +{
- local dev=$1
- local src=192.0.2.1
- local dst=192.0.2.2
- local key="0x3132333435363738393031323334353664636261"
- RET=0
- ip -n "$ns" xfrm state flush
- ip -n "$ns" xfrm state add proto esp src "$src" dst "$dst" spi 9 \
mode transport reqid 42 aead "rfc4106(gcm(aes))" "$key" 128 \sel src "$src"/24 dst "$dst"/24 offload dev "$dev" dir out- # shellcheck disable=SC2034
- ip -n "$ns" xfrm state list | grep -q "crypto offload parameters: dev $dev dir" || RET=1
- log_test "$dev" "xfrm offload"
+}
+do_test() +{
- local dev=$1
IMO, it makes more sense to put "RET=0" in the same function that calls log_test() (like you have it in check_xfrm()), so I would put it here...
- set_offload veth0 "on"
- set_offload veth1 "on"
- check_offload "$dev" "true"
- log_test "$dev" "enable offload"
... and here (instead of in check_offload())
- set_offload veth0 "off"
- set_offload veth1 "off"
- check_offload "$dev" "false"
- log_test "$dev" "disable offload"
+}
+trap cleanup EXIT +setup_veth +setup_bond +do_test bond0 +setup_team +do_test team0 +setup_bridge +do_test br0
+# Check NETIF_F_HW_ESP +# Only test bond as team and bridge haven't implemented xfrm offload +setup_netdevsim +setup_bond
+check_xfrm bond0
2.50.1
On Sun, Aug 31, 2025 at 06:52:01PM +0300, Ido Schimmel wrote:
On Fri, Aug 29, 2025 at 09:54:30AM +0000, Hangbin Liu wrote:
make sure the virtual interface offload setting is correct after changing lower devices.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
tools/testing/selftests/net/config | 2 + tools/testing/selftests/net/vdev_offload.sh | 174 ++++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100755 tools/testing/selftests/net/vdev_offload.sh
Need to add to the Makefile
Sigh, I already added it to Makefile in my local branch, but forgot to re-format-patch. Thanks for catching it.
- # Common features
- # NETIF_F_SG: tx-scatter-gather
- ip netns exec "$ns" ethtool -K "$dev" tx-scatter-gather "$state" &> /dev/null
Why the redirection here? I don't see it in other places
When I tested local, I got message like
Actual changes: tx-scatter-gather: on tx-generic-segmentation: on [not requested] tx-tcp-segmentation: on [not requested] tx-tcp-ecn-segmentation: on [not requested] tx-tcp-mangleid-segmentation: on [not requested] tx-tcp6-segmentation: on [not requested]
So I redirected the log.
+do_test() +{
- local dev=$1
IMO, it makes more sense to put "RET=0" in the same function that calls log_test() (like you have it in check_xfrm()), so I would put it here...
- set_offload veth0 "on"
- set_offload veth1 "on"
- check_offload "$dev" "true"
- log_test "$dev" "enable offload"
... and here (instead of in check_offload())
OK, I will
Thanks Hangbin
linux-kselftest-mirror@lists.linaro.org
-
Hangbin Liu -
Ido Schimmel -
Sabrina Dubroca -
Stanislav Fomichev