[PATCH net 0/3] Fix issues with Heper-V network offload code

[PATCH net 0/3] Fix issues with Heper-V network offload code

[K. Y. Srinivasan]

WS2008 R2 does not support udp checksum offload. Furthermore, ws2012 and
ws2012 r2 have issues offloading udp checksum from Linux guests.
This patch-set addresses these issues as well as other bug fixes.
Please apply.

K. Y. Srinivasan (3):
  Drivers: net: hyperv: Allocate memory for all possible per-pecket
    information
  Drivers: net: hyperv: Negotiate suitable ndis version for offload
    support
  Drivers: net: hyperv: Address UDP checksum issues

 drivers/net/hyperv/hyperv_net.h   |    1 +
 drivers/net/hyperv/netvsc.c       |    2 +-
 drivers/net/hyperv/netvsc_drv.c   |   23 +++++++++++++++++++++--
 drivers/net/hyperv/rndis_filter.c |   12 +++++++++++-
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
1.7.4.1

[PATCH net 0/3] Fix issues with Heper-V network offload code

[K. Y. Srinivasan]

WS2008 R2 does not support udp checksum offload. Furthermore, ws2012 and
ws2012 r2 have issues offloading udp checksum from Linux guests.
This patch-set addresses these issues as well as other bug fixes.
Please apply.

K. Y. Srinivasan (3):
  Drivers: net: hyperv: Allocate memory for all possible per-pecket
    information
  Drivers: net: hyperv: Negotiate suitable ndis version for offload
    support
  Drivers: net: hyperv: Address UDP checksum issues

 drivers/net/hyperv/hyperv_net.h   |    1 +
 drivers/net/hyperv/netvsc.c       |    2 +-
 drivers/net/hyperv/netvsc_drv.c   |   23 +++++++++++++++++++++--
 drivers/net/hyperv/rndis_filter.c |   12 +++++++++++-
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
1.7.4.1

[PATCH net 1/3] Drivers: net: hyperv: Allocate memory for all possible per-pecket information

[K. Y. Srinivasan]

An outgoing packet can potentially need per-packet information for
all the offloads and VLAN tagging. Fix this issue.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 4e4cf9e..6f39baa 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -319,7 +319,9 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	packet = kzalloc(sizeof(struct hv_netvsc_packet) +
 			 (num_data_pgs * sizeof(struct hv_page_buffer)) +
 			 sizeof(struct rndis_message) +
-			 NDIS_VLAN_PPI_SIZE, GFP_ATOMIC);
+			 NDIS_VLAN_PPI_SIZE +
+			 NDIS_CSUM_PPI_SIZE +
+			 NDIS_LSO_PPI_SIZE, GFP_ATOMIC);
 	if (!packet) {
 		/* out of memory, drop packet */
 		netdev_err(net, "unable to allocate hv_netvsc_packet\n");
-- 
1.7.4.1

[PATCH net 2/3] Drivers: net: hyperv: Negotiate suitable ndis version for offload support

[K. Y. Srinivasan]

Ws2008R2 supports ndis_version 6.1 and 6.1 is the minimal version required
for various offloads. Negotiate ndis_version 6.1 when on ws2008r2.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index daddea2..f7629ec 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -344,7 +344,7 @@ static int netvsc_connect_vsp(struct hv_device *device)
 	memset(init_packet, 0, sizeof(struct nvsp_message));
 
 	if (net_device->nvsp_version <= NVSP_PROTOCOL_VERSION_4)
-		ndis_version = 0x00050001;
+		ndis_version = 0x00060001;
 	else
 		ndis_version = 0x0006001e;
 
-- 
1.7.4.1

[PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum issues

[K. Y. Srinivasan]

ws2008r2 does not support UDP checksum offload. Thus, we cannnot turn on
UDP offload in the host. Also, on ws2012 and ws2012 r2, there appear to be
an issue with UDP checksum offload.
Fix this issue by computing the UDP checksum in the Hyper-V driver.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |    1 +
 drivers/net/hyperv/netvsc_drv.c   |   19 ++++++++++++++++++-
 drivers/net/hyperv/rndis_filter.c |   12 +++++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 13010b4..d18f711d 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -747,6 +747,7 @@ struct ndis_oject_header {
 #define NDIS_TCP_LARGE_SEND_OFFLOAD_IPV4	0
 #define NDIS_TCP_LARGE_SEND_OFFLOAD_IPV6	1
 
+#define VERSION_4_OFFLOAD_SIZE			22
 /*
  * New offload OIDs for NDIS 6
  */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 6f39baa..8e3919b 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -398,7 +398,24 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 		csum_info->transmit.tcp_checksum = 1;
 		csum_info->transmit.tcp_header_offset = hdr_offset;
 	} else if (net_trans_info & INFO_UDP) {
-		csum_info->transmit.udp_checksum = 1;
+		/* UDP checksum offload is not supported on ws2008r2.
+		 * Furthermore, on ws2012 and ws2012r2, there are some
+		 * issues with udp checksum offload from Linux guests.
+		 * (these are host issues).
+		 * For now compute the checksum here.
+		 */
+		struct udphdr *uh = udp_hdr(skb);
+		u16 udp_len = ntohs(uh->len);
+
+		uh->check = 0;
+		uh->check = csum_tcpudp_magic(ip_hdr(skb)->saddr,
+					      ip_hdr(skb)->daddr,
+					      udp_len, IPPROTO_UDP,
+					      csum_partial(uh, udp_len, 0));
+		if (uh->check == 0)
+			uh->check = CSUM_MANGLED_0;
+
+		csum_info->transmit.udp_checksum = 0;
 	}
 	goto do_send;
 
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 4a37e3d..143a98c 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -641,6 +641,16 @@ int rndis_filter_set_offload_params(struct hv_device *hdev,
 	struct rndis_set_complete *set_complete;
 	u32 extlen = sizeof(struct ndis_offload_params);
 	int ret, t;
+	u32 vsp_version = nvdev->nvsp_version;
+
+	if (vsp_version <= NVSP_PROTOCOL_VERSION_4) {
+		extlen = VERSION_4_OFFLOAD_SIZE;
+		/* On NVSP_PROTOCOL_VERSION_4 and below, we do not support
+		 * UDP checksum offload.
+		 */
+		req_offloads->udp_ip_v4_csum = 0;
+		req_offloads->udp_ip_v6_csum = 0;
+	}
 
 	request = get_rndis_request(rdev, RNDIS_MSG_SET,
 		RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen);
@@ -674,7 +684,7 @@ int rndis_filter_set_offload_params(struct hv_device *hdev,
 	} else {
 		set_complete = &request->response_msg.msg.set_complete;
 		if (set_complete->status != RNDIS_STATUS_SUCCESS) {
-			netdev_err(ndev, "Fail to set MAC on host side:0x%x\n",
+			netdev_err(ndev, "Fail to set offload on host side:0x%x\n",
 				   set_complete->status);
 			ret = -EINVAL;
 		}
-- 
1.7.4.1

Re: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum issues

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Tue,  8 Apr 2014 12:46:11 -0700

>  	} else if (net_trans_info & INFO_UDP) {
> -		csum_info->transmit.udp_checksum = 1;
> +		/* UDP checksum offload is not supported on ws2008r2.
> +		 * Furthermore, on ws2012 and ws2012r2, there are some
> +		 * issues with udp checksum offload from Linux guests.
> +		 * (these are host issues).
> +		 * For now compute the checksum here.
> +		 */
> +		struct udphdr *uh = udp_hdr(skb);
> +		u16 udp_len = ntohs(uh->len);
> +
> +		uh->check = 0;
> +		uh->check = csum_tcpudp_magic(ip_hdr(skb)->saddr,
> +					      ip_hdr(skb)->daddr,
> +					      udp_len, IPPROTO_UDP,
> +					      csum_partial(uh, udp_len, 0));
> +		if (uh->check == 0)
> +			uh->check = CSUM_MANGLED_0;
> +
> +		csum_info->transmit.udp_checksum = 0;

You absolutely cannot mangle packet header contents without first
COW'ing the SKB to make sure it's writable.

Otherwise network taps like tcpdump, and any other entity with a reference
to the packet, can see inconsistent state.

RE: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum issues

[KY Srinivasan]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, April 9, 2014 10:01 AM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum
> issues
> 
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Tue,  8 Apr 2014 12:46:11 -0700
> 
> >  	} else if (net_trans_info & INFO_UDP) {
> > -		csum_info->transmit.udp_checksum = 1;
> > +		/* UDP checksum offload is not supported on ws2008r2.
> > +		 * Furthermore, on ws2012 and ws2012r2, there are some
> > +		 * issues with udp checksum offload from Linux guests.
> > +		 * (these are host issues).
> > +		 * For now compute the checksum here.
> > +		 */
> > +		struct udphdr *uh = udp_hdr(skb);
> > +		u16 udp_len = ntohs(uh->len);
> > +
> > +		uh->check = 0;
> > +		uh->check = csum_tcpudp_magic(ip_hdr(skb)->saddr,
> > +					      ip_hdr(skb)->daddr,
> > +					      udp_len, IPPROTO_UDP,
> > +					      csum_partial(uh, udp_len, 0));
> > +		if (uh->check == 0)
> > +			uh->check = CSUM_MANGLED_0;
> > +
> > +		csum_info->transmit.udp_checksum = 0;
> 
> You absolutely cannot mangle packet header contents without first COW'ing
> the SKB to make sure it's writable.
> 
> Otherwise network taps like tcpdump, and any other entity with a reference
> to the packet, can see inconsistent state.

Thanks Dave. I will COW the SKB before changing the checksum value. I do have a question though.
I looked at a bunch of hardware drivers and they modify the header information, specially checksum
field and I could not see where they had COWed the skb. I was grepping for skb_cow in these drivers.


Regards,

K. Y

Re: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum issues

[David Miller]

From: KY Srinivasan <kys@microsoft.com>
Date: Wed, 9 Apr 2014 18:08:47 +0000

> Thanks Dave. I will COW the SKB before changing the checksum
> value. I do have a question though.  I looked at a bunch of hardware
> drivers and they modify the header information, specially checksum
> field and I could not see where they had COWed the skb. I was
> grepping for skb_cow in these drivers.

Look for skb_cow_head(), for example, the tg3.c driver uses this.

RE: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum issues

[KY Srinivasan]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, April 9, 2014 11:37 AM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum
> issues
> 
> From: KY Srinivasan <kys@microsoft.com>
> Date: Wed, 9 Apr 2014 18:08:47 +0000
> 
> > Thanks Dave. I will COW the SKB before changing the checksum value. I
> > do have a question though.  I looked at a bunch of hardware drivers
> > and they modify the header information, specially checksum field and I
> > could not see where they had COWed the skb. I was grepping for skb_cow
> > in these drivers.
> 
> Look for skb_cow_head(), for example, the tg3.c driver uses this.
> 
Thank you. I will update this patch. Do you want me to resend the whole set or just
this patch.

K. Y

Re: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum issues

[David Miller]

From: KY Srinivasan <kys@microsoft.com>
Date: Wed, 9 Apr 2014 18:39:57 +0000

> 
> 
>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> Sent: Wednesday, April 9, 2014 11:37 AM
>> To: KY Srinivasan
>> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com
>> Subject: Re: [PATCH net 3/3] Drivers: net: hyperv: Address UDP checksum
>> issues
>> 
>> From: KY Srinivasan <kys@microsoft.com>
>> Date: Wed, 9 Apr 2014 18:08:47 +0000
>> 
>> > Thanks Dave. I will COW the SKB before changing the checksum value. I
>> > do have a question though.  I looked at a bunch of hardware drivers
>> > and they modify the header information, specially checksum field and I
>> > could not see where they had COWed the skb. I was grepping for skb_cow
>> > in these drivers.
>> 
>> Look for skb_cow_head(), for example, the tg3.c driver uses this.
>> 
> Thank you. I will update this patch. Do you want me to resend the whole set or just
> this patch.

You never need to ask me this.

Always, when there is an update to any part of a series, re-submit the entire
series.