Message ID | 1396986371-12137-4-git-send-email-kys@microsoft.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }