| Message ID | 1394104390-23477-5-git-send-email-kys@microsoft.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: "K. Y. Srinivasan" <kys@microsoft.com> Date: Thu, 6 Mar 2014 03:13:09 -0800 > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4, > + bool *is_tcp, bool *is_udp, u32 *trans_off) > +{ Returning so many values like this is awkward, at best. Why not return a well defined bitmask: #define TRANSPORT_INFO_SUCCESS 0x00000001 #define TRANSPORT_INFO_TCP 0x00000002 #define TRANSPORT_INFO_UDP 0x00000004 Then the only value you have to return by reference is trans_off. -- 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: Friday, March 7, 2014 1:04 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 5/6] Drivers: net: hyperv: Enable send side checksum > offload > > From: "K. Y. Srinivasan" <kys@microsoft.com> > Date: Thu, 6 Mar 2014 03:13:09 -0800 > > > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4, > > + bool *is_tcp, bool *is_udp, u32 *trans_off) { > > Returning so many values like this is awkward, at best. > > Why not return a well defined bitmask: > > #define TRANSPORT_INFO_SUCCESS 0x00000001 > #define TRANSPORT_INFO_TCP 0x00000002 > #define TRANSPORT_INFO_UDP 0x00000004 > > Then the only value you have to return by reference is trans_off. I also need information on the IP version as well. If it is ok with you, I will use the return value to get information on the network protocol and return information on transport via reference - protocol and the offset. 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: Thu, 6 Mar 2014 20:29:13 +0000 > > >> -----Original Message----- >> From: David Miller [mailto:davem@davemloft.net] >> Sent: Friday, March 7, 2014 1:04 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 5/6] Drivers: net: hyperv: Enable send side checksum >> offload >> >> From: "K. Y. Srinivasan" <kys@microsoft.com> >> Date: Thu, 6 Mar 2014 03:13:09 -0800 >> >> > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4, >> > + bool *is_tcp, bool *is_udp, u32 *trans_off) { >> >> Returning so many values like this is awkward, at best. >> >> Why not return a well defined bitmask: >> >> #define TRANSPORT_INFO_SUCCESS 0x00000001 >> #define TRANSPORT_INFO_TCP 0x00000002 >> #define TRANSPORT_INFO_UDP 0x00000004 >> >> Then the only value you have to return by reference is trans_off. > > I also need information on the IP version as well. If it is ok with you, I will use the return > value to get information on the network protocol and return information on transport via > reference - protocol and the offset. Just add "TRANSPORT_INFO_IPv4" as another bitmask in the return value. -- 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: Friday, March 7, 2014 2:19 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 5/6] Drivers: net: hyperv: Enable send side checksum > offload > > From: KY Srinivasan <kys@microsoft.com> > Date: Thu, 6 Mar 2014 20:29:13 +0000 > > > > > > >> -----Original Message----- > >> From: David Miller [mailto:davem@davemloft.net] > >> Sent: Friday, March 7, 2014 1:04 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 5/6] Drivers: net: hyperv: Enable send side > >> checksum offload > >> > >> From: "K. Y. Srinivasan" <kys@microsoft.com> > >> Date: Thu, 6 Mar 2014 03:13:09 -0800 > >> > >> > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4, > >> > + bool *is_tcp, bool *is_udp, u32 *trans_off) { > >> > >> Returning so many values like this is awkward, at best. > >> > >> Why not return a well defined bitmask: > >> > >> #define TRANSPORT_INFO_SUCCESS 0x00000001 > >> #define TRANSPORT_INFO_TCP 0x00000002 > >> #define TRANSPORT_INFO_UDP 0x00000004 > >> > >> Then the only value you have to return by reference is trans_off. > > > > I also need information on the IP version as well. If it is ok with > > you, I will use the return value to get information on the network > > protocol and return information on transport via reference - protocol and > the offset. > > Just add "TRANSPORT_INFO_IPv4" as another bitmask in the return value. The network information is independent of the transport information - I can partition the return value to encode all the possible network and transport combination and subsequently parse the network and transport information. I will go ahead and code it up this way. 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
On Thu, 2014-03-06 at 03:13 -0800, K. Y. Srinivasan wrote: [...] > @@ -593,8 +658,9 @@ static int netvsc_probe(struct hv_device *dev, > net->netdev_ops = &device_ops; > > /* TODO: Add GSO and Checksum offload */ > - net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG; > - net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM; > + net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM; > + net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM | > + NETIF_F_IP_CSUM; Missing NETIF_F_IPV6_CSUM? Ben. > SET_ETHTOOL_OPS(net, ðtool_ops); > SET_NETDEV_DEV(net, &dev->device);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 98562fc..e8159a8 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -259,6 +259,36 @@ static int netvsc_get_slots(struct sk_buff *skb) return slots + frag_slots; } +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4, + bool *is_tcp, bool *is_udp, u32 *trans_off) +{ + *is_tcp = false; + *is_udp = false; + + if ((eth_hdr(skb)->h_proto != htons(ETH_P_IP)) && + (eth_hdr(skb)->h_proto != htons(ETH_P_IPV6))) { + return false; + } + + *trans_off = skb_transport_offset(skb); + + if ((eth_hdr(skb)->h_proto == htons(ETH_P_IP))) { + struct iphdr *iphdr = ip_hdr(skb); + *is_v4 = true; + if (iphdr->protocol == IPPROTO_TCP) + *is_tcp = true; + else if (iphdr->protocol == IPPROTO_UDP) + *is_udp = true; + } else { + *is_v4 = false; + if (ipv6_hdr(skb)->nexthdr == IPPROTO_TCP) + *is_tcp = true; + else if (ipv6_hdr(skb)->nexthdr == IPPROTO_UDP) + *is_udp = true; + } + return true; +} + static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) { struct net_device_context *net_device_ctx = netdev_priv(net); @@ -270,6 +300,12 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) u32 rndis_msg_size; bool isvlan; struct rndis_per_packet_info *ppi; + struct ndis_tcp_ip_checksum_info *csum_info; + int hdr_offset; + bool is_v4; + bool is_tcp; + bool is_udp; + /* * We will atmost need two pages to describe the rndis @@ -338,6 +374,35 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) VLAN_PRIO_SHIFT; } + if (!get_net_transport_info(skb, &is_v4, &is_tcp, &is_udp, &hdr_offset)) + goto do_send; + /* + * Setup the sendside checksum offload only if this is not a + * GSO packet. + */ + if (skb_is_gso(skb)) + goto do_send; + + rndis_msg_size += NDIS_CSUM_PPI_SIZE; + ppi = init_ppi_data(rndis_msg, NDIS_CSUM_PPI_SIZE, + TCPIP_CHKSUM_PKTINFO); + + csum_info = (struct ndis_tcp_ip_checksum_info *)((ulong)ppi + + ppi->ppi_offset); + + if (is_v4) + csum_info->transmit.is_ipv4 = 1; + else + csum_info->transmit.is_ipv6 = 1; + + if (is_tcp) { + csum_info->transmit.tcp_checksum = 1; + csum_info->transmit.tcp_header_offset = hdr_offset; + } else if (is_udp) { + csum_info->transmit.udp_checksum = 1; + } + +do_send: /* Start filling in the page buffers with the rndis hdr */ rndis_msg->msg_len += rndis_msg_size; packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size, @@ -593,8 +658,9 @@ static int netvsc_probe(struct hv_device *dev, net->netdev_ops = &device_ops; /* TODO: Add GSO and Checksum offload */ - net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG; - net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM; + net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM; + net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM | + NETIF_F_IP_CSUM; SET_ETHTOOL_OPS(net, ðtool_ops); SET_NETDEV_DEV(net, &dev->device);