TSO and checksum fixes for i40e

PATCH Superseded Alexander Duyck 3 participants 26 messages Jan 22, 2016

Subsystems (2)

Network drivers Intel Ethernet

lore

Share & tools

lore

Alexander Duyck

author

2016-01-22 00:26 UTC #0 lore raw

v2 cover TSO and checksum fixes for i40e

This patch set is meant to improve the performance and reliability of i40e when it comes to performing TSO and Tx checksum offloads related to tunnels.

I have tested it with a number of combinations of v4 over v6 and v6 over v4 for VXLANs. With GRE I have only been able to test with v4 as the base as it appears that offloads are currently not supported for GRE over IPv6. With this patch set I resolved a number of issues and I am now able to perform TSO for any of them as long as the outer UDP checksum is 0. It should also now be supported if the outer checksum is enabled in the case of the XL722, though I cannot test it.

One additional item I have identified as an issue for the v2 patchset is the fact that Rx checksums were not working for IPv6 based tunnels due to the fact that the ports were not being registered. From what I can tell there was no reason for this so I have enabled IPv6 tunnels to register their port numbers so that they can take full advantage of Rx offloads.

As far as testing this patch set I would recommend setting up a series of tunnels between two endpointers. For VXLAN both v4 and v6 based tunnels can be used, with and without checksums. For GRE a v4 tunnel can be used with and without checksum.

---

Alexander Duyck (15):
i40e/i40evf: Drop outer checksum offload that was not requested i40e/i40evf: Use u64 values instead of casting them in TSO function i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path i40e/i40evf: Consolidate all header changes into TSO function i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path i40e/i40evf: Add support for IPv4 encapsulated in IPv6 i40e/i40evf: Handle IPv6 extension headers in checksum offload i40e/i40evf: Do not write to descriptor unless we complete i40e/i40evf: Add exception handling for Tx checksum
i40e/i40evf: Clean-up Rx packet checksum handling
i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
i40e: Fix ATR in relation to tunnels
i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels
i40e: Update feature flags to reflect newly enabled features
i40evf: Update feature flags to reflect newly enabled features

 drivers/net/ethernet/intel/i40e/i40e_main.c     |   30 +-
 drivers/net/ethernet/intel/i40e/i40e_txrx.c     |  378 ++++++++++++-----------
 drivers/net/ethernet/intel/i40e/i40e_txrx.h     |    1 
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c   |  358 ++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h   |    1 
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23 +
 6 files changed, 422 insertions(+), 369 deletions(-)

Alexander Duyck

author

2016-01-22 00:27 UTC #1 lore raw

PATCH v2 1/15 i40e/i40evf: Drop outer checksum offload that was not requested

The i40e and i40evf drivers contained code for inserting an outer checksum on UDP tunnels. The issue however is that the upper levels of the stack never requested such an offload and it results in possible errors.

In addition the same logic was being applied to the Rx side where it was attempting to validate the outer checksum, but the logic there was incorrect in that it was testing for the resultant sum to be equal to the header checksum instead of being equal to 0.

Since this code is so massively flawed, and doing things that we didn't ask for it to do I am just dropping it, and will bring it back later to use as an offload for SKB_GSO_UDP_TUNNEL_CSUM which can make use of such a feature.

As far as the Rx feature I am dropping it completely since it would need to be massively expanded and applied to IPv4 and IPv6 checksums for all parts, not just the one that supports Tx checksum offload for the outer.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_main.c   |    2 -
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   47 +++----------------------
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |    1 -
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   46 +++---------------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h |    1 -
 5 files changed, 10 insertions(+), 87 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -2

index 06b2204860a9..1316b9958e7e 100644
@@ -7465,8 +7465,6 @@ static int i40e_alloc_rings(struct i40e_vsi *vsi)
 		tx_ring->dcb_tc = 0;
 		if (vsi->back->flags & I40E_FLAG_WB_ON_ITR_CAPABLE)
 			tx_ring->flags = I40E_TXR_FLAGS_WB_ON_ITR;
-		if (vsi->back->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
-			tx_ring->flags |= I40E_TXR_FLAGS_OUTER_UDP_CSUM;
 		vsi->tx_rings[i] = tx_ring;
 
 		rx_ring = &tx_ring[1];

drivers/net/ethernet/intel/i40e/i40e_txrx.c +5 ~~-42~~

66 lines not on this page · raw message

drivers/net/ethernet/intel/i40e/i40e_txrx.h +0 -1

index 45496dfa5fd7..e1f4dedf6754 100644
@@ -277,7 +277,6 @@ struct i40e_ring {
 
 	u16 flags;
 #define I40E_TXR_FLAGS_WB_ON_ITR	BIT(0)
-#define I40E_TXR_FLAGS_OUTER_UDP_CSUM	BIT(1)
 #define I40E_TXR_FLAGS_LAST_XMIT_MORE_SET BIT(2)
 
 	/* stats structs */

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +5 ~~-41~~

65 lines not on this page · raw message

drivers/net/ethernet/intel/i40evf/i40e_txrx.h +0 -1

index bbd98b785ae0..ce1898fe4094 100644
@@ -275,7 +275,6 @@ struct i40e_ring {
 
 	u16 flags;
 #define I40E_TXR_FLAGS_WB_ON_ITR	BIT(0)
-#define I40E_TXR_FLAGS_OUTER_UDP_CSUM	BIT(1)
 
 	/* stats structs */
 	struct i40e_queue_stats	stats;

Alexander Duyck

author

2016-01-22 00:27 UTC #2 lore raw

PATCH v2 2/15 i40e/i40evf: Use u64 values instead of casting them in TSO function

In reply to Alexander Duyck

Instead of casing u32 values to u64 it makes more sense to just start out with u64 values in the first place. This way we don't need to create a mess with all of the casts needed to populate a 64b value.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |    9 ++++-----
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |    9 ++++-----
 2 files changed, 8 insertions(+), 10 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +4 -5

index c92198e7f5bc..d201bedaca27 100644
@@ -2267,7 +2267,7 @@ out:
 static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		    u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
 {
-	u32 cd_cmd, cd_tso_len, cd_mss;
+	u64 cd_cmd, cd_tso_len, cd_mss;
 	struct ipv6hdr *ipv6h;
 	struct tcphdr *tcph;
 	struct iphdr *iph;

@@ -2309,10 +2309,9 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	cd_cmd = I40E_TX_CTX_DESC_TSO;
 	cd_tso_len = skb->len - *hdr_len;
 	cd_mss = skb_shinfo(skb)->gso_size;
-	*cd_type_cmd_tso_mss |= ((u64)cd_cmd << I40E_TXD_CTX_QW1_CMD_SHIFT) |
-				((u64)cd_tso_len <<
-				 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
-				((u64)cd_mss << I40E_TXD_CTX_QW1_MSS_SHIFT);
+	*cd_type_cmd_tso_mss |= (cd_cmd << I40E_TXD_CTX_QW1_CMD_SHIFT) |
+				(cd_tso_len << I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
+				(cd_mss << I40E_TXD_CTX_QW1_MSS_SHIFT);
 	return 1;
 }

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +4 -5

index f7ac254b79f0..07d6b5d5a2ae 100644
@@ -1527,7 +1527,7 @@ out:
 static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		    u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
 {
-	u32 cd_cmd, cd_tso_len, cd_mss;
+	u64 cd_cmd, cd_tso_len, cd_mss;
 	struct ipv6hdr *ipv6h;
 	struct tcphdr *tcph;
 	struct iphdr *iph;

@@ -1569,10 +1569,9 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	cd_cmd = I40E_TX_CTX_DESC_TSO;
 	cd_tso_len = skb->len - *hdr_len;
 	cd_mss = skb_shinfo(skb)->gso_size;
-	*cd_type_cmd_tso_mss |= ((u64)cd_cmd << I40E_TXD_CTX_QW1_CMD_SHIFT) |
-				((u64)cd_tso_len <<
-				 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
-				((u64)cd_mss << I40E_TXD_CTX_QW1_MSS_SHIFT);
+	*cd_type_cmd_tso_mss |= (cd_cmd << I40E_TXD_CTX_QW1_CMD_SHIFT) |
+				(cd_tso_len << I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
+				(cd_mss << I40E_TXD_CTX_QW1_MSS_SHIFT);
 	return 1;
 }

Alexander Duyck

author

2016-01-22 00:27 UTC #3 lore raw

PATCH v2 3/15 i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path

In reply to Alexander Duyck

This patch makes it so that the L4 header offsets and such can be ignored when dealing with the L3 checksum and length update. This is done making use of two things.

First we can just use the offset from the L4 header to the start of the packet to determine the L4 offset, and from that we can then make use of the data offset to determine the full length of the headers.

As far as adjusting the checksum to remove the length we can simply add the inverse of the length instead of having to recompute the entire pseudo-header without the length. In the case of an IPv6 header this should be significantly cheaper since we can make use of a value we already needed instead of having to read the source and destination address out of the packet.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   31 +++++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   31 +++++++++++++++----------
 2 files changed, 36 insertions(+), 26 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +18 ~~-13~~

index d201bedaca27..b09859e55996 100644
@@ -2269,9 +2269,12 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 {
 	u64 cd_cmd, cd_tso_len, cd_mss;
 	struct ipv6hdr *ipv6h;
-	struct tcphdr *tcph;
 	struct iphdr *iph;
-	u32 l4len;
+	union {
+		struct tcphdr *tcp;
+		unsigned char *hdr;
+	} l4;
+	u32 paylen, l4_offset;
 	int err;
 
 	if (skb->ip_summed != CHECKSUM_PARTIAL)

@@ -2286,24 +2289,26 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
 	iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb);
 	ipv6h = skb->encapsulation ? inner_ipv6_hdr(skb) : ipv6_hdr(skb);
+	l4.hdr = skb->encapsulation ? skb_inner_transport_header(skb) :
+				      skb_transport_header(skb);
 
 	if (iph->version == 4) {
-		tcph = skb->encapsulation ? inner_tcp_hdr(skb) : tcp_hdr(skb);
 		iph->tot_len = 0;
 		iph->check = 0;
-		tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-						 0, IPPROTO_TCP, 0);
-	} else if (ipv6h->version == 6) {
-		tcph = skb->encapsulation ? inner_tcp_hdr(skb) : tcp_hdr(skb);
+	} else {
 		ipv6h->payload_len = 0;
-		tcph->check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
-					       0, IPPROTO_TCP, 0);
 	}
 
-	l4len = skb->encapsulation ? inner_tcp_hdrlen(skb) : tcp_hdrlen(skb);
-	*hdr_len = (skb->encapsulation
-		    ? (skb_inner_transport_header(skb) - skb->data)
-		    : skb_transport_offset(skb)) + l4len;
+	/* determine offset of inner transport header */
+	l4_offset = l4.hdr - skb->data;
+
+	/* remove payload length from inner checksum */
+	paylen = (__force u16)l4.tcp->check;
+	paylen += ntohs(1) * (u16)~(skb->len - l4_offset);
+	l4.tcp->check = ~csum_fold((__force __wsum)paylen);
+
+	/* compute length of segmentation header */
+	*hdr_len = (l4.tcp->doff * 4) + l4_offset;
 
 	/* find the field values */
 	cd_cmd = I40E_TX_CTX_DESC_TSO;

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +18 ~~-13~~

index 07d6b5d5a2ae..b0dc21af3cc8 100644
@@ -1529,9 +1529,12 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 {
 	u64 cd_cmd, cd_tso_len, cd_mss;
 	struct ipv6hdr *ipv6h;
-	struct tcphdr *tcph;
 	struct iphdr *iph;
-	u32 l4len;
+	union {
+		struct tcphdr *tcp;
+		unsigned char *hdr;
+	} l4;
+	u32 paylen, l4_offset;
 	int err;
 
 	if (skb->ip_summed != CHECKSUM_PARTIAL)

@@ -1546,24 +1549,26 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
 	iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb);
 	ipv6h = skb->encapsulation ? inner_ipv6_hdr(skb) : ipv6_hdr(skb);
+	l4.hdr = skb->encapsulation ? skb_inner_transport_header(skb) :
+				      skb_transport_header(skb);
 
 	if (iph->version == 4) {
-		tcph = skb->encapsulation ? inner_tcp_hdr(skb) : tcp_hdr(skb);
 		iph->tot_len = 0;
 		iph->check = 0;
-		tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-						 0, IPPROTO_TCP, 0);
-	} else if (ipv6h->version == 6) {
-		tcph = skb->encapsulation ? inner_tcp_hdr(skb) : tcp_hdr(skb);
+	} else {
 		ipv6h->payload_len = 0;
-		tcph->check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
-					       0, IPPROTO_TCP, 0);
 	}
 
-	l4len = skb->encapsulation ? inner_tcp_hdrlen(skb) : tcp_hdrlen(skb);
-	*hdr_len = (skb->encapsulation
-		    ? (skb_inner_transport_header(skb) - skb->data)
-		    : skb_transport_offset(skb)) + l4len;
+	/* determine offset of inner transport header */
+	l4_offset = l4.hdr - skb->data;
+
+	/* remove payload length from inner checksum */
+	paylen = (__force u16)l4.tcp->check;
+	paylen += ntohs(1) * (u16)~(skb->len - l4_offset);
+	l4.tcp->check = ~csum_fold((__force __wsum)paylen);
+
+	/* compute length of segmentation header */
+	*hdr_len = (l4.tcp->doff * 4) + l4_offset;
 
 	/* find the field values */
 	cd_cmd = I40E_TX_CTX_DESC_TSO;

Alexander Duyck

author

2016-01-22 00:27 UTC #4 lore raw

PATCH v2 4/15 i40e/i40evf: Consolidate all header changes into TSO function

In reply to Alexander Duyck

This patch goes through and pulls all of the spots where we were updating either the TCP or IP checksums in the TSO and checksum path into the TSO function. The general idea here is that we should only be updating the header after we verify we have completed a skb_cow_head check to verify the head is writable.

One other advantage to doing this is that it makes things much more obvious. For example, in the case of IPv6 there was one spot where the offset of the IPv4 header checksum was being updated which is obviously incorrect.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   44 +++++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   44 +++++++++++++++----------
 2 files changed, 52 insertions(+), 36 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +26 ~~-18~~

index b09859e55996..c84ba5d4634a 100644
@@ -2268,8 +2268,11 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		    u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
 {
 	u64 cd_cmd, cd_tso_len, cd_mss;
-	struct ipv6hdr *ipv6h;
-	struct iphdr *iph;
+	union {
+		struct iphdr *v4;
+		struct ipv6hdr *v6;
+		unsigned char *hdr;
+	} ip;
 	union {
 		struct tcphdr *tcp;
 		unsigned char *hdr;

@@ -2287,16 +2290,29 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	if (err < 0)
 		return err;
 
-	iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb);
-	ipv6h = skb->encapsulation ? inner_ipv6_hdr(skb) : ipv6_hdr(skb);
-	l4.hdr = skb->encapsulation ? skb_inner_transport_header(skb) :
-				      skb_transport_header(skb);
+	ip.hdr = skb_network_header(skb);
+	l4.hdr = skb_transport_header(skb);
 
-	if (iph->version == 4) {
-		iph->tot_len = 0;
-		iph->check = 0;
+	/* initialize outer IP header fields */
+	if (ip.v4->version == 4) {
+		ip.v4->tot_len = 0;
+		ip.v4->check = 0;
 	} else {
-		ipv6h->payload_len = 0;
+		ip.v6->payload_len = 0;
+	}
+
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL | SKB_GSO_GRE)) {
+		/* reset pointers to inner headers */
+		ip.hdr = skb_inner_network_header(skb);
+		l4.hdr = skb_inner_transport_header(skb);
+
+		/* initialize inner IP header fields */
+		if (ip.v4->version == 4) {
+			ip.v4->tot_len = 0;
+			ip.v4->check = 0;
+		} else {
+			ip.v6->payload_len = 0;
+		}
 	}
 
 	/* determine offset of inner transport header */

@@ -2381,15 +2397,11 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 	struct iphdr *this_ip_hdr;
 	u32 network_hdr_len;
 	u8 l4_hdr = 0;
-	struct udphdr *oudph = NULL;
-	struct iphdr *oiph = NULL;
 	u32 l4_tunnel = 0;
 
 	if (skb->encapsulation) {
 		switch (ip_hdr(skb)->protocol) {
 		case IPPROTO_UDP:
-			oudph = udp_hdr(skb);
-			oiph = ip_hdr(skb);
 			l4_tunnel = I40E_TXD_CTX_UDP_TUNNELING;
 			*tx_flags |= I40E_TX_FLAGS_TUNNEL;
 			break;

@@ -2408,15 +2420,12 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		if (*tx_flags & I40E_TX_FLAGS_IPV4) {
 			if (*tx_flags & I40E_TX_FLAGS_TSO) {
 				*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
-				ip_hdr(skb)->check = 0;
 			} else {
 				*cd_tunneling |=
 					 I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
 			}
 		} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
 			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
-			if (*tx_flags & I40E_TX_FLAGS_TSO)
-				ip_hdr(skb)->check = 0;
 		}
 
 		/* Now set the ctx descriptor fields */

@@ -2445,7 +2454,6 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		 */
 		if (*tx_flags & I40E_TX_FLAGS_TSO) {
 			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4_CSUM;
-			this_ip_hdr->check = 0;
 		} else {
 			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4;
 		}

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +26 ~~-18~~

87 lines not on this page · raw message

Alexander Duyck

author

2016-01-22 00:27 UTC #5 lore raw

PATCH v2 5/15 i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path

In reply to Alexander Duyck

The Tx checksum path was maintaining a set of 3 pointers and two lengths in order to prepare the packet for being checksummed. The thing is we only really needed 2 pointers, and the lengths that were being maintained can easily be computed.

As such we can replace the IPv4 and IPv6 header pointers with one single union that represents both, or a generic pointer to the start of the network header. For the L4 headers we can do the same with TCP and a generic pointer to the start of the transport header. The length of the TCP header is obtained by simply multiplying doff by 4, and the network header length can be obtained by subtracting the network header pointer from the transport header pointer.

While I was at it I renamed l4_hdr to l4_proto to make it a bit more clear and less likely to be confused with l4.hdr which is the transport header pointer.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   51 +++++++++++++------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   52 +++++++++++++------------
 2 files changed, 52 insertions(+), 51 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +26 ~~-25~~

index c84ba5d4634a..ef632fd60486 100644
@@ -2392,12 +2392,21 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 				struct i40e_ring *tx_ring,
 				u32 *cd_tunneling)
 {
-	struct ipv6hdr *this_ipv6_hdr;
-	unsigned int this_tcp_hdrlen;
-	struct iphdr *this_ip_hdr;
-	u32 network_hdr_len;
-	u8 l4_hdr = 0;
+	union {
+		struct iphdr *v4;
+		struct ipv6hdr *v6;
+		unsigned char *hdr;
+	} ip;
+	union {
+		struct tcphdr *tcp;
+		struct udphdr *udp;
+		unsigned char *hdr;
+	} l4;
 	u32 l4_tunnel = 0;
+	u8 l4_proto = 0;
+
+	ip.hdr = skb_network_header(skb);
+	l4.hdr = skb_transport_header(skb);
 
 	if (skb->encapsulation) {
 		switch (ip_hdr(skb)->protocol) {

@@ -2412,10 +2421,10 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		default:
 			return;
 		}
-		network_hdr_len = skb_inner_network_header_len(skb);
-		this_ip_hdr = inner_ip_hdr(skb);
-		this_ipv6_hdr = inner_ipv6_hdr(skb);
-		this_tcp_hdrlen = inner_tcp_hdrlen(skb);
+
+		/* switch L4 header pointer from outer to inner */
+		ip.hdr = skb_inner_network_header(skb);
+		l4.hdr = skb_inner_transport_header(skb);
 
 		if (*tx_flags & I40E_TX_FLAGS_IPV4) {
 			if (*tx_flags & I40E_TX_FLAGS_TSO) {

@@ -2435,20 +2444,15 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 				   ((skb_inner_network_offset(skb) -
 					skb_transport_offset(skb)) >> 1) <<
 				   I40E_TXD_CTX_QW0_NATLEN_SHIFT;
-		if (this_ip_hdr->version == 6) {
+		if (ip.v6->version == 6) {
 			*tx_flags &= ~I40E_TX_FLAGS_IPV4;
 			*tx_flags |= I40E_TX_FLAGS_IPV6;
 		}
-	} else {
-		network_hdr_len = skb_network_header_len(skb);
-		this_ip_hdr = ip_hdr(skb);
-		this_ipv6_hdr = ipv6_hdr(skb);
-		this_tcp_hdrlen = tcp_hdrlen(skb);
 	}
 
 	/* Enable IP checksum offloads */
 	if (*tx_flags & I40E_TX_FLAGS_IPV4) {
-		l4_hdr = this_ip_hdr->protocol;
+		l4_proto = ip.v4->protocol;
 		/* the stack computes the IP header already, the only time we
 		 * need the hardware to recompute it is in the case of TSO.
 		 */

@@ -2457,26 +2461,23 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		} else {
 			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4;
 		}
-		/* Now set the td_offset for IP header length */
-		*td_offset = (network_hdr_len >> 2) <<
-			      I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
 	} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
-		l4_hdr = this_ipv6_hdr->nexthdr;
+		l4_proto = ip.v6->nexthdr;
 		*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV6;
-		/* Now set the td_offset for IP header length */
-		*td_offset = (network_hdr_len >> 2) <<
-			      I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
 	}
+
+	/* Now set the td_offset for IP header length */
+	*td_offset = ((l4.hdr - ip.hdr) / 4) << I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
 	/* words in MACLEN + dwords in IPLEN + dwords in L4Len */
 	*td_offset |= (skb_network_offset(skb) >> 1) <<
 		       I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
 
 	/* Enable L4 checksum offloads */
-	switch (l4_hdr) {
+	switch (l4_proto) {
 	case IPPROTO_TCP:
 		/* enable checksum offloads */
 		*td_cmd |= I40E_TX_DESC_CMD_L4T_EOFT_TCP;
-		*td_offset |= (this_tcp_hdrlen >> 2) <<
+		*td_offset |= l4.tcp->doff <<
 			       I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
 		break;
 	case IPPROTO_SCTP:

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +26 ~~-26~~

95 lines not on this page · raw message

Alexander Duyck

author

2016-01-22 00:27 UTC #6 lore raw

PATCH v2 6/15 i40e/i40evf: Add support for IPv4 encapsulated in IPv6

In reply to Alexander Duyck

This patch fixes two issues. First was the fact that iphdr(skb)->protocl was being used to test for the outer transport protocol. This completely breaks IPv6 support. Second was the fact that we cleared the flag for v4 going to v6, but we didn't take care of txflags going the other way. As such we would have the v6 flag still set even if the inner header was v4.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   38 ++++++++++++++---------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   42 ++++++++++++++++---------
 2 files changed, 50 insertions(+), 30 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +23 ~~-15~~

index ef632fd60486..8e723125e31e 100644
@@ -2409,7 +2409,22 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 	l4.hdr = skb_transport_header(skb);
 
 	if (skb->encapsulation) {
-		switch (ip_hdr(skb)->protocol) {
+		/* define outer network header type */
+		if (*tx_flags & I40E_TX_FLAGS_IPV4) {
+			if (*tx_flags & I40E_TX_FLAGS_TSO) {
+				*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
+			} else {
+				*cd_tunneling |=
+					 I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
+			}
+			l4_proto = ip.v4->protocol;
+		} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
+			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
+			l4_proto = ip.v6->nexthdr;
+		}
+
+		/* define outer transport */
+		switch (l4_proto) {
 		case IPPROTO_UDP:
 			l4_tunnel = I40E_TXD_CTX_UDP_TUNNELING;
 			*tx_flags |= I40E_TX_FLAGS_TUNNEL;

@@ -2425,17 +2440,7 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		/* switch L4 header pointer from outer to inner */
 		ip.hdr = skb_inner_network_header(skb);
 		l4.hdr = skb_inner_transport_header(skb);
-
-		if (*tx_flags & I40E_TX_FLAGS_IPV4) {
-			if (*tx_flags & I40E_TX_FLAGS_TSO) {
-				*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
-			} else {
-				*cd_tunneling |=
-					 I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
-			}
-		} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
-			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
-		}
+		l4_proto = 0;
 
 		/* Now set the ctx descriptor fields */
 		*cd_tunneling |= (skb_network_header_len(skb) >> 2) <<

@@ -2444,10 +2449,13 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 				   ((skb_inner_network_offset(skb) -
 					skb_transport_offset(skb)) >> 1) <<
 				   I40E_TXD_CTX_QW0_NATLEN_SHIFT;
-		if (ip.v6->version == 6) {
-			*tx_flags &= ~I40E_TX_FLAGS_IPV4;
+
+		/* reset type as we transition from outer to inner headers */
+		*tx_flags &= ~(I40E_TX_FLAGS_IPV4 | I40E_TX_FLAGS_IPV6);
+		if (ip.v4->version == 4)
+			*tx_flags |= I40E_TX_FLAGS_IPV4;
+		if (ip.v6->version == 6)
 			*tx_flags |= I40E_TX_FLAGS_IPV6;
-		}
 	}
 
 	/* Enable IP checksum offloads */

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +27 ~~-15~~

65 lines not on this page · raw message

Alexander Duyck

author

2016-01-22 00:27 UTC #7 lore raw

PATCH v2 7/15 i40e/i40evf: Handle IPv6 extension headers in checksum offload

In reply to Alexander Duyck

This patch adds support for IPv6 extension headers in setting up the Tx checksum. Without this patch extension headers would cause IPv6 traffic to fail as the transport protocol could not be identified.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   14 +++++++++++++-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   14 +++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +13 -1

index 8e723125e31e..2e9ae018d639 100644
@@ -2402,7 +2402,9 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		struct udphdr *udp;
 		unsigned char *hdr;
 	} l4;
+	unsigned char *exthdr;
 	u32 l4_tunnel = 0;
+	__be16 frag_off;
 	u8 l4_proto = 0;
 
 	ip.hdr = skb_network_header(skb);

@@ -2420,7 +2422,12 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			l4_proto = ip.v4->protocol;
 		} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
 			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
+
+			exthdr = ip.hdr + sizeof(*ip.v6);
 			l4_proto = ip.v6->nexthdr;
+			if (l4.hdr != exthdr)
+				ipv6_skip_exthdr(skb, exthdr - skb->data,
+						 &l4_proto, &frag_off);
 		}
 
 		/* define outer transport */

@@ -2470,8 +2477,13 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4;
 		}
 	} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
-		l4_proto = ip.v6->nexthdr;
 		*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV6;
+
+		exthdr = ip.hdr + sizeof(*ip.v6);
+		l4_proto = ip.v6->nexthdr;
+		if (l4.hdr != exthdr)
+			ipv6_skip_exthdr(skb, exthdr - skb->data,
+					 &l4_proto, &frag_off);
 	}
 
 	/* Now set the td_offset for IP header length */

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +13 -1

index 34c7e3ffba1a..d37cba28ecfa 100644
@@ -1619,7 +1619,9 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		struct udphdr *udp;
 		unsigned char *hdr;
 	} l4;
+	unsigned char *exthdr;
 	u32 l4_tunnel = 0;
+	__be16 frag_off;
 	u8 l4_proto = 0;
 
 	ip.hdr = skb_network_header(skb);

@@ -1637,7 +1639,12 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			l4_proto = ip.v4->protocol;
 		} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
 			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
+
+			exthdr = ip.hdr + sizeof(*ip.v6);
 			l4_proto = ip.v6->nexthdr;
+			if (l4.hdr != exthdr)
+				ipv6_skip_exthdr(skb, exthdr - skb->data,
+						 &l4_proto, &frag_off);
 		}
 
 		/* define outer transport */

@@ -1687,8 +1694,13 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4;
 		}
 	} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
-		l4_proto = ip.v6->nexthdr;
 		*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV6;
+
+		exthdr = ip.hdr + sizeof(*ip.v6);
+		l4_proto = ip.v6->nexthdr;
+		if (l4.hdr != exthdr)
+			ipv6_skip_exthdr(skb, exthdr - skb->data,
+					 &l4_proto, &frag_off);
 	}
 
 	/* Now set the td_offset for IP header length */

Alexander Duyck

author

2016-01-22 00:27 UTC #8 lore raw

PATCH v2 8/15 i40e/i40evf: Do not write to descriptor unless we complete

In reply to Alexander Duyck

This patch defers writing to the Tx descriptor bits until we know we have successuflly completed a given operation. So for example we defer updating the tunnelling portion of the context descriptor until we have fully identified the type.

The advantage to this approach is that we can assemble values as we go instead of having to try and cludge everything together all at once. As a result we can significantly clean up the tunneling configuration for instance as we can just do a pointer walk and do the math for the distance between each set of points.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   80 +++++++++++++------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   80 +++++++++++++------------
 2 files changed, 84 insertions(+), 76 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +42 ~~-38~~

index 2e9ae018d639..a7f5df3c3c79 100644
@@ -2403,25 +2403,27 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		unsigned char *hdr;
 	} l4;
 	unsigned char *exthdr;
-	u32 l4_tunnel = 0;
+	u32 offset, cmd = 0, tunnel = 0;
 	__be16 frag_off;
 	u8 l4_proto = 0;
 
 	ip.hdr = skb_network_header(skb);
 	l4.hdr = skb_transport_header(skb);
 
+	/* words in MACLEN + dwords in IPLEN + dwords in L4Len */
+	offset = ((ip.hdr - skb->data) / 2) <<
+		 I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+
 	if (skb->encapsulation) {
 		/* define outer network header type */
 		if (*tx_flags & I40E_TX_FLAGS_IPV4) {
-			if (*tx_flags & I40E_TX_FLAGS_TSO) {
-				*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
-			} else {
-				*cd_tunneling |=
-					 I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
-			}
+			tunnel |= (*tx_flags & I40E_TX_FLAGS_TSO) ?
+				  I40E_TX_CTX_EXT_IP_IPV4 :
+				  I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
+
 			l4_proto = ip.v4->protocol;
 		} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
-			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
+			tunnel |= I40E_TX_CTX_EXT_IP_IPV6;
 
 			exthdr = ip.hdr + sizeof(*ip.v6);
 			l4_proto = ip.v6->nexthdr;

@@ -2430,33 +2432,38 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 						 &l4_proto, &frag_off);
 		}
 
+		/* compute outer L3 header size */
+		tunnel |= ((l4.hdr - ip.hdr) / 4) <<
+			  I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT;
+
+		/* switch IP header pointer from outer to inner header */
+		ip.hdr = skb_inner_network_header(skb);
+
 		/* define outer transport */
 		switch (l4_proto) {
 		case IPPROTO_UDP:
-			l4_tunnel = I40E_TXD_CTX_UDP_TUNNELING;
+			tunnel |= I40E_TXD_CTX_UDP_TUNNELING;
 			*tx_flags |= I40E_TX_FLAGS_TUNNEL;
 			break;
 		case IPPROTO_GRE:
-			l4_tunnel = I40E_TXD_CTX_GRE_TUNNELING;
+			tunnel |= I40E_TXD_CTX_GRE_TUNNELING;
 			*tx_flags |= I40E_TX_FLAGS_TUNNEL;
 			break;
 		default:
 			return;
 		}
 
+		/* compute tunnel header size */
+		tunnel |= ((ip.hdr - l4.hdr) / 2) <<
+			  I40E_TXD_CTX_QW0_NATLEN_SHIFT;
+
+		/* record tunnel offload values */
+		*cd_tunneling |= tunnel;
+
 		/* switch L4 header pointer from outer to inner */
-		ip.hdr = skb_inner_network_header(skb);
 		l4.hdr = skb_inner_transport_header(skb);
 		l4_proto = 0;
 
-		/* Now set the ctx descriptor fields */
-		*cd_tunneling |= (skb_network_header_len(skb) >> 2) <<
-				   I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT      |
-				   l4_tunnel                             |
-				   ((skb_inner_network_offset(skb) -
-					skb_transport_offset(skb)) >> 1) <<
-				   I40E_TXD_CTX_QW0_NATLEN_SHIFT;
-
 		/* reset type as we transition from outer to inner headers */
 		*tx_flags &= ~(I40E_TX_FLAGS_IPV4 | I40E_TX_FLAGS_IPV6);
 		if (ip.v4->version == 4)

@@ -2471,13 +2478,11 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		/* the stack computes the IP header already, the only time we
 		 * need the hardware to recompute it is in the case of TSO.
 		 */
-		if (*tx_flags & I40E_TX_FLAGS_TSO) {
-			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4_CSUM;
-		} else {
-			*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4;
-		}
+		cmd |= (*tx_flags & I40E_TX_FLAGS_TSO) ?
+		       I40E_TX_DESC_CMD_IIPT_IPV4_CSUM :
+		       I40E_TX_DESC_CMD_IIPT_IPV4;
 	} else if (*tx_flags & I40E_TX_FLAGS_IPV6) {
-		*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV6;
+		cmd |= I40E_TX_DESC_CMD_IIPT_IPV6;
 
 		exthdr = ip.hdr + sizeof(*ip.v6);
 		l4_proto = ip.v6->nexthdr;

@@ -2487,34 +2492,33 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 	}
 
 	/* Now set the td_offset for IP header length */
-	*td_offset = ((l4.hdr - ip.hdr) / 4) << I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
-	/* words in MACLEN + dwords in IPLEN + dwords in L4Len */
-	*td_offset |= (skb_network_offset(skb) >> 1) <<
-		       I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+	offset |= ((l4.hdr - ip.hdr) / 4) << I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
 
 	/* Enable L4 checksum offloads */
 	switch (l4_proto) {
 	case IPPROTO_TCP:
 		/* enable checksum offloads */
-		*td_cmd |= I40E_TX_DESC_CMD_L4T_EOFT_TCP;
-		*td_offset |= l4.tcp->doff <<
-			       I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
+		cmd |= I40E_TX_DESC_CMD_L4T_EOFT_TCP;
+		offset |= l4.tcp->doff << I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
 		break;
 	case IPPROTO_SCTP:
 		/* enable SCTP checksum offload */
-		*td_cmd |= I40E_TX_DESC_CMD_L4T_EOFT_SCTP;
-		*td_offset |= (sizeof(struct sctphdr) >> 2) <<
-			       I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
+		cmd |= I40E_TX_DESC_CMD_L4T_EOFT_SCTP;
+		offset |= (sizeof(struct sctphdr) >> 2) <<
+			  I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
 		break;
 	case IPPROTO_UDP:
 		/* enable UDP checksum offload */
-		*td_cmd |= I40E_TX_DESC_CMD_L4T_EOFT_UDP;
-		*td_offset |= (sizeof(struct udphdr) >> 2) <<
-			       I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
+		cmd |= I40E_TX_DESC_CMD_L4T_EOFT_UDP;
+		offset |= (sizeof(struct udphdr) >> 2) <<
+			  I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
 		break;
 	default:
 		break;
 	}
+
+	*td_cmd |= cmd;
+	*td_offset |= offset;
 }
 
 /**

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +42 ~~-38~~

147 lines not on this page · raw message

Singhai

2016-01-22 21:23 UTC #9 lore raw

PATCH v2 8/15 i40e/i40evf: Do not write to descriptor unless we complete

Review

Quoted 7 lines · Alexander Duyck

  	__be16 frag_off;
  	u8 l4_proto = 0;
  
  	ip.hdr = skb_network_header(skb);
  	l4.hdr = skb_transport_header(skb);

(quoted diff) +1 -0

+	/* words in MACLEN + dwords in IPLEN + dwords in L4Len */

May be the comment should say words in MACLEN and as the other fields get added later, build upon it...

drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags, · line 2494

Quoted 11 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +3 -0

+	offset = ((ip.hdr - skb->data) / 2) <<
+		 I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+
 
 
  		exthdr = ip.hdr + sizeof(*ip.v6);
  		l4_proto = ip.v6->nexthdr;

@@ -2487,34 +2492,33 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
  	}
 
  	/* Now set the td_offset for IP header length */

Comment needs to be fixed

Quoted 7 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +1 -4

-	*td_offset = ((l4.hdr - ip.hdr) / 4) << I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
-	/* words in MACLEN + dwords in IPLEN + dwords in L4Len */
-	*td_offset |= (skb_network_offset(skb) >> 1) <<
-		       I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+	offset |= ((l4.hdr - ip.hdr) / 4) << I40E_TX_DESC_LENGTH_IPLEN_SHIFT;

Alexander Duyck

2016-01-22 21:26 UTC #10 lore raw

PATCH v2 8/15 i40e/i40evf: Do not write to descriptor unless we complete

Quoted 11 lines · Anjali

Quoted 6 lines · Alexander Duyck
        __be16 frag_off;
        u8 l4_proto = 0;
        ip.hdr = skb_network_header(skb);
        l4.hdr = skb_transport_header(skb);
+ /* words in MACLEN + dwords in IPLEN + dwords in L4Len */

Quoted 2 lines

May be the comment should say words in MACLEN and as the other fields get added later, build upon it...

You are right, this is just MACLEN. I can update the comment for a v3.

Quoted 6 lines

Quoted 4 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +3 -0

+       offset = ((ip.hdr - skb->data) / 2) <<
+                I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+

Quoted 9 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_tx_enable_csum(struct sk_buff · line 2494
Quoted 6 lines
                exthdr = ip.hdr + sizeof(*ip.v6);
                l4_proto = ip.v6->nexthdr;
drivers/net/ethernet/intel/i40e/i40e_txrx.c +0 -0
@@ -2487,34 +2492,33 @@ static void i40e_tx_enable_csum(struct sk_buff
 *skb, u32 *tx_flags,
        }
        /* Now set the td_offset for IP header length */
Comment needs to be fixed

Will update for v3.

Quoted 15 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags, · line 2496
Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c +1 -4
-       *td_offset = ((l4.hdr - ip.hdr) / 4) <<
 I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
-       /* words in MACLEN + dwords in IPLEN + dwords in L4Len */
-       *td_offset |= (skb_network_offset(skb) >> 1) <<
-                      I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+       offset |= ((l4.hdr - ip.hdr) / 4) <<
 I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
 
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Singhai

2016-01-22 21:28 UTC #11 lore raw

PATCH v2 8/15 i40e/i40evf: Do not write to descriptor unless we complete

Thanks a lot Alex for the series! Much cleaner and you caught a lot of IPv6 issues.

Anjali

Quoted 61 lines

-----Original Message-----
From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
Sent: Friday, January 22, 2016 1:27 PM
To: Singhai, Anjali
Cc: Alexander Duyck; intel-wired-lan; Kirsher, Jeffrey T
Subject: Re: [Intel-wired-lan] [next PATCH v2 08/15] i40e/i40evf: Do not write to descriptor unless we complete
Quoted 4 lines · Anjali
Quoted 1 line · Alexander Duyck
Quoted 8 lines
Quoted 5 lines
        __be16 frag_off;
        u8 l4_proto = 0;
        ip.hdr = skb_network_header(skb);
        l4.hdr = skb_transport_header(skb);
+ /* words in MACLEN + dwords in IPLEN + dwords in L4Len */
May be the comment should say words in MACLEN and as the other fields
get
Quoted 1 line
added later, build upon it...
You are right, this is just MACLEN. I can update the comment for a v3.
Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_tx_enable_csum(struct
Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c +3 -0
+       offset = ((ip.hdr - skb->data) / 2) <<
+                I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+
 
 
                exthdr = ip.hdr + sizeof(*ip.v6);
                l4_proto = ip.v6->nexthdr;
@@ -2487,34 +2492,33 @@ static void i40e_tx_enable_csum(struct
sk_buff
Quoted 6 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags, · line 2494
Quoted 3 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c +0 -0
 *skb, u32 *tx_flags,
        }
        /* Now set the td_offset for IP header length */
Comment needs to be fixed
Will update for v3.
Quoted 15 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags, · line 2496
Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_txrx.c +1 -4
-       *td_offset = ((l4.hdr - ip.hdr) / 4) <<
 I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
-       /* words in MACLEN + dwords in IPLEN + dwords in L4Len */
-       *td_offset |= (skb_network_offset(skb) >> 1) <<
-                      I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
+       offset |= ((l4.hdr - ip.hdr) / 4) <<
 I40E_TX_DESC_LENGTH_IPLEN_SHIFT;
 
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Alexander Duyck

2016-01-22 21:37 UTC #12 lore raw

PATCH v2 8/15 i40e/i40evf: Do not write to descriptor unless we complete

Quoted 1 line · Anjali

Thanks a lot Alex for the series! Much cleaner and you caught a lot of IPv6 issues.

No problem. Thanks for reviewing the patches.

I will have a v3 ready by this afternoon with most of the suggestions you have made addressed.

- Alex

Alexander Duyck

author

2016-01-22 00:27 UTC #13 lore raw

PATCH v2 9/15 i40e/i40evf: Add exception handling for Tx checksum

In reply to Alexander Duyck

Add exception handling to the Tx checksum path so that we can handle cases of TSO where the frame is bad, or Tx checksum where we didn't recognize a protocol

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   31 ++++++++++++++++--------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   32 +++++++++++++++++--------
 2 files changed, 43 insertions(+), 20 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +21 ~~-10~~

index a7f5df3c3c79..b13b637b97fe 100644
@@ -2387,10 +2387,10 @@ static int i40e_tsyn(struct i40e_ring *tx_ring, struct sk_buff *skb,
  * @tx_ring: Tx descriptor ring
  * @cd_tunneling: ptr to context desc bits
  **/
-static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
-				u32 *td_cmd, u32 *td_offset,
-				struct i40e_ring *tx_ring,
-				u32 *cd_tunneling)
+static int i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
+			       u32 *td_cmd, u32 *td_offset,
+			       struct i40e_ring *tx_ring,
+			       u32 *cd_tunneling)
 {
 	union {
 		struct iphdr *v4;

@@ -2450,7 +2450,11 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			*tx_flags |= I40E_TX_FLAGS_TUNNEL;
 			break;
 		default:
-			return;
+			if (*tx_flags & I40E_TX_FLAGS_TSO)
+				return -1;
+
+			skb_checksum_help(skb);
+			return 0;
 		}
 
 		/* compute tunnel header size */

@@ -2514,11 +2518,16 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			  I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
 		break;
 	default:
-		break;
+		if (*tx_flags & I40E_TX_FLAGS_TSO)
+			return -1;
+		skb_checksum_help(skb);
+		return 0;
 	}
 
 	*td_cmd |= cmd;
 	*td_offset |= offset;
+
+	return 1;
 }
 
 /**

@@ -2956,10 +2965,12 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 
 	/* Always offload the checksum, since it's in the data descriptor */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		tx_flags |= I40E_TX_FLAGS_CSUM;
-
-		i40e_tx_enable_csum(skb, &tx_flags, &td_cmd, &td_offset,
-				    tx_ring, &cd_tunneling);
+		tso = i40e_tx_enable_csum(skb, &tx_flags, &td_cmd, &td_offset,
+					  tx_ring, &cd_tunneling);
+		if (tso < 0)
+			goto out_drop;
+		else if (tso)
+			tx_flags |= I40E_TX_FLAGS_CSUM;
 	}
 
 	i40e_create_tx_ctx(tx_ring, cd_type_cmd_tso_mss,

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +22 ~~-10~~

index bd990dc5b7c6..b9272234959b 100644
@@ -1602,12 +1602,13 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
  * @tx_flags: pointer to Tx flags currently set
  * @td_cmd: Tx descriptor command bits to set
  * @td_offset: Tx descriptor header offsets to set
+ * @tx_ring: Tx descriptor ring
  * @cd_tunneling: ptr to context desc bits
  **/
-static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
-				u32 *td_cmd, u32 *td_offset,
-				struct i40e_ring *tx_ring,
-				u32 *cd_tunneling)
+static int i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
+			       u32 *td_cmd, u32 *td_offset,
+			       struct i40e_ring *tx_ring,
+			       u32 *cd_tunneling)
 {
 	union {
 		struct iphdr *v4;

@@ -1667,7 +1668,11 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			*tx_flags |= I40E_TX_FLAGS_TUNNEL;
 			break;
 		default:
-			return;
+			if (*tx_flags & I40E_TX_FLAGS_TSO)
+				return -1;
+
+			skb_checksum_help(skb);
+			return 0;
 		}
 
 		/* compute tunnel header size */

@@ -1731,11 +1736,16 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			  I40E_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
 		break;
 	default:
-		break;
+		if (*tx_flags & I40E_TX_FLAGS_TSO)
+			return -1;
+		skb_checksum_help(skb);
+		return 0;
 	}
 
 	*td_cmd |= cmd;
 	*td_offset |= offset;
+
+	return 1;
 }
 
 /**

@@ -2152,10 +2162,12 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 
 	/* Always offload the checksum, since it's in the data descriptor */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		tx_flags |= I40E_TX_FLAGS_CSUM;
-
-		i40e_tx_enable_csum(skb, &tx_flags, &td_cmd, &td_offset,
-				    tx_ring, &cd_tunneling);
+		tso = i40e_tx_enable_csum(skb, &tx_flags, &td_cmd, &td_offset,
+					  tx_ring, &cd_tunneling);
+		if (tso < 0)
+			goto out_drop;
+		else if (tso)
+			tx_flags |= I40E_TX_FLAGS_CSUM;
 	}
 
 	i40e_create_tx_ctx(tx_ring, cd_type_cmd_tso_mss,

Singhai

2016-01-22 21:25 UTC #14 lore raw

PATCH v2 9/15 i40e/i40evf: Add exception handling for Tx checksum

Quoted 5 lines · Alexander Duyck

/**

drivers/net/ethernet/intel/i40e/i40e_txrx.c +0 -0

@@ -2152,10 +2162,12 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 
  	/* Always offload the checksum, since it's in the data descriptor */
  	if (skb->ip_summed == CHECKSUM_PARTIAL) {

drivers/net/ethernet/intel/i40e/i40e_txrx.c static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb, · line 2170

Quoted 10 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +6 -4

-		tx_flags |= I40E_TX_FLAGS_CSUM;
-
-		i40e_tx_enable_csum(skb, &tx_flags, &td_cmd, &td_offset,
-				    tx_ring, &cd_tunneling);
+		tso = i40e_tx_enable_csum(skb, &tx_flags, &td_cmd, &td_offset,
+					  tx_ring, &cd_tunneling);
+		if (tso < 0)
+			goto out_drop;
+		else if (tso)
+			tx_flags |= I40E_TX_FLAGS_CSUM;

Flag is not used

Quoted 8 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +0 -0
  	}
 
  	i40e_create_tx_ctx(tx_ring, cd_type_cmd_tso_mss,
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Alexander Duyck

author

2016-01-22 00:27 UTC #15 lore raw

PATCH v2 10/15 i40e/i40evf: Clean-up Rx packet checksum handling

In reply to Alexander Duyck

This is mostly a minor clean-up for the Rx checksum path in order to avoid some of the unnecessary conditional checks that were being applied.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   23 ++++++++++-------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   23 ++++++++++-------------
 2 files changed, 20 insertions(+), 26 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +10 ~~-13~~

index b13b637b97fe..40a4fca627d5 100644
@@ -1389,13 +1389,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 				    u16 rx_ptype)
 {
 	struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
-	bool ipv4 = false, ipv6 = false;
-	bool ipv4_tunnel, ipv6_tunnel;
-
-	ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
-		     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
-	ipv6_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
-		     (rx_ptype <= I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+	bool ipv4, ipv6, ipv4_tunnel, ipv6_tunnel;
 
 	skb->ip_summed = CHECKSUM_NONE;

@@ -1411,12 +1405,10 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	if (!(decoded.known && decoded.outer_ip))
 		return;
 
-	if (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP &&
-	    decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV4)
-		ipv4 = true;
-	else if (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP &&
-		 decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV6)
-		ipv6 = true;
+	ipv4 = (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP) &&
+	       (decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV4);
+	ipv6 = (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP) &&
+	       (decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV6);
 
 	if (ipv4 &&
 	    (rx_error & (BIT(I40E_RX_DESC_ERROR_IPE_SHIFT) |

@@ -1447,6 +1439,11 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	 * inner checksum report CHECKSUM_UNNECESSARY.
 	 */
 
+	ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
+		     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
+	ipv6_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
+		     (rx_ptype <= I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	skb->csum_level = ipv4_tunnel || ipv6_tunnel;

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +10 ~~-13~~

index b9272234959b..f1036845072b 100644
@@ -861,13 +861,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 				    u16 rx_ptype)
 {
 	struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
-	bool ipv4 = false, ipv6 = false;
-	bool ipv4_tunnel, ipv6_tunnel;
-
-	ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
-		     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
-	ipv6_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
-		     (rx_ptype <= I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+	bool ipv4, ipv6, ipv4_tunnel, ipv6_tunnel;
 
 	skb->ip_summed = CHECKSUM_NONE;

@@ -883,12 +877,10 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	if (!(decoded.known && decoded.outer_ip))
 		return;
 
-	if (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP &&
-	    decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV4)
-		ipv4 = true;
-	else if (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP &&
-		 decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV6)
-		ipv6 = true;
+	ipv4 = (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP) &&
+	       (decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV4);
+	ipv6 = (decoded.outer_ip == I40E_RX_PTYPE_OUTER_IP) &&
+	       (decoded.outer_ip_ver == I40E_RX_PTYPE_OUTER_IPV6);
 
 	if (ipv4 &&
 	    (rx_error & (BIT(I40E_RX_DESC_ERROR_IPE_SHIFT) |

@@ -919,6 +911,11 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	 * inner checksum report CHECKSUM_UNNECESSARY.
 	 */
 
+	ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
+		     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
+	ipv6_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
+		     (rx_ptype <= I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	skb->csum_level = ipv4_tunnel || ipv6_tunnel;

Alexander Duyck

author

2016-01-22 00:28 UTC #16 lore raw

PATCH v2 11/15 i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM

In reply to Alexander Duyck

The XL722 has support for providing the outer UDP tunnel checksum on transmits. Make use of this feature to support segmenting UDP tunnels with outer checksums enabled.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   19 ++++++++++++++++++-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   19 ++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +18 -1

index 40a4fca627d5..34dfa79e76db 100644
@@ -2272,6 +2272,7 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	} ip;
 	union {
 		struct tcphdr *tcp;
+		struct udphdr *udp;
 		unsigned char *hdr;
 	} l4;
 	u32 paylen, l4_offset;

@@ -2298,7 +2299,18 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		ip.v6->payload_len = 0;
 	}
 
-	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL | SKB_GSO_GRE)) {
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL | SKB_GSO_GRE |
+					 SKB_GSO_UDP_TUNNEL_CSUM)) {
+		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM) {
+			/* determine offset of outer transport header */
+			l4_offset = l4.hdr - skb->data;
+
+			/* remove payload length from outer checksum */
+			paylen = (__force u16)l4.udp->check;
+			paylen += ntohs(1) * (u16)~(skb->len - l4_offset);
+			l4.udp->check = ~csum_fold((__force __wsum)paylen);
+		}
+
 		/* reset pointers to inner headers */
 		ip.hdr = skb_inner_network_header(skb);
 		l4.hdr = skb_inner_transport_header(skb);

@@ -2458,6 +2470,11 @@ static int i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		tunnel |= ((ip.hdr - l4.hdr) / 2) <<
 			  I40E_TXD_CTX_QW0_NATLEN_SHIFT;
 
+		/* indicate if we need to offload outer UDP header */
+		if ((*tx_flags & I40E_TX_FLAGS_TSO) &&
+		    (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM))
+			tunnel |= I40E_TXD_CTX_QW0_L4T_CS_MASK;
+
 		/* record tunnel offload values */
 		*cd_tunneling |= tunnel;

drivers/net/ethernet/intel/i40evf/i40e_txrx.c +18 -1

index f1036845072b..ec4e008c7529 100644
@@ -1532,6 +1532,7 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	} ip;
 	union {
 		struct tcphdr *tcp;
+		struct udphdr *udp;
 		unsigned char *hdr;
 	} l4;
 	u32 paylen, l4_offset;

@@ -1558,7 +1559,18 @@ static int i40e_tso(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		ip.v6->payload_len = 0;
 	}
 
-	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL | SKB_GSO_GRE)) {
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL | SKB_GSO_GRE |
+					 SKB_GSO_UDP_TUNNEL_CSUM)) {
+		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM) {
+			/* determine offset of outer transport header */
+			l4_offset = l4.hdr - skb->data;
+
+			/* remove payload length from outer checksum */
+			paylen = (__force u16)l4.udp->check;
+			paylen += ntohs(1) * (u16)~(skb->len - l4_offset);
+			l4.udp->check = ~csum_fold((__force __wsum)paylen);
+		}
+
 		/* reset pointers to inner headers */
 		ip.hdr = skb_inner_network_header(skb);
 		l4.hdr = skb_inner_transport_header(skb);

@@ -1676,6 +1688,11 @@ static int i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 		tunnel |= ((ip.hdr - l4.hdr) / 2) <<
 			  I40E_TXD_CTX_QW0_NATLEN_SHIFT;
 
+		/* indicate if we need to offload outer UDP header */
+		if ((*tx_flags & I40E_TX_FLAGS_TSO) &&
+		    (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM))
+			tunnel |= I40E_TXD_CTX_QW0_L4T_CS_MASK;
+
 		/* record tunnel offload values */
 		*cd_tunneling |= tunnel;

Alexander Duyck

author

2016-01-22 00:28 UTC #17 lore raw

PATCH v2 12/15 i40e: Fix ATR in relation to tunnels

In reply to Alexander Duyck

This patch contains a number of fixes to make certain that we are using the correct protocols when parsing both the inner and outer headers of a frame that is mixed between IPv4 and IPv6 for inner and outer.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_txrx.c +9 ~~-14~~

index 34dfa79e76db..5a8db8de7675 100644
@@ -2045,6 +2045,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	struct tcphdr *th;
 	unsigned int hlen;
 	u32 flex_ptype, dtype_cmd;
+	u8 l4_proto;
 	u16 i;
 
 	/* make sure ATR is enabled */

@@ -2058,6 +2059,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	if (!tx_ring->atr_sample_rate)
 		return;
 
+	/* Currently only IPv4/IPv6 with TCP is supported */
 	if (!(tx_flags & (I40E_TX_FLAGS_IPV4 | I40E_TX_FLAGS_IPV6)))
 		return;

@@ -2068,26 +2070,19 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 		/* snag network header to get L4 type and address */
 		hdr.network = skb_network_header(skb);
 
-		/* Currently only IPv4/IPv6 with TCP is supported
-		 * access ihl as u8 to avoid unaligned access on ia64
-		 */
+		/* access ihl as u8 to avoid unaligned access on ia64 */
 		if (tx_flags & I40E_TX_FLAGS_IPV4)
 			hlen = (hdr.network[0] & 0x0F) << 2;
-		else if (protocol == htons(ETH_P_IPV6))
-			hlen = sizeof(struct ipv6hdr);
 		else
-			return;
+			hlen = sizeof(struct ipv6hdr);
 	}
 
-	/* Currently only IPv4/IPv6 with TCP is supported
-	 * Note: tx_flags gets modified to reflect inner protocols in
+	/* Note: tx_flags gets modified to reflect inner protocols in
 	 * tx_enable_csum function if encap is enabled.
 	 */
-	if ((tx_flags & I40E_TX_FLAGS_IPV4) &&
-	    (hdr.ipv4->protocol != IPPROTO_TCP))
-		return;
-	else if ((tx_flags & I40E_TX_FLAGS_IPV6) &&
-		 (hdr.ipv6->nexthdr != IPPROTO_TCP))
+	l4_proto = (tx_flags & I40E_TX_FLAGS_IPV4) ? hdr.ipv4->protocol :
+						     hdr.ipv6->nexthdr;
+	if (l4_proto != IPPROTO_TCP)
 		return;
 
 	th = (struct tcphdr *)(hdr.network + hlen);

@@ -2124,7 +2119,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
 	flex_ptype = (tx_ring->queue_index << I40E_TXD_FLTR_QW0_QINDEX_SHIFT) &
 		      I40E_TXD_FLTR_QW0_QINDEX_MASK;
-	flex_ptype |= (protocol == htons(ETH_P_IP)) ?
+	flex_ptype |= (tx_flags & I40E_TX_FLAGS_IPV4) ?
 		      (I40E_FILTER_PCTYPE_NONF_IPV4_TCP <<
 		       I40E_TXD_FLTR_QW0_PCTYPE_SHIFT) :
 		      (I40E_FILTER_PCTYPE_NONF_IPV6_TCP <<

Singhai

2016-01-22 21:17 UTC #18 lore raw

PATCH v2 12/15 i40e: Fix ATR in relation to tunnels

drivers/net/ethernet/intel/i40e/i40e_txrx.c static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb, · line 2127

Quoted 6 lines · Alexander Duyck

  	th = (struct tcphdr *)(hdr.network + hlen);

drivers/net/ethernet/intel/i40e/i40e_txrx.c +0 -1

@@ -2124,7 +2119,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 
  	flex_ptype = (tx_ring->queue_index << I40E_TXD_FLTR_QW0_QINDEX_SHIFT) &
  		      I40E_TXD_FLTR_QW0_QINDEX_MASK;
-	flex_ptype |= (protocol == htons(ETH_P_IP)) ?

Protocol, incoming parameter is no longer needed should be dropped from the function parameters.

Quoted 6 lines

drivers/net/ethernet/intel/i40e/i40e_txrx.c +1 -0

+	flex_ptype |= (tx_flags & I40E_TX_FLAGS_IPV4) ?
  		      (I40E_FILTER_PCTYPE_NONF_IPV4_TCP <<
  		       I40E_TXD_FLTR_QW0_PCTYPE_SHIFT) :
  		      (I40E_FILTER_PCTYPE_NONF_IPV6_TCP <<

Alexander Duyck

author

2016-01-22 00:28 UTC #19 lore raw

PATCH v2 13/15 i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels

In reply to Alexander Duyck

All of the documentation in the datasheets for the XL710 do not call out any reason to exclude support for IPv6 based tunnels. As such I am dropping the code that was excluding these tunnel types from having their port numbers recognized. This way we can take advantage of things such as checksum offload for inner headers over IPv6 based VXLAN or GENEVE tunnels.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_main.c |   12 ------------
 1 file changed, 12 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_main.c +0 ~~-12~~

index 1316b9958e7e..e3a903f01d73 100644
@@ -8617,9 +8617,6 @@ static void i40e_add_vxlan_port(struct net_device *netdev,
 	u8 next_idx;
 	u8 idx;
 
-	if (sa_family == AF_INET6)
-		return;
-
 	idx = i40e_get_udp_port_idx(pf, port);
 
 	/* Check if port already exists */

@@ -8659,9 +8656,6 @@ static void i40e_del_vxlan_port(struct net_device *netdev,
 	struct i40e_pf *pf = vsi->back;
 	u8 idx;
 
-	if (sa_family == AF_INET6)
-		return;
-
 	idx = i40e_get_udp_port_idx(pf, port);
 
 	/* Check if port already exists */

@@ -8698,9 +8692,6 @@ static void i40e_add_geneve_port(struct net_device *netdev,
 	if (!(pf->flags & I40E_FLAG_GENEVE_OFFLOAD_CAPABLE))
 		return;
 
-	if (sa_family == AF_INET6)
-		return;
-
 	idx = i40e_get_udp_port_idx(pf, port);
 
 	/* Check if port already exists */

@@ -8742,9 +8733,6 @@ static void i40e_del_geneve_port(struct net_device *netdev,
 	struct i40e_pf *pf = vsi->back;
 	u8 idx;
 
-	if (sa_family == AF_INET6)
-		return;
-
 	if (!(pf->flags & I40E_FLAG_GENEVE_OFFLOAD_CAPABLE))
 		return;

Alexander Duyck

author

2016-01-22 00:28 UTC #20 lore raw

PATCH v2 14/15 i40e: Update feature flags to reflect newly enabled features

In reply to Alexander Duyck

Recent changes should have enabled support for IPv6 based tunnels and support for TSO with outer UDP checksums. As such we can update the feature flags to reflect that.

In addition we can clean-up the flags that aren't needed such as SCTP and RXCSUM since having the bits there doesn't add any value.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_main.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_main.c +10 -6

index e3a903f01d73..1d969acf9517 100644
@@ -9023,12 +9023,14 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
 	np = netdev_priv(netdev);
 	np->vsi = vsi;
 
-	netdev->hw_enc_features |= NETIF_F_IP_CSUM	  |
-				   NETIF_F_RXCSUM	  |
-				   NETIF_F_SCTP_CRC	  |
-				   NETIF_F_GSO_UDP_TUNNEL |
-				   NETIF_F_GSO_GRE	  |
-				   NETIF_F_TSO		  |
+	netdev->hw_enc_features |= NETIF_F_IP_CSUM	       |
+				   NETIF_F_IPV6_CSUM	       |
+				   NETIF_F_TSO		       |
+				   NETIF_F_TSO6		       |
+				   NETIF_F_TSO_ECN	       |
+				   NETIF_F_GSO_GRE	       |
+				   NETIF_F_GSO_UDP_TUNNEL      |
+				   NETIF_F_GSO_UDP_TUNNEL_CSUM |
 				   0;
 
 	netdev->features = NETIF_F_SG		       |

@@ -9050,6 +9052,8 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
 
 	if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
 		netdev->features |= NETIF_F_NTUPLE;
+	if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
+		netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
 
 	/* copy netdev features into list of user selectable features */
 	netdev->hw_features |= netdev->features;

Singhai

2016-01-22 21:15 UTC #21 lore raw

PATCH v2 14/15 i40e: Update feature flags to reflect newly enabled features

Quoted 25 lines · Alexander Duyck

Recent changes should have enabled support for IPv6 based tunnels and support for TSO with outer UDP checksums. As such we can update the feature flags to reflect that.
In addition we can clean-up the flags that aren't needed such as SCTP and RXCSUM since having the bits there doesn't add any value.
Signed-off-by Alexander Duyck <[email protected]>
---
  drivers/net/ethernet/intel/i40e/i40e_main.c |   16 ++++++++++------
  1 file changed, 10 insertions(+), 6 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_main.c +0 -5
index e3a903f01d73..1d969acf9517 100644
@@ -9023,12 +9023,14 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
  	np = netdev_priv(netdev);
  	np->vsi = vsi;
 
-	netdev->hw_enc_features |= NETIF_F_IP_CSUM	  |
-				   NETIF_F_RXCSUM	  |
-				   NETIF_F_SCTP_CRC	  |
-				   NETIF_F_GSO_UDP_TUNNEL |
-				   NETIF_F_GSO_GRE	  |

drivers/net/ethernet/intel/i40e/i40e_main.c static int i40e_config_netdev(struct i40e_vsi *vsi) · line 9034

Quoted 10 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +8 -1

-				   NETIF_F_TSO		  |
+	netdev->hw_enc_features |= NETIF_F_IP_CSUM	       |
+				   NETIF_F_IPV6_CSUM	       |
+				   NETIF_F_TSO		       |
+				   NETIF_F_TSO6		       |
+				   NETIF_F_TSO_ECN	       |
+				   NETIF_F_GSO_GRE	       |
+				   NETIF_F_GSO_UDP_TUNNEL      |
+				   NETIF_F_GSO_UDP_TUNNEL_CSUM |
  				   0;

TUNNEL_CSUM in this case requires the PF_FLAG check for OUTER_UDP_CSUM as well

Quoted 15 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +2 -0

 	netdev->features = NETIF_F_SG		       |

@@ -9050,6 +9052,8 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
 
  	if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
  		netdev->features |= NETIF_F_NTUPLE;
+	if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
+		netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
 
  	/* copy netdev features into list of user selectable features */
  	netdev->hw_features |= netdev->features;

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Alexander Duyck

2016-01-22 21:34 UTC #22 lore raw

PATCH v2 14/15 i40e: Update feature flags to reflect newly enabled features

Quoted 36 lines · Anjali

Quoted 33 lines · Alexander Duyck

Recent changes should have enabled support for IPv6 based tunnels and support for TSO with outer UDP checksums. As such we can update the feature flags to reflect that.

In addition we can clean-up the flags that aren't needed such as SCTP and RXCSUM since having the bits there doesn't add any value.

Signed-off-by Alexander Duyck <[email protected]>

---
  drivers/net/ethernet/intel/i40e/i40e_main.c |   16 ++++++++++------
  1 file changed, 10 insertions(+), 6 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_main.c +5 -5

index e3a903f01d73..1d969acf9517 100644
@@ -9023,12 +9023,14 @@ static int i40e_config_netdev(struct i40e_vsi
 *vsi)
        np = netdev_priv(netdev);
        np->vsi = vsi;
  -     netdev->hw_enc_features |= NETIF_F_IP_CSUM        |
-                                  NETIF_F_RXCSUM         |
-                                  NETIF_F_SCTP_CRC       |
-                                  NETIF_F_GSO_UDP_TUNNEL |
-                                  NETIF_F_GSO_GRE        |
-                                  NETIF_F_TSO            |
+       netdev->hw_enc_features |= NETIF_F_IP_CSUM             |
+                                  NETIF_F_IPV6_CSUM           |
+                                  NETIF_F_TSO                 |
+                                  NETIF_F_TSO6                |
+                                  NETIF_F_TSO_ECN             |

Quoted 8 lines

drivers/net/ethernet/intel/i40e/i40e_main.c static int i40e_config_netdev(struct i40e_vsi *vsi) · line 9034

Quoted 4 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +3 -0

+                                  NETIF_F_GSO_GRE             |
+                                  NETIF_F_GSO_UDP_TUNNEL      |
+                                  NETIF_F_GSO_UDP_TUNNEL_CSUM |
                                   0;

TUNNEL_CSUM in this case requires the PF_FLAG check for OUTER_UDP_CSUM as well

Actually it doesn't. The hw_enc_features is just a mask. So if the device cannot enable TUNNEL_CSUM via the check below, then the feature will never be exposed to the tunnel anyway. For hw_features it matters, but for hw_enc_features it doesn't.

Quoted 19 lines

Quoted 9 lines
drivers/net/ethernet/intel/i40e/i40e_main.c +2 -0
        netdev->features = NETIF_F_SG                  |
@@ -9050,6 +9052,8 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
        if (!(pf->flags & I40E_FLAG_MFP_ENABLED))
                netdev->features |= NETIF_F_NTUPLE;
+       if (pf->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
+               netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
        /* copy netdev features into list of user selectable features */
        netdev->hw_features |= netdev->features;
 
Quoted 4 lines
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Alexander Duyck

author

2016-01-22 00:28 UTC #23 lore raw

PATCH v2 15/15 i40evf: Update feature flags to reflect newly enabled features

In reply to Alexander Duyck

Recent changes should have enabled support for IPv6 based tunnels and support for TSO with outer UDP checksums. As such we can update the feature flags to reflect that.

In addition we can clean-up the flags that aren't needed such as SCTP and RXCSUM since having the bits there doesn't add any value.

I also found one spot where we were setting the same flag twice. It looks like it was probably a git merge error that resulted in the line being duplicated. As such I have dropped it in this patch.

Signed-off-by Alexander Duyck <[email protected]>

---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

drivers/net/ethernet/intel/i40evf/i40evf_main.c +19 -4

index 41369a30dfb8..3396fe32cc6d 100644
@@ -2337,9 +2337,24 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
 			    NETIF_F_IPV6_CSUM |
 			    NETIF_F_TSO |
 			    NETIF_F_TSO6 |
+			    NETIF_F_TSO_ECN |
+			    NETIF_F_GSO_GRE	       |
+			    NETIF_F_GSO_UDP_TUNNEL |
 			    NETIF_F_RXCSUM |
 			    NETIF_F_GRO;
 
+	netdev->hw_enc_features |= NETIF_F_IP_CSUM	       |
+				   NETIF_F_IPV6_CSUM	       |
+				   NETIF_F_TSO		       |
+				   NETIF_F_TSO6		       |
+				   NETIF_F_TSO_ECN	       |
+				   NETIF_F_GSO_GRE	       |
+				   NETIF_F_GSO_UDP_TUNNEL      |
+				   NETIF_F_GSO_UDP_TUNNEL_CSUM;
+
+	if (adapter->flags & I40EVF_FLAG_OUTER_UDP_CSUM_CAPABLE)
+		netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
+
 	/* copy netdev features into list of user selectable features */
 	netdev->hw_features |= netdev->features;
 	netdev->hw_features &= ~NETIF_F_RXCSUM;

@@ -2478,6 +2493,10 @@ static void i40evf_init_task(struct work_struct *work)
 	default:
 		goto err_alloc;
 	}
+
+	if (hw->mac.type == I40E_MAC_X722_VF)
+		adapter->flags |= I40EVF_FLAG_OUTER_UDP_CSUM_CAPABLE;
+
 	if (i40evf_process_config(adapter))
 		goto err_alloc;
 	adapter->current_op = I40E_VIRTCHNL_OP_UNKNOWN;

@@ -2519,10 +2538,6 @@ static void i40evf_init_task(struct work_struct *work)
 		goto err_sw_init;
 	i40evf_map_rings_to_vectors(adapter);
 	if (adapter->vf_res->vf_offload_flags &
-		    I40E_VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
-		adapter->flags |= I40EVF_FLAG_WB_ON_ITR_CAPABLE;
-
-	if (adapter->vf_res->vf_offload_flags &
 	    I40E_VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
 		adapter->flags |= I40EVF_FLAG_WB_ON_ITR_CAPABLE;

Singhai

2016-01-22 21:27 UTC #24 lore raw

PATCH v2 15/15 i40evf: Update feature flags to reflect newly enabled features

We haven't SVed any of the tunnel offloads in the VF, I am assuming you tried this.

Quoted 72 lines · Alexander Duyck

Recent changes should have enabled support for IPv6 based tunnels and support for TSO with outer UDP checksums. As such we can update the feature flags to reflect that.

In addition we can clean-up the flags that aren't needed such as SCTP and RXCSUM since having the bits there doesn't add any value.

I also found one spot where we were setting the same flag twice. It looks like it was probably a git merge error that resulted in the line being duplicated. As such I have dropped it in this patch.

Signed-off-by Alexander Duyck <[email protected]>

---
  drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23 +++++++++++++++++++----
  1 file changed, 19 insertions(+), 4 deletions(-)

drivers/net/ethernet/intel/i40evf/i40evf_main.c +19 -4

index 41369a30dfb8..3396fe32cc6d 100644
@@ -2337,9 +2337,24 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
  			    NETIF_F_IPV6_CSUM |
  			    NETIF_F_TSO |
  			    NETIF_F_TSO6 |
+			    NETIF_F_TSO_ECN |
+			    NETIF_F_GSO_GRE	       |
+			    NETIF_F_GSO_UDP_TUNNEL |
  			    NETIF_F_RXCSUM |
  			    NETIF_F_GRO;
 
+	netdev->hw_enc_features |= NETIF_F_IP_CSUM	       |
+				   NETIF_F_IPV6_CSUM	       |
+				   NETIF_F_TSO		       |
+				   NETIF_F_TSO6		       |
+				   NETIF_F_TSO_ECN	       |
+				   NETIF_F_GSO_GRE	       |
+				   NETIF_F_GSO_UDP_TUNNEL      |
+				   NETIF_F_GSO_UDP_TUNNEL_CSUM;
+
+	if (adapter->flags & I40EVF_FLAG_OUTER_UDP_CSUM_CAPABLE)
+		netdev->features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
+
  	/* copy netdev features into list of user selectable features */
  	netdev->hw_features |= netdev->features;
  	netdev->hw_features &= ~NETIF_F_RXCSUM;

@@ -2478,6 +2493,10 @@ static void i40evf_init_task(struct work_struct *work)
  	default:
  		goto err_alloc;
  	}
+
+	if (hw->mac.type == I40E_MAC_X722_VF)
+		adapter->flags |= I40EVF_FLAG_OUTER_UDP_CSUM_CAPABLE;
+
  	if (i40evf_process_config(adapter))
  		goto err_alloc;
  	adapter->current_op = I40E_VIRTCHNL_OP_UNKNOWN;

@@ -2519,10 +2538,6 @@ static void i40evf_init_task(struct work_struct *work)
  		goto err_sw_init;
  	i40evf_map_rings_to_vectors(adapter);
  	if (adapter->vf_res->vf_offload_flags &
-		    I40E_VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
-		adapter->flags |= I40EVF_FLAG_WB_ON_ITR_CAPABLE;
-
-	if (adapter->vf_res->vf_offload_flags &
  	    I40E_VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
  		adapter->flags |= I40EVF_FLAG_WB_ON_ITR_CAPABLE;

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Alexander Duyck

2016-01-22 21:36 UTC #25 lore raw

PATCH v2 15/15 i40evf: Update feature flags to reflect newly enabled features

Quoted 2 lines · Anjali

We haven't SVed any of the tunnel offloads in the VF, I am assuming you tried this.

Yes, I've been testing it with an XL710. For the most part if it is PF or VF it shouldn't really matter. The descriptor format is all the same for the Tx rings so the offload functionality all works the same.