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

[annie li <annie.li@oracle.com>]

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