[PATCH v2 0/3] RSS fix for ixgbe/ixgbevf

[PATCH v2 0/3] RSS fix for ixgbe/ixgbevf

[Fan Du]

This patchset provides RSS hash value for upper stack by indicating
whether it's a L3 or L4 hash. Thus other component of network
stack could utilize this hash value provided by NIC without caculating
by themselves.

Test:
* Simple iperf test using ixgbe for 82599ES
* Build test only for ixgbevf
* make C=1 CF=-D__CHECK_ENDIAN__, no warnings introduced by this patchset

Changelog:
v2:
  - Add patch3 to make all hash value from RSS part.
v1:
  - Code style issue suggeted by Tantilov, Emil S.
  - Initial version

Fan Du (3):
  ixgbe: Specify rx hash type wrt rx desc RSS type
  ixgbevf: Set rx hash type for ingress packets
  ixgbe: Don't report flow director filter's status

 drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c    |    2 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   25 ++++++++++++++++---
 drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 +++++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   27 +++++++++++++++++++++
 4 files changed, 60 insertions(+), 6 deletions(-)

[PATCH v2 1/3] ixgbe: Specify rx hash type wrt rx desc RSS type

[Fan Du]

RSS could be leveraged by taking account L4 src/dst ports
as ingredients, thus ingress skb rx hash type should honor
such the real configuration.

Signed-off-by: Fan Du <fan.du@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d3f4b0c..5df896b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1357,14 +1357,31 @@ static int __ixgbe_notify_dca(struct device *dev, void *data)
 }
 
 #endif /* CONFIG_IXGBE_DCA */
+
+#define IXGBE_RSS_L4_TYPES_MASK \
+	((1ul << IXGBE_RXDADV_RSSTYPE_IPV4_TCP) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV4_UDP) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV6_TCP) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV6_UDP))
+
 static inline void ixgbe_rx_hash(struct ixgbe_ring *ring,
 				 union ixgbe_adv_rx_desc *rx_desc,
 				 struct sk_buff *skb)
 {
-	if (ring->netdev->features & NETIF_F_RXHASH)
-		skb_set_hash(skb,
-			     le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
-			     PKT_HASH_TYPE_L3);
+	u16 rss_type;
+
+	if (!(ring->netdev->features & NETIF_F_RXHASH))
+		return;
+
+	rss_type = le16_to_cpu(rx_desc->wb.lower.lo_dword.hs_rss.pkt_info) &
+		   IXGBE_RXDADV_RSSTYPE_MASK;
+
+	if (!rss_type)
+		return;
+
+	skb_set_hash(skb, le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
+		     (IXGBE_RSS_L4_TYPES_MASK & (1ul << rss_type)) ?
+		     PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3);
 }
 
 #ifdef IXGBE_FCOE
-- 
1.7.1

[PATCH v2 2/3] ixgbevf: Set rx hash type for ingress packets

[Fan Du]

Set hash type for ingress packets according to NIC
advanced receive descriptors RSS type part.

Signed-off-by: Fan Du <fan.du@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 +++++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   27 +++++++++++++++++++++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/defines.h b/drivers/net/ethernet/intel/ixgbevf/defines.h
index 770e21a..af28ef3 100644
--- a/drivers/net/ethernet/intel/ixgbevf/defines.h
+++ b/drivers/net/ethernet/intel/ixgbevf/defines.h
@@ -161,6 +161,18 @@ typedef u32 ixgbe_link_speed;
 #define IXGBE_RXDADV_SPLITHEADER_EN	0x00001000
 #define IXGBE_RXDADV_SPH		0x8000
 
+/* RSS Hash results */
+#define IXGBE_RXDADV_RSSTYPE_NONE       0x00000000
+#define IXGBE_RXDADV_RSSTYPE_IPV4_TCP   0x00000001
+#define IXGBE_RXDADV_RSSTYPE_IPV4       0x00000002
+#define IXGBE_RXDADV_RSSTYPE_IPV6_TCP   0x00000003
+#define IXGBE_RXDADV_RSSTYPE_IPV6_EX    0x00000004
+#define IXGBE_RXDADV_RSSTYPE_IPV6       0x00000005
+#define IXGBE_RXDADV_RSSTYPE_IPV6_TCP_EX 0x00000006
+#define IXGBE_RXDADV_RSSTYPE_IPV4_UDP   0x00000007
+#define IXGBE_RXDADV_RSSTYPE_IPV6_UDP   0x00000008
+#define IXGBE_RXDADV_RSSTYPE_IPV6_UDP_EX 0x00000009
+
 #define IXGBE_RXD_ERR_FRAME_ERR_MASK ( \
 				      IXGBE_RXD_ERR_CE |  \
 				      IXGBE_RXD_ERR_LE |  \
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index a16d267..2a4ae83 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -457,6 +457,32 @@ static void ixgbevf_rx_skb(struct ixgbevf_q_vector *q_vector,
 	napi_gro_receive(&q_vector->napi, skb);
 }
 
+#define IXGBE_RSS_L4_TYPES_MASK \
+	((1ul << IXGBE_RXDADV_RSSTYPE_IPV4_TCP) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV4_UDP) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV6_TCP) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV6_UDP))
+
+static inline void ixgbevf_rx_hash(struct ixgbevf_ring *ring,
+				   union ixgbe_adv_rx_desc *rx_desc,
+				   struct sk_buff *skb)
+{
+	u16 rss_type;
+
+	if (!(ring->netdev->features & NETIF_F_RXHASH))
+		return;
+
+	rss_type = le16_to_cpu(rx_desc->wb.lower.lo_dword.hs_rss.pkt_info) &
+		   IXGBE_RXDADV_RSSTYPE_MASK;
+
+	if (!rss_type)
+		return;
+
+	skb_set_hash(skb, le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
+		     (IXGBE_RSS_L4_TYPES_MASK & (1ul << rss_type)) ?
+		     PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3);
+}
+
 /**
  * ixgbevf_rx_checksum - indicate in skb if hw indicated a good cksum
  * @ring: structure containig ring specific data
@@ -506,6 +532,7 @@ static void ixgbevf_process_skb_fields(struct ixgbevf_ring *rx_ring,
 				       union ixgbe_adv_rx_desc *rx_desc,
 				       struct sk_buff *skb)
 {
+	ixgbevf_rx_hash(rx_ring, rx_desc, skb);
 	ixgbevf_rx_checksum(rx_ring, rx_desc, skb);
 
 	if (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_VP)) {
-- 
1.7.1

[PATCH v2 3/3] ixgbe: Don't report flow director filter's status

[Fan Du]

For two reasons I want to disable this:
1. Not any part actually check the report status(Alexander Duyck)
2. To report hash value of a packet to stack,
   RSS -> 32bits hash value
   Perfect match fdir filter -> 13bits hash value
   Hashed-based fdir filter -> 31bits hash value

   fdir filter might hash on masked tuples for IP address,
   so it's still not desirable for usage.

So for now, just stick to RSS 32bits hash value.

Signed-off-by: Fan Du <fan.du@intel.com>
Suggested-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
index e0c3639..de83b6f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c
@@ -1394,14 +1394,12 @@ s32 ixgbe_init_fdir_perfect_82599(struct ixgbe_hw *hw, u32 fdirctrl)
 	/*
 	 * Continue setup of fdirctrl register bits:
 	 *  Turn perfect match filtering on
-	 *  Report hash in RSS field of Rx wb descriptor
 	 *  Initialize the drop queue
 	 *  Move the flexible bytes to use the ethertype - shift 6 words
 	 *  Set the maximum length per hash bucket to 0xA filters
 	 *  Send interrupt when 64 (0x4 * 16) filters are left
 	 */
 	fdirctrl |= IXGBE_FDIRCTRL_PERFECT_MATCH |
-		    IXGBE_FDIRCTRL_REPORT_STATUS |
 		    (IXGBE_FDIR_DROP_QUEUE << IXGBE_FDIRCTRL_DROP_Q_SHIFT) |
 		    (0x6 << IXGBE_FDIRCTRL_FLEX_SHIFT) |
 		    (0xA << IXGBE_FDIRCTRL_MAX_LENGTH_SHIFT) |
-- 
1.7.1

Re: [PATCH v2 0/3] RSS fix for ixgbe/ixgbevf

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

On Wed, 2015-04-29 at 10:57 +0800, Fan Du wrote:
> This patchset provides RSS hash value for upper stack by indicating
> whether it's a L3 or L4 hash. Thus other component of network
> stack could utilize this hash value provided by NIC without caculating
> by themselves.
> 
> Test:
> * Simple iperf test using ixgbe for 82599ES
> * Build test only for ixgbevf
> * make C=1 CF=-D__CHECK_ENDIAN__, no warnings introduced by this patchset
> 
> Changelog:
> v2:
>   - Add patch3 to make all hash value from RSS part.
> v1:
>   - Code style issue suggeted by Tantilov, Emil S.
>   - Initial version
> 
> Fan Du (3):
>   ixgbe: Specify rx hash type wrt rx desc RSS type
>   ixgbevf: Set rx hash type for ingress packets
>   ixgbe: Don't report flow director filter's status
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c    |    2 -
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   25 ++++++++++++++++---
>  drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 +++++++++
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   27 +++++++++++++++++++++
>  4 files changed, 60 insertions(+), 6 deletions(-)
> 

Thanks Fan, I have added your series to my queue for further review and
testing.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [Intel-wired-lan] [PATCH v2 0/3] RSS fix for ixgbe/ixgbevf

[Schmitt, Phillip J]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Thursday, April 30, 2015 6:00 AM
> To: Du, Fan
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH v2 0/3] RSS fix for ixgbe/ixgbevf
> 
> On Wed, 2015-04-29 at 10:57 +0800, Fan Du wrote:
> > This patchset provides RSS hash value for upper stack by indicating
> > whether it's a L3 or L4 hash. Thus other component of network stack
> > could utilize this hash value provided by NIC without caculating by
> > themselves.
> >
> > Test:
> > * Simple iperf test using ixgbe for 82599ES
> > * Build test only for ixgbevf
> > * make C=1 CF=-D__CHECK_ENDIAN__, no warnings introduced by this
> > patchset
> >
> > Changelog:
> > v2:
> >   - Add patch3 to make all hash value from RSS part.
> > v1:
> >   - Code style issue suggeted by Tantilov, Emil S.
> >   - Initial version
> >
> > Fan Du (3):
> >   ixgbe: Specify rx hash type wrt rx desc RSS type
> >   ixgbevf: Set rx hash type for ingress packets
> >   ixgbe: Don't report flow director filter's status
> >
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c    |    2 -
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   25 ++++++++++++++++---
> >  drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 +++++++++
> >  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   27
> +++++++++++++++++++++
> >  4 files changed, 60 insertions(+), 6 deletions(-)
> >
> 
> Thanks Fan, I have added your series to my queue for further review and testing.

Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>

RE: [Intel-wired-lan] [PATCH v2 1/3] ixgbe: Specify rx hash type wrt rx desc RSS type

[Schmitt, Phillip J]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Fan Du
> Sent: Tuesday, April 28, 2015 7:58 PM
> To: Kirsher, Jeffrey T
> Cc: netdev@vger.kernel.org; Du, Fan; intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH v2 1/3] ixgbe: Specify rx hash type wrt rx desc
> RSS type
> 
> RSS could be leveraged by taking account L4 src/dst ports as ingredients, thus
> ingress skb rx hash type should honor such the real configuration.
> 
> Signed-off-by: Fan Du <fan.du@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   25 +++++++++++++++++++++-
> ---
>  1 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d3f4b0c..5df896b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>

RE: [Intel-wired-lan] [PATCH v2 2/3] ixgbevf: Set rx hash type for ingress packets

[Schmitt, Phillip J]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Fan Du
> Sent: Tuesday, April 28, 2015 7:58 PM
> To: Kirsher, Jeffrey T
> Cc: netdev@vger.kernel.org; Du, Fan; intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH v2 2/3] ixgbevf: Set rx hash type for ingress
> packets
> 
> Set hash type for ingress packets according to NIC advanced receive descriptors
> RSS type part.
> 
> Signed-off-by: Fan Du <fan.du@intel.com>
> ---
>  drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 +++++++++
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   27
> +++++++++++++++++++++
>  2 files changed, 39 insertions(+), 0 deletions(-)

Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>

RE: [Intel-wired-lan] [PATCH v2 3/3] ixgbe: Don't report flow director filter's status

[Schmitt, Phillip J]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Fan Du
> Sent: Tuesday, April 28, 2015 7:58 PM
> To: Kirsher, Jeffrey T
> Cc: netdev@vger.kernel.org; Du, Fan; intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH v2 3/3] ixgbe: Don't report flow director filter's
> status
> 
> For two reasons I want to disable this:
> 1. Not any part actually check the report status(Alexander Duyck) 2. To report
> hash value of a packet to stack,
>    RSS -> 32bits hash value
>    Perfect match fdir filter -> 13bits hash value
>    Hashed-based fdir filter -> 31bits hash value
> 
>    fdir filter might hash on masked tuples for IP address,
>    so it's still not desirable for usage.
> 
> So for now, just stick to RSS 32bits hash value.
> 
> Signed-off-by: Fan Du <fan.du@intel.com>
> Suggested-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)

Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>