Skip to contentSkip to conversation

ixgbe: Specify rx hash type wrt rx desc RSS type

PATCH New Fan Du 4 participants 9 messages Apr 14, 2015 – Apr 15, 2015

Subsystems (2)
Network drivers Intel Ethernet
lore
Share & tools
lore

Fan Du

author
#0 lore raw

PATCH 1/1 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 <[email protected]>

---
Sorry for resending, I forgot to subscribe intel-wired-lan at lists.osuosl.org as previous mail is being held by list moderator.

note:
1. I checked ixgbe_pci_tbl for 82598, 82599 x540 series, RSS type in adavanced descriptor is all supported. x550 is in trial version, so not checked.

2. ixgbevf looks like didn't even set rx hash type
I'm not sure why this is left behind. IMHO it should be there.

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   23 +++++++++++++++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    1 +
 2 files changed, 22 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +21 -2
index 395dc6b..8915992 100644
@@ -1357,14 +1357,33 @@ static int __ixgbe_notify_dca(struct device *dev, void *data)
 }
 
 #endif /* CONFIG_IXGBE_DCA */
+static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
+{
+	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	case IXGBE_RXDADV_RSSTYPE_IPV4_TCP:
+	case IXGBE_RXDADV_RSSTYPE_IPV4_UDP:
+	case IXGBE_RXDADV_RSSTYPE_IPV6_TCP:
+	case IXGBE_RXDADV_RSSTYPE_IPV6_UDP:
+		return PKT_HASH_TYPE_L4;
+	case IXGBE_RXDADV_RSSTYPE_IPV4:
+	case IXGBE_RXDADV_RSSTYPE_IPV6:
+		return PKT_HASH_TYPE_L3;
+	default:
+		return PKT_HASH_TYPE_NONE;
+	}
+}
+
 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)
+	if (ring->netdev->features & NETIF_F_RXHASH) {
+		__le16 pkt_info = rx_desc->wb.lower.lo_dword.hs_rss.pkt_info;
+
 		skb_set_hash(skb,
 			     le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
-			     PKT_HASH_TYPE_L3);
+			     ixgbe_get_hash_type(pkt_info));
+	}
 }
 
 #ifdef IXGBE_FCOE
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h +1 -0
index c3ddc94..97d600e 100644
@@ -2225,6 +2225,7 @@ enum {
 #define IXGBE_RXDADV_RSSTYPE_IPV4_UDP   0x00000007
 #define IXGBE_RXDADV_RSSTYPE_IPV6_UDP   0x00000008
 #define IXGBE_RXDADV_RSSTYPE_IPV6_UDP_EX 0x00000009
+#define IXGBE_RXDADV_RSSTYPE_MASK       0x0000000F
 
 /* RSS Packet Types as indicated in the receive descriptor. */
 #define IXGBE_RXDADV_PKTTYPE_NONE       0x00000000
Signature · 1 line
1.7.1
Applied
#1 lore raw

PATCH 1/1 ixgbe: Specify rx hash type wrt rx desc RSS type

Quoted 15 lines · 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 <[email protected]>

---
Sorry for resending, I forgot to subscribe
intel-wired-lan at lists.osuosl.org
as previous mail is being held by list moderator.

note:
1. I checked ixgbe_pci_tbl for 82598, 82599 x540 series, RSS type in adavanced descriptor is all supported. x550 is in trial version, so not checked.

Quoted 8 lines

2. ixgbevf looks like didn't even set rx hash type
I'm not sure why this is left behind. IMHO it should be there.

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   23
+++++++++++++++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    1 +
 2 files changed, 22 insertions(+), 2 deletions(-)

I have applied your patch to my queue

Signature · 9 lines
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git
dev-queue
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150414/7d37f7aa/attachment.asc>
#2 lore raw

PATCH 1/1 ixgbe: Specify rx hash type wrt rx desc RSS type

In reply to Fan Du
Quoted 14 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Fan Du
Sent: Monday, April 13, 2015 11:56 PM
Subject: [Intel-wired-lan] [PATCH] 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 <[email protected]>

---
Sorry for resending, I forgot to subscribe intel-wired-lan at lists.osuosl.org as previous mail is being held by list moderator.

Quoted 7 lines

note:
1. I checked ixgbe_pci_tbl for 82598, 82599 x540 series, RSS type in adavanced descriptor is all supported. x550 is in trial version, so not checked.

2. ixgbevf looks like didn't even set rx hash type
I'm not sure why this is left behind. IMHO it should be there.

I will look into it.

Quoted 50 lines

---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +++++++++++++++++++++--
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +

2 files changed, 22 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +21 -2
index 395dc6b..8915992 100644
@@ -1357,14 +1357,33 @@ static int __ixgbe_notify_dca(struct device *dev, void *data)
 }
 
 #endif /* CONFIG_IXGBE_DCA */
+static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
+{
+	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	case IXGBE_RXDADV_RSSTYPE_IPV4_TCP:
+	case IXGBE_RXDADV_RSSTYPE_IPV4_UDP:
+	case IXGBE_RXDADV_RSSTYPE_IPV6_TCP:
+	case IXGBE_RXDADV_RSSTYPE_IPV6_UDP:
+		return PKT_HASH_TYPE_L4;
+	case IXGBE_RXDADV_RSSTYPE_IPV4:
+	case IXGBE_RXDADV_RSSTYPE_IPV6:
+		return PKT_HASH_TYPE_L3;
+	default:
+		return PKT_HASH_TYPE_NONE;
+	}
+}
+
 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)
+	if (ring->netdev->features & NETIF_F_RXHASH) {
+		__le16 pkt_info = rx_desc->wb.lower.lo_dword.hs_rss.pkt_info;
+
 		skb_set_hash(skb,
 			     le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
-			     PKT_HASH_TYPE_L3);
+			     ixgbe_get_hash_type(pkt_info));
+	}
 }
 
 #ifdef IXGBE_FCOE
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h +0 -0
index c3ddc94..97d600e 100644

drivers/net/ethernet/intel/ixgbe/ixgbe_type.h enum { · line 2227

Quoted 3 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h +0 -0
@@ -2225,6 +2225,7 @@ enum {
 #define IXGBE_RXDADV_RSSTYPE_IPV4_UDP   0x00000007
 #define IXGBE_RXDADV_RSSTYPE_IPV6_UDP   0x00000008
 #define IXGBE_RXDADV_RSSTYPE_IPV6_UDP_EX 0x00000009

+#define IXGBE_RXDADV_RSSTYPE_MASK 0x0000000F

This define already exists in ixgbe_type.h.

The out of tree driver has this already implemented. I guess we did not push it upstream:

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +23 -4
index 66adbd0..96703de 100644
@@ -1357,14 +1357,33 @@ 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) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV6_TCP_EX) | \
+	 (1ul << IXGBE_RXDADV_RSSTYPE_IPV6_UDP_EX))
+
 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

Thanks,
Emil

author
#3 lore raw

PATCH 1/1 ixgbevf: Set rx hash type for ingress packets

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

also use le16_to_cpu forcing type conversion to slient endian check warnings.

Signed-off-by Fan Du <[email protected]>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |    2 +-
 drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 ++++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   30 +++++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
index 8915992..1b3b5fb 100644
@@ -1359,7 +1359,7 @@ static int __ixgbe_notify_dca(struct device *dev, void *data)
 #endif /* CONFIG_IXGBE_DCA */
 static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
 {
-	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	switch (le16_to_cpu(pkt_info) & IXGBE_RXDADV_RSSTYPE_MASK) {
 	case IXGBE_RXDADV_RSSTYPE_IPV4_TCP:
 	case IXGBE_RXDADV_RSSTYPE_IPV4_UDP:
 	case IXGBE_RXDADV_RSSTYPE_IPV6_TCP:
drivers/net/ethernet/intel/ixgbevf/defines.h +12 -0
index 770e21a..040f7ca 100644
@@ -151,7 +151,19 @@ typedef u32 ixgbe_link_speed;
 #define IXGBE_RXDADV_STAT_FCSTAT_FCPRSP	0x00000020 /* 10: Recv. FCP_RSP */
 #define IXGBE_RXDADV_STAT_FCSTAT_DDP	0x00000030 /* 11: Ctxt w/ DDP */
 
+/* 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_RXDADV_RSSTYPE_MASK	0x0000000F
+
 #define IXGBE_RXDADV_PKTTYPE_MASK	0x0000FFF0
 #define IXGBE_RXDADV_PKTTYPE_MASK_EX	0x0001FFF0
 #define IXGBE_RXDADV_HDRBUFLEN_MASK	0x00007FE0
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +30 -0
index 4ee15ad..e529744 100644
@@ -492,6 +492,35 @@ static inline void ixgbevf_rx_checksum(struct ixgbevf_ring *ring,
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 }
 
+static inline enum pkt_hash_types ixgbevf_get_hash_type(__le16 pkt_info)
+{
+	switch ((le16_to_cpu(pkt_info) & IXGBE_RXDADV_RSSTYPE_MASK)) {
+	case IXGBE_RXDADV_RSSTYPE_IPV4_TCP:
+	case IXGBE_RXDADV_RSSTYPE_IPV4_UDP:
+	case IXGBE_RXDADV_RSSTYPE_IPV6_TCP:
+	case IXGBE_RXDADV_RSSTYPE_IPV6_UDP:
+		return PKT_HASH_TYPE_L4;
+	case IXGBE_RXDADV_RSSTYPE_IPV4:
+	case IXGBE_RXDADV_RSSTYPE_IPV6:
+		return PKT_HASH_TYPE_L3;
+	default:
+		return PKT_HASH_TYPE_NONE;
+	}
+}
+
+static inline void ixgbevf_rx_hash(struct ixgbevf_ring *ring,
+				   union ixgbe_adv_rx_desc *rx_desc,
+				   struct sk_buff *skb)
+{
+	if (ring->netdev->features & NETIF_F_RXHASH) {
+		__le16 pkt_info = rx_desc->wb.lower.lo_dword.hs_rss.pkt_info;
+
+		skb_set_hash(skb,
+			     le32_to_cpu(rx_desc->wb.lower.hi_dword.rss),
+			     ixgbevf_get_hash_type(pkt_info));
+	}
+}
+
 /**
  * ixgbevf_process_skb_fields - Populate skb header fields from Rx descriptor
  * @rx_ring: rx descriptor ring packet is being transacted on

@@ -507,6 +536,7 @@ static void ixgbevf_process_skb_fields(struct ixgbevf_ring *rx_ring,
 				       struct sk_buff *skb)
 {
 	ixgbevf_rx_checksum(rx_ring, rx_desc, skb);
+	ixgbevf_rx_hash(rx_ring, rx_desc, skb);
 
 	if (ixgbevf_test_staterr(rx_desc, IXGBE_RXD_STAT_VP)) {
 		u16 vid = le16_to_cpu(rx_desc->wb.upper.vlan);
Signature · 1 line
1.7.1
#4 lore raw

PATCH 1/1 ixgbevf: Set rx hash type for ingress packets

Quoted 24 lines

-----Original Message-----
From: Du, Fan
Sent: Tuesday, April 14, 2015 8:12 PM
To: Kirsher, Jeffrey T
Subject: [PATCH] ixgbevf: Set rx hash type for ingress packets

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

also use le16_to_cpu forcing type conversion to slient endian check warnings.

Signed-off-by Fan Du <[email protected]>

---

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |    2 +-
drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 ++++++++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   30 +++++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
index 8915992..1b3b5fb 100644

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __ixgbe_notify_dca(struct device *dev, void *data) · line 1362

Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
@@ -1359,7 +1359,7 @@ static int __ixgbe_notify_dca(struct device *dev, void *data)
 #endif /* CONFIG_IXGBE_DCA */
 static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
 {
-	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	switch (le16_to_cpu(pkt_info) & IXGBE_RXDADV_RSSTYPE_MASK) {

This appears to be a fix for your ixgbe patch and does not belong here. The proper way to handle it is to submit
version 2. Also checkout my reply to your ixgbe patch - it has the logic we use in ixgbe and there are some differences
that we need to address before we can accept this. Like the IPV4/6 extensions defines are not included and in the case of rss_type being 0 RSS is nor performed and so we probably should not call set_skb_hash() in that situation.

Thanks,
Emil

#5 lore raw

PATCH 1/1 ixgbevf: Set rx hash type for ingress packets

Quoted 37 lines

-----Original Message-----
From: Tantilov, Emil S
Sent: Wednesday, April 15, 2015 11:42 AM
To: Du, Fan; Kirsher, Jeffrey T
Cc: intel-wired-lan at lists.osuosl.org; e1000-devel at lists.sourceforge.net; fengyuleidian0615 at gmail.com
Subject: RE: [PATCH] ixgbevf: Set rx hash type for ingress packets

Quoted 9 lines

-----Original Message-----
From: Du, Fan
Sent: Tuesday, April 14, 2015 8:12 PM
To: Kirsher, Jeffrey T
Subject: [PATCH] ixgbevf: Set rx hash type for ingress packets

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

Quoted 8 lines

also use le16_to_cpu forcing type conversion to slient endian check warnings.

Signed-off-by Fan Du <[email protected]>

---

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |    2 +-
drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 ++++++++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   30

+++++++++++++++++++++

Review

Quoted 3 lines
3 files changed, 43 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __ixgbe_notify_dca(struct device *dev,

Quoted 4 lines

index 8915992..1b3b5fb 100644

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
@@ -1359,7 +1359,7 @@ static int __ixgbe_notify_dca(struct device *dev,

void *data)

Quoted 9 lines

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __ixgbe_notify_dca(struct device *dev, void *data) · line 1362

Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
 #endif /* CONFIG_IXGBE_DCA */
 static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
 {
-	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	switch (le16_to_cpu(pkt_info) & IXGBE_RXDADV_RSSTYPE_MASK) {

This appears to be a fix for your ixgbe patch and does not belong here. The proper way to handle it is to submit
version 2.

ok, I will send v2 for ixgbe patch, but Jeffrey has to drop previous ixgbe path he has queued.

Quoted 4 lines

Also checkout my reply to your ixgbe patch - it has the logic we use in ixgbe and there are some differences
that we need to address before we can accept this. Like the IPV4/6 extensions defines are not included

All RSSTYPE suffixed with '_EX' are marked as reserved, which hold no valid meaning from 82599 data sheet labeled February 2015 Revision 3.1 331520-002.

Is there any other official document states what does those bits mean?

Quoted 2 lines

and in the case of rss_type being 0 RSS is nor performed and so we probably should not call set_skb_hash() in that situation.

Hmm, this relies on the fact skb hash parts are cleared at the allocation of skb structure. yes we should not set hash for PKT_HASH_TYPE_NONE, however bnx2x/i40e all call set_skb_hash to clear skb hash parts for PKT_HASH_TYPE_NONE as a defensive programming probably.

I'm ok with either way, I will fix this part if you insist to do so. which would you prefer?

Quoted 3 lines

Thanks,
Emil

#6 lore raw

PATCH 1/1 ixgbevf: Set rx hash type for ingress packets

Quoted 7 lines · Fan
Quoted 1 line

This appears to be a fix for your ixgbe patch and does not belong

here. The

Quoted 2 lines

proper way to handle it is to submit
version 2.

ok, I will send v2 for ixgbe patch, but Jeffrey has to drop previous ixgbe path he has queued.

I was going to mention what Emil has already stated... Please provide a v2 of your original patch, with the necessary fixes integrated.

I have dropped both of your patches and await v2.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150415/a386cfb8/attachment.asc>

#7 lore raw

PATCH 1/1 ixgbevf: Set rx hash type for ingress packets

In reply to Du
Quoted 59 lines

-----Original Message-----
From: Du, Fan
Sent: Wednesday, April 15, 2015 12:28 AM
Subject: RE: [PATCH] ixgbevf: Set rx hash type for ingress packets

Quoted 35 lines

-----Original Message-----
From: Tantilov, Emil S
Sent: Wednesday, April 15, 2015 11:42 AM
To: Du, Fan; Kirsher, Jeffrey T
Cc: intel-wired-lan at lists.osuosl.org; e1000-devel at lists.sourceforge.net; fengyuleidian0615 at gmail.com
Subject: RE: [PATCH] ixgbevf: Set rx hash type for ingress packets

Quoted 9 lines

-----Original Message-----
From: Du, Fan
Sent: Tuesday, April 14, 2015 8:12 PM
To: Kirsher, Jeffrey T
Subject: [PATCH] ixgbevf: Set rx hash type for ingress packets

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

Quoted 8 lines

also use le16_to_cpu forcing type conversion to slient endian check warnings.

Signed-off-by Fan Du <[email protected]>

---

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |    2 +-
drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 ++++++++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   30

+++++++++++++++++++++

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __ixgbe_notify_dca(struct device *dev,

Quoted 8 lines
3 files changed, 43 insertions(+), 1 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
index 8915992..1b3b5fb 100644
@@ -1359,7 +1359,7 @@ static int __ixgbe_notify_dca(struct device *dev,

void *data)

Quoted 9 lines

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __ixgbe_notify_dca(struct device *dev, void *data) · line 1362

Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
 #endif /* CONFIG_IXGBE_DCA */
 static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
 {
-	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	switch (le16_to_cpu(pkt_info) & IXGBE_RXDADV_RSSTYPE_MASK) {

This appears to be a fix for your ixgbe patch and does not belong here. The proper way to handle it is to submit
version 2.

ok, I will send v2 for ixgbe patch, but Jeffrey has to drop previous ixgbe path he has queued.

Quoted 4 lines

Also checkout my reply to your ixgbe patch - it has the logic we use in ixgbe and there are some differences
that we need to address before we can accept this. Like the IPV4/6 extensions defines are not included

Quoted 4 lines

All RSSTYPE suffixed with '_EX' are marked as reserved, which hold no valid meaning from 82599 data sheet labeled February 2015 Revision 3.1 331520-002.

Is there any other official document states what does those bits mean?

They are defined, so they do probably have meaning, although you're right that it is not in the datasheet. I'll have to check.

Quoted 8 lines
Quoted 2 lines

and in the case of rss_type being 0 RSS is nor performed and so we probably should not call set_skb_hash() in that situation.

Hmm, this relies on the fact skb hash parts are cleared at the allocation of skb structure. yes we should not set hash for PKT_HASH_TYPE_NONE, however bnx2x/i40e all call set_skb_hash to clear skb hash parts for PKT_HASH_TYPE_NONE as a defensive programming probably. I'm ok with either way, I will fix this part if you insist to do so.
which would you prefer?

I'd rather we keep the logic we use in the out of tree driver (see my other email).

Thanks,
Emil

#8 lore raw

PATCH 1/1 ixgbevf: Set rx hash type for ingress packets

In reply to Tantilov
Quoted 33 lines · Emil S
Quoted 24 lines

-----Original Message-----
From: Du, Fan
Sent: Tuesday, April 14, 2015 8:12 PM
To: Kirsher, Jeffrey T
Subject: [PATCH] ixgbevf: Set rx hash type for ingress packets

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

also use le16_to_cpu forcing type conversion to slient endian check warnings.

Signed-off-by Fan Du <[email protected]>

---

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |    2 +-
drivers/net/ethernet/intel/ixgbevf/defines.h      |   12 ++++++++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   30 +++++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
index 8915992..1b3b5fb 100644

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __ixgbe_notify_dca(struct device *dev, void *data) · line 1362

Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
@@ -1359,7 +1359,7 @@ static int __ixgbe_notify_dca(struct device *dev, void *data)
 #endif /* CONFIG_IXGBE_DCA */
 static inline enum pkt_hash_types ixgbe_get_hash_type(__le16 pkt_info)
 {
-	switch (pkt_info & cpu_to_le16(IXGBE_RXDADV_RSSTYPE_MASK)) {
+	switch (le16_to_cpu(pkt_info) & IXGBE_RXDADV_RSSTYPE_MASK) {

This appears to be a fix for your ixgbe patch and does not belong here. The proper way to handle it is to submit
version 2. Also checkout my reply to your ixgbe patch - it has the logic we use in ixgbe and there are some differences
that we need to address before we can accept this. Like the IPV4/6 extensions defines are not included and in the case of rss_type being 0 RSS is nor performed and so we probably should not call set_skb_hash() in that situation.

Quoted 3 lines

Thanks,
Emil

Also instead of byte swapping the value, why not byte swap the cases in the switch statement? Then the byte swapping can all be done at compile time instead of having to do it at run-time on big endian systems.

- Alex