hi, qi

On 5/23/2020 10:03 AM, Zhang, Qi Z wrote:

-----Original Message-----
From: Guo, Jia <jia....@intel.com>
Sent: Saturday, May 23, 2020 7:18 AM
To: Xing, Beilei <beilei.x...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>;
Wu, Jingjing <jingjing...@intel.com>
Cc: Ye, Xiaolong <xiaolong...@intel.com>; dev@dpdk.org; Guo, Jia
<jia....@intel.com>
Subject: [dpdk-dev] net/iavf: fix protocol field selector configure

When VFs configure the rss rule by virtchnl, it need to set bit mask into the
field selector for the protocol, then PF got the configure massage and parse
the field selector to the corresponding protocol field.

Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")

Signed-off-by: Jeff Guo <jia....@intel.com>
---
  drivers/net/iavf/iavf_hash.c | 474 +++++++++++++++++++++--------------
  1 file changed, 280 insertions(+), 194 deletions(-)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c index
af528863b..1e0ffad4c 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -43,6 +43,7 @@ struct iavf_hash_match_type {
        enum iavf_pattern_hint_type phint_type;
        uint64_t hash_type;
        struct virtchnl_proto_hdrs *proto_hdrs;
+       uint32_t link_type;
If this is just a hint for gtpu link type.
Better to rename to "gtpu_hint"
And use the already defined enum but not uint32_t.


ok.


  };

  struct iavf_rss_meta {
@@ -147,8 +148,11 @@ static struct iavf_pattern_match_item
iavf_hash_pattern_list[] = {
        {iavf_pattern_empty, IAVF_INSET_NONE, &phint_empty},  };

-#define        GTP_EH_PDU_LINK_UP              1
-#define        GTP_EH_PDU_LINK_DWN             0
+enum iavf_pattern_link_type {
Rename to iavf_gtpu_hint.


seem it is better.


+       IAVF_PATTERN_LINK_DOWN,
+       IAVF_PATTERN_LINK_UP,
+       IAVF_PATTERN_LINK_NONE,
+};
The configure is for GTP down/up link,
The name "xxx_LINK_DOWN", and "xxx_LINK_UP" is a little bit misleading.
Could be
IAVF_GTPU_HINT_UPLINK.
IAVF_GTPU_HINT_DOWNLINK.
IAVF_GTPU_HINT_N/A


ok, but i will choose  IAVF_GTPU_HINT_NONE but not IAVF_GTPU_HINT_N/A for common and it should also knowledgeable.


  #define TUNNEL_LEVEL_OUTER            0
  #define TUNNEL_LEVEL_FIRST_INNER      1
@@ -160,103 +164,112 @@ static struct iavf_pattern_match_item
iavf_hash_pattern_list[] = {
  #define BUFF_NOUSED                   0
  #define FIELD_FOR_PROTO_ONLY          0

+#define FIELD_SELECTOR(proto_hdr_field)        (1UL << ((proto_hdr_field) % \
+                                        (1UL << PROTO_HDR_SHIFT)))
Could be simplified to.
#define FIELD_SELECTOR(proto_hdr_field) \
        (1UL << (proto_hdr_field & PROTO_HDR_FIELD_MASK))


Use the currently macro to make it simple, sounds good. Thanks.


......

Regards
Qi

Reply via email to