| Message ID | 20240816075915.6245-1-nbd@nbd.name (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC] net: remove NETIF_F_GSO_FRAGLIST from NETIF_F_GSO_SOFTWARE | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Felix Fietkau wrote: > Several drivers set NETIF_F_GSO_SOFTWARE, but mangle fraglist GRO packets > in a way that they can't be properly segmented anymore. Can you share a bit more concrete detail: which driver, for instance, and how does it mangle the packet? I assume something with inserting or deleting tunnel headers. But the fraglist skbs should be able to reproduce the modified header in skb_segment_list? > In order to properly deal with this, remove fraglist GSO from > NETIF_F_GSO_SOFTWARE and switch to NETIF_F_GSO_SOFTWARE_ALL (which includes > fraglist GSO) in places where it's safe to add. > > Signed-off-by: Felix Fietkau <nbd@nbd.name>
On 16.08.24 16:13, Willem de Bruijn wrote: > Felix Fietkau wrote: >> Several drivers set NETIF_F_GSO_SOFTWARE, but mangle fraglist GRO packets >> in a way that they can't be properly segmented anymore. > > Can you share a bit more concrete detail: which driver, for instance, and > how does it mangle the packet? > > I assume something with inserting or deleting tunnel headers. But the > fraglist skbs should be able to reproduce the modified header in > skb_segment_list? I've received bug reports from a variety of people, often with very little context, usually crashing while segmenting fraglist skbs. The ones that people were able to easily reproduce involved bridging with vxlan or virtio net devices. People using those two reported that my patch fixed their crashes. I will try to gather more information about the root cause of the crashes. - Felix
From: Felix Fietkau <nbd@nbd.name> Date: Fri, 16 Aug 2024 09:59:15 +0200 Please use scripts/get_maintainer.pl next time. I remember it was me who added fraglist to GSO_SOFTWARE, but I'm not Cced =\ > Several drivers set NETIF_F_GSO_SOFTWARE, but mangle fraglist GRO packets Which ones precisely? > in a way that they can't be properly segmented anymore. > In order to properly deal with this, remove fraglist GSO from > NETIF_F_GSO_SOFTWARE and switch to NETIF_F_GSO_SOFTWARE_ALL (which includes > fraglist GSO) in places where it's safe to add. Why isn't bridge changed? It's only a software layer, I don't think it break fraglist skbs anyhow. > > Signed-off-by: Felix Fietkau <nbd@nbd.name> > --- > drivers/net/dummy.c | 2 +- > drivers/net/loopback.c | 2 +- > drivers/net/macvlan.c | 2 +- > include/linux/netdev_features.h | 5 +++-- > net/8021q/vlan.h | 2 +- > net/8021q/vlan_dev.c | 4 ++-- > net/core/sock.c | 2 +- > net/mac80211/ieee80211_i.h | 2 +- > net/openvswitch/vport-internal_dev.c | 2 +- > 9 files changed, 12 insertions(+), 11 deletions(-) Thanks, Olek
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c index d29b5d7af0d7..e2d0837c953c 100644 --- a/drivers/net/dummy.c +++ b/drivers/net/dummy.c @@ -110,7 +110,7 @@ static void dummy_setup(struct net_device *dev) dev->flags &= ~IFF_MULTICAST; dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE; dev->features |= NETIF_F_SG | NETIF_F_FRAGLIST; - dev->features |= NETIF_F_GSO_SOFTWARE; + dev->features |= NETIF_F_GSO_SOFTWARE_ALL; dev->features |= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX; dev->features |= NETIF_F_GSO_ENCAP_ALL; dev->hw_features |= dev->features; diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c index 2b486e7c749c..50cd6eb93eb7 100644 --- a/drivers/net/loopback.c +++ b/drivers/net/loopback.c @@ -172,7 +172,7 @@ static void gen_lo_setup(struct net_device *dev, dev->flags = IFF_LOOPBACK; dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE; netif_keep_dst(dev); - dev->hw_features = NETIF_F_GSO_SOFTWARE; + dev->hw_features = NETIF_F_GSO_SOFTWARE_ALL; dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | NETIF_F_HW_CSUM diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 24298a33e0e9..cd0b50199354 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -897,7 +897,7 @@ static int macvlan_hwtstamp_set(struct net_device *dev, static struct lock_class_key macvlan_netdev_addr_lock_key; #define ALWAYS_ON_OFFLOADS \ - (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \ + (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE_ALL | \ NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL) #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX) diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h index 7c2d77d75a88..8b6d7f532065 100644 --- a/include/linux/netdev_features.h +++ b/include/linux/netdev_features.h @@ -219,13 +219,14 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start) /* List of features with software fallbacks. */ #define NETIF_F_GSO_SOFTWARE (NETIF_F_ALL_TSO | NETIF_F_GSO_SCTP | \ - NETIF_F_GSO_UDP_L4 | NETIF_F_GSO_FRAGLIST) + NETIF_F_GSO_UDP_L4) +#define NETIF_F_GSO_SOFTWARE_ALL (NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_FRAGLIST) /* * If one device supports one of these features, then enable them * for all in netdev_increment_features. */ -#define NETIF_F_ONE_FOR_ALL (NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ROBUST | \ +#define NETIF_F_ONE_FOR_ALL (NETIF_F_GSO_SOFTWARE_ALL | NETIF_F_GSO_ROBUST | \ NETIF_F_SG | NETIF_F_HIGHDMA | \ NETIF_F_FRAGLIST | NETIF_F_VLAN_CHALLENGED) diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h index 5eaf38875554..4ce1e8cf6b2e 100644 --- a/net/8021q/vlan.h +++ b/net/8021q/vlan.h @@ -108,7 +108,7 @@ static inline netdev_features_t vlan_tnl_features(struct net_device *real_dev) netdev_features_t ret; ret = real_dev->hw_enc_features & - (NETIF_F_CSUM_MASK | NETIF_F_GSO_SOFTWARE | + (NETIF_F_CSUM_MASK | NETIF_F_GSO_SOFTWARE_ALL | NETIF_F_GSO_ENCAP_ALL); if ((ret & NETIF_F_GSO_ENCAP_ALL) && (ret & NETIF_F_CSUM_MASK)) diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index 217be32426b5..194fb71e8463 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -561,7 +561,7 @@ static int vlan_dev_init(struct net_device *dev) dev->state |= (1 << __LINK_STATE_NOCARRIER); dev->hw_features = NETIF_F_HW_CSUM | NETIF_F_SG | - NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | + NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE_ALL | NETIF_F_GSO_ENCAP_ALL | NETIF_F_HIGHDMA | NETIF_F_SCTP_CRC | NETIF_F_ALL_FCOE; @@ -654,7 +654,7 @@ static netdev_features_t vlan_dev_fix_features(struct net_device *dev, if (lower_features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM)) lower_features |= NETIF_F_HW_CSUM; features = netdev_intersect_features(features, lower_features); - features |= old_features & (NETIF_F_SOFT_FEATURES | NETIF_F_GSO_SOFTWARE); + features |= old_features & (NETIF_F_SOFT_FEATURES | NETIF_F_GSO_SOFTWARE_ALL); features |= NETIF_F_LLTX; return features; diff --git a/net/core/sock.c b/net/core/sock.c index 9abc4fe25953..1973e3092ed8 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -2451,7 +2451,7 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst) if (sk_is_tcp(sk)) sk->sk_route_caps |= NETIF_F_GSO; if (sk->sk_route_caps & NETIF_F_GSO) - sk->sk_route_caps |= NETIF_F_GSO_SOFTWARE; + sk->sk_route_caps |= NETIF_F_GSO_SOFTWARE_ALL; if (unlikely(sk->sk_gso_disabled)) sk->sk_route_caps &= ~NETIF_F_GSO_MASK; if (sk_can_gso(sk)) { diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index a3485e4c6132..8de4f8cabf3a 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -2009,7 +2009,7 @@ void ieee80211_color_collision_detection_work(struct work_struct *work); /* interface handling */ #define MAC80211_SUPPORTED_FEATURES_TX (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | \ NETIF_F_HW_CSUM | NETIF_F_SG | \ - NETIF_F_HIGHDMA | NETIF_F_GSO_SOFTWARE | \ + NETIF_F_HIGHDMA | NETIF_F_GSO_SOFTWARE_ALL | \ NETIF_F_HW_TC) #define MAC80211_SUPPORTED_FEATURES_RX (NETIF_F_RXCSUM) #define MAC80211_SUPPORTED_FEATURES (MAC80211_SUPPORTED_FEATURES_TX | \ diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c index 4b33133cbdff..6c4b231c3957 100644 --- a/net/openvswitch/vport-internal_dev.c +++ b/net/openvswitch/vport-internal_dev.c @@ -109,7 +109,7 @@ static void do_setup(struct net_device *netdev) netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | - NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ENCAP_ALL; + NETIF_F_GSO_SOFTWARE_ALL | NETIF_F_GSO_ENCAP_ALL; netdev->vlan_features = netdev->features; netdev->hw_enc_features = netdev->features;
Several drivers set NETIF_F_GSO_SOFTWARE, but mangle fraglist GRO packets in a way that they can't be properly segmented anymore. In order to properly deal with this, remove fraglist GSO from NETIF_F_GSO_SOFTWARE and switch to NETIF_F_GSO_SOFTWARE_ALL (which includes fraglist GSO) in places where it's safe to add. Signed-off-by: Felix Fietkau <nbd@nbd.name> --- drivers/net/dummy.c | 2 +- drivers/net/loopback.c | 2 +- drivers/net/macvlan.c | 2 +- include/linux/netdev_features.h | 5 +++-- net/8021q/vlan.h | 2 +- net/8021q/vlan_dev.c | 4 ++-- net/core/sock.c | 2 +- net/mac80211/ieee80211_i.h | 2 +- net/openvswitch/vport-internal_dev.c | 2 +- 9 files changed, 12 insertions(+), 11 deletions(-)