From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0B1DEA0577; Tue, 14 Apr 2020 05:43:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6CC911C05C; Tue, 14 Apr 2020 05:43:04 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id D56971C039 for ; Tue, 14 Apr 2020 05:43:02 +0200 (CEST) IronPort-SDR: +ZzdxQDDZuaaPeGazCeCuvUKmcaRcxvdrUbrW+Kh/Ky27gPmhMW/v26FrwxqCwRx9vJA6fDi8a pMlhqSfYfPjg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Apr 2020 20:43:02 -0700 IronPort-SDR: yorND99k0prs4CAHDLSYXnq8MLq+sSluFDmO7qE+vMDMD8NFVUuFpBAJFtAqdODB35MnMXFFGx 57hQ1Jk9618Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,381,1580803200"; d="scan'208";a="298585962" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.67.68.150]) ([10.67.68.150]) by FMSMGA003.fm.intel.com with ESMTP; 13 Apr 2020 20:43:00 -0700 To: "Zhang, Qi Z" , "Ye, Xiaolong" Cc: "dev@dpdk.org" , "Wu, Jingjing" , "Cao, Yahui" , "Su, Simei" References: <20200318170401.7938-5-jia.guo@intel.com> <20200411000945.15311-1-jia.guo@intel.com> <20200411000945.15311-3-jia.guo@intel.com> <039ED4275CED7440929022BC67E70611547F7C92@SHSMSX103.ccr.corp.intel.com> From: Jeff Guo Message-ID: Date: Tue, 14 Apr 2020 11:42:59 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <039ED4275CED7440929022BC67E70611547F7C92@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [dpdk-dev v3 2/4] net/iavf: add RSS configuration for VFs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" hi, qi On 4/13/2020 10:02 AM, Zhang, Qi Z wrote: > Hi Jeff: > >> -----Original Message----- >> From: Guo, Jia >> Sent: Saturday, April 11, 2020 8:10 AM >> To: Ye, Xiaolong ; Zhang, Qi Z >> Cc: dev@dpdk.org; Wu, Jingjing ; Cao, Yahui >> ; Su, Simei ; Guo, Jia >> >> 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 >> --- >> 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.