All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [next PATCH 0/3] ixgbe/i40e: Fix ATR functionality related to raw sockets and IPv6 extension headers
@ 2016-01-26  3:31 Alexander Duyck
  2016-01-26  3:32 ` [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ " Alexander Duyck
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Alexander Duyck @ 2016-01-26  3:31 UTC (permalink / raw)
  To: intel-wired-lan

This patch set addresses several things.

First the i40e driver was lacking extension header parsing for ATR so I
went ahead and added support via the ipv6_find_hdr call.

Second the vxlan_port code in ixgbe was a bit convoluted and needed to be
cleaned up in order to allow me to address an issue in the ATR code.

Finally the ixgbe ATR code has been generating bogus rules for a while now
when raw sockets are being used.  The last patch resolves this and cleans
up the IPv6 extended header parsing to match the approach taken for i40e.

---

Alexander Duyck (3):
      i40e: Add support for ATR w/ IPv6 extension headers
      ixgbe: Store VXLAN port number in network order
      ixgbe: Fix ATR so that it correctly handles IPv6 extension headers


 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   30 ++++----
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    8 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   90 ++++++++-----------------
 3 files changed, 44 insertions(+), 84 deletions(-)

--

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

* [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ IPv6 extension headers
  2016-01-26  3:31 [Intel-wired-lan] [next PATCH 0/3] ixgbe/i40e: Fix ATR functionality related to raw sockets and IPv6 extension headers Alexander Duyck
@ 2016-01-26  3:32 ` Alexander Duyck
  2016-01-27 18:49   ` Bowers, AndrewX
  2016-01-26  3:36 ` [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number in network order Alexander Duyck
  2016-01-26  3:39 ` [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers Alexander Duyck
  2 siblings, 1 reply; 7+ messages in thread
From: Alexander Duyck @ 2016-01-26  3:32 UTC (permalink / raw)
  To: intel-wired-lan

This patch updates the code for determining the L4 protocol and L3 header
length so that when IPv6 extension headers are being used we can determine
the offset and type of the L4 protocol.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---

Testing Hints:
Test with mix of tunnel and non-tunnel headers to verify that frames
containing IPv6 extension headers are properly triggering ATR match events.

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

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index ed8d13637c15..1616a85fd25a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2044,7 +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;
+	int l4_proto;
 	u16 i;
 
 	/* make sure ATR is enabled */
@@ -2062,25 +2062,23 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	if (!(tx_flags & (I40E_TX_FLAGS_IPV4 | I40E_TX_FLAGS_IPV6)))
 		return;
 
-	if (tx_flags & I40E_TX_FLAGS_TUNNEL) {
-		hdr.network = skb_inner_network_header(skb);
-		hlen = skb_inner_network_header_len(skb);
-	} else {
-		/* snag network header to get L4 type and address */
-		hdr.network = skb_network_header(skb);
-
-		/* access ihl as u8 to avoid unaligned access on ia64 */
-		if (tx_flags & I40E_TX_FLAGS_IPV4)
-			hlen = (hdr.network[0] & 0x0F) << 2;
-		else
-			hlen = sizeof(struct ipv6hdr);
-	}
+	/* snag network header to get L4 type and address */
+	hdr.network = (tx_flags & I40E_TX_FLAGS_TUNNEL) ?
+		      skb_inner_network_header(skb) : skb_network_header(skb);
 
 	/* Note: tx_flags gets modified to reflect inner protocols in
 	 * tx_enable_csum function if encap is enabled.
 	 */
-	l4_proto = (tx_flags & I40E_TX_FLAGS_IPV4) ? hdr.ipv4->protocol :
-						     hdr.ipv6->nexthdr;
+	if (tx_flags & I40E_TX_FLAGS_IPV4) {
+		/* access ihl as u8 to avoid unaligned access on ia64 */
+		hlen = (hdr.network[0] & 0x0F) << 2;
+		l4_proto = hdr.ipv4->protocol;
+	} else {
+		hlen = hdr.network - skb->data;
+		l4_proto = ipv6_find_hdr(skb, &hlen, IPPROTO_TCP, NULL, NULL);
+		hlen -= hdr.network - skb->data;
+	}
+
 	if (l4_proto != IPPROTO_TCP)
 		return;
 


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

* [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number in network order
  2016-01-26  3:31 [Intel-wired-lan] [next PATCH 0/3] ixgbe/i40e: Fix ATR functionality related to raw sockets and IPv6 extension headers Alexander Duyck
  2016-01-26  3:32 ` [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ " Alexander Duyck
@ 2016-01-26  3:36 ` Alexander Duyck
  2016-03-01 18:23   ` Bowers, AndrewX
  2016-01-26  3:39 ` [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers Alexander Duyck
  2 siblings, 1 reply; 7+ messages in thread
From: Alexander Duyck @ 2016-01-26  3:36 UTC (permalink / raw)
  To: intel-wired-lan

The VXLAN port number should be stored in network order instead of in host
order as it is accessed from the hot-path in ATR.  This way we can avoid
having to do any byte swaps in order to validate the port number.

I moved the vxlan_port value into a hole in the read-mostly region of the
adapter struct.  This way it should be in a warm cache-line instead of in
some isolated region in memory when it needs to be accessed.

In addition I went through and stripped a bunch of unneeded ifdef flags
since having an extra variable present doesn't really hurt anything and
makes the code easier to read.  I also went through and dropped the
NETIF_F_RXCSUM flag which was being set in hw_encap_features but provides
no value as the flag is not evaluated in the Rx path.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---

Testing Hints:
Test with X550 to verify that VXLAN offloads are still occurring as they should.

 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    8 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   61 +++++++------------------
 2 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 75dfbc36e138..12ae5963af64 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -661,9 +661,7 @@ struct ixgbe_adapter {
 #define IXGBE_FLAG2_RSS_FIELD_IPV6_UDP		(u32)(1 << 9)
 #define IXGBE_FLAG2_PTP_PPS_ENABLED		(u32)(1 << 10)
 #define IXGBE_FLAG2_PHY_INTERRUPT		(u32)(1 << 11)
-#ifdef CONFIG_IXGBE_VXLAN
 #define IXGBE_FLAG2_VXLAN_REREG_NEEDED		BIT(12)
-#endif
 #define IXGBE_FLAG2_VLAN_PROMISC		BIT(13)
 
 	/* Tx fast path data */
@@ -675,6 +673,9 @@ struct ixgbe_adapter {
 	int num_rx_queues;
 	u16 rx_itr_setting;
 
+	/* Port number used to identify VXLAN traffic */
+	__be16 vxlan_port;
+
 	/* TX */
 	struct ixgbe_ring *tx_ring[MAX_TX_QUEUES] ____cacheline_aligned_in_smp;
 
@@ -782,9 +783,6 @@ struct ixgbe_adapter {
 	u32 timer_event_accumulator;
 	u32 vferr_refcount;
 	struct ixgbe_mac_addr *mac_table;
-#ifdef CONFIG_IXGBE_VXLAN
-	u16 vxlan_port;
-#endif
 	struct kobject *info_kobj;
 #ifdef CONFIG_IXGBE_HWMON
 	struct hwmon_buff *ixgbe_hwmon_buff;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 41bd7c4cecc1..88368d2f7e1a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4519,9 +4519,7 @@ static void ixgbe_clear_vxlan_port(struct ixgbe_adapter *adapter)
 	case ixgbe_mac_X550:
 	case ixgbe_mac_X550EM_x:
 		IXGBE_WRITE_REG(&adapter->hw, IXGBE_VXLANCTRL, 0);
-#ifdef CONFIG_IXGBE_VXLAN
 		adapter->vxlan_port = 0;
-#endif
 		break;
 	default:
 		break;
@@ -7504,9 +7502,6 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 	} hdr;
 	struct tcphdr *th;
 	struct sk_buff *skb;
-#ifdef CONFIG_IXGBE_VXLAN
-	u8 encap = false;
-#endif /* CONFIG_IXGBE_VXLAN */
 	__be16 vlan_id;
 
 	/* if ring doesn't have a interrupt vector, cannot perform ATR */
@@ -7522,28 +7517,21 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 	/* snag network header to get L4 type and address */
 	skb = first->skb;
 	hdr.network = skb_network_header(skb);
-	if (!skb->encapsulation) {
-		th = tcp_hdr(skb);
-	} else {
+	th = tcp_hdr(skb);
 #ifdef CONFIG_IXGBE_VXLAN
+	if (skb->encapsulation &&
+	    first->protocol == htons(ETH_P_IP) &&
+	    hdr.ipv4->protocol != IPPROTO_UDP) {
 		struct ixgbe_adapter *adapter = q_vector->adapter;
 
-		if (!adapter->vxlan_port)
-			return;
-		if (first->protocol != htons(ETH_P_IP) ||
-		    hdr.ipv4->version != IPVERSION ||
-		    hdr.ipv4->protocol != IPPROTO_UDP) {
-			return;
+		/* verify the port is recognized as VXLAN */
+		if (adapter->vxlan_port &&
+		    udp_hdr(skb)->dest == adapter->vxlan_port) {
+			hdr.network = skb_inner_network_header(skb);
+			th = inner_tcp_hdr(skb);
 		}
-		if (ntohs(udp_hdr(skb)->dest) != adapter->vxlan_port)
-			return;
-		encap = true;
-		hdr.network = skb_inner_network_header(skb);
-		th = inner_tcp_hdr(skb);
-#else
-		return;
-#endif /* CONFIG_IXGBE_VXLAN */
 	}
+#endif /* CONFIG_IXGBE_VXLAN */
 
 	/* Currently only IPv4/IPv6 with TCP is supported */
 	switch (hdr.ipv4->version) {
@@ -7625,10 +7613,8 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 		break;
 	}
 
-#ifdef CONFIG_IXGBE_VXLAN
-	if (encap)
+	if (hdr.network != skb_network_header(skb))
 		input.formatted.flow_type |= IXGBE_ATR_L4TYPE_TUNNEL_MASK;
-#endif /* CONFIG_IXGBE_VXLAN */
 
 	/* This assumes the Rx queue and Tx queue are bound to the same CPU */
 	ixgbe_fdir_add_signature_filter_82599(&q_vector->adapter->hw,
@@ -8287,7 +8273,6 @@ static void ixgbe_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 	struct ixgbe_hw *hw = &adapter->hw;
-	u16 new_port = ntohs(port);
 
 	if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
 		return;
@@ -8295,18 +8280,18 @@ static void ixgbe_add_vxlan_port(struct net_device *dev, sa_family_t sa_family,
 	if (sa_family == AF_INET6)
 		return;
 
-	if (adapter->vxlan_port == new_port)
+	if (adapter->vxlan_port == port)
 		return;
 
 	if (adapter->vxlan_port) {
 		netdev_info(dev,
 			    "Hit Max num of VXLAN ports, not adding port %d\n",
-			    new_port);
+			    ntohs(port));
 		return;
 	}
 
-	adapter->vxlan_port = new_port;
-	IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, new_port);
+	adapter->vxlan_port = port;
+	IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, ntohs(port));
 }
 
 /**
@@ -8319,7 +8304,6 @@ static void ixgbe_del_vxlan_port(struct net_device *dev, sa_family_t sa_family,
 				 __be16 port)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
-	u16 new_port = ntohs(port);
 
 	if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
 		return;
@@ -8327,9 +8311,9 @@ static void ixgbe_del_vxlan_port(struct net_device *dev, sa_family_t sa_family,
 	if (sa_family == AF_INET6)
 		return;
 
-	if (adapter->vxlan_port != new_port) {
+	if (adapter->vxlan_port != port) {
 		netdev_info(dev, "Port %d was not found, not deleting\n",
-			    new_port);
+			    ntohs(port));
 		return;
 	}
 
@@ -8973,17 +8957,6 @@ skip_sriov:
 	netdev->priv_flags |= IFF_UNICAST_FLT;
 	netdev->priv_flags |= IFF_SUPP_NOFCS;
 
-#ifdef CONFIG_IXGBE_VXLAN
-	switch (adapter->hw.mac.type) {
-	case ixgbe_mac_X550:
-	case ixgbe_mac_X550EM_x:
-		netdev->hw_enc_features |= NETIF_F_RXCSUM;
-		break;
-	default:
-		break;
-	}
-#endif /* CONFIG_IXGBE_VXLAN */
-
 #ifdef CONFIG_IXGBE_DCB
 	netdev->dcbnl_ops = &dcbnl_ops;
 #endif


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

* [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers
  2016-01-26  3:31 [Intel-wired-lan] [next PATCH 0/3] ixgbe/i40e: Fix ATR functionality related to raw sockets and IPv6 extension headers Alexander Duyck
  2016-01-26  3:32 ` [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ " Alexander Duyck
  2016-01-26  3:36 ` [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number in network order Alexander Duyck
@ 2016-01-26  3:39 ` Alexander Duyck
  2016-03-01 19:10   ` Bowers, AndrewX
  2 siblings, 1 reply; 7+ messages in thread
From: Alexander Duyck @ 2016-01-26  3:39 UTC (permalink / raw)
  To: intel-wired-lan

The ATR code was assuming that it would be able to use tcp_hdr for
every TCP frame that came through.  However this isn't the case as it
is possible for a frame to arrive that is TCP but sent through something
like a raw socket.  As a result the driver was setting up bad filters in
which tcp_hdr was really pointing to the network header so the data was
all invalid.

In order to correct this I have added a bit of parsing logic that will
determine the TCP header location based off of the network header and
either the offset in the case of the IPv4 header, or a walk through the
IPv6 extension headers until it encounters the header that indicates
IPPROTO_TCP.  In addition I have added checks to verify that the lowest
protocol provided is recognized as IPv4 or IPv6 to help mitigate raw
sockets using ETH_P_ALL from having ATR applied to them.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

---

Testing Hints:
In order for me to verify the bug was there I ended up having to use tools
such as tcpreplay and some raw socket apps to send TCP frames.  Without
instrumenting the driver this can be hard to see.  One approach might be to
enable 9K jumbo frames between two hosts and then start using TCP replay or
something similar to start walking through the same port and IP numbers,
but sending larger and larger packets.  On a system without this patch you
should see the following behavior:
  1.  IP total length is interpreted as destination port
  2.  Second octet of IP source address is interpreted as TCP flags.
      - Fin resides at bit 0, so odd values will not cause issue.
      - Syn is in second bit, so it is 20X more likely to cause issue
        - In other words test with 190.2.100.X, not 190.1.100.X

Testing with a range of frame sizes should start triggering table resets
without any flow director matches due to how the headers are being
misread.

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   45 ++++++++++++-------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 88368d2f7e1a..f3191b574dd3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7501,8 +7501,10 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 		struct ipv6hdr *ipv6;
 	} hdr;
 	struct tcphdr *th;
+	unsigned int hlen;
 	struct sk_buff *skb;
 	__be16 vlan_id;
+	int l4_proto;
 
 	/* if ring doesn't have a interrupt vector, cannot perform ATR */
 	if (!q_vector)
@@ -7514,10 +7516,14 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 
 	ring->atr_count++;
 
+	/* currently only IPv4/IPv6 with TCP is supported */
+	if ((first->protocol != htons(ETH_P_IP)) &&
+	    (first->protocol != htons(ETH_P_IPV6)))
+		return;
+
 	/* snag network header to get L4 type and address */
 	skb = first->skb;
 	hdr.network = skb_network_header(skb);
-	th = tcp_hdr(skb);
 #ifdef CONFIG_IXGBE_VXLAN
 	if (skb->encapsulation &&
 	    first->protocol == htons(ETH_P_IP) &&
@@ -7526,43 +7532,34 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 
 		/* verify the port is recognized as VXLAN */
 		if (adapter->vxlan_port &&
-		    udp_hdr(skb)->dest == adapter->vxlan_port) {
+		    udp_hdr(skb)->dest == adapter->vxlan_port)
 			hdr.network = skb_inner_network_header(skb);
-			th = inner_tcp_hdr(skb);
-		}
 	}
 #endif /* CONFIG_IXGBE_VXLAN */
 
 	/* Currently only IPv4/IPv6 with TCP is supported */
 	switch (hdr.ipv4->version) {
 	case IPVERSION:
-		if (hdr.ipv4->protocol != IPPROTO_TCP)
-			return;
+		/* access ihl as u8 to avoid unaligned access on ia64 */
+		hlen = (hdr.network[0] & 0x0F) << 2;
+		l4_proto = hdr.ipv4->protocol;
 		break;
 	case 6:
-		if (likely((unsigned char *)th - hdr.network ==
-			   sizeof(struct ipv6hdr))) {
-			if (hdr.ipv6->nexthdr != IPPROTO_TCP)
-				return;
-		} else {
-			__be16 frag_off;
-			u8 l4_hdr;
-
-			ipv6_skip_exthdr(skb, hdr.network - skb->data +
-					      sizeof(struct ipv6hdr),
-					 &l4_hdr, &frag_off);
-			if (unlikely(frag_off))
-				return;
-			if (l4_hdr != IPPROTO_TCP)
-				return;
-		}
+		hlen = hdr.network - skb->data;
+		l4_proto = ipv6_find_hdr(skb, &hlen, IPPROTO_TCP, NULL, NULL);
+		hlen -= hdr.network - skb->data;
 		break;
 	default:
 		return;
 	}
 
-	/* skip this packet since it is invalid or the socket is closing */
-	if (!th || th->fin)
+	if (l4_proto != IPPROTO_TCP)
+		return;
+
+	th = (struct tcphdr *)(hdr.network + hlen);
+
+	/* skip this packet since the socket is closing */
+	if (th->fin)
 		return;
 
 	/* sample on all syn packets or once every atr sample count */


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

* [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ IPv6 extension headers
  2016-01-26  3:32 ` [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ " Alexander Duyck
@ 2016-01-27 18:49   ` Bowers, AndrewX
  0 siblings, 0 replies; 7+ messages in thread
From: Bowers, AndrewX @ 2016-01-27 18:49 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: Monday, January 25, 2016 7:33 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ IPv6
> extension headers
> 
> This patch updates the code for determining the L4 protocol and L3 header
> length so that when IPv6 extension headers are being used we can
> determine the offset and type of the L4 protocol.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> 
> Testing Hints:
> Test with mix of tunnel and non-tunnel headers to verify that frames
> containing IPv6 extension headers are properly triggering ATR match events.
> 
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c |   30 +++++++++++++------------
> --
>  1 file changed, 14 insertions(+), 16 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
ATR events properly triggered with IPv6 tunnel and non-tunnel traffic, counters increment as expected

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

* [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number in network order
  2016-01-26  3:36 ` [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number in network order Alexander Duyck
@ 2016-03-01 18:23   ` Bowers, AndrewX
  0 siblings, 0 replies; 7+ messages in thread
From: Bowers, AndrewX @ 2016-03-01 18:23 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: Monday, January 25, 2016 7:36 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number
> in network order
> 
> The VXLAN port number should be stored in network order instead of in host
> order as it is accessed from the hot-path in ATR.  This way we can avoid
> having to do any byte swaps in order to validate the port number.
> 
> I moved the vxlan_port value into a hole in the read-mostly region of the
> adapter struct.  This way it should be in a warm cache-line instead of in some
> isolated region in memory when it needs to be accessed.
> 
> In addition I went through and stripped a bunch of unneeded ifdef flags since
> having an extra variable present doesn't really hurt anything and makes the
> code easier to read.  I also went through and dropped the NETIF_F_RXCSUM
> flag which was being set in hw_encap_features but provides no value as the
> flag is not evaluated in the Rx path.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> 
> Testing Hints:
> Test with X550 to verify that VXLAN offloads are still occurring as they should.
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    8 +--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   61 +++++++-----------------
> -
>  2 files changed, 20 insertions(+), 49 deletions(-)

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

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

* [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers
  2016-01-26  3:39 ` [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers Alexander Duyck
@ 2016-03-01 19:10   ` Bowers, AndrewX
  0 siblings, 0 replies; 7+ messages in thread
From: Bowers, AndrewX @ 2016-03-01 19:10 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: Monday, January 25, 2016 7:40 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Subject: [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly
> handles IPv6 extension headers
> 
> The ATR code was assuming that it would be able to use tcp_hdr for every
> TCP frame that came through.  However this isn't the case as it is possible for
> a frame to arrive that is TCP but sent through something like a raw socket.  As
> a result the driver was setting up bad filters in which tcp_hdr was really
> pointing to the network header so the data was all invalid.
> 
> In order to correct this I have added a bit of parsing logic that will determine
> the TCP header location based off of the network header and either the
> offset in the case of the IPv4 header, or a walk through the
> IPv6 extension headers until it encounters the header that indicates
> IPPROTO_TCP.  In addition I have added checks to verify that the lowest
> protocol provided is recognized as IPv4 or IPv6 to help mitigate raw sockets
> using ETH_P_ALL from having ATR applied to them.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> 
> ---
> 
> Testing Hints:
> In order for me to verify the bug was there I ended up having to use tools
> such as tcpreplay and some raw socket apps to send TCP frames.  Without
> instrumenting the driver this can be hard to see.  One approach might be to
> enable 9K jumbo frames between two hosts and then start using TCP replay
> or something similar to start walking through the same port and IP numbers,
> but sending larger and larger packets.  On a system without this patch you
> should see the following behavior:
>   1.  IP total length is interpreted as destination port
>   2.  Second octet of IP source address is interpreted as TCP flags.
>       - Fin resides at bit 0, so odd values will not cause issue.
>       - Syn is in second bit, so it is 20X more likely to cause issue
>         - In other words test with 190.2.100.X, not 190.1.100.X
> 
> Testing with a range of frame sizes should start triggering table resets
> without any flow director matches due to how the headers are being
> misread.
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   45 ++++++++++++---------
> ----
>  1 file changed, 21 insertions(+), 24 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Traffic handled correctly IPv4/v6

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

end of thread, other threads:[~2016-03-01 19:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-26  3:31 [Intel-wired-lan] [next PATCH 0/3] ixgbe/i40e: Fix ATR functionality related to raw sockets and IPv6 extension headers Alexander Duyck
2016-01-26  3:32 ` [Intel-wired-lan] [next PATCH 1/3] i40e: Add support for ATR w/ " Alexander Duyck
2016-01-27 18:49   ` Bowers, AndrewX
2016-01-26  3:36 ` [Intel-wired-lan] [next PATCH 2/3] ixgbe: Store VXLAN port number in network order Alexander Duyck
2016-03-01 18:23   ` Bowers, AndrewX
2016-01-26  3:39 ` [Intel-wired-lan] [next PATCH 3/3] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers Alexander Duyck
2016-03-01 19:10   ` 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.