All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e
@ 2016-01-25  5:16 Alexander Duyck
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested Alexander Duyck
                   ` (14 more replies)
  0 siblings, 15 replies; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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.

v2: Expanded coverage to include i40evf
    Enabled IPv6 tunnel support for Rx
    Moved XL722 support from generic checksum to TSO
v3: Fixed comments to match code in checksum path
    Dropped unused flag for checksum
    Dropped protocol parameter since it was unused in ATR function

Testing Hints:
I would recommend setting up a series of tunnels between two endpoints.
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.

Significant performance improvements should be seen for IPv6 based tunnels,
or IPv4 tunnels passing IPv6 traffic with netperf.

The XL722 should no longer insert checksums for tunnels that do not have
udpcsum set.  This can be verified via tcpdump on the link partner.  XL722
should show significant performance improvements for transmit with tunnels
that have udpcsum with netperf TCP_STREAM testing.

The i40evf driver will show significant Tx performance improvements for
tunnels.  This can be verified with netperf.

---

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     |  385 ++++++++++++-----------
 drivers/net/ethernet/intel/i40e/i40e_txrx.h     |    2 
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c   |  360 ++++++++++++----------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h   |    2 
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23 +
 6 files changed, 424 insertions(+), 378 deletions(-)

--

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 16:00   ` Bowers, AndrewX
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values instead of casting them in TSO function Alexander Duyck
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 06b2204860a9..1316b9958e7e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -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];
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 7702e5faf609..c92198e7f5bc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1391,9 +1391,6 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
 	bool ipv4 = false, ipv6 = false;
 	bool ipv4_tunnel, ipv6_tunnel;
-	__wsum rx_udp_csum;
-	struct iphdr *iph;
-	__sum16 csum;
 
 	ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
 		     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
@@ -1443,37 +1440,12 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	if (rx_error & BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))
 		return;
 
-	/* If VXLAN/GENEVE traffic has an outer UDPv4 checksum we need to check
-	 * it in the driver, hardware does not do it for us.
-	 * Since L3L4P bit was set we assume a valid IHL value (>=5)
-	 * so the total length of IPv4 header is IHL*4 bytes
-	 * The UDP_0 bit *may* bet set if the *inner* header is UDP
+	/* The hardware supported by this driver does not validate outer
+	 * checksums for tunneled VXLAN or GENEVE frames.  I don't agree
+	 * with it but the specification states that you "MAY validate", it
+	 * doesn't make it a hard requirement so if we have validated the
+	 * inner checksum report CHECKSUM_UNNECESSARY.
 	 */
-	if (!(vsi->back->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE) &&
-	    (ipv4_tunnel)) {
-		skb->transport_header = skb->mac_header +
-					sizeof(struct ethhdr) +
-					(ip_hdr(skb)->ihl * 4);
-
-		/* Add 4 bytes for VLAN tagged packets */
-		skb->transport_header += (skb->protocol == htons(ETH_P_8021Q) ||
-					  skb->protocol == htons(ETH_P_8021AD))
-					  ? VLAN_HLEN : 0;
-
-		if ((ip_hdr(skb)->protocol == IPPROTO_UDP) &&
-		    (udp_hdr(skb)->check != 0)) {
-			rx_udp_csum = udp_csum(skb);
-			iph = ip_hdr(skb);
-			csum = csum_tcpudp_magic(
-					iph->saddr, iph->daddr,
-					(skb->len - skb_transport_offset(skb)),
-					IPPROTO_UDP, rx_udp_csum);
-
-			if (udp_hdr(skb)->check != csum)
-				goto checksum_fail;
-
-		} /* else its GRE and so no outer UDP header */
-	}
 
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	skb->csum_level = ipv4_tunnel || ipv6_tunnel;
@@ -2454,15 +2426,6 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			*tx_flags &= ~I40E_TX_FLAGS_IPV4;
 			*tx_flags |= I40E_TX_FLAGS_IPV6;
 		}
-		if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
-		    (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
-		    (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
-			oudph->check = ~csum_tcpudp_magic(oiph->saddr,
-					oiph->daddr,
-					(skb->len - skb_transport_offset(skb)),
-					IPPROTO_UDP, 0);
-			*cd_tunneling |= I40E_TXD_CTX_QW0_L4T_CS_MASK;
-		}
 	} else {
 		network_hdr_len = skb_network_header_len(skb);
 		this_ip_hdr = ip_hdr(skb);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 45496dfa5fd7..e1f4dedf6754 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -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 */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index ecd2df28d8b8..f7ac254b79f0 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -863,9 +863,6 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
 	bool ipv4 = false, ipv6 = false;
 	bool ipv4_tunnel, ipv6_tunnel;
-	__wsum rx_udp_csum;
-	struct iphdr *iph;
-	__sum16 csum;
 
 	ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
 		     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
@@ -915,36 +912,12 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
 	if (rx_error & BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))
 		return;
 
-	/* If VXLAN traffic has an outer UDPv4 checksum we need to check
-	 * it in the driver, hardware does not do it for us.
-	 * Since L3L4P bit was set we assume a valid IHL value (>=5)
-	 * so the total length of IPv4 header is IHL*4 bytes
-	 * The UDP_0 bit *may* bet set if the *inner* header is UDP
+	/* The hardware supported by this driver does not validate outer
+	 * checksums for tunneled VXLAN or GENEVE frames.  I don't agree
+	 * with it but the specification states that you "MAY validate", it
+	 * doesn't make it a hard requirement so if we have validated the
+	 * inner checksum report CHECKSUM_UNNECESSARY.
 	 */
-	if (ipv4_tunnel) {
-		skb->transport_header = skb->mac_header +
-					sizeof(struct ethhdr) +
-					(ip_hdr(skb)->ihl * 4);
-
-		/* Add 4 bytes for VLAN tagged packets */
-		skb->transport_header += (skb->protocol == htons(ETH_P_8021Q) ||
-					  skb->protocol == htons(ETH_P_8021AD))
-					  ? VLAN_HLEN : 0;
-
-		if ((ip_hdr(skb)->protocol == IPPROTO_UDP) &&
-		    (udp_hdr(skb)->check != 0)) {
-			rx_udp_csum = udp_csum(skb);
-			iph = ip_hdr(skb);
-			csum = csum_tcpudp_magic(iph->saddr, iph->daddr,
-						 (skb->len -
-						  skb_transport_offset(skb)),
-						 IPPROTO_UDP, rx_udp_csum);
-
-			if (udp_hdr(skb)->check != csum)
-				goto checksum_fail;
-
-		} /* else its GRE and so no outer UDP header */
-	}
 
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	skb->csum_level = ipv4_tunnel || ipv6_tunnel;
@@ -1667,15 +1640,6 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 			*tx_flags |= I40E_TX_FLAGS_IPV6;
 		}
 
-		if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
-		    (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
-		    (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
-			oudph->check = ~csum_tcpudp_magic(oiph->saddr,
-					oiph->daddr,
-					(skb->len - skb_transport_offset(skb)),
-					IPPROTO_UDP, 0);
-			*cd_tunneling |= I40E_TXD_CTX_QW0_L4T_CS_MASK;
-		}
 	} else {
 		network_hdr_len = skb_network_header_len(skb);
 		this_ip_hdr = ip_hdr(skb);
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index bbd98b785ae0..ce1898fe4094 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -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;


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values instead of casting them in TSO function
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 16:03   ` Bowers, AndrewX
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path Alexander Duyck
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c92198e7f5bc..d201bedaca27 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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;
 }
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index f7ac254b79f0..07d6b5d5a2ae 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -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;
 }
 


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested Alexander Duyck
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values instead of casting them in TSO function Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 16:09   ` Bowers, AndrewX
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all header changes into TSO function Alexander Duyck
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index d201bedaca27..b09859e55996 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 07d6b5d5a2ae..b0dc21af3cc8 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -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;


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all header changes into TSO function
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (2 preceding siblings ...)
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 16:14   ` Bowers, AndrewX
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path Alexander Duyck
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index b09859e55996..c84ba5d4634a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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;
 		}
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index b0dc21af3cc8..1b2788093425 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1528,8 +1528,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;
@@ -1547,16 +1550,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 */
@@ -1598,15 +1614,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;
-	struct iphdr *oiph;
 	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;
@@ -1621,15 +1633,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 */
@@ -1659,7 +1668,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;
 		}


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (3 preceding siblings ...)
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all header changes into TSO function Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 16:27   ` Bowers, AndrewX
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for IPv4 encapsulated in IPv6 Alexander Duyck
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c84ba5d4634a..ef632fd60486 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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:
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 1b2788093425..b1c8ada663aa 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1609,12 +1609,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) {
@@ -1625,10 +1634,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) {
@@ -1648,21 +1657,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.
 		 */
@@ -1671,26 +1674,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:


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for IPv4 encapsulated in IPv6
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (4 preceding siblings ...)
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 18:04   ` Bowers, AndrewX
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6 extension headers in checksum offload Alexander Duyck
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index ef632fd60486..8e723125e31e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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 */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index b1c8ada663aa..34c7e3ffba1a 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1626,11 +1626,30 @@ 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;
 			break;
+		case IPPROTO_GRE:
+			l4_tunnel = I40E_TXD_CTX_GRE_TUNNELING;
+			*tx_flags |= I40E_TX_FLAGS_TUNNEL;
+			break;
 		default:
 			return;
 		}
@@ -1638,17 +1657,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) <<
@@ -1657,10 +1666,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 */


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6 extension headers in checksum offload
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (5 preceding siblings ...)
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for IPv4 encapsulated in IPv6 Alexander Duyck
@ 2016-01-25  5:16 ` Alexander Duyck
  2016-01-27 18:05   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to descriptor unless we complete Alexander Duyck
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:16 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 8e723125e31e..2e9ae018d639 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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 */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 34c7e3ffba1a..d37cba28ecfa 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -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 */


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to descriptor unless we complete
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (6 preceding siblings ...)
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6 extension headers in checksum offload Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-27 18:07   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception handling for Tx checksum Alexander Duyck
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   81 +++++++++++++------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   81 +++++++++++++------------
 2 files changed, 84 insertions(+), 78 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 2e9ae018d639..713b39f4f6f8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2403,25 +2403,26 @@ 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);
 
+	/* compute outer L2 header size */
+	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 +2431,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 +2477,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;
@@ -2486,35 +2490,34 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 					 &l4_proto, &frag_off);
 	}
 
-	/* 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;
+	/* compute inner L3 header size */
+	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;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index d37cba28ecfa..35aa07f8b9da 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1620,25 +1620,26 @@ 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);
 
+	/* compute outer L2 header size */
+	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;
@@ -1647,33 +1648,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)
@@ -1688,13 +1694,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;
@@ -1703,35 +1707,34 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 					 &l4_proto, &frag_off);
 	}
 
-	/* 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;
+	/* compute inner L3 header size */
+	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;
 }
 
 /**


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception handling for Tx checksum
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (7 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to descriptor unless we complete Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-27 18:08   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx packet checksum handling Alexander Duyck
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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

Drop I40E_TX_FLAGS_CSUM as it is unused, move the CHECKSUM_PARTIAL check
into the function itself so that we can decrease indent.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   34 ++++++++++++++++--------
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |    1 -
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   35 ++++++++++++++++---------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h |    1 -
 4 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 713b39f4f6f8..094e0153c7c6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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;
@@ -2407,6 +2407,9 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 	__be16 frag_off;
 	u8 l4_proto = 0;
 
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		return 0;
+
 	ip.hdr = skb_network_header(skb);
 	l4.hdr = skb_transport_header(skb);
 
@@ -2449,7 +2452,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 */
@@ -2513,11 +2520,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;
 }
 
 /**
@@ -2954,12 +2966,10 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 	td_cmd |= I40E_TX_DESC_CMD_ICRC;
 
 	/* 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;
 
 	i40e_create_tx_ctx(tx_ring, cd_type_cmd_tso_mss,
 			   cd_tunneling, cd_l2tag2);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index e1f4dedf6754..837d14f0184e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -153,7 +153,6 @@ enum i40e_dyn_idx_t {
 #define DESC_NEEDED (MAX_SKB_FRAGS + 4)
 #define I40E_MIN_DESC_PENDING	4
 
-#define I40E_TX_FLAGS_CSUM		BIT(0)
 #define I40E_TX_FLAGS_HW_VLAN		BIT(1)
 #define I40E_TX_FLAGS_SW_VLAN		BIT(2)
 #define I40E_TX_FLAGS_TSO		BIT(3)
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 35aa07f8b9da..b8d7c7a29070 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -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;
@@ -1624,6 +1625,9 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
 	__be16 frag_off;
 	u8 l4_proto = 0;
 
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		return 0;
+
 	ip.hdr = skb_network_header(skb);
 	l4.hdr = skb_transport_header(skb);
 
@@ -1666,7 +1670,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 */
@@ -1730,11 +1738,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;
 }
 
 /**
@@ -2150,12 +2163,10 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 	td_cmd |= I40E_TX_DESC_CMD_ICRC;
 
 	/* 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;
 
 	i40e_create_tx_ctx(tx_ring, cd_type_cmd_tso_mss,
 			   cd_tunneling, cd_l2tag2);
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index ce1898fe4094..561ba66d3ba8 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -153,7 +153,6 @@ enum i40e_dyn_idx_t {
 #define DESC_NEEDED (MAX_SKB_FRAGS + 4)
 #define I40E_MIN_DESC_PENDING	4
 
-#define I40E_TX_FLAGS_CSUM		BIT(0)
 #define I40E_TX_FLAGS_HW_VLAN		BIT(1)
 #define I40E_TX_FLAGS_SW_VLAN		BIT(2)
 #define I40E_TX_FLAGS_TSO		BIT(3)


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx packet checksum handling
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (8 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception handling for Tx checksum Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-27 18:09   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM Alexander Duyck
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 094e0153c7c6..890d2f580434 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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;
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index b8d7c7a29070..126b748c0ff1 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -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;
 


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (9 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx packet checksum handling Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-27 18:17   ` Bowers, AndrewX
  2016-02-02 22:49   ` Jesse Brandeburg
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels Alexander Duyck
                   ` (3 subsequent siblings)
  14 siblings, 2 replies; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 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(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 890d2f580434..6a76c169c07e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -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);
@@ -2460,6 +2472,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;
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 126b748c0ff1..a4f03f6752cc 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -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);
@@ -1678,6 +1690,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;
 


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (10 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-25 19:27   ` Patil, Kiran
  2016-01-27 18:18   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels Alexander Duyck
                   ` (2 subsequent siblings)
  14 siblings, 2 replies; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |   28 +++++++++++----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 6a76c169c07e..ed8d13637c15 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2030,10 +2030,9 @@ tx_only:
  * @tx_ring:  ring to add programming descriptor to
  * @skb:      send buffer
  * @tx_flags: send tx flags
- * @protocol: wire protocol
  **/
 static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
-		     u32 tx_flags, __be16 protocol)
+		     u32 tx_flags)
 {
 	struct i40e_filter_program_desc *fdir_desc;
 	struct i40e_pf *pf = tx_ring->vsi->back;
@@ -2045,6 +2044,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 +2058,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 +2069,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 +2118,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 <<
@@ -2992,7 +2986,7 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 	 *
 	 * NOTE: this must always be directly before the data descriptor.
 	 */
-	i40e_atr(tx_ring, skb, tx_flags, protocol);
+	i40e_atr(tx_ring, skb, tx_flags);
 
 	i40e_tx_map(tx_ring, skb, first, tx_flags, hdr_len,
 		    td_cmd, td_offset);


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (11 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-27 18:26   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to reflect newly enabled features Alexander Duyck
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 15/15] i40evf: " Alexander Duyck
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1316b9958e7e..e3a903f01d73 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -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;
 


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to reflect newly enabled features
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (12 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-27 18:44   ` Bowers, AndrewX
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 15/15] i40evf: " Alexander Duyck
  14 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index e3a903f01d73..1d969acf9517 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -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;


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 15/15] i40evf: Update feature flags to reflect newly enabled features
  2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
                   ` (13 preceding siblings ...)
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to reflect newly enabled features Alexander Duyck
@ 2016-01-25  5:17 ` Alexander Duyck
  2016-01-25 19:43   ` Singhai, Anjali
  2016-01-27 18:45   ` Bowers, AndrewX
  14 siblings, 2 replies; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25  5:17 UTC (permalink / raw)
  To: intel-wired-lan

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 <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 41369a30dfb8..3396fe32cc6d 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -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;
 


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels Alexander Duyck
@ 2016-01-25 19:27   ` Patil, Kiran
  2016-01-25 22:21     ` Alexander Duyck
  2016-01-27 18:18   ` Bowers, AndrewX
  1 sibling, 1 reply; 39+ messages in thread
From: Patil, Kiran @ 2016-01-25 19:27 UTC (permalink / raw)
  To: intel-wired-lan



Thanks Alex for fixing it. Please see in-line comments/suggestion.

-- Kiran P.

On 1/24/2016 9:17 PM, Alexander Duyck wrote:
> 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 <aduyck@mirantis.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_txrx.c |   28 +++++++++++----------------
>   1 file changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 6a76c169c07e..ed8d13637c15 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2030,10 +2030,9 @@ tx_only:
>    * @tx_ring:  ring to add programming descriptor to
>    * @skb:      send buffer
>    * @tx_flags: send tx flags
> - * @protocol: wire protocol
>    **/
>   static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
> -		     u32 tx_flags, __be16 protocol)
> +		     u32 tx_flags)
>   {
>   	struct i40e_filter_program_desc *fdir_desc;
>   	struct i40e_pf *pf = tx_ring->vsi->back;
> @@ -2045,6 +2044,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 +2058,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 +2069,19 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
                       union {
                               unsigned char *transport;
                               struct tcphdr *th; /* if needed */
                               struct udphdr *uh; /* if needed */
                      } l4_hdr;

                     /* Obtain start of transport */
                      if ((tx_flags & I40E_TX_FLAGS_TUNNEL))
                                 l4_hdr.transport = 
skb_inner_transport_header(skb);
                     else
                                 l4_hdr.transport = 
skb_transport_header(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;
                     We need to skip extended IPV6 header as well and 
obtain correct value for
                     l4_proto & hlen (may be something like this):

                     {
                             __be16 frag_off;
                             u8 next_hdr;
                             l4_proto = hdr.ipv6->nexthdr;
                             if (unlikely((l4_hdr.transport - 
hdr.network) == sizeof(struct ipv6hdr))) {
                                     off = ipv6_skip_exthdr(skb, 
hdr.network - skb->data + sizeof(struct ipv6hdr),
&l4_proto,&frag_off);
                                     if (unlikely(off < 0))
                                                     return; /* 
Unexpected */
                                     if (unlikely(frag_off))
                                             l4_proto= NEXTHDR_FRAGMENT;

                                     /* Update 'hlen' for the code below 
to work properly */
                                     hlen = l4_hdr.transport - hdr.network;
                             }
                     }
> +	if (l4_proto != IPPROTO_TCP)
>   		return;
>   
>   	th = (struct tcphdr *)(hdr.network + hlen);
> @@ -2124,7 +2118,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 <<
> @@ -2992,7 +2986,7 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
>   	 *
>   	 * NOTE: this must always be directly before the data descriptor.
>   	 */
> -	i40e_atr(tx_ring, skb, tx_flags, protocol);
> +	i40e_atr(tx_ring, skb, tx_flags);
>   
>   	i40e_tx_map(tx_ring, skb, first, tx_flags, hdr_len,
>   		    td_cmd, td_offset);
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 15/15] i40evf: Update feature flags to reflect newly enabled features
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 15/15] i40evf: " Alexander Duyck
@ 2016-01-25 19:43   ` Singhai, Anjali
  2016-01-27 18:45   ` Bowers, AndrewX
  1 sibling, 0 replies; 39+ messages in thread
From: Singhai, Anjali @ 2016-01-25 19:43 UTC (permalink / raw)
  To: intel-wired-lan

Acked-by: Anjali Singhai Jain <anjali.singhai@intel.com>

On 1/24/2016 9:17 PM, Alexander Duyck wrote:
> 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 <aduyck@mirantis.com>
> ---
>   drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23 +++++++++++++++++++----
>   1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> index 41369a30dfb8..3396fe32cc6d 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> @@ -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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels
  2016-01-25 19:27   ` Patil, Kiran
@ 2016-01-25 22:21     ` Alexander Duyck
  2016-01-26  0:16       ` Patil, Kiran
  0 siblings, 1 reply; 39+ messages in thread
From: Alexander Duyck @ 2016-01-25 22:21 UTC (permalink / raw)
  To: intel-wired-lan

On Mon, Jan 25, 2016 at 11:27 AM, Patil, Kiran <kiran.patil@intel.com> wrote:
>
>
> Thanks Alex for fixing it. Please see in-line comments/suggestion.
>
> -- Kiran P.
>
>
> On 1/24/2016 9:17 PM, Alexander Duyck wrote:
>>
>> 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 <aduyck@mirantis.com>
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e_txrx.c |   28
>> +++++++++++----------------
>>   1 file changed, 11 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> index 6a76c169c07e..ed8d13637c15 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> @@ -2030,10 +2030,9 @@ tx_only:
>>    * @tx_ring:  ring to add programming descriptor to
>>    * @skb:      send buffer
>>    * @tx_flags: send tx flags
>> - * @protocol: wire protocol
>>    **/
>>   static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
>> -                    u32 tx_flags, __be16 protocol)
>> +                    u32 tx_flags)
>>   {
>>         struct i40e_filter_program_desc *fdir_desc;
>>         struct i40e_pf *pf = tx_ring->vsi->back;
>> @@ -2045,6 +2044,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 +2058,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 +2069,19 @@ static void i40e_atr(struct i40e_ring *tx_ring,
>> struct sk_buff *skb,
>
>                       union {
>                               unsigned char *transport;
>                               struct tcphdr *th; /* if needed */
>                               struct udphdr *uh; /* if needed */
>                      } l4_hdr;

I'm not sure what the point of this is.  See the comment above which I
moved from a few lines below about "Currently only IPv4/IPv6 with TCP
is supported".  There is only TCP header, the transport header is okay
to have but that is essentially what we compute via network + hlen
anyway.

>                     /* Obtain start of transport */
>                      if ((tx_flags & I40E_TX_FLAGS_TUNNEL))
>                                 l4_hdr.transport =
> skb_inner_transport_header(skb);
>                     else
>                                 l4_hdr.transport =
> skb_transport_header(skb);

If an offload is not requested there is no guarantee that the
transport header will be populated.  For example if someone is pumping
out raw frames with no offloads the only header offsets that will be
set are for network(L3) and MAC(L2).

>>                 /* 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;
>
>                     We need to skip extended IPV6 header as well and obtain
> correct value for
>                     l4_proto & hlen (may be something like this):
>
>                     {
>                             __be16 frag_off;
>                             u8 next_hdr;
>                             l4_proto = hdr.ipv6->nexthdr;
>                             if (unlikely((l4_hdr.transport - hdr.network) ==
> sizeof(struct ipv6hdr))) {
>                                     off = ipv6_skip_exthdr(skb, hdr.network
> - skb->data + sizeof(struct ipv6hdr),
> &l4_proto,&frag_off);
>                                     if (unlikely(off < 0))
>                                                     return; /* Unexpected */
>                                     if (unlikely(frag_off))
>                                             l4_proto= NEXTHDR_FRAGMENT;
>
>                                     /* Update 'hlen' for the code below to
> work properly */
>                                     hlen = l4_hdr.transport - hdr.network;
>                             }
>                     }

Once again.  See the comment about IPv4/v6 and TCP only.  Your code as
suggested breaks that.

Also instead of using ipv6_skip_exthdr we would be better off in this
case using ipv6_find_hdr since we know we are looking for TCP, but we
don't know where the TCP header actually is as we aren't guaranteed
skb_transport_header will exist.

For now I think it would be better to leave this patch as is since it
is fixing a number of bugs that were in the code without introducing
any new ones.  I will submit a follow-on patch that can add support
for ATR in the case of IPv6 extension headers being present.

- Alex

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels
  2016-01-25 22:21     ` Alexander Duyck
@ 2016-01-26  0:16       ` Patil, Kiran
  0 siblings, 0 replies; 39+ messages in thread
From: Patil, Kiran @ 2016-01-26  0:16 UTC (permalink / raw)
  To: intel-wired-lan

Acked-by: Kiran Patil <kiran.patil@intel.com>


On 1/25/2016 2:21 PM, Alexander Duyck wrote:
> On Mon, Jan 25, 2016 at 11:27 AM, Patil, Kiran <kiran.patil@intel.com> wrote:
>>
>> Thanks Alex for fixing it. Please see in-line comments/suggestion.
>>
>> -- Kiran P.
>>
>>
>> On 1/24/2016 9:17 PM, Alexander Duyck wrote:
>>> 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 <aduyck@mirantis.com>
>>> ---
>>>    drivers/net/ethernet/intel/i40e/i40e_txrx.c |   28
>>> +++++++++++----------------
>>>    1 file changed, 11 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> index 6a76c169c07e..ed8d13637c15 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> @@ -2030,10 +2030,9 @@ tx_only:
>>>     * @tx_ring:  ring to add programming descriptor to
>>>     * @skb:      send buffer
>>>     * @tx_flags: send tx flags
>>> - * @protocol: wire protocol
>>>     **/
>>>    static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
>>> -                    u32 tx_flags, __be16 protocol)
>>> +                    u32 tx_flags)
>>>    {
>>>          struct i40e_filter_program_desc *fdir_desc;
>>>          struct i40e_pf *pf = tx_ring->vsi->back;
>>> @@ -2045,6 +2044,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 +2058,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 +2069,19 @@ static void i40e_atr(struct i40e_ring *tx_ring,
>>> struct sk_buff *skb,
>>                        union {
>>                                unsigned char *transport;
>>                                struct tcphdr *th; /* if needed */
>>                                struct udphdr *uh; /* if needed */
>>                       } l4_hdr;
> I'm not sure what the point of this is.  See the comment above which I
> moved from a few lines below about "Currently only IPv4/IPv6 with TCP
> is supported".  There is only TCP header, the transport header is okay
> to have but that is essentially what we compute via network + hlen
> anyway.
Agree. For now, looks like we support IPv4/IPv6 with TCP, hence it is 
not needed.
>>                      /* Obtain start of transport */
>>                       if ((tx_flags & I40E_TX_FLAGS_TUNNEL))
>>                                  l4_hdr.transport =
>> skb_inner_transport_header(skb);
>>                      else
>>                                  l4_hdr.transport =
>> skb_transport_header(skb);
> If an offload is not requested there is no guarantee that the
> transport header will be populated.  For example if someone is pumping
> out raw frames with no offloads the only header offsets that will be
> set are for network(L3) and MAC(L2).
Thanks for good info. I didn't knew about it.
>
>>>                  /* 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;
>>                      We need to skip extended IPV6 header as well and obtain
>> correct value for
>>                      l4_proto & hlen (may be something like this):
>>
>>                      {
>>                              __be16 frag_off;
>>                              u8 next_hdr;
>>                              l4_proto = hdr.ipv6->nexthdr;
>>                              if (unlikely((l4_hdr.transport - hdr.network) ==
>> sizeof(struct ipv6hdr))) {
>>                                      off = ipv6_skip_exthdr(skb, hdr.network
>> - skb->data + sizeof(struct ipv6hdr),
>> &l4_proto,&frag_off);
>>                                      if (unlikely(off < 0))
>>                                                      return; /* Unexpected */
>>                                      if (unlikely(frag_off))
>>                                              l4_proto= NEXTHDR_FRAGMENT;
>>
>>                                      /* Update 'hlen' for the code below to
>> work properly */
>>                                      hlen = l4_hdr.transport - hdr.network;
>>                              }
>>                      }
> Once again.  See the comment about IPv4/v6 and TCP only.  Your code as
> suggested breaks that.
>
> Also instead of using ipv6_skip_exthdr we would be better off in this
> case using ipv6_find_hdr since we know we are looking for TCP, but we
> don't know where the TCP header actually is as we aren't guaranteed
> skb_transport_header will exist.
Agree that we can use ipv6_find_hdr since we know what we are looking 
forward, that will solve the problem of what is the value for 
'next_protocol'. Certainly this patch make sense and follow-on patch can 
address extended IPv6 header support.

>
> For now I think it would be better to leave this patch as is since it
> is fixing a number of bugs that were in the code without introducing
> any new ones.  I will submit a follow-on patch that can add support
> for ATR in the case of IPv6 extension headers being present.
>
> - Alex

Thanks,
-- Kiran P.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160125/6984ba8f/attachment-0001.html>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested Alexander Duyck
@ 2016-01-27 16:00   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 16:00 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:16 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 01/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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Outer checksum not present on UDP tunnel traffic.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values instead of casting them in TSO function
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values instead of casting them in TSO function Alexander Duyck
@ 2016-01-27 16:03   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 16:03 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:16 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values
> instead of casting them in TSO function
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Code change properly applied

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path Alexander Duyck
@ 2016-01-27 16:09   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 16:09 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:16 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4
> header and checksum from L3 bits in TSO path
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
TSO works as expected, code changes properly applied

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all header changes into TSO function
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all header changes into TSO function Alexander Duyck
@ 2016-01-27 16:14   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 16:14 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all
> header changes into TSO function
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
TSO works as expected, checksum offload works as expected, code changes properly applied

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path Alexander Duyck
@ 2016-01-27 16:27   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 16:27 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header
> pointers with unions of pointers in Tx checksum path
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
checksum offload works as expected

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for IPv4 encapsulated in IPv6
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for IPv4 encapsulated in IPv6 Alexander Duyck
@ 2016-01-27 18:04   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:04 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for
> IPv4 encapsulated in IPv6
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
IPv4 encapsulated in IPv6 works properly

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6 extension headers in checksum offload
  2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6 extension headers in checksum offload Alexander Duyck
@ 2016-01-27 18:05   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:05 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6
> extension headers in checksum offload
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
IPv6 works properly

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to descriptor unless we complete
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to descriptor unless we complete Alexander Duyck
@ 2016-01-27 18:07   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:07 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to
> descriptor unless we complete
> 
> 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 <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   81 +++++++++++++----------
> --
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   81 +++++++++++++---------
> ---
>  2 files changed, 84 insertions(+), 78 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Tunnelling, offloads work correctly

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception handling for Tx checksum
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception handling for Tx checksum Alexander Duyck
@ 2016-01-27 18:08   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:08 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception
> handling for Tx checksum
> 
> 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
> 
> Drop I40E_TX_FLAGS_CSUM as it is unused, move the CHECKSUM_PARTIAL
> check into the function itself so that we can decrease indent.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   34 ++++++++++++++++------
> --
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h   |    1 -
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c |   35 ++++++++++++++++----
> -----
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.h |    1 -
>  4 files changed, 45 insertions(+), 26 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Tx checksum/offloading/TSO works correctly

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx packet checksum handling
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx packet checksum handling Alexander Duyck
@ 2016-01-27 18:09   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:09 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx
> packet checksum handling
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Rx checksumming works correctly

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM Alexander Duyck
@ 2016-01-27 18:17   ` Bowers, AndrewX
  2016-02-02 22:49   ` Jesse Brandeburg
  1 sibling, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:17 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:17 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support
> for SKB_GSO_UDP_TUNNEL_CSUM
> 
> 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 <aduyck@mirantis.com>
> ---
>  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(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Outer checksum present when enabled on X722

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels Alexander Duyck
  2016-01-25 19:27   ` Patil, Kiran
@ 2016-01-27 18:18   ` Bowers, AndrewX
  1 sibling, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:18 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:18 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to
> tunnels
> 
> 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 <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c |   28 +++++++++++---------------
> -
>  1 file changed, 11 insertions(+), 17 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Tunnel protocols applied correctly to packet headers, ATR counters show ATR activity as expected

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels Alexander Duyck
@ 2016-01-27 18:26   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:26 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:18 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support
> for IPv6 VXLAN or GENEVE tunnels
> 
> 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 <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c |   12 ------------
>  1 file changed, 12 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Code changes correctly applied

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to reflect newly enabled features
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to reflect newly enabled features Alexander Duyck
@ 2016-01-27 18:44   ` Bowers, AndrewX
  0 siblings, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:44 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:18 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to
> reflect newly enabled features
> 
> 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 <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
ethtool shows proper flags, code changes present

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 15/15] i40evf: Update feature flags to reflect newly enabled features
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 15/15] i40evf: " Alexander Duyck
  2016-01-25 19:43   ` Singhai, Anjali
@ 2016-01-27 18:45   ` Bowers, AndrewX
  1 sibling, 0 replies; 39+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:45 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Sunday, January 24, 2016 9:18 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH v3 15/15] i40evf: Update feature flags
> to reflect newly enabled features
> 
> 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 <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c |   23
> +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Code changes properly applied

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
  2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM Alexander Duyck
  2016-01-27 18:17   ` Bowers, AndrewX
@ 2016-02-02 22:49   ` Jesse Brandeburg
  2016-02-02 23:10     ` Jesse Brandeburg
  2016-02-03  0:06     ` Alexander Duyck
  1 sibling, 2 replies; 39+ messages in thread
From: Jesse Brandeburg @ 2016-02-02 22:49 UTC (permalink / raw)
  To: intel-wired-lan

On Sun, 24 Jan 2016 21:17:29 -0800
Alexander Duyck <aduyck@mirantis.com> wrote:

> The XL722 has support for providing the outer UDP tunnel checksum on

X722, not XL722

> transmits.  Make use of this feature to support segmenting UDP tunnels with
> outer checksums enabled.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---

...

> +		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);

Can we get a comment about how this is supposed to work?  Doesn't it
have endian problems?  I understand these lines are removing the payload
length from the checksum by getting the length of the payload,
inverting it and removing it from the checksum, and then below,
updating the checksum, but it is really hard to follow why you use
ntohs(1) without some explanation.

> +			l4.udp->check = ~csum_fold((__force __wsum)paylen);
> +		}
> +


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
  2016-02-02 22:49   ` Jesse Brandeburg
@ 2016-02-02 23:10     ` Jesse Brandeburg
  2016-02-02 23:18       ` Jesse Brandeburg
  2016-02-03  0:06     ` Alexander Duyck
  1 sibling, 1 reply; 39+ messages in thread
From: Jesse Brandeburg @ 2016-02-02 23:10 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2 Feb 2016 14:49:24 -0800
Jesse Brandeburg <jesse.brandeburg@intel.com> wrote:

> On Sun, 24 Jan 2016 21:17:29 -0800
> Alexander Duyck <aduyck@mirantis.com> wrote:
> 
> > The XL722 has support for providing the outer UDP tunnel checksum on
> 
> X722, not XL722
> 
> > transmits.  Make use of this feature to support segmenting UDP tunnels with
> > outer checksums enabled.
> > 
> > Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> > ---
> 
> ...
> 
> > +		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);
> 
> Can we get a comment about how this is supposed to work?  Doesn't it
> have endian problems?  I understand these lines are removing the payload
> length from the checksum by getting the length of the payload,
> inverting it and removing it from the checksum, and then below,
> updating the checksum, but it is really hard to follow why you use
> ntohs(1) without some explanation.
> 
> > +			l4.udp->check = ~csum_fold((__force __wsum)paylen);
> > +		}
> > +
> 

And, isn't this patch missing the check that enables outer UDP only in
the case of X722?

I expected to see an i40e_main.c change to tell the stack we support
UDP_TUNNEL_CSUM only on X722.


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
  2016-02-02 23:10     ` Jesse Brandeburg
@ 2016-02-02 23:18       ` Jesse Brandeburg
  0 siblings, 0 replies; 39+ messages in thread
From: Jesse Brandeburg @ 2016-02-02 23:18 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2 Feb 2016 15:10:00 -0800
Jesse Brandeburg <jesse.brandeburg@intel.com> wrote:

> On Tue, 2 Feb 2016 14:49:24 -0800
> Jesse Brandeburg <jesse.brandeburg@intel.com> wrote:
> 
> And, isn't this patch missing the check that enables outer UDP only in
> the case of X722?
> 
> I expected to see an i40e_main.c change to tell the stack we support
> UDP_TUNNEL_CSUM only on X722.

Doh, I see it now in patch 13/15.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM
  2016-02-02 22:49   ` Jesse Brandeburg
  2016-02-02 23:10     ` Jesse Brandeburg
@ 2016-02-03  0:06     ` Alexander Duyck
  1 sibling, 0 replies; 39+ messages in thread
From: Alexander Duyck @ 2016-02-03  0:06 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, Feb 2, 2016 at 2:49 PM, Jesse Brandeburg
<jesse.brandeburg@intel.com> wrote:
> On Sun, 24 Jan 2016 21:17:29 -0800
> Alexander Duyck <aduyck@mirantis.com> wrote:
>
>> The XL722 has support for providing the outer UDP tunnel checksum on
>
> X722, not XL722
>
>> transmits.  Make use of this feature to support segmenting UDP tunnels with
>> outer checksums enabled.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>
> ...
>
>> +             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);
>
> Can we get a comment about how this is supposed to work?  Doesn't it
> have endian problems?  I understand these lines are removing the payload
> length from the checksum by getting the length of the payload,
> inverting it and removing it from the checksum, and then below,
> updating the checksum, but it is really hard to follow why you use
> ntohs(1) without some explanation.

The logic is actually based off of
csum_tcpudp_nofold(http://lxr.free-electrons.com/source/lib/checksum.c#L193).
The byte swapping is taken care of via a combination of the ntohs
which converts to a shift, and the csum_fold call below.  What happens
is that by shifting by 8 we push the lower byte into the upper slot,
and the upper slot into the next 16 bits.  Then when csum_fold is run
it shifts the upper 16 bits down by 16 and adds them to the lower 16
which takes care of the other half of the rotation.

>> +                     l4.udp->check = ~csum_fold((__force __wsum)paylen);
>> +             }
>> +

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2016-02-03  0:06 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-25  5:16 [Intel-wired-lan] [next PATCH v3 00/15] TSO and checksum fixes for i40e Alexander Duyck
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 01/15] i40e/i40evf: Drop outer checksum offload that was not requested Alexander Duyck
2016-01-27 16:00   ` Bowers, AndrewX
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 02/15] i40e/i40evf: Use u64 values instead of casting them in TSO function Alexander Duyck
2016-01-27 16:03   ` Bowers, AndrewX
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 03/15] i40e/i40evf: Factor out L4 header and checksum from L3 bits in TSO path Alexander Duyck
2016-01-27 16:09   ` Bowers, AndrewX
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 04/15] i40e/i40evf: Consolidate all header changes into TSO function Alexander Duyck
2016-01-27 16:14   ` Bowers, AndrewX
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 05/15] i40e/i40evf: Replace header pointers with unions of pointers in Tx checksum path Alexander Duyck
2016-01-27 16:27   ` Bowers, AndrewX
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 06/15] i40e/i40evf: Add support for IPv4 encapsulated in IPv6 Alexander Duyck
2016-01-27 18:04   ` Bowers, AndrewX
2016-01-25  5:16 ` [Intel-wired-lan] [next PATCH v3 07/15] i40e/i40evf: Handle IPv6 extension headers in checksum offload Alexander Duyck
2016-01-27 18:05   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 08/15] i40e/i40evf: Do not write to descriptor unless we complete Alexander Duyck
2016-01-27 18:07   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 09/15] i40e/i40evf: Add exception handling for Tx checksum Alexander Duyck
2016-01-27 18:08   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 10/15] i40e/i40evf: Clean-up Rx packet checksum handling Alexander Duyck
2016-01-27 18:09   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 11/15] i40e/i40evf: Enable support for SKB_GSO_UDP_TUNNEL_CSUM Alexander Duyck
2016-01-27 18:17   ` Bowers, AndrewX
2016-02-02 22:49   ` Jesse Brandeburg
2016-02-02 23:10     ` Jesse Brandeburg
2016-02-02 23:18       ` Jesse Brandeburg
2016-02-03  0:06     ` Alexander Duyck
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 12/15] i40e: Fix ATR in relation to tunnels Alexander Duyck
2016-01-25 19:27   ` Patil, Kiran
2016-01-25 22:21     ` Alexander Duyck
2016-01-26  0:16       ` Patil, Kiran
2016-01-27 18:18   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 13/15] i40e: Do not drop support for IPv6 VXLAN or GENEVE tunnels Alexander Duyck
2016-01-27 18:26   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 14/15] i40e: Update feature flags to reflect newly enabled features Alexander Duyck
2016-01-27 18:44   ` Bowers, AndrewX
2016-01-25  5:17 ` [Intel-wired-lan] [next PATCH v3 15/15] i40evf: " Alexander Duyck
2016-01-25 19:43   ` Singhai, Anjali
2016-01-27 18:45   ` Bowers, AndrewX

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.