hi, qi

On 4/13/2020 10:02 AM, Zhang, Qi Z wrote:
Hi Jeff:

-----Original Message-----
From: Guo, Jia <jia....@intel.com>
Sent: Saturday, April 11, 2020 8:10 AM
To: Ye, Xiaolong <xiaolong...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
Cc: dev@dpdk.org; Wu, Jingjing <jingjing...@intel.com>; Cao, Yahui
<yahui....@intel.com>; Su, Simei <simei...@intel.com>; Guo, Jia
<jia....@intel.com>
Subject: [dpdk-dev v3 2/4] net/iavf: add RSS configuration for VFs

The VF must be capable of configuring RSS. Add a virtchnl handler to parse a
specific RSS configuration, and process the configuration for VFs, such as add
or delete a RSS rule.

Signed-off-by: Jeff Guo <jia....@intel.com>
---
v3->v2:
1.add doc in release note
2.refine some naming base on virtchnl definition.
---
  doc/guides/rel_notes/release_20_05.rst |    2 +
  drivers/net/iavf/Makefile              |    1 +
  drivers/net/iavf/iavf.h                |    2 +
  drivers/net/iavf/iavf_hash.c           | 1108
++++++++++++++++++++++++
  drivers/net/iavf/iavf_vchnl.c          |   33 +-
  drivers/net/iavf/meson.build           |    1 +
  6 files changed, 1142 insertions(+), 5 deletions(-)  create mode 100644
drivers/net/iavf/iavf_hash.c

diff --git a/doc/guides/rel_notes/release_20_05.rst
b/doc/guides/rel_notes/release_20_05.rst
index 4b81893ff..b5962d8e4 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -98,6 +98,8 @@ New Features
    Update the Intel iavf driver with new features and improvements,
including:

    * Added generic filter support.
+  * Added advanced RSS configuration for VFs.
+

  Removed Items
  -------------
diff --git a/drivers/net/iavf/Makefile b/drivers/net/iavf/Makefile index
1bf0f26b5..7b0093a3e 100644
--- a/drivers/net/iavf/Makefile
+++ b/drivers/net/iavf/Makefile
@@ -24,6 +24,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) +=
iavf_ethdev.c
  SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) += iavf_vchnl.c
  SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) += iavf_rxtx.c
  SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) += iavf_generic_flow.c
+SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) += iavf_hash.c
  ifeq ($(CONFIG_RTE_ARCH_X86), y)
  SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) += iavf_rxtx_vec_sse.c  endif diff
--git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
78bdaff20..d813296d3 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -252,4 +252,6 @@ int iavf_config_promisc(struct iavf_adapter
*adapter, bool enable_unicast,  int iavf_add_del_eth_addr(struct
iavf_adapter *adapter,
                         struct rte_ether_addr *addr, bool add);  int
iavf_add_del_vlan(struct iavf_adapter *adapter, uint16_t vlanid, bool add);
+int iavf_add_del_rss_cfg(struct iavf_adapter *adapter,
+                        struct virtchnl_rss_cfg *rss_cfg, bool add);
  #endif /* _IAVF_ETHDEV_H_ */
diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c new
file mode 100644 index 000000000..f831738cc
--- /dev/null
+++ b/drivers/net/iavf/iavf_hash.c
@@ -0,0 +1,1108 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Intel Corporation
+ */
+
+
+static uint64_t
+iavf_rss_hf_refine(uint64_t rss_hf, const struct rte_flow_item pattern[],
+                  const struct rte_flow_action *action,
+                  struct rte_flow_error *error)
+{
+       const struct rte_flow_item *item;
+
+       for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++)
{
+               if (item->type == RTE_FLOW_ITEM_TYPE_GTP_PSC) {
+                       const struct rte_flow_action_rss *rss = action->conf;
+                       const struct rte_flow_item_gtp_psc *psc = item->spec;
+
+                       if (psc && ((psc->pdu_type == GTP_EH_PDU_LINK_UP &&
+                                    (rss->types & ETH_RSS_L3_SRC_ONLY)) ||
Why we need to check SRC_ONLY here, the purpose of the function is to refine 
the rss_hf,
Should we'd better input set check to the stage " Find matched proto hdrs according 
to hash type" later


agree with you the point that we don't need to check the SRC/DST here, because we could exactly check it later.


+                                   (!psc->pdu_type &&
Better use GTP_EH_PDU_LINK_DOWN


make sense.


+                                    (rss->types & ETH_RSS_L3_DST_ONLY)))) {
+                               rss_hf |= ETH_RSS_GTPU;
+                       } else {
+                               rte_flow_error_set(error, EINVAL,
+                                                  
RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
+                                                  pattern,
+                                                  "Invalid input set");
+                               return -rte_errno;
+                       }
+               }
+       }
+
+       return rss_hf;
+}
+
+static int
+iavf_hash_parse_action(struct iavf_pattern_match_item
*pattern_match_item,
+                      const struct rte_flow_item pattern[],
+                      const struct rte_flow_action actions[],
+                      void **meta, struct rte_flow_error *error) {
+       struct iavf_rss_meta *rss_meta = (struct iavf_rss_meta *)*meta;
+       struct rss_type_match_hdr *m = (struct rss_type_match_hdr *)
+                               (pattern_match_item->meta);
+       uint32_t type_list_len = RTE_DIM(iavf_hash_type_list);
+       struct iavf_hash_match_type *type_match_item;
+       enum rte_flow_action_type action_type;
+       const struct rte_flow_action_rss *rss;
+       const struct rte_flow_action *action;
+       uint64_t rss_hf;
+       uint16_t i;
+       bool item_found = false;
+
+       /* Supported action is RSS. */
+       for (action = actions; action->type !=
+               RTE_FLOW_ACTION_TYPE_END; action++) {
+               action_type = action->type;
+               switch (action_type) {
+               case RTE_FLOW_ACTION_TYPE_RSS:
+                       rss = action->conf;
+                       rss_hf = rss->types;
+
+                       /**
+                        * Check simultaneous use of SRC_ONLY and DST_ONLY
+                        * of the same level.
+                        */
+                       rss_hf = rte_eth_rss_hf_refine(rss_hf);
+
+                       /**
+                        * Check the item spec with the rss action and
+                        * refine rss hash field.
+                        */
+                       rss_hf = iavf_rss_hf_refine(rss_hf, pattern, action,
+                                                   error);
+
+                       /* Check if pattern is empty. */
+                       if (pattern_match_item->pattern_list !=
+                               iavf_pattern_empty && rss->func ==
+                               RTE_ETH_HASH_FUNCTION_SIMPLE_XOR)
+                               return rte_flow_error_set(error, ENOTSUP,
+                                       RTE_FLOW_ERROR_TYPE_ACTION, action,
+                                       "Not supported flow");
+
+                       /* Check if rss types match pattern. */
+                       if (rss->func != RTE_ETH_HASH_FUNCTION_SIMPLE_XOR) {
+                               if (((rss_hf & ETH_RSS_IPV4) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_NONFRAG_IPV4_UDP) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_NONFRAG_IPV4_TCP) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_NONFRAG_IPV4_SCTP) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_IPV6) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_NONFRAG_IPV6_UDP) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_NONFRAG_IPV6_TCP) !=
+                                       m->eth_rss_hint) &&
+                                   ((rss_hf & ETH_RSS_NONFRAG_IPV6_SCTP) !=
+                                       m->eth_rss_hint))
+                                       return rte_flow_error_set(error,
+                                       ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
+                                       action, "Not supported RSS types");
+                       }
Why we need above check ? what happens if the input set does not include any IP 
layer for example session id of PFCP?

......


PFCP is also include IP later, but yes, it should support for much case case, such as only ether.



+static int
+iavf_hash_create(__rte_unused struct iavf_adapter *ad,
+                __rte_unused struct rte_flow *flow, void *meta,
+                __rte_unused struct rte_flow_error *error) {
+       struct iavf_rss_meta *rss_meta = (struct iavf_rss_meta *)meta;
+       struct virtchnl_rss_cfg *rss_cfg;
+       int ret = 0;
+
+       rss_cfg = rte_zmalloc("iavf rss rule",
+                             sizeof(struct virtchnl_rss_cfg), 0);
+       if (!rss_cfg) {
+               rte_flow_error_set(error, EINVAL,
+                                  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+                                  "No memory for rss rule");
+               return -ENOMEM;
+       }
+
+       rss_cfg->proto_hdrs = *rss_meta->proto_hdrs;
+       rss_cfg->rss_algorithm = rss_meta->hash_function;
+
+       ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
+       if (!ret) {
+               flow->rule = rss_cfg;
+       } else {
+               PMD_DRV_LOG(ERR, "fail to add RSS configure");
+               rte_free(rss_cfg);
+       }
The "meta" be created by iavf_hash_parse_pattern_action is assumed to be freed 
here,


sure.


Reply via email to