[Intel-wired-lan] [PATCH] ixgbevf: Set rx hash type for ingress packets

[Du, Fan]

>-----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
>
>>-----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 <fan.du at intel.com>
>>---
>> 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(-)
>>
>>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>index 8915992..1b3b5fb 100644
>>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>@@ -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. 

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

>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?

>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?

>Thanks,
>Emil
>