thr3ads.net - Virtualization - [PATCH 10/10] Revert "drivers/net: Disable UFO through virtio" [Dec 2014]

If this information is useful, please help other people find it:
Share via:

Vladislav Yasevich

2014-Dec-17 18:20 UTC

[PATCH 00/10] Split UFO into v4 and v6 versions.

UFO support in the kernel applies to both IPv4 and IPv6 protocols
with the same device feature.  However some devices may not be able
to support one of the offloads.  For this we split the UFO offload
feature into 2 pieces.  NETIF_F_UFO now controlls the IPv4 part and
this series introduces NETIF_F_UFO6.

As a result of this work, we can now re-enable NETIF_F_UFO on
virtio_net devices and restore UDP over IPv4 performance for guests.
We also continue to support legacy guests that assume that UFO6
support included into UFO(4).

Without this work, migrating a guest to a 3.18 kernel fails.

Vladislav Yasevich (10):
  core: Split out UFO6 support
  net:  Correctly mark IPv6 UFO offload type.
  ovs: Enable handling of UFO6 packets.
  loopback: Turn on UFO6 support.
  veth: Enable UFO6 support.
  macvlan: Enable UFO6 support.
  s2io: Enable UFO6 support.
  tun: Re-uanble UFO support.
  macvtap: Re-enable UFO support
  Revert "drivers/net: Disable UFO through virtio"

 drivers/net/ethernet/neterion/s2io.c |  6 +++---
 drivers/net/loopback.c               |  4 ++--
 drivers/net/macvlan.c                |  2 +-
 drivers/net/macvtap.c                | 20 ++++++++++++++------
 drivers/net/tun.c                    | 26 ++++++++++++++------------
 drivers/net/veth.c                   |  2 +-
 drivers/net/virtio_net.c             | 24 ++++++++++--------------
 include/linux/netdev_features.h      |  7 +++++--
 include/linux/netdevice.h            |  1 +
 include/linux/skbuff.h               |  1 +
 net/core/dev.c                       | 35 +++++++++++++++++++----------------
 net/core/ethtool.c                   |  2 +-
 net/ipv6/ip6_offload.c               |  1 +
 net/ipv6/ip6_output.c                |  4 ++--
 net/ipv6/udp_offload.c               |  3 ++-
 net/mpls/mpls_gso.c                  |  1 +
 net/openvswitch/datapath.c           |  3 ++-
 net/openvswitch/flow.c               |  2 +-
 18 files changed, 81 insertions(+), 63 deletions(-)

-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 01/10] core: Split out UFO6 support

Split IPv6 support for UFO into its own feature similiar to TSO.
This will later allow us to re-enable UFO support for virtio-net
devices.

Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 include/linux/netdev_features.h |  7 +++++--
 include/linux/netdevice.h       |  1 +
 include/linux/skbuff.h          |  1 +
 net/core/dev.c                  | 35 +++++++++++++++++++----------------
 net/core/ethtool.c              |  2 +-
 5 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index dcfdecb..a078945 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -48,8 +48,9 @@ enum {
 	NETIF_F_GSO_UDP_TUNNEL_BIT,	/* ... UDP TUNNEL with TSO */
 	NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
 	NETIF_F_GSO_MPLS_BIT,		/* ... MPLS segmentation */
+	NETIF_F_UFO6_BIT,		/* ... UDPv6 fragmentation */
 	/**/NETIF_F_GSO_LAST =		/* last bit, see GSO_MASK */
-		NETIF_F_GSO_MPLS_BIT,
+		NETIF_F_UFO6_BIT,
 
 	NETIF_F_FCOE_CRC_BIT,		/* FCoE CRC32 */
 	NETIF_F_SCTP_CSUM_BIT,		/* SCTP checksum offload */
@@ -109,6 +110,7 @@ enum {
 #define NETIF_F_TSO_ECN		__NETIF_F(TSO_ECN)
 #define NETIF_F_TSO		__NETIF_F(TSO)
 #define NETIF_F_UFO		__NETIF_F(UFO)
+#define NETIF_F_UFO6		__NETIF_F(UFO6)
 #define NETIF_F_VLAN_CHALLENGED	__NETIF_F(VLAN_CHALLENGED)
 #define NETIF_F_RXFCS		__NETIF_F(RXFCS)
 #define NETIF_F_RXALL		__NETIF_F(RXALL)
@@ -141,7 +143,7 @@ enum {
 
 /* List of features with software fallbacks. */
 #define NETIF_F_GSO_SOFTWARE	(NETIF_F_TSO | NETIF_F_TSO_ECN | \
-				 NETIF_F_TSO6 | NETIF_F_UFO)
+				 NETIF_F_TSO6 | NETIF_F_UFO | NETIF_F_UFO6)
 
 #define NETIF_F_GEN_CSUM	NETIF_F_HW_CSUM
 #define NETIF_F_V4_CSUM		(NETIF_F_GEN_CSUM | NETIF_F_IP_CSUM)
@@ -149,6 +151,7 @@ enum {
 #define NETIF_F_ALL_CSUM	(NETIF_F_V4_CSUM | NETIF_F_V6_CSUM)
 
 #define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+#define NETIF_F_ALL_UFO		(NETIF_F_UFO | NETIF_F_UFO6)
 
 #define NETIF_F_ALL_FCOE	(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
 				 NETIF_F_FSO)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 74fd5d3..86af10a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3559,6 +3559,7 @@ static inline bool net_gso_ok(netdev_features_t features,
int gso_type)
 	/* check flags correspondence */
 	BUILD_BUG_ON(SKB_GSO_TCPV4   != (NETIF_F_TSO >> NETIF_F_GSO_SHIFT));
 	BUILD_BUG_ON(SKB_GSO_UDP     != (NETIF_F_UFO >> NETIF_F_GSO_SHIFT));
+	BUILD_BUG_ON(SKB_GSO_UDP6    != (NETIF_F_UFO6 >> NETIF_F_GSO_SHIFT));
 	BUILD_BUG_ON(SKB_GSO_DODGY   != (NETIF_F_GSO_ROBUST >>
NETIF_F_GSO_SHIFT));
 	BUILD_BUG_ON(SKB_GSO_TCP_ECN != (NETIF_F_TSO_ECN >> NETIF_F_GSO_SHIFT));
 	BUILD_BUG_ON(SKB_GSO_TCPV6   != (NETIF_F_TSO6 >> NETIF_F_GSO_SHIFT));
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6c8b6f6..8538b67 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -372,6 +372,7 @@ enum {
 
 	SKB_GSO_MPLS = 1 << 12,
 
+	SKB_GSO_UDP6 = 1 << 13
 };
 
 #if BITS_PER_LONG > 32
diff --git a/net/core/dev.c b/net/core/dev.c
index 945bbd0..fa4d2ee 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5929,6 +5929,12 @@ static netdev_features_t netdev_fix_features(struct
net_device *dev,
 		features &= ~NETIF_F_ALL_TSO;
 	}
 
+	/* UFO requires that SG is present as well */
+	if ((features & NETIF_F_ALL_UFO) && !(features & NETIF_F_SG))
{
+		netdev_dbg(dev, "Dropping UFO features since no SG feature.\n");
+		features &= ~NETIF_F_ALL_UFO;
+	}
+
 	if ((features & NETIF_F_TSO) && !(features & NETIF_F_HW_CSUM)
&&
 					!(features & NETIF_F_IP_CSUM)) {
 		netdev_dbg(dev, "Dropping TSO features since no CSUM feature.\n");
@@ -5952,24 +5958,21 @@ static netdev_features_t netdev_fix_features(struct
net_device *dev,
 		features &= ~NETIF_F_GSO;
 	}
 
-	/* UFO needs SG and checksumming */
-	if (features & NETIF_F_UFO) {
-		/* maybe split UFO into V4 and V6? */
-		if (!((features & NETIF_F_GEN_CSUM) ||
-		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
-			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
-			netdev_dbg(dev,
-				"Dropping NETIF_F_UFO since no checksum offload features.\n");
-			features &= ~NETIF_F_UFO;
-		}
-
-		if (!(features & NETIF_F_SG)) {
-			netdev_dbg(dev,
-				"Dropping NETIF_F_UFO since no NETIF_F_SG feature.\n");
-			features &= ~NETIF_F_UFO;
-		}
+	/* UFO also needs checksumming */
+	if ((features & NETIF_F_UFO) && !(features & NETIF_F_GEN_CSUM)
&&
+					!(features & NETIF_F_IP_CSUM)) {
+		netdev_dbg(dev,
+			   "Dropping NETIF_F_UFO since no checksum offload features.\n");
+		features &= ~NETIF_F_UFO;
+	}
+	if ((features & NETIF_F_UFO6) && !(features &
NETIF_F_GEN_CSUM) &&
+					 !(features & NETIF_F_IPV6_CSUM)) {
+		netdev_dbg(dev,
+			   "Dropping NETIF_F_UFO6 since no checksum offload features.\n");
+		features &= ~NETIF_F_UFO6;
 	}
 
+
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	if (dev->netdev_ops->ndo_busy_poll)
 		features |= NETIF_F_BUSY_POLL;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 06dfb29..93eff41 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -223,7 +223,7 @@ static netdev_features_t ethtool_get_feature_mask(u32
eth_cmd)
 		return NETIF_F_ALL_TSO;
 	case ETHTOOL_GUFO:
 	case ETHTOOL_SUFO:
-		return NETIF_F_UFO;
+		return NETIF_F_ALL_UFO;
 	case ETHTOOL_GGSO:
 	case ETHTOOL_SGSO:
 		return NETIF_F_GSO;
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 02/10] net: Correctly mark IPv6 UFO offload type.

If the device supports UFO6 features, mark the offloaded ipv6 udp
traffic appropriately.

Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 net/ipv6/ip6_offload.c | 1 +
 net/ipv6/ip6_output.c  | 4 ++--
 net/ipv6/udp_offload.c | 3 ++-
 net/mpls/mpls_gso.c    | 1 +
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 01e12d0..bd985d5 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -81,6 +81,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 		       SKB_GSO_UDP_TUNNEL_CSUM |
 		       SKB_GSO_MPLS |
 		       SKB_GSO_TCPV6 |
+		       SKB_GSO_UDP6 |
 		       0)))
 		goto out;
 
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 8e950c2..83f5c04 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1089,7 +1089,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 	 */
 	skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
 				     sizeof(struct frag_hdr)) & ~7;
-	skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
+	skb_shinfo(skb)->gso_type = SKB_GSO_UDP6;
 	ipv6_select_ident(&fhdr, rt);
 	skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
 
@@ -1296,7 +1296,7 @@ emsgsize:
 	if (((length > mtu) ||
 	     (skb && skb_is_gso(skb))) &&
 	    (sk->sk_protocol == IPPROTO_UDP) &&
-	    (rt->dst.dev->features & NETIF_F_UFO)) {
+	    (rt->dst.dev->features & NETIF_F_UFO6)) {
 		err = ip6_ufo_append_data(sk, getfrag, from, length,
 					  hh_len, fragheaderlen,
 					  transhdrlen, mtu, flags, rt);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 6b8f543..00d723e 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -39,6 +39,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 		int type = skb_shinfo(skb)->gso_type;
 
 		if (unlikely(type & ~(SKB_GSO_UDP |
+				      SKB_GSO_UDP6 |
 				      SKB_GSO_DODGY |
 				      SKB_GSO_UDP_TUNNEL |
 				      SKB_GSO_UDP_TUNNEL_CSUM |
@@ -47,7 +48,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 				      SKB_GSO_IPIP |
 				      SKB_GSO_SIT |
 				      SKB_GSO_MPLS) ||
-			     !(type & (SKB_GSO_UDP))))
+			     !(type & (SKB_GSO_UDP | SKB_GSO_UDP6))))
 			goto out;
 
 		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
diff --git a/net/mpls/mpls_gso.c b/net/mpls/mpls_gso.c
index e3545f2..27343f0 100644
--- a/net/mpls/mpls_gso.c
+++ b/net/mpls/mpls_gso.c
@@ -30,6 +30,7 @@ static struct sk_buff *mpls_gso_segment(struct sk_buff *skb,
 				~(SKB_GSO_TCPV4 |
 				  SKB_GSO_TCPV6 |
 				  SKB_GSO_UDP |
+				  SKB_GSO_UDP6 |
 				  SKB_GSO_DODGY |
 				  SKB_GSO_TCP_ECN |
 				  SKB_GSO_GRE |
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 03/10] ovs: Enable handling of UFO6 packets.

Since UFO6 packets can now be identified by SKB_GSO_UDP6, add proper checks
to handel UFO6 flows.
Legacy applications may still have UFO6 packets identified by SKB_GSO_UDP,
so we need to continue to handle them correclty.

Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 net/openvswitch/datapath.c | 3 ++-
 net/openvswitch/flow.c     | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index f9e556b..b43fc60 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -334,7 +334,8 @@ static int queue_gso_packets(struct datapath *dp, struct
sk_buff *skb,
 		if (err)
 			break;
 
-		if (skb == segs && gso_type & SKB_GSO_UDP) {
+		if (skb == segs &&
+		    ((gso_type & SKB_GSO_UDP) || (gso_type & SKB_GSO_UDP6))) {
 			/* The initial flow key extracted by ovs_flow_extract()
 			 * in this case is for a first fragment, so we need to
 			 * properly mark later fragments.
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 2b78789..d03adf4 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -602,7 +602,7 @@ static int key_extract(struct sk_buff *skb, struct
sw_flow_key *key)
 
 		if (key->ip.frag == OVS_FRAG_TYPE_LATER)
 			return 0;
-		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
+		if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP6))
 			key->ip.frag = OVS_FRAG_TYPE_FIRST;
 
 		/* Transport layer. */
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 04/10] loopback: Turn on UFO6 support.

Turn on UFO6 support.

Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 drivers/net/loopback.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index c76283c..762c28a 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -170,10 +170,10 @@ static void loopback_setup(struct net_device *dev)
 	dev->flags		= IFF_LOOPBACK;
 	dev->priv_flags		|= IFF_LIVE_ADDR_CHANGE;
 	netif_keep_dst(dev);
-	dev->hw_features	= NETIF_F_ALL_TSO | NETIF_F_UFO;
+	dev->hw_features	= NETIF_F_ALL_TSO | NETIF_F_ALL_UFO;
 	dev->features 		= NETIF_F_SG | NETIF_F_FRAGLIST
 		| NETIF_F_ALL_TSO
-		| NETIF_F_UFO
+		| NETIF_F_ALL_UFO
 		| NETIF_F_HW_CSUM
 		| NETIF_F_RXCSUM
 		| NETIF_F_SCTP_CSUM
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 05/10] veth: Enable UFO6 support.

Turn on UFO6 feature.

Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 drivers/net/veth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8ad5965..0052db5 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -280,7 +280,7 @@ static const struct net_device_ops veth_netdev_ops = {
 #define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_ALL_TSO |    \
 		       NETIF_F_HW_CSUM | NETIF_F_RXCSUM | NETIF_F_HIGHDMA | \
 		       NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |	    \
-		       NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT | NETIF_F_UFO	|   \
+		       NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT | NETIF_F_ALL_UFO | \
 		       NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX | \
 		       NETIF_F_HW_VLAN_STAG_TX | NETIF_F_HW_VLAN_STAG_RX )
 
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 06/10] macvlan: Enable UFO6 support.

Turn on UFO6 feature.

Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 drivers/net/macvlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index bfb0b6e..807b98d 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -746,7 +746,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
 
 #define MACVLAN_FEATURES \
 	(NETIF_F_SG | NETIF_F_ALL_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
-	 NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | NETIF_F_GSO_ROBUST | \
+	 NETIF_F_GSO | NETIF_F_TSO | NETIF_F_ALL_UFO | NETIF_F_GSO_ROBUST | \
 	 NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \
 	 NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER)
 
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 07/10] s2io: Enable UFO6 support.

CC: Jon Mason <jdmason at kudzu.us>
Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 drivers/net/ethernet/neterion/s2io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/neterion/s2io.c
b/drivers/net/ethernet/neterion/s2io.c
index f5e4b82..d823bb7 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -4140,7 +4140,7 @@ static netdev_tx_t s2io_xmit(struct sk_buff *skb, struct
net_device *dev)
 	}
 
 	frg_len = skb_headlen(skb);
-	if (offload_type == SKB_GSO_UDP) {
+	if (offload_type == SKB_GSO_UDP || offload_type == SKB_GSO_UDP6) {
 		int ufo_size;
 
 		ufo_size = s2io_udp_mss(skb);
@@ -7917,9 +7917,9 @@ s2io_init_nic(struct pci_dev *pdev, const struct
pci_device_id *pre)
 	dev->features |= dev->hw_features |
 		NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
 	if (sp->device_type & XFRAME_II_DEVICE) {
-		dev->hw_features |= NETIF_F_UFO;
+		dev->hw_features |= NETIF_F_ALL_UFO;
 		if (ufo)
-			dev->features |= NETIF_F_UFO;
+			dev->features |= NETIF_F_ALL_UFO;
 	}
 	if (sp->high_dma_flag == true)
 		dev->features |= NETIF_F_HIGHDMA;
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 08/10] tun: Re-uanble UFO support.

Now that UFO is split into v4 and v6 parts, we can bring
back v4 support without any trouble.

Continue to handle legacy applications by selecting the
IPv6 fragment id but do not change the gso type.  Thist
makes sure that two legacy VMs may still communicate.

Based on original work from Ben Hutchings.

Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
CC: Ben Hutchings <ben at decadent.org.uk>
Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 drivers/net/tun.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9dd3746..8c32fca 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -175,7 +175,7 @@ struct tun_struct {
 	struct net_device	*dev;
 	netdev_features_t	set_features;
 #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
-			  NETIF_F_TSO6)
+			  NETIF_F_TSO6|NETIF_F_UFO)
 
 	int			vnet_hdr_sz;
 	int			sndbuf;
@@ -1152,20 +1152,15 @@ static ssize_t tun_get_user(struct tun_struct *tun,
struct tun_file *tfile,
 			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
 			break;
 		case VIRTIO_NET_HDR_GSO_UDP:
-		{
-			static bool warned;
-
-			if (!warned) {
-				warned = true;
-				netdev_warn(tun->dev,
-					    "%s: using disabled UFO feature; please fix this program\n",
-					    current->comm);
-			}
 			skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
-			if (skb->protocol == htons(ETH_P_IPV6))
+			if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
+				/* This allows legacy application to work.
+				 * Do not change the gso_type as it may
+				 * not be upderstood by legacy applications.
+				 */
 				ipv6_proxy_select_ident(skb);
+			}
 			break;
-		}
 		default:
 			tun->dev->stats.rx_frame_errors++;
 			kfree_skb(skb);
@@ -1273,6 +1268,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
 			else if (sinfo->gso_type & SKB_GSO_TCPV6)
 				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+			else if (sinfo->gso_type & SKB_GSO_UDP)
+				gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
 			else {
 				pr_err("unexpected GSO type: "
 				       "0x%x, gso_size %d, hdr_len %d\n",
@@ -1780,6 +1777,11 @@ static int set_offload(struct tun_struct *tun, unsigned
long arg)
 				features |= NETIF_F_TSO6;
 			arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
 		}
+
+		if (arg & TUN_F_UFO) {
+			features |= NETIF_F_UFO;
+			arg &= ~TUN_F_UFO;
+		}
 	}
 
 	/* This gives the user a way to test for new features in future by
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 09/10] macvtap: Re-enable UFO support

Now that UFO is split into v4 and v6 parts, we can bring
back v4 support.  Continue to handle legacy applications
by selecting the ipv6 fagment id but do not change the
gso type.  This allows 2 legacy VMs to continue to communicate.

Based on original work from Ben Hutchings.

Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
CC: Ben Hutchings <ben at decadent.org.uk>
Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
---
 drivers/net/macvtap.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 880cc09..75febd4 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -66,7 +66,7 @@ static struct cdev macvtap_cdev;
 static const struct proto_ops macvtap_socket_ops;
 
 #define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
-		      NETIF_F_TSO6)
+		      NETIF_F_TSO6 | NETIF_F_UFO)
 #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
 #define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
 
@@ -570,11 +570,14 @@ static int macvtap_skb_from_vnet_hdr(struct sk_buff *skb,
 			gso_type = SKB_GSO_TCPV6;
 			break;
 		case VIRTIO_NET_HDR_GSO_UDP:
-			pr_warn_once("macvtap: %s: using disabled UFO feature; please fix this
program\n",
-				     current->comm);
 			gso_type = SKB_GSO_UDP;
-			if (skb->protocol == htons(ETH_P_IPV6))
+			if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
+				/* This is to support legacy appliacations.
+				 * Do not change the gso_type as legacy apps
+				 * may not know about the new type.
+				 */
 				ipv6_proxy_select_ident(skb);
+			}
 			break;
 		default:
 			return -EINVAL;
@@ -619,6 +622,8 @@ static void macvtap_skb_to_vnet_hdr(const struct sk_buff
*skb,
 			vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
 		else if (sinfo->gso_type & SKB_GSO_TCPV6)
 			vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+		else if (sinfo->gso_type & SKB_GSO_UDP)
+			vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP;
 		else
 			BUG();
 		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
@@ -955,6 +960,9 @@ static int set_offload(struct macvtap_queue *q, unsigned
long arg)
 			if (arg & TUN_F_TSO6)
 				feature_mask |= NETIF_F_TSO6;
 		}
+
+		if (arg & TUN_F_UFO)
+			feature_mask |= NETIF_F_UFO;
 	}
 
 	/* tun/tap driver inverts the usage for TSO offloads, where
@@ -965,7 +973,7 @@ static int set_offload(struct macvtap_queue *q, unsigned
long arg)
 	 * When user space turns off TSO, we turn off GSO/LRO so that
 	 * user-space will not receive TSO frames.
 	 */
-	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
+	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
 		features |= RX_OFFLOADS;
 	else
 		features &= ~RX_OFFLOADS;
@@ -1066,7 +1074,7 @@ static long macvtap_ioctl(struct file *file, unsigned int
cmd,
 	case TUNSETOFFLOAD:
 		/* let the user check for future flags */
 		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
-			    TUN_F_TSO_ECN))
+			    TUN_F_TSO_ECN | TUN_F_UFO))
 			return -EINVAL;
 
 		rtnl_lock();
-- 
1.9.3

Vladislav Yasevich

2014-Dec-17 18:20 UTC

head link

[PATCH 10/10] Revert "drivers/net: Disable UFO through virtio"

This reverts commit 3d0ad09412ffe00c9afa201d01effdb6023d09b4.
Now that we've split UFO into v4 and v6 version, we can turn
back UFO support for ipv4.  Full IPv6 support will come later as
it requires extending vnet header structure.

Any older VM that assumes IPv6 support is included in UFO
will continue to use UFO and the host will generate fragment
ids for it, thus preserving connectivity.

Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
CC: Ben Hutchings <ben at decadent.org.uk>
Signed-off-by: Vlad Yasevich <vyasevic at redhat.com>
---
 drivers/net/virtio_net.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b0bc8ea..534b633 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -491,17 +491,8 @@ static void receive_buf(struct receive_queue *rq, void
*buf, unsigned int len)
 			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
 			break;
 		case VIRTIO_NET_HDR_GSO_UDP:
-		{
-			static bool warned;
-
-			if (!warned) {
-				warned = true;
-				netdev_warn(dev,
-					    "host using disabled UFO feature; please fix it\n");
-			}
 			skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
 			break;
-		}
 		case VIRTIO_NET_HDR_GSO_TCPV6:
 			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
 			break;
@@ -890,6 +881,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
*skb)
 			hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
 		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
 			hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+		else if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
+			hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_UDP;
 		else
 			BUG();
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
@@ -1749,7 +1742,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 			dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
 
 		if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
-			dev->hw_features |= NETIF_F_TSO
+			dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
 				| NETIF_F_TSO_ECN | NETIF_F_TSO6;
 		}
 		/* Individual feature bits: what can host handle? */
@@ -1759,9 +1752,11 @@ static int virtnet_probe(struct virtio_device *vdev)
 			dev->hw_features |= NETIF_F_TSO6;
 		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
 			dev->hw_features |= NETIF_F_TSO_ECN;
+		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO))
+			dev->hw_features |= NETIF_F_UFO;
 
 		if (gso)
-			dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
+			dev->features |= dev->hw_features & (NETIF_F_ALL_TSO|NETIF_F_UFO);
 		/* (!csum && gso) case will be fixed by register_netdev() */
 	}
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
@@ -1799,7 +1794,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 	/* If we can receive ANY GSO packets, we must allocate large ones. */
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
 	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
-	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
+	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN) ||
+	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UFO))
 		vi->big_packets = true;
 
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
@@ -1993,9 +1989,9 @@ static struct virtio_device_id id_table[] = {
 static unsigned int features[] = {
 	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
 	VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
-	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6,
+	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
 	VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
-	VIRTIO_NET_F_GUEST_ECN,
+	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
 	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
 	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
 	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
-- 
1.9.3

Ben Hutchings

2014-Dec-17 20:10 UTC

head link

[PATCH 01/10] core: Split out UFO6 support

On Wed, 2014-12-17 at 13:20 -0500, Vladislav Yasevich
wrote:> Split IPv6 support for UFO into its own feature similiar to TSO.
> This will later allow us to re-enable UFO support for virtio-net
> devices.
[...]> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6c8b6f6..8538b67 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -372,6 +372,7 @@ enum {
>  
>  	SKB_GSO_MPLS = 1 << 12,
>  
> +	SKB_GSO_UDP6 = 1 << 13
It seems like it would be cleaner to use the names SKB_GSO_UDPV{4,6},
similarly to SKB_GSO_TCPV{4,6}.
>  };
>  
>  #if BITS_PER_LONG > 32
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 945bbd0..fa4d2ee 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[...]> @@ -5952,24 +5958,21 @@ static netdev_features_t netdev_fix_features(struct
net_device *dev,
[...]> +	/* UFO also needs checksumming */
> +	if ((features & NETIF_F_UFO) && !(features &
NETIF_F_GEN_CSUM) &&
> +					!(features & NETIF_F_IP_CSUM)) {
You can use !(features & NETIF_F_V4_CSUM) instead of the last two terms.
> +		netdev_dbg(dev,
> +			   "Dropping NETIF_F_UFO since no checksum offload
features.\n");
> +		features &= ~NETIF_F_UFO;
> +	}
> +	if ((features & NETIF_F_UFO6) && !(features &
NETIF_F_GEN_CSUM) &&
> +					 !(features & NETIF_F_IPV6_CSUM)) {[...]

Similarly you can use !(features & NETIF_F_V6_CSUM) instead of the last
two terms.

Aside from those minor points, this looks fine.

Ben.

-- 
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 811 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20141217/e4ebbe58/attachment-0001.sig>

Sergei Shtylyov

2014-Dec-17 20:17 UTC

head link

[PATCH 03/10] ovs: Enable handling of UFO6 packets.

Hello.

On 12/17/2014 09:20 PM, Vladislav Yasevich wrote:
> Since UFO6 packets can now be identified by SKB_GSO_UDP6, add proper checks
> to handel UFO6 flows.
> Legacy applications may still have UFO6 packets identified by SKB_GSO_UDP,
> so we need to continue to handle them correclty.
> Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
> ---
>   net/openvswitch/datapath.c | 3 ++-
>   net/openvswitch/flow.c     | 2 +-
>   2 files changed, 3 insertions(+), 2 deletions(-)
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index f9e556b..b43fc60 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -334,7 +334,8 @@ static int queue_gso_packets(struct datapath *dp,
struct sk_buff *skb,
>   		if (err)
>   			break;
>
> -		if (skb == segs && gso_type & SKB_GSO_UDP) {
> +		if (skb == segs &&
> +		    ((gso_type & SKB_GSO_UDP) || (gso_type & SKB_GSO_UDP6))) {
    'gso_type & (SKB_GSO_UDP | SKB_GSO_UDP6)' would be shorter...
>   			/* The initial flow key extracted by ovs_flow_extract()
>   			 * in this case is for a first fragment, so we need to
>   			 * properly mark later fragments.
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 2b78789..d03adf4 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -602,7 +602,7 @@ static int key_extract(struct sk_buff *skb, struct
sw_flow_key *key)
>
>   		if (key->ip.frag == OVS_FRAG_TYPE_LATER)
>   			return 0;
> -		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
> +		if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP6))
    .... like here.

[...]

WBR, Sergei

Michael S. Tsirkin

2014-Dec-17 22:26 UTC

head link

[PATCH 03/10] ovs: Enable handling of UFO6 packets.

On Wed, Dec 17, 2014 at 01:20:48PM -0500, Vladislav Yasevich
wrote:> Since UFO6 packets can now be identified by SKB_GSO_UDP6, add proper checks
> to handel UFO6 flows.
s/handel/handle/
> Legacy applications may still have UFO6 packets identified by SKB_GSO_UDP,
> so we need to continue to handle them correclty.
> 
> Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
> ---
>  net/openvswitch/datapath.c | 3 ++-
>  net/openvswitch/flow.c     | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index f9e556b..b43fc60 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -334,7 +334,8 @@ static int queue_gso_packets(struct datapath *dp,
struct sk_buff *skb,
>  		if (err)
>  			break;
>  
> -		if (skb == segs && gso_type & SKB_GSO_UDP) {
> +		if (skb == segs &&
> +		    ((gso_type & SKB_GSO_UDP) || (gso_type & SKB_GSO_UDP6))) {
>  			/* The initial flow key extracted by ovs_flow_extract()
>  			 * in this case is for a first fragment, so we need to
>  			 * properly mark later fragments.
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 2b78789..d03adf4 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -602,7 +602,7 @@ static int key_extract(struct sk_buff *skb, struct
sw_flow_key *key)
>  
>  		if (key->ip.frag == OVS_FRAG_TYPE_LATER)
>  			return 0;
> -		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
> +		if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP6))
>  			key->ip.frag = OVS_FRAG_TYPE_FIRST;
>  
>  		/* Transport layer. */
> -- 
> 1.9.3

Michael S. Tsirkin

2014-Dec-17 22:33 UTC

head link

[PATCH 08/10] tun: Re-uanble UFO support.

subs: re-enable

On Wed, Dec 17, 2014 at 01:20:53PM -0500, Vladislav Yasevich
wrote:> Now that UFO is split into v4 and v6 parts, we can bring
> back v4 support without any trouble.
> 
> Continue to handle legacy applications by selecting the
> IPv6 fragment id but do not change the gso type.  Thist
s/Thist/this/
> makes sure that two legacy VMs may still communicate.
This means IPv6 skbs with UFO (not UFO6) flag set are present
in the stack.
If possible, I think it would be better to make GSO type correct: UFO6,
and then convert to UFO when copying to guest.

A similar approach should be possible  for OVS?

> Based on original work from Ben Hutchings.
> 
> Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
> CC: Ben Hutchings <ben at decadent.org.uk>
> Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
> ---
>  drivers/net/tun.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 9dd3746..8c32fca 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -175,7 +175,7 @@ struct tun_struct {
>  	struct net_device	*dev;
>  	netdev_features_t	set_features;
>  #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
> -			  NETIF_F_TSO6)
> +			  NETIF_F_TSO6|NETIF_F_UFO)
>  
>  	int			vnet_hdr_sz;
>  	int			sndbuf;
> @@ -1152,20 +1152,15 @@ static ssize_t tun_get_user(struct tun_struct *tun,
struct tun_file *tfile,
>  			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>  			break;
>  		case VIRTIO_NET_HDR_GSO_UDP:
> -		{
> -			static bool warned;
> -
> -			if (!warned) {
> -				warned = true;
> -				netdev_warn(tun->dev,
> -					    "%s: using disabled UFO feature; please fix this
program\n",
> -					    current->comm);
> -			}
>  			skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> -			if (skb->protocol == htons(ETH_P_IPV6))
> +			if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
> +				/* This allows legacy application to work.
> +				 * Do not change the gso_type as it may
> +				 * not be upderstood by legacy applications.
Shouldn't we handle legacy applications when passing packets to
userspace?
> +				 */
>  				ipv6_proxy_select_ident(skb);
> +			}
>  			break;
> -		}
>  		default:
>  			tun->dev->stats.rx_frame_errors++;
>  			kfree_skb(skb);
> @@ -1273,6 +1268,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>  				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
>  			else if (sinfo->gso_type & SKB_GSO_TCPV6)
>  				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> +			else if (sinfo->gso_type & SKB_GSO_UDP)
> +				gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
>  			else {
>  				pr_err("unexpected GSO type: "
>  				       "0x%x, gso_size %d, hdr_len %d\n",
> @@ -1780,6 +1777,11 @@ static int set_offload(struct tun_struct *tun,
unsigned long arg)
>  				features |= NETIF_F_TSO6;
>  			arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
>  		}
> +
> +		if (arg & TUN_F_UFO) {
> +			features |= NETIF_F_UFO;
> +			arg &= ~TUN_F_UFO;
> +		}
>  	}
>  
>  	/* This gives the user a way to test for new features in future by
> -- 
> 1.9.3

Michael S. Tsirkin

2014-Dec-17 22:41 UTC

head link

[PATCH 09/10] macvtap: Re-enable UFO support

On Wed, Dec 17, 2014 at 01:20:54PM -0500, Vladislav Yasevich
wrote:> Now that UFO is split into v4 and v6 parts, we can bring
> back v4 support.  Continue to handle legacy applications
> by selecting the ipv6 fagment id but do not change the
> gso type.  This allows 2 legacy VMs to continue to communicate.
> 
> Based on original work from Ben Hutchings.
> 
> Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
> CC: Ben Hutchings <ben at decadent.org.uk>
> Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
> ---
>  drivers/net/macvtap.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 880cc09..75febd4 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -66,7 +66,7 @@ static struct cdev macvtap_cdev;
>  static const struct proto_ops macvtap_socket_ops;
>  
>  #define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> -		      NETIF_F_TSO6)
> +		      NETIF_F_TSO6 | NETIF_F_UFO)
>  #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
>  #define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
>  
> @@ -570,11 +570,14 @@ static int macvtap_skb_from_vnet_hdr(struct sk_buff
*skb,
>  			gso_type = SKB_GSO_TCPV6;
>  			break;
>  		case VIRTIO_NET_HDR_GSO_UDP:
> -			pr_warn_once("macvtap: %s: using disabled UFO feature; please fix
this program\n",
> -				     current->comm);
>  			gso_type = SKB_GSO_UDP;
> -			if (skb->protocol == htons(ETH_P_IPV6))
> +			if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
> +				/* This is to support legacy appliacations.
> +				 * Do not change the gso_type as legacy apps
> +				 * may not know about the new type.
> +				 */
>  				ipv6_proxy_select_ident(skb);
> +			}
>  			break;
>  		default:
>  			return -EINVAL;
> @@ -619,6 +622,8 @@ static void macvtap_skb_to_vnet_hdr(const struct
sk_buff *skb,
>  			vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
>  		else if (sinfo->gso_type & SKB_GSO_TCPV6)
>  			vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> +		else if (sinfo->gso_type & SKB_GSO_UDP)
> +			vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP;
>  		else
>  			BUG();
>  		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> @@ -955,6 +960,9 @@ static int set_offload(struct macvtap_queue *q,
unsigned long arg)
>  			if (arg & TUN_F_TSO6)
>  				feature_mask |= NETIF_F_TSO6;
>  		}
> +
> +		if (arg & TUN_F_UFO)
> +			feature_mask |= NETIF_F_UFO;
>  	}
>  
>  	/* tun/tap driver inverts the usage for TSO offloads, where
> @@ -965,7 +973,7 @@ static int set_offload(struct macvtap_queue *q,
unsigned long arg)
>  	 * When user space turns off TSO, we turn off GSO/LRO so that
>  	 * user-space will not receive TSO frames.
>  	 */
> -	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
> +	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
>  		features |= RX_OFFLOADS;
>  	else
>  		features &= ~RX_OFFLOADS;
By the way this logic is completely broken, even without your patch,
isn't it?

It says: enable LRO+GRO if any of NETIF_F_TSO | NETIF_F_TSO6 |
NETIF_F_UFO set.

So what happens if I enable TSO only?
LRO gets enabled so I can still get TSO6 packets.


This really should be:

	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO) =		
(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO)


fixing this probably should be a separate patch before your
series, and Cc stable.

> @@ -1066,7 +1074,7 @@ static long macvtap_ioctl(struct file *file, unsigned
int cmd,
>  	case TUNSETOFFLOAD:
>  		/* let the user check for future flags */
>  		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> -			    TUN_F_TSO_ECN))
> +			    TUN_F_TSO_ECN | TUN_F_UFO))
>  			return -EINVAL;
>  
>  		rtnl_lock();
> -- 
> 1.9.3

Michael S. Tsirkin

2014-Dec-17 22:44 UTC

head link

[PATCH 10/10] Revert "drivers/net: Disable UFO through virtio"

On Wed, Dec 17, 2014 at 01:20:55PM -0500, Vladislav Yasevich
wrote:> This reverts commit 3d0ad09412ffe00c9afa201d01effdb6023d09b4.
> Now that we've split UFO into v4 and v6 version, we can turn
> back UFO support for ipv4.  Full IPv6 support will come later as
> it requires extending vnet header structure.
> 
> Any older VM that assumes IPv6 support is included in UFO
> will continue to use UFO and the host will generate fragment
> ids for it, thus preserving connectivity.
> 
> Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
> CC: Ben Hutchings <ben at decadent.org.uk>
> Signed-off-by: Vlad Yasevich <vyasevic at redhat.com>
> ---
>  drivers/net/virtio_net.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b0bc8ea..534b633 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -491,17 +491,8 @@ static void receive_buf(struct receive_queue *rq, void
*buf, unsigned int len)
>  			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>  			break;
>  		case VIRTIO_NET_HDR_GSO_UDP:
> -		{
> -			static bool warned;
> -
> -			if (!warned) {
> -				warned = true;
> -				netdev_warn(dev,
> -					    "host using disabled UFO feature; please fix it\n");
> -			}
>  			skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
>  			break;

This might not be true: could be a legacy host.
I think we need to check for IPv6 and set it
correctly to SKB_GSO_UDP/SKB_GSO_UDP6?

> -		}
>  		case VIRTIO_NET_HDR_GSO_TCPV6:
>  			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>  			break;
> @@ -890,6 +881,8 @@ static int xmit_skb(struct send_queue *sq, struct
sk_buff *skb)
>  			hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
>  		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
>  			hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> +		else if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
> +			hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_UDP;
>  		else
>  			BUG();
>  		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
> @@ -1749,7 +1742,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>  			dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
>  
>  		if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> -			dev->hw_features |= NETIF_F_TSO
> +			dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
>  				| NETIF_F_TSO_ECN | NETIF_F_TSO6;
>  		}
>  		/* Individual feature bits: what can host handle? */
> @@ -1759,9 +1752,11 @@ static int virtnet_probe(struct virtio_device *vdev)
>  			dev->hw_features |= NETIF_F_TSO6;
>  		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
>  			dev->hw_features |= NETIF_F_TSO_ECN;
> +		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO))
> +			dev->hw_features |= NETIF_F_UFO;
>  
>  		if (gso)
> -			dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
> +			dev->features |= dev->hw_features &
(NETIF_F_ALL_TSO|NETIF_F_UFO);
>  		/* (!csum && gso) case will be fixed by register_netdev() */
>  	}
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
> @@ -1799,7 +1794,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	/* If we can receive ANY GSO packets, we must allocate large ones. */
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>  	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
> -	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
> +	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN) ||
> +	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UFO))
>  		vi->big_packets = true;
>  
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> @@ -1993,9 +1989,9 @@ static struct virtio_device_id id_table[] = {
>  static unsigned int features[] = {
>  	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
>  	VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
> -	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6,
> +	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
>  	VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
> -	VIRTIO_NET_F_GUEST_ECN,
> +	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> -- 
> 1.9.3

Michael S. Tsirkin

2014-Dec-17 22:45 UTC

head link

[PATCH 01/10] core: Split out UFO6 support

On Wed, Dec 17, 2014 at 01:20:46PM -0500, Vladislav Yasevich
wrote:> Split IPv6 support for UFO into its own feature similiar to TSO.
> This will later allow us to re-enable UFO support for virtio-net
> devices.
> 
> Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
> ---
>  include/linux/netdev_features.h |  7 +++++--
>  include/linux/netdevice.h       |  1 +
>  include/linux/skbuff.h          |  1 +
>  net/core/dev.c                  | 35 +++++++++++++++++++----------------
>  net/core/ethtool.c              |  2 +-
>  5 files changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/netdev_features.h
b/include/linux/netdev_features.h
> index dcfdecb..a078945 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -48,8 +48,9 @@ enum {
>  	NETIF_F_GSO_UDP_TUNNEL_BIT,	/* ... UDP TUNNEL with TSO */
>  	NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
>  	NETIF_F_GSO_MPLS_BIT,		/* ... MPLS segmentation */
> +	NETIF_F_UFO6_BIT,		/* ... UDPv6 fragmentation */
>  	/**/NETIF_F_GSO_LAST =		/* last bit, see GSO_MASK */
> -		NETIF_F_GSO_MPLS_BIT,
> +		NETIF_F_UFO6_BIT,
>  
>  	NETIF_F_FCOE_CRC_BIT,		/* FCoE CRC32 */
>  	NETIF_F_SCTP_CSUM_BIT,		/* SCTP checksum offload */
> @@ -109,6 +110,7 @@ enum {
>  #define NETIF_F_TSO_ECN		__NETIF_F(TSO_ECN)
>  #define NETIF_F_TSO		__NETIF_F(TSO)
>  #define NETIF_F_UFO		__NETIF_F(UFO)
> +#define NETIF_F_UFO6		__NETIF_F(UFO6)
>  #define NETIF_F_VLAN_CHALLENGED	__NETIF_F(VLAN_CHALLENGED)
>  #define NETIF_F_RXFCS		__NETIF_F(RXFCS)
>  #define NETIF_F_RXALL		__NETIF_F(RXALL)
> @@ -141,7 +143,7 @@ enum {
>  
>  /* List of features with software fallbacks. */
>  #define NETIF_F_GSO_SOFTWARE	(NETIF_F_TSO | NETIF_F_TSO_ECN | \
> -				 NETIF_F_TSO6 | NETIF_F_UFO)
> +				 NETIF_F_TSO6 | NETIF_F_UFO | NETIF_F_UFO6)
>  
>  #define NETIF_F_GEN_CSUM	NETIF_F_HW_CSUM
>  #define NETIF_F_V4_CSUM		(NETIF_F_GEN_CSUM | NETIF_F_IP_CSUM)
> @@ -149,6 +151,7 @@ enum {
>  #define NETIF_F_ALL_CSUM	(NETIF_F_V4_CSUM | NETIF_F_V6_CSUM)
>  
>  #define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
> +#define NETIF_F_ALL_UFO		(NETIF_F_UFO | NETIF_F_UFO6)
>  
>  #define NETIF_F_ALL_FCOE	(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
>  				 NETIF_F_FSO)
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 74fd5d3..86af10a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3559,6 +3559,7 @@ static inline bool net_gso_ok(netdev_features_t
features, int gso_type)
>  	/* check flags correspondence */
>  	BUILD_BUG_ON(SKB_GSO_TCPV4   != (NETIF_F_TSO >>
NETIF_F_GSO_SHIFT));
>  	BUILD_BUG_ON(SKB_GSO_UDP     != (NETIF_F_UFO >>
NETIF_F_GSO_SHIFT));
> +	BUILD_BUG_ON(SKB_GSO_UDP6    != (NETIF_F_UFO6 >>
NETIF_F_GSO_SHIFT));
>  	BUILD_BUG_ON(SKB_GSO_DODGY   != (NETIF_F_GSO_ROBUST >>
NETIF_F_GSO_SHIFT));
>  	BUILD_BUG_ON(SKB_GSO_TCP_ECN != (NETIF_F_TSO_ECN >>
NETIF_F_GSO_SHIFT));
>  	BUILD_BUG_ON(SKB_GSO_TCPV6   != (NETIF_F_TSO6 >>
NETIF_F_GSO_SHIFT));
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6c8b6f6..8538b67 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -372,6 +372,7 @@ enum {
>  
>  	SKB_GSO_MPLS = 1 << 12,
>  
> +	SKB_GSO_UDP6 = 1 << 13
>  };
>  
>  #if BITS_PER_LONG > 32
So this implies anything getting GSO packets e.g.
from userspace now needs to check IP version to
set GSO type correctly.

I think you missed some places that do this, e.g. af_packet
sockets.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 945bbd0..fa4d2ee 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5929,6 +5929,12 @@ static netdev_features_t netdev_fix_features(struct
net_device *dev,
>  		features &= ~NETIF_F_ALL_TSO;
>  	}
>  
> +	/* UFO requires that SG is present as well */
> +	if ((features & NETIF_F_ALL_UFO) && !(features &
NETIF_F_SG)) {
> +		netdev_dbg(dev, "Dropping UFO features since no SG
feature.\n");
> +		features &= ~NETIF_F_ALL_UFO;
> +	}
> +
>  	if ((features & NETIF_F_TSO) && !(features &
NETIF_F_HW_CSUM) &&
>  					!(features & NETIF_F_IP_CSUM)) {
>  		netdev_dbg(dev, "Dropping TSO features since no CSUM
feature.\n");
> @@ -5952,24 +5958,21 @@ static netdev_features_t netdev_fix_features(struct
net_device *dev,
>  		features &= ~NETIF_F_GSO;
>  	}
>  
> -	/* UFO needs SG and checksumming */
> -	if (features & NETIF_F_UFO) {
> -		/* maybe split UFO into V4 and V6? */
> -		if (!((features & NETIF_F_GEN_CSUM) ||
> -		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
> -			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
> -			netdev_dbg(dev,
> -				"Dropping NETIF_F_UFO since no checksum offload
features.\n");
> -			features &= ~NETIF_F_UFO;
> -		}
> -
> -		if (!(features & NETIF_F_SG)) {
> -			netdev_dbg(dev,
> -				"Dropping NETIF_F_UFO since no NETIF_F_SG feature.\n");
> -			features &= ~NETIF_F_UFO;
> -		}
> +	/* UFO also needs checksumming */
> +	if ((features & NETIF_F_UFO) && !(features &
NETIF_F_GEN_CSUM) &&
> +					!(features & NETIF_F_IP_CSUM)) {
> +		netdev_dbg(dev,
> +			   "Dropping NETIF_F_UFO since no checksum offload
features.\n");
> +		features &= ~NETIF_F_UFO;
> +	}
> +	if ((features & NETIF_F_UFO6) && !(features &
NETIF_F_GEN_CSUM) &&
> +					 !(features & NETIF_F_IPV6_CSUM)) {
> +		netdev_dbg(dev,
> +			   "Dropping NETIF_F_UFO6 since no checksum offload
features.\n");
> +		features &= ~NETIF_F_UFO6;
>  	}
>  
> +
>  #ifdef CONFIG_NET_RX_BUSY_POLL
>  	if (dev->netdev_ops->ndo_busy_poll)
>  		features |= NETIF_F_BUSY_POLL;
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 06dfb29..93eff41 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -223,7 +223,7 @@ static netdev_features_t ethtool_get_feature_mask(u32
eth_cmd)
>  		return NETIF_F_ALL_TSO;
>  	case ETHTOOL_GUFO:
>  	case ETHTOOL_SUFO:
> -		return NETIF_F_UFO;
> +		return NETIF_F_ALL_UFO;
>  	case ETHTOOL_GGSO:
>  	case ETHTOOL_SGSO:
>  		return NETIF_F_GSO;
> -- 
> 1.9.3

Jason Wang

2014-Dec-18 05:28 UTC

head link

[PATCH 00/10] Split UFO into v4 and v6 versions.

----- Original Message -----> UFO support in the kernel applies to both IPv4 and IPv6 protocols
> with the same device feature.  However some devices may not be able
> to support one of the offloads.  For this we split the UFO offload
> feature into 2 pieces.  NETIF_F_UFO now controlls the IPv4 part and
> this series introduces NETIF_F_UFO6.
> 
> As a result of this work, we can now re-enable NETIF_F_UFO on
> virtio_net devices and restore UDP over IPv4 performance for guests.
> We also continue to support legacy guests that assume that UFO6
> support included into UFO(4).
> 
> Without this work, migrating a guest to a 3.18 kernel fails.
> 
This series eliminate the ambiguous NETIF_F_UFO.

But VIRTIO_NET_F_{HOST|GUEST}_UFO and VIRTIO_NET_F_HDR_GSO_UDP is still
ambigious. I know it was used to keep compatibility for legacy guest. But
what's the future plan? Differentiate UFOv4 and UFOv6 in virtio features and
gso type in vnet header looks sufficient?

With the series, we can't send UFOv6 packet but legacy guest can. How about
fix the UFOv6 also in this series?

Thanks> Vladislav Yasevich (10):
>   core: Split out UFO6 support
>   net:  Correctly mark IPv6 UFO offload type.
>   ovs: Enable handling of UFO6 packets.
>   loopback: Turn on UFO6 support.
>   veth: Enable UFO6 support.
>   macvlan: Enable UFO6 support.
>   s2io: Enable UFO6 support.
>   tun: Re-uanble UFO support.
>   macvtap: Re-enable UFO support
>   Revert "drivers/net: Disable UFO through virtio"
> 
>  drivers/net/ethernet/neterion/s2io.c |  6 +++---
>  drivers/net/loopback.c               |  4 ++--
>  drivers/net/macvlan.c                |  2 +-
>  drivers/net/macvtap.c                | 20 ++++++++++++++------
>  drivers/net/tun.c                    | 26 ++++++++++++++------------
>  drivers/net/veth.c                   |  2 +-
>  drivers/net/virtio_net.c             | 24 ++++++++++--------------
>  include/linux/netdev_features.h      |  7 +++++--
>  include/linux/netdevice.h            |  1 +
>  include/linux/skbuff.h               |  1 +
>  net/core/dev.c                       | 35
>  +++++++++++++++++++----------------
>  net/core/ethtool.c                   |  2 +-
>  net/ipv6/ip6_offload.c               |  1 +
>  net/ipv6/ip6_output.c                |  4 ++--
>  net/ipv6/udp_offload.c               |  3 ++-
>  net/mpls/mpls_gso.c                  |  1 +
>  net/openvswitch/datapath.c           |  3 ++-
>  net/openvswitch/flow.c               |  2 +-
>  18 files changed, 81 insertions(+), 63 deletions(-)
> 
> --
> 1.9.3
> 
> _______________________________________________
> Virtualization mailing list
> Virtualization at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>

Jason Wang

2014-Dec-18 05:51 UTC

head link

[PATCH 08/10] tun: Re-uanble UFO support.

----- Original Message -----> Now that UFO is split into v4 and v6 parts, we can bring
> back v4 support without any trouble.
> 
> Continue to handle legacy applications by selecting the
> IPv6 fragment id but do not change the gso type.  Thist
> makes sure that two legacy VMs may still communicate.
> 
> Based on original work from Ben Hutchings.
> 
> Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
> CC: Ben Hutchings <ben at decadent.org.uk>
> Signed-off-by: Vladislav Yasevich <vyasevic at redhat.com>
> ---
>  drivers/net/tun.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 9dd3746..8c32fca 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -175,7 +175,7 @@ struct tun_struct {
>  	struct net_device	*dev;
>  	netdev_features_t	set_features;
>  #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
> -			  NETIF_F_TSO6)
> +			  NETIF_F_TSO6|NETIF_F_UFO)
>  
>  	int			vnet_hdr_sz;
>  	int			sndbuf;
> @@ -1152,20 +1152,15 @@ static ssize_t tun_get_user(struct tun_struct *tun,
> struct tun_file *tfile,
>  			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>  			break;
>  		case VIRTIO_NET_HDR_GSO_UDP:
> -		{
> -			static bool warned;
> -
> -			if (!warned) {
> -				warned = true;
> -				netdev_warn(tun->dev,
> -					    "%s: using disabled UFO feature; please fix this
program\n",
> -					    current->comm);
> -			}
>  			skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> -			if (skb->protocol == htons(ETH_P_IPV6))
> +			if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
This probably means UDPv6 with vlan does not work well, looks like another
independent fixe and also for stable.> +				/* This allows legacy application to work.
> +				 * Do not change the gso_type as it may
> +				 * not be upderstood by legacy applications.
> +				 */
Isn't this an issue that we use SKB_GSO_UDP for a UDPv6 packet? Especially
consider we want to fix UFOv6 in the future? We probably can use
SKB_GSO_UDP6 here and try to fix it in tun_put_user() if a legacy userspace
is detected.>  				ipv6_proxy_select_ident(skb);
Question still for vlan, is network header correctly set here? Looks like
ipv6_proxy_select_ident() depends on correct network header to
work.> +			}
>  			break;
> -		}
>  		default:
>  			tun->dev->stats.rx_frame_errors++;
>  			kfree_skb(skb);
> @@ -1273,6 +1268,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>  				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
>  			else if (sinfo->gso_type & SKB_GSO_TCPV6)
>  				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> +			else if (sinfo->gso_type & SKB_GSO_UDP)
> +				gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
>  			else {
>  				pr_err("unexpected GSO type: "
>  				       "0x%x, gso_size %d, hdr_len %d\n",
> @@ -1780,6 +1777,11 @@ static int set_offload(struct tun_struct *tun,
> unsigned long arg)
>  				features |= NETIF_F_TSO6;
>  			arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
>  		}
> +
> +		if (arg & TUN_F_UFO) {
> +			features |= NETIF_F_UFO;
> +			arg &= ~TUN_F_UFO;
> +		}
>  	}
>  
>  	/* This gives the user a way to test for new features in future by
> --
> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev"
in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Ben Hutchings

2014-Dec-24 18:11 UTC

head link

[PATCH 00/10] Split UFO into v4 and v6 versions.

On Thu, 2014-12-18 at 00:28 -0500, Jason Wang wrote:> 
> ----- Original Message -----
> > UFO support in the kernel applies to both IPv4 and IPv6 protocols
> > with the same device feature.  However some devices may not be able
> > to support one of the offloads.  For this we split the UFO offload
> > feature into 2 pieces.  NETIF_F_UFO now controlls the IPv4 part and
> > this series introduces NETIF_F_UFO6.
> > 
> > As a result of this work, we can now re-enable NETIF_F_UFO on
> > virtio_net devices and restore UDP over IPv4 performance for guests.
> > We also continue to support legacy guests that assume that UFO6
> > support included into UFO(4).
> > 
> > Without this work, migrating a guest to a 3.18 kernel fails.
> > 
> 
> This series eliminate the ambiguous NETIF_F_UFO.
> 
> But VIRTIO_NET_F_{HOST|GUEST}_UFO and VIRTIO_NET_F_HDR_GSO_UDP is still
> ambigious. I know it was used to keep compatibility for legacy guest. But
> what's the future plan? Differentiate UFOv4 and UFOv6 in virtio
features and
> gso type in vnet header looks sufficient?[...]

The IPv6 fragmentation ID needs to be added to the vnet header, to do
UFOv6 properly.  If it wasn't for that lack, we wouldn't have to split
the feature flag.

Ben.

-- 
Ben Hutchings
If more than one person is responsible for a bug, no one is at fault.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 811 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20141224/2119f037/attachment.sig>

Apparently Analagous Threads

Search for more possibly parallel threads

Virtualization - Dec 2014 - [PATCH 10/10] Revert "drivers/net: Disable UFO through virtio"

[PATCH 00/10] Split UFO into v4 and v6 versions.

[PATCH 01/10] core: Split out UFO6 support

[PATCH 02/10] net: Correctly mark IPv6 UFO offload type.

[PATCH 03/10] ovs: Enable handling of UFO6 packets.

[PATCH 04/10] loopback: Turn on UFO6 support.

[PATCH 05/10] veth: Enable UFO6 support.

[PATCH 06/10] macvlan: Enable UFO6 support.

[PATCH 07/10] s2io: Enable UFO6 support.

[PATCH 08/10] tun: Re-uanble UFO support.

[PATCH 09/10] macvtap: Re-enable UFO support

[PATCH 10/10] Revert "drivers/net: Disable UFO through virtio"

[PATCH 01/10] core: Split out UFO6 support

[PATCH 03/10] ovs: Enable handling of UFO6 packets.

[PATCH 03/10] ovs: Enable handling of UFO6 packets.

[PATCH 08/10] tun: Re-uanble UFO support.

[PATCH 09/10] macvtap: Re-enable UFO support

[PATCH 10/10] Revert "drivers/net: Disable UFO through virtio"

[PATCH 01/10] core: Split out UFO6 support

[PATCH 00/10] Split UFO into v4 and v6 versions.

[PATCH 08/10] tun: Re-uanble UFO support.

[PATCH 00/10] Split UFO into v4 and v6 versions.

Apparently Analagous Threads