[PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Paul Durrant]

This patch adds support for IPv6 checksum offload and GSO when those
features are available in the backend.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---

v2:
 - Addressed comments raised by Ian Campbell

 drivers/net/xen-netfront.c |  226 ++++++++++++++++++++++++++++++++++++++++----
 include/linux/ipv6.h       |    2 +
 2 files changed, 211 insertions(+), 17 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index dd1011e..afadfb5 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -616,7 +616,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		tx->flags |= XEN_NETTXF_extra_info;
 
 		gso->u.gso.size = skb_shinfo(skb)->gso_size;
-		gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
+		gso->u.gso.type = (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) ?
+			          XEN_NETIF_GSO_TYPE_TCPV6 :
+			          XEN_NETIF_GSO_TYPE_TCPV4;
 		gso->u.gso.pad = 0;
 		gso->u.gso.features = 0;
 
@@ -808,15 +810,18 @@ static int xennet_set_skb_gso(struct sk_buff *skb,
 		return -EINVAL;
 	}
 
-	/* Currently only TCPv4 S.O. is supported. */
-	if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) {
+	if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 &&
+	    gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) {
 		if (net_ratelimit())
 			pr_warn("Bad GSO type %d\n", gso->u.gso.type);
 		return -EINVAL;
 	}
 
 	skb_shinfo(skb)->gso_size = gso->u.gso.size;
-	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+	skb_shinfo(skb)->gso_type =
+		(gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ?
+		SKB_GSO_TCPV4 :
+		SKB_GSO_TCPV6;
 
 	/* Header must be checked, and gso_segs computed. */
 	skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
@@ -856,11 +861,34 @@ static RING_IDX xennet_fill_frags(struct netfront_info *np,
 	return cons;
 }
 
-static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
+static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int len)
 {
-	struct iphdr *iph;
-	int err = -EPROTO;
+	int target;
+
+	if (!skb_is_nonlinear(skb) || skb_headlen(skb) >= len)
+		return true;
+
+	/* If we need to pullup then pullup to the max, so we
+	 * won't need to do it again.
+	 */
+	target = min_t(int, skb->len, MAX_TCP_HEADER);
+	__pskb_pull_tail(skb, target - skb_headlen(skb));
+
+	if (skb_headlen(skb) < len) {
+		net_err_ratelimited("Failed to pullup packet header\n");
+		return false;    
+	}
+
+	return true;
+}
+
+static int checksum_setup_ip(struct net_device *dev, struct sk_buff *skb)
+{
+	struct iphdr *iph = (void *)skb->data;
+	unsigned int header_size;
+	unsigned int off;
 	int recalculate_partial_csum = 0;
+	int err = -EPROTO;
 
 	/*
 	 * A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
@@ -879,40 +907,56 @@ static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
 		return 0;
 
-	if (skb->protocol != htons(ETH_P_IP))
+	off = sizeof(struct iphdr);
+
+	header_size = skb->network_header + off;
+	if (!maybe_pull_tail(skb, header_size))
 		goto out;
 
-	iph = (void *)skb->data;
+	off = iph->ihl * 4;
 
 	switch (iph->protocol) {
 	case IPPROTO_TCP:
-		if (!skb_partial_csum_set(skb, 4 * iph->ihl,
+		if (!skb_partial_csum_set(skb, off,
 					  offsetof(struct tcphdr, check)))
 			goto out;
 
 		if (recalculate_partial_csum) {
 			struct tcphdr *tcph = tcp_hdr(skb);
+
+			header_size = skb->network_header +
+				off +
+				sizeof(struct tcphdr);
+			if (!maybe_pull_tail(skb, header_size))
+				goto out;
+
 			tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-							 skb->len - iph->ihl*4,
+							 skb->len - off,
 							 IPPROTO_TCP, 0);
 		}
 		break;
 	case IPPROTO_UDP:
-		if (!skb_partial_csum_set(skb, 4 * iph->ihl,
+		if (!skb_partial_csum_set(skb, off,
 					  offsetof(struct udphdr, check)))
 			goto out;
 
 		if (recalculate_partial_csum) {
 			struct udphdr *udph = udp_hdr(skb);
+
+			header_size = skb->network_header +
+				off +
+				sizeof(struct udphdr);
+			if (!maybe_pull_tail(skb, header_size))
+				goto out;
+
 			udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-							 skb->len - iph->ihl*4,
+							 skb->len - off,
 							 IPPROTO_UDP, 0);
 		}
 		break;
 	default:
-		if (net_ratelimit())
-			pr_err("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
-			       iph->protocol);
+		net_err_ratelimited("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
+				    iph->protocol);
 		goto out;
 	}
 
@@ -922,6 +966,122 @@ out:
 	return err;
 }
 
+static int checksum_setup_ipv6(struct net_device *dev, struct sk_buff *skb)
+{
+	struct ipv6hdr *ipv6h = (void *)skb->data;
+	u8 nexthdr;
+	unsigned int header_size;
+	unsigned int off;
+	bool fragment;
+	bool done;
+	int err = -EPROTO;
+
+	done = false;
+
+	/* A non-CHECKSUM_PARTIAL SKB does not require setup. */
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		return 0;
+
+	off = sizeof(struct ipv6hdr);
+
+	header_size = skb->network_header + off;
+	if (!maybe_pull_tail(skb, header_size))
+		goto out;
+
+	nexthdr = ipv6h->nexthdr;
+
+	while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len)) &&
+	       !done) {
+		switch (nexthdr) {
+		case IPPROTO_DSTOPTS:
+		case IPPROTO_HOPOPTS:
+		case IPPROTO_ROUTING: {
+			struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
+
+			header_size = skb->network_header +
+				off +
+				sizeof(struct ipv6_opt_hdr);
+			if (!maybe_pull_tail(skb, header_size))
+				goto out;
+
+			nexthdr = hp->nexthdr;
+			off += ipv6_optlen(hp);
+			break;
+		}
+		case IPPROTO_AH: {
+			struct ip_auth_hdr *hp = (void *)(skb->data + off);
+
+			header_size = skb->network_header +
+				off +
+				sizeof(struct ip_auth_hdr);
+			if (!maybe_pull_tail(skb, header_size))
+				goto out;
+
+			nexthdr = hp->nexthdr;
+			off += ipv6_ahlen(hp);
+			break;
+		}
+		case IPPROTO_FRAGMENT:
+			fragment = true;
+			/* fall through */
+		default:
+			done = true;
+			break;
+		}
+	}
+
+	if (!done) {
+		net_err_ratelimited("Failed to parse packet header\n");
+		goto out;
+	}
+
+	if (fragment) {
+		net_err_ratelimited("Packet is a fragment!\n");
+		goto out;
+	}
+
+	switch (nexthdr) {
+	case IPPROTO_TCP:
+		if (!skb_partial_csum_set(skb, off,
+					  offsetof(struct tcphdr, check)))
+			goto out;
+		break;
+	case IPPROTO_UDP:
+		if (!skb_partial_csum_set(skb, off,
+					  offsetof(struct udphdr, check)))
+			goto out;
+		break;
+	default:
+		net_err_ratelimited("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
+				    nexthdr);
+		goto out;
+	}
+
+	err = 0;
+
+out:
+	return err;
+}
+
+static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
+{
+	int err;
+
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		err = checksum_setup_ip(dev, skb);
+		break;
+	case htons(ETH_P_IPV6):
+		err = checksum_setup_ipv6(dev, skb);
+		break;
+	default:
+		err = -EPROTO;
+		break;
+	}
+
+	return err;
+}
+
 static int handle_incoming_queue(struct net_device *dev,
 				 struct sk_buff_head *rxq)
 {
@@ -1232,6 +1392,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
 			features &= ~NETIF_F_SG;
 	}
 
+	if (features & NETIF_F_IPV6_CSUM) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
+				 "feature-ipv6-csum-offload", "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_IPV6_CSUM;
+	}
+
 	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
@@ -1241,6 +1410,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
 			features &= ~NETIF_F_TSO;
 	}
 
+	if (features & NETIF_F_TSO6) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
+				 "feature-gso-tcpv6", "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_TSO6;
+	}
+
 	return features;
 }
 
@@ -1373,7 +1551,9 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
 	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
 				  NETIF_F_GSO_ROBUST;
-	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
+	netdev->hw_features	= NETIF_F_SG | 
+		                  NETIF_F_IPV6_CSUM |
+		                  NETIF_F_TSO | NETIF_F_TSO6;
 
 	/*
          * Assume that all hw features are available for now. This set
@@ -1751,6 +1931,18 @@ again:
 		goto abort_transaction;
 	}
 
+	err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", "%d", 1);
+	if (err) {
+		message = "writing feature-gso-tcpv6";
+		goto abort_transaction;
+	}
+
+	err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-offload", "%d", 1);
+	if (err) {
+		message = "writing feature-gso-tcpv6";
+		goto abort_transaction;
+	}
+
 	err = xenbus_transaction_end(xbt, 0);
 	if (err) {
 		if (err == -EAGAIN)
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5d89d1b..10f1b03 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -4,6 +4,8 @@
 #include <uapi/linux/ipv6.h>
 
 #define ipv6_optlen(p)  (((p)->hdrlen+1) << 3)
+#define ipv6_ahlen(p)   (((p)->hdrlen+2) << 2);
+
 /*
  * This structure contains configuration options per IPv6 link.
  */
-- 
1.7.10.4

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Konrad Rzeszutek Wilk]

On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> This patch adds support for IPv6 checksum offload and GSO when those
> features are available in the backend.

Wei, Annie, thoughts?
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
> 
> v2:
>  - Addressed comments raised by Ian Campbell
> 
>  drivers/net/xen-netfront.c |  226 ++++++++++++++++++++++++++++++++++++++++----
>  include/linux/ipv6.h       |    2 +
>  2 files changed, 211 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index dd1011e..afadfb5 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -616,7 +616,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  		tx->flags |= XEN_NETTXF_extra_info;
>  
>  		gso->u.gso.size = skb_shinfo(skb)->gso_size;
> -		gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
> +		gso->u.gso.type = (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) ?
> +			          XEN_NETIF_GSO_TYPE_TCPV6 :
> +			          XEN_NETIF_GSO_TYPE_TCPV4;
>  		gso->u.gso.pad = 0;
>  		gso->u.gso.features = 0;
>  
> @@ -808,15 +810,18 @@ static int xennet_set_skb_gso(struct sk_buff *skb,
>  		return -EINVAL;
>  	}
>  
> -	/* Currently only TCPv4 S.O. is supported. */
> -	if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) {
> +	if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 &&
> +	    gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) {
>  		if (net_ratelimit())
>  			pr_warn("Bad GSO type %d\n", gso->u.gso.type);
>  		return -EINVAL;
>  	}
>  
>  	skb_shinfo(skb)->gso_size = gso->u.gso.size;
> -	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> +	skb_shinfo(skb)->gso_type =
> +		(gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ?
> +		SKB_GSO_TCPV4 :
> +		SKB_GSO_TCPV6;
>  
>  	/* Header must be checked, and gso_segs computed. */
>  	skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
> @@ -856,11 +861,34 @@ static RING_IDX xennet_fill_frags(struct netfront_info *np,
>  	return cons;
>  }
>  
> -static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
> +static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int len)
>  {
> -	struct iphdr *iph;
> -	int err = -EPROTO;
> +	int target;
> +
> +	if (!skb_is_nonlinear(skb) || skb_headlen(skb) >= len)
> +		return true;
> +
> +	/* If we need to pullup then pullup to the max, so we
> +	 * won't need to do it again.
> +	 */
> +	target = min_t(int, skb->len, MAX_TCP_HEADER);
> +	__pskb_pull_tail(skb, target - skb_headlen(skb));
> +
> +	if (skb_headlen(skb) < len) {
> +		net_err_ratelimited("Failed to pullup packet header\n");
> +		return false;    
> +	}
> +
> +	return true;
> +}
> +
> +static int checksum_setup_ip(struct net_device *dev, struct sk_buff *skb)
> +{
> +	struct iphdr *iph = (void *)skb->data;
> +	unsigned int header_size;
> +	unsigned int off;
>  	int recalculate_partial_csum = 0;
> +	int err = -EPROTO;
>  
>  	/*
>  	 * A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
> @@ -879,40 +907,56 @@ static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
>  	if (skb->ip_summed != CHECKSUM_PARTIAL)
>  		return 0;
>  
> -	if (skb->protocol != htons(ETH_P_IP))
> +	off = sizeof(struct iphdr);
> +
> +	header_size = skb->network_header + off;
> +	if (!maybe_pull_tail(skb, header_size))
>  		goto out;
>  
> -	iph = (void *)skb->data;
> +	off = iph->ihl * 4;
>  
>  	switch (iph->protocol) {
>  	case IPPROTO_TCP:
> -		if (!skb_partial_csum_set(skb, 4 * iph->ihl,
> +		if (!skb_partial_csum_set(skb, off,
>  					  offsetof(struct tcphdr, check)))
>  			goto out;
>  
>  		if (recalculate_partial_csum) {
>  			struct tcphdr *tcph = tcp_hdr(skb);
> +
> +			header_size = skb->network_header +
> +				off +
> +				sizeof(struct tcphdr);
> +			if (!maybe_pull_tail(skb, header_size))
> +				goto out;
> +
>  			tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
> -							 skb->len - iph->ihl*4,
> +							 skb->len - off,
>  							 IPPROTO_TCP, 0);
>  		}
>  		break;
>  	case IPPROTO_UDP:
> -		if (!skb_partial_csum_set(skb, 4 * iph->ihl,
> +		if (!skb_partial_csum_set(skb, off,
>  					  offsetof(struct udphdr, check)))
>  			goto out;
>  
>  		if (recalculate_partial_csum) {
>  			struct udphdr *udph = udp_hdr(skb);
> +
> +			header_size = skb->network_header +
> +				off +
> +				sizeof(struct udphdr);
> +			if (!maybe_pull_tail(skb, header_size))
> +				goto out;
> +
>  			udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
> -							 skb->len - iph->ihl*4,
> +							 skb->len - off,
>  							 IPPROTO_UDP, 0);
>  		}
>  		break;
>  	default:
> -		if (net_ratelimit())
> -			pr_err("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
> -			       iph->protocol);
> +		net_err_ratelimited("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
> +				    iph->protocol);
>  		goto out;
>  	}
>  
> @@ -922,6 +966,122 @@ out:
>  	return err;
>  }
>  
> +static int checksum_setup_ipv6(struct net_device *dev, struct sk_buff *skb)
> +{
> +	struct ipv6hdr *ipv6h = (void *)skb->data;
> +	u8 nexthdr;
> +	unsigned int header_size;
> +	unsigned int off;
> +	bool fragment;
> +	bool done;
> +	int err = -EPROTO;
> +
> +	done = false;
> +
> +	/* A non-CHECKSUM_PARTIAL SKB does not require setup. */
> +	if (skb->ip_summed != CHECKSUM_PARTIAL)
> +		return 0;
> +
> +	off = sizeof(struct ipv6hdr);
> +
> +	header_size = skb->network_header + off;
> +	if (!maybe_pull_tail(skb, header_size))
> +		goto out;
> +
> +	nexthdr = ipv6h->nexthdr;
> +
> +	while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len)) &&
> +	       !done) {
> +		switch (nexthdr) {
> +		case IPPROTO_DSTOPTS:
> +		case IPPROTO_HOPOPTS:
> +		case IPPROTO_ROUTING: {
> +			struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
> +
> +			header_size = skb->network_header +
> +				off +
> +				sizeof(struct ipv6_opt_hdr);
> +			if (!maybe_pull_tail(skb, header_size))
> +				goto out;
> +
> +			nexthdr = hp->nexthdr;
> +			off += ipv6_optlen(hp);
> +			break;
> +		}
> +		case IPPROTO_AH: {
> +			struct ip_auth_hdr *hp = (void *)(skb->data + off);
> +
> +			header_size = skb->network_header +
> +				off +
> +				sizeof(struct ip_auth_hdr);
> +			if (!maybe_pull_tail(skb, header_size))
> +				goto out;
> +
> +			nexthdr = hp->nexthdr;
> +			off += ipv6_ahlen(hp);
> +			break;
> +		}
> +		case IPPROTO_FRAGMENT:
> +			fragment = true;
> +			/* fall through */
> +		default:
> +			done = true;
> +			break;
> +		}
> +	}
> +
> +	if (!done) {
> +		net_err_ratelimited("Failed to parse packet header\n");
> +		goto out;
> +	}
> +
> +	if (fragment) {
> +		net_err_ratelimited("Packet is a fragment!\n");
> +		goto out;
> +	}
> +
> +	switch (nexthdr) {
> +	case IPPROTO_TCP:
> +		if (!skb_partial_csum_set(skb, off,
> +					  offsetof(struct tcphdr, check)))
> +			goto out;
> +		break;
> +	case IPPROTO_UDP:
> +		if (!skb_partial_csum_set(skb, off,
> +					  offsetof(struct udphdr, check)))
> +			goto out;
> +		break;
> +	default:
> +		net_err_ratelimited("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
> +				    nexthdr);
> +		goto out;
> +	}
> +
> +	err = 0;
> +
> +out:
> +	return err;
> +}
> +
> +static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
> +{
> +	int err;
> +
> +	switch (skb->protocol) {
> +	case htons(ETH_P_IP):
> +		err = checksum_setup_ip(dev, skb);
> +		break;
> +	case htons(ETH_P_IPV6):
> +		err = checksum_setup_ipv6(dev, skb);
> +		break;
> +	default:
> +		err = -EPROTO;
> +		break;
> +	}
> +
> +	return err;
> +}
> +
>  static int handle_incoming_queue(struct net_device *dev,
>  				 struct sk_buff_head *rxq)
>  {
> @@ -1232,6 +1392,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
>  			features &= ~NETIF_F_SG;
>  	}
>  
> +	if (features & NETIF_F_IPV6_CSUM) {
> +		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> +				 "feature-ipv6-csum-offload", "%d", &val) < 0)
> +			val = 0;
> +
> +		if (!val)
> +			features &= ~NETIF_F_IPV6_CSUM;
> +	}
> +
>  	if (features & NETIF_F_TSO) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>  				 "feature-gso-tcpv4", "%d", &val) < 0)
> @@ -1241,6 +1410,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
>  			features &= ~NETIF_F_TSO;
>  	}
>  
> +	if (features & NETIF_F_TSO6) {
> +		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> +				 "feature-gso-tcpv6", "%d", &val) < 0)
> +			val = 0;
> +
> +		if (!val)
> +			features &= ~NETIF_F_TSO6;
> +	}
> +
>  	return features;
>  }
>  
> @@ -1373,7 +1551,9 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
>  	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
>  	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
>  				  NETIF_F_GSO_ROBUST;
> -	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
> +	netdev->hw_features	= NETIF_F_SG | 
> +		                  NETIF_F_IPV6_CSUM |
> +		                  NETIF_F_TSO | NETIF_F_TSO6;
>  
>  	/*
>           * Assume that all hw features are available for now. This set
> @@ -1751,6 +1931,18 @@ again:
>  		goto abort_transaction;
>  	}
>  
> +	err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", "%d", 1);
> +	if (err) {
> +		message = "writing feature-gso-tcpv6";
> +		goto abort_transaction;
> +	}
> +
> +	err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-offload", "%d", 1);
> +	if (err) {
> +		message = "writing feature-gso-tcpv6";
> +		goto abort_transaction;
> +	}
> +
>  	err = xenbus_transaction_end(xbt, 0);
>  	if (err) {
>  		if (err == -EAGAIN)
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index 5d89d1b..10f1b03 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -4,6 +4,8 @@
>  #include <uapi/linux/ipv6.h>
>  
>  #define ipv6_optlen(p)  (((p)->hdrlen+1) << 3)
> +#define ipv6_ahlen(p)   (((p)->hdrlen+2) << 2);
> +
>  /*
>   * This structure contains configuration options per IPv6 link.
>   */
> -- 
> 1.7.10.4
> 

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Wei Liu]

On Mon, Nov 25, 2013 at 10:19:26AM -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> > This patch adds support for IPv6 checksum offload and GSO when those
> > features are available in the backend.
> 
> Wei, Annie, thoughts?

It looks mostly the same as the backend code so I don't have much to
say. But I suppose there will be a V3 to have a macro called
IPV6_MAX_HEADER, if I understand this thread correctly.

Wei.

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 25 November 2013 15:37
> To: Konrad Rzeszutek Wilk
> Cc: Paul Durrant; annie li; Wei Liu; xen-devel@lists.xen.org; Boris Ostrovsky;
> David Vrabel; Ian Campbell
> Subject: Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads
> 
> On Mon, Nov 25, 2013 at 10:19:26AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> > > This patch adds support for IPv6 checksum offload and GSO when those
> > > features are available in the backend.
> >
> > Wei, Annie, thoughts?
> 
> It looks mostly the same as the backend code so I don't have much to
> say. But I suppose there will be a V3 to have a macro called
> IPV6_MAX_HEADER, if I understand this thread correctly.
> 

There will. I'll try to post some time this week.

  Paul

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Wei Liu]

On Mon, Nov 25, 2013 at 03:37:23PM +0000, Wei Liu wrote:
> On Mon, Nov 25, 2013 at 10:19:26AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> > > This patch adds support for IPv6 checksum offload and GSO when those
> > > features are available in the backend.
> > 
> > Wei, Annie, thoughts?
> 
> It looks mostly the same as the backend code so I don't have much to
> say. But I suppose there will be a V3 to have a macro called
> IPV6_MAX_HEADER, if I understand this thread correctly.

Paul, if you're going to do this, does it make sense to have that macro
in Xen's header? Backend and frontend both the same constant.

Wei.

> 
> Wei.

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 25 November 2013 15:53
> To: Konrad Rzeszutek Wilk
> Cc: Paul Durrant; annie li; Wei Liu; xen-devel@lists.xen.org; Boris Ostrovsky;
> David Vrabel; Ian Campbell
> Subject: Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads
> 
> On Mon, Nov 25, 2013 at 03:37:23PM +0000, Wei Liu wrote:
> > On Mon, Nov 25, 2013 at 10:19:26AM -0500, Konrad Rzeszutek Wilk wrote:
> > > On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> > > > This patch adds support for IPv6 checksum offload and GSO when those
> > > > features are available in the backend.
> > >
> > > Wei, Annie, thoughts?
> >
> > It looks mostly the same as the backend code so I don't have much to
> > say. But I suppose there will be a V3 to have a macro called
> > IPV6_MAX_HEADER, if I understand this thread correctly.
> 
> Paul, if you're going to do this, does it make sense to have that macro
> in Xen's header? Backend and frontend both the same constant.
> 

The header parse and checksum offset code should be core IMO. I think that's where the #define should go. I'll file patch to pull out the common code from netfront and netback later. For the moment I'll keep the definition local.

  Paul

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[annie li]

On 2013/11/25 23:19, Konrad Rzeszutek Wilk wrote:
> On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
>> This patch adds support for IPv6 checksum offload and GSO when those
>> features are available in the backend.
> Wei, Annie, thoughts?

Seems fine except for following two comments :-)

>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: David Vrabel <david.vrabel@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> ---
>>
>> v2:
>>   - Addressed comments raised by Ian Campbell
>>
>>   drivers/net/xen-netfront.c |  226 ++++++++++++++++++++++++++++++++++++++++----
>>   include/linux/ipv6.h       |    2 +
>>   2 files changed, 211 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> index dd1011e..afadfb5 100644
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
>> @@ -616,7 +616,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>   		tx->flags |= XEN_NETTXF_extra_info;
>>   
>>   		gso->u.gso.size = skb_shinfo(skb)->gso_size;
>> -		gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
>> +		gso->u.gso.type = (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) ?
>> +			          XEN_NETIF_GSO_TYPE_TCPV6 :
>> +			          XEN_NETIF_GSO_TYPE_TCPV4;
>>   		gso->u.gso.pad = 0;
>>   		gso->u.gso.features = 0;

...snip...

>>   static int handle_incoming_queue(struct net_device *dev,
>>   				 struct sk_buff_head *rxq)
>>   {
>> @@ -1232,6 +1392,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
>>   			features &= ~NETIF_F_SG;
>>   	}
>>   
>> +	if (features & NETIF_F_IPV6_CSUM) {
>> +		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>> +				 "feature-ipv6-csum-offload", "%d", &val) < 0)
>> +			val = 0;
>> +
>> +		if (!val)
>> +			features &= ~NETIF_F_IPV6_CSUM;
>> +	}
>> +
>>   	if (features & NETIF_F_TSO) {
>>   		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>>   				 "feature-gso-tcpv4", "%d", &val) < 0)
>> @@ -1241,6 +1410,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
>>   			features &= ~NETIF_F_TSO;
>>   	}
>>   
>> +	if (features & NETIF_F_TSO6) {
>> +		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>> +				 "feature-gso-tcpv6", "%d", &val) < 0)
>> +			val = 0;
>> +
>> +		if (!val)
>> +			features &= ~NETIF_F_TSO6;
>> +	}
>> +
>>   	return features;
>>   }
>>   
>> @@ -1373,7 +1551,9 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
>>   	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
>>   	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
>>   				  NETIF_F_GSO_ROBUST;
>> -	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
>> +	netdev->hw_features	= NETIF_F_SG |
>> +		                  NETIF_F_IPV6_CSUM |

Are you missing NETIF_F_IP_CSUM here?

>> +		                  NETIF_F_TSO | NETIF_F_TSO6;
>>   
>>   	/*
>>            * Assume that all hw features are available for now. This set
>> @@ -1751,6 +1931,18 @@ again:
>>   		goto abort_transaction;
>>   	}
>>   
>> +	err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", "%d", 1);
>> +	if (err) {
>> +		message = "writing feature-gso-tcpv6";
>> +		goto abort_transaction;
>> +	}
>> +
>> +	err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-offload", "%d", 1);
>> +	if (err) {
>> +		message = "writing feature-gso-tcpv6";

Change "feature-gso-tcpv6" to "feature-ipv6-csum-offload"?

Thanks
Annie

Re: [PATCH net-next v2] xen-netfront: Add support for IPv6 offloads

[Paul Durrant]

> -----Original Message-----
> From: annie li [mailto:annie.li@oracle.com]
> Sent: 26 November 2013 01:58
> To: Konrad Rzeszutek Wilk
> Cc: Paul Durrant; Wei Liu; Boris Ostrovsky; David Vrabel; Ian Campbell; xen-
> devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH net-next v2] xen-netfront: Add support for
> IPv6 offloads
> 
> 
> On 2013/11/25 23:19, Konrad Rzeszutek Wilk wrote:
> > On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> >> This patch adds support for IPv6 checksum offload and GSO when those
> >> features are available in the backend.
> > Wei, Annie, thoughts?
> 
> Seems fine except for following two comments :-)
> 
> >> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> >> Cc: David Vrabel <david.vrabel@citrix.com>
> >> Cc: Ian Campbell <ian.campbell@citrix.com>
> >> ---
> >>
> >> v2:
> >>   - Addressed comments raised by Ian Campbell
> >>
> >>   drivers/net/xen-netfront.c |  226
> ++++++++++++++++++++++++++++++++++++++++----
> >>   include/linux/ipv6.h       |    2 +
> >>   2 files changed, 211 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >> index dd1011e..afadfb5 100644
> >> --- a/drivers/net/xen-netfront.c
> >> +++ b/drivers/net/xen-netfront.c
> >> @@ -616,7 +616,9 @@ static int xennet_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> >>   		tx->flags |= XEN_NETTXF_extra_info;
> >>
> >>   		gso->u.gso.size = skb_shinfo(skb)->gso_size;
> >> -		gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
> >> +		gso->u.gso.type = (skb_shinfo(skb)->gso_type &
> SKB_GSO_TCPV6) ?
> >> +			          XEN_NETIF_GSO_TYPE_TCPV6 :
> >> +			          XEN_NETIF_GSO_TYPE_TCPV4;
> >>   		gso->u.gso.pad = 0;
> >>   		gso->u.gso.features = 0;
> 
> ...snip...
> 
> >>   static int handle_incoming_queue(struct net_device *dev,
> >>   				 struct sk_buff_head *rxq)
> >>   {
> >> @@ -1232,6 +1392,15 @@ static netdev_features_t
> xennet_fix_features(struct net_device *dev,
> >>   			features &= ~NETIF_F_SG;
> >>   	}
> >>
> >> +	if (features & NETIF_F_IPV6_CSUM) {
> >> +		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> >> +				 "feature-ipv6-csum-offload", "%d", &val) <
> 0)
> >> +			val = 0;
> >> +
> >> +		if (!val)
> >> +			features &= ~NETIF_F_IPV6_CSUM;
> >> +	}
> >> +
> >>   	if (features & NETIF_F_TSO) {
> >>   		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> >>   				 "feature-gso-tcpv4", "%d", &val) < 0)
> >> @@ -1241,6 +1410,15 @@ static netdev_features_t
> xennet_fix_features(struct net_device *dev,
> >>   			features &= ~NETIF_F_TSO;
> >>   	}
> >>
> >> +	if (features & NETIF_F_TSO6) {
> >> +		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
> >> +				 "feature-gso-tcpv6", "%d", &val) < 0)
> >> +			val = 0;
> >> +
> >> +		if (!val)
> >> +			features &= ~NETIF_F_TSO6;
> >> +	}
> >> +
> >>   	return features;
> >>   }
> >>
> >> @@ -1373,7 +1551,9 @@ static struct net_device
> *xennet_create_dev(struct xenbus_device *dev)
> >>   	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> >>   	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> >>   				  NETIF_F_GSO_ROBUST;
> >> -	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG |
> NETIF_F_TSO;
> >> +	netdev->hw_features	= NETIF_F_SG |
> >> +		                  NETIF_F_IPV6_CSUM |
> 
> Are you missing NETIF_F_IP_CSUM here?
> 

Unfortunately the context didn't quite stretch that far, but just below there is a line which ORs netdev->features and netdev->hw_features together so that fact that NETIF_F_IP_CSUM is in netdev->features is sufficient and its presence in hw_features seems incorrect since hw_features is the set that can be modified by xenstore negotiation and, while it's possible to disable checksum by negotiation, netfront doesn't do that.

> >> +		                  NETIF_F_TSO | NETIF_F_TSO6;
> >>
> >>   	/*
> >>            * Assume that all hw features are available for now. This set
> >> @@ -1751,6 +1931,18 @@ again:
> >>   		goto abort_transaction;
> >>   	}
> >>
> >> +	err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6",
> "%d", 1);
> >> +	if (err) {
> >> +		message = "writing feature-gso-tcpv6";
> >> +		goto abort_transaction;
> >> +	}
> >> +
> >> +	err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-
> offload", "%d", 1);
> >> +	if (err) {
> >> +		message = "writing feature-gso-tcpv6";
> 
> Change "feature-gso-tcpv6" to "feature-ipv6-csum-offload"?
> 

Good catch! Thanks,

  Paul

> Thanks
> Annie

[PATCH net-next v2] xen-netfront: add support for IPv6 offloads

[Paul Durrant]

This patch adds support for IPv6 checksum offload and GSO when those
features are available in the backend.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
v2:
- Use xenbus_write rather than xenbus_printf

 drivers/net/xen-netfront.c |   48 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index c41537b..d7bee8a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -617,7 +617,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		tx->flags |= XEN_NETTXF_extra_info;
 
 		gso->u.gso.size = skb_shinfo(skb)->gso_size;
-		gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4;
+		gso->u.gso.type = (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) ?
+			XEN_NETIF_GSO_TYPE_TCPV6 :
+			XEN_NETIF_GSO_TYPE_TCPV4;
 		gso->u.gso.pad = 0;
 		gso->u.gso.features = 0;
 
@@ -809,15 +811,18 @@ static int xennet_set_skb_gso(struct sk_buff *skb,
 		return -EINVAL;
 	}
 
-	/* Currently only TCPv4 S.O. is supported. */
-	if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) {
+	if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 &&
+	    gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) {
 		if (net_ratelimit())
 			pr_warn("Bad GSO type %d\n", gso->u.gso.type);
 		return -EINVAL;
 	}
 
 	skb_shinfo(skb)->gso_size = gso->u.gso.size;
-	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+	skb_shinfo(skb)->gso_type =
+		(gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ?
+		SKB_GSO_TCPV4 :
+		SKB_GSO_TCPV6;
 
 	/* Header must be checked, and gso_segs computed. */
 	skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
@@ -1191,6 +1196,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
 			features &= ~NETIF_F_SG;
 	}
 
+	if (features & NETIF_F_IPV6_CSUM) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
+				 "feature-ipv6-csum-offload", "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_IPV6_CSUM;
+	}
+
 	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
@@ -1200,6 +1214,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
 			features &= ~NETIF_F_TSO;
 	}
 
+	if (features & NETIF_F_TSO6) {
+		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
+				 "feature-gso-tcpv6", "%d", &val) < 0)
+			val = 0;
+
+		if (!val)
+			features &= ~NETIF_F_TSO6;
+	}
+
 	return features;
 }
 
@@ -1338,7 +1361,9 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
 	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
 				  NETIF_F_GSO_ROBUST;
-	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
+	netdev->hw_features	= NETIF_F_SG |
+				  NETIF_F_IPV6_CSUM |
+				  NETIF_F_TSO | NETIF_F_TSO6;
 
 	/*
          * Assume that all hw features are available for now. This set
@@ -1716,6 +1741,19 @@ again:
 		goto abort_transaction;
 	}
 
+	err = xenbus_write(xbt, dev->nodename, "feature-gso-tcpv6", "1");
+	if (err) {
+		message = "writing feature-gso-tcpv6";
+		goto abort_transaction;
+	}
+
+	err = xenbus_write(xbt, dev->nodename, "feature-ipv6-csum-offload",
+			   "1");
+	if (err) {
+		message = "writing feature-ipv6-csum-offload";
+		goto abort_transaction;
+	}
+
 	err = xenbus_transaction_end(xbt, 0);
 	if (err) {
 		if (err == -EAGAIN)
-- 
1.7.10.4

Re: [PATCH net-next v2] xen-netfront: add support for IPv6 offloads

[David Miller]

From: Paul Durrant <paul.durrant@citrix.com>
Date: Wed, 15 Jan 2014 17:30:33 +0000

> This patch adds support for IPv6 checksum offload and GSO when those
> features are available in the backend.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Applied.