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 E73DAA0577; Tue, 14 Apr 2020 09:41:38 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C4D9C1C13B; Tue, 14 Apr 2020 09:41:36 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id B5F8B1C13A for ; Tue, 14 Apr 2020 09:41:34 +0200 (CEST) IronPort-SDR: T2pD9H3Jal8zTM83gZt8SsdusHGxnGkTpLVt+VTgM8GP9yGVWcjJbimVbTl1hl2DUHgYE9Y8Oz 45RiY6mPEAeg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2020 00:41:32 -0700 IronPort-SDR: HRrIKHpBl5T+EOPfbrnqcumatTMg12Nx0cRhp79oiua5jxoTZsGSb56yebcY6CI9ND3wdmBEU9 34nWdRRjCNCQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,381,1580803200"; d="scan'208";a="243749732" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by fmsmga007.fm.intel.com with ESMTP; 14 Apr 2020 00:41:31 -0700 Date: Tue, 14 Apr 2020 15:37:29 +0800 From: Ye Xiaolong To: Simei Su Cc: qi.z.zhang@intel.com, jingjing.wu@intel.com, dev@dpdk.org, yahui.cao@intel.com Message-ID: <20200414073729.GA27150@intel.com> References: <1585834375-157346-1-git-send-email-simei.su@intel.com> <1586513905-437173-1-git-send-email-simei.su@intel.com> <1586513905-437173-2-git-send-email-simei.su@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1586513905-437173-2-git-send-email-simei.su@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/iavf: add support for FDIR basic rule 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" On 04/10, Simei Su wrote: >This patch adds FDIR create/destroy/validate function in AVF. >Common pattern and queue/qgroup/passthru/drop actions are supported. > >Signed-off-by: Simei Su >--- > doc/guides/rel_notes/release_20_05.rst | 1 + > drivers/net/iavf/Makefile | 1 + > drivers/net/iavf/iavf.h | 17 + > drivers/net/iavf/iavf_fdir.c | 749 +++++++++++++++++++++++++++++++++ > drivers/net/iavf/iavf_vchnl.c | 154 ++++++- > drivers/net/iavf/meson.build | 1 + > 6 files changed, 922 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/iavf/iavf_fdir.c > >diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst >index b5962d8..17299ef 100644 >--- a/doc/guides/rel_notes/release_20_05.rst >+++ b/doc/guides/rel_notes/release_20_05.rst >@@ -99,6 +99,7 @@ New Features > > * Added generic filter support. > * Added advanced RSS configuration for VFs. >+ * Added advanced iavf with FDIR capability. > > > Removed Items >diff --git a/drivers/net/iavf/Makefile b/drivers/net/iavf/Makefile >index 7b0093a..b2b75d7 100644 >--- a/drivers/net/iavf/Makefile >+++ b/drivers/net/iavf/Makefile >@@ -25,6 +25,7 @@ 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 >+SRCS-$(CONFIG_RTE_LIBRTE_IAVF_PMD) += iavf_fdir.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 d813296..2f84a1f 100644 >--- a/drivers/net/iavf/iavf.h >+++ b/drivers/net/iavf/iavf.h >@@ -92,6 +92,17 @@ struct iavf_vsi { > struct iavf_flow_parser_node; > TAILQ_HEAD(iavf_parser_list, iavf_flow_parser_node); > >+struct iavf_fdir_conf { >+ struct virtchnl_fdir_add add_fltr; >+ struct virtchnl_fdir_del del_fltr; >+ uint64_t input_set; >+ uint32_t flow_id; >+}; >+ >+struct iavf_fdir_info { >+ struct iavf_fdir_conf conf; >+}; Why we need struct iavf_fdir_info since it has only one member? >+ > /* TODO: is that correct to assume the max number to be 16 ?*/ > #define IAVF_MAX_MSIX_VECTORS 16 > >@@ -131,6 +142,8 @@ struct iavf_info { > rte_spinlock_t flow_ops_lock; > struct iavf_parser_list rss_parser_list; > struct iavf_parser_list dist_parser_list; >+ >+ struct iavf_fdir_info fdir; /* flow director info */ > }; > > #define IAVF_MAX_PKT_TYPE 1024 >@@ -254,4 +267,8 @@ int iavf_add_del_eth_addr(struct iavf_adapter *adapter, > 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); >+int iavf_fdir_add(struct iavf_adapter *adapter, struct iavf_fdir_conf *filter); >+int iavf_fdir_del(struct iavf_adapter *adapter, struct iavf_fdir_conf *filter); >+int iavf_fdir_check(struct iavf_adapter *adapter, >+ struct iavf_fdir_conf *filter); > #endif /* _IAVF_ETHDEV_H_ */ >diff --git a/drivers/net/iavf/iavf_fdir.c b/drivers/net/iavf/iavf_fdir.c >new file mode 100644 >index 0000000..f2b10d7 >--- /dev/null >+++ b/drivers/net/iavf/iavf_fdir.c >@@ -0,0 +1,749 @@ >+/* SPDX-License-Identifier: BSD-3-Clause >+ * Copyright(c) 2019 Intel Corporation Should be 2020. >+ */ >+ >+#include >+#include >+#include >+#include >+#include >+#include >+#include >+ >+#include >+#include >+#include >+#include >+ >+#include "iavf.h" >+#include "iavf_generic_flow.h" >+#include "virtchnl.h" >+ >+#define IAVF_FDIR_MAX_QREGION_SIZE 128 >+ >+#define IAVF_FDIR_IPV6_TC_OFFSET 20 >+#define IAVF_IPV6_TC_MASK (0xFF << IAVF_FDIR_IPV6_TC_OFFSET) >+ >+#define IAVF_FDIR_INSET_ETH (\ >+ IAVF_INSET_ETHERTYPE) >+ >+#define IAVF_FDIR_INSET_ETH_IPV4 (\ >+ IAVF_INSET_IPV4_SRC | IAVF_INSET_IPV4_DST | \ >+ IAVF_INSET_IPV4_PROTO | IAVF_INSET_IPV4_TOS | \ >+ IAVF_INSET_IPV4_TTL) >+ >+#define IAVF_FDIR_INSET_ETH_IPV4_UDP (\ >+ IAVF_INSET_IPV4_SRC | IAVF_INSET_IPV4_DST | \ >+ IAVF_INSET_IPV4_TOS | IAVF_INSET_IPV4_TTL | \ >+ IAVF_INSET_UDP_SRC_PORT | IAVF_INSET_UDP_DST_PORT) >+ >+#define IAVF_FDIR_INSET_ETH_IPV4_TCP (\ >+ IAVF_INSET_IPV4_SRC | IAVF_INSET_IPV4_DST | \ >+ IAVF_INSET_IPV4_TOS | IAVF_INSET_IPV4_TTL | \ >+ IAVF_INSET_TCP_SRC_PORT | IAVF_INSET_TCP_DST_PORT) >+ >+#define IAVF_FDIR_INSET_ETH_IPV4_SCTP (\ >+ IAVF_INSET_IPV4_SRC | IAVF_INSET_IPV4_DST | \ >+ IAVF_INSET_IPV4_TOS | IAVF_INSET_IPV4_TTL | \ >+ IAVF_INSET_SCTP_SRC_PORT | IAVF_INSET_SCTP_DST_PORT) >+ >+#define IAVF_FDIR_INSET_ETH_IPV6 (\ >+ IAVF_INSET_IPV6_SRC | IAVF_INSET_IPV6_DST | \ >+ IAVF_INSET_IPV6_NEXT_HDR | IAVF_INSET_IPV6_TC | \ >+ IAVF_INSET_IPV6_HOP_LIMIT) >+ >+#define IAVF_FDIR_INSET_ETH_IPV6_UDP (\ >+ IAVF_INSET_IPV6_SRC | IAVF_INSET_IPV6_DST | \ >+ IAVF_INSET_IPV6_TC | IAVF_INSET_IPV6_HOP_LIMIT | \ >+ IAVF_INSET_UDP_SRC_PORT | IAVF_INSET_UDP_DST_PORT) >+ >+#define IAVF_FDIR_INSET_ETH_IPV6_TCP (\ >+ IAVF_INSET_IPV6_SRC | IAVF_INSET_IPV6_DST | \ >+ IAVF_INSET_IPV6_TC | IAVF_INSET_IPV6_HOP_LIMIT | \ >+ IAVF_INSET_TCP_SRC_PORT | IAVF_INSET_TCP_DST_PORT) >+ >+#define IAVF_FDIR_INSET_ETH_IPV6_SCTP (\ >+ IAVF_INSET_IPV6_SRC | IAVF_INSET_IPV6_DST | \ >+ IAVF_INSET_IPV6_TC | IAVF_INSET_IPV6_HOP_LIMIT | \ >+ IAVF_INSET_SCTP_SRC_PORT | IAVF_INSET_SCTP_DST_PORT) >+ >+static struct iavf_pattern_match_item iavf_fdir_pattern[] = { >+ {iavf_pattern_ethertype, IAVF_FDIR_INSET_ETH, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv4, IAVF_FDIR_INSET_ETH_IPV4, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv4_udp, IAVF_FDIR_INSET_ETH_IPV4_UDP, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv4_tcp, IAVF_FDIR_INSET_ETH_IPV4_TCP, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv4_sctp, IAVF_FDIR_INSET_ETH_IPV4_SCTP, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv6, IAVF_FDIR_INSET_ETH_IPV6, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv6_udp, IAVF_FDIR_INSET_ETH_IPV6_UDP, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv6_tcp, IAVF_FDIR_INSET_ETH_IPV6_TCP, IAVF_INSET_NONE}, >+ {iavf_pattern_eth_ipv6_sctp, IAVF_FDIR_INSET_ETH_IPV6_SCTP, IAVF_INSET_NONE}, >+}; >+ >+static struct iavf_flow_parser iavf_fdir_parser; >+ >+static int >+iavf_fdir_init(struct iavf_adapter *ad) >+{ >+ struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad); >+ struct iavf_flow_parser *parser; >+ >+ if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_FDIR_PF) Need to check whether vf->vf_res is NULL, otherwise it may cause segfault. >+ parser = &iavf_fdir_parser; >+ else >+ return -ENOTSUP; >+ >+ return iavf_register_parser(parser, ad); >+} >+ >+static void >+iavf_fdir_uninit(struct iavf_adapter *ad) >+{ >+ struct iavf_flow_parser *parser; >+ >+ parser = &iavf_fdir_parser; >+ >+ iavf_unregister_parser(parser, ad); Simplify to iavf_unregister_parser(&iavf_fdir_parser, ad) ? >+} >+ >+static int >+iavf_fdir_create(struct iavf_adapter *ad, >+ struct rte_flow *flow, >+ void *meta, >+ struct rte_flow_error *error) >+{ >+ struct iavf_fdir_conf *filter = meta; >+ struct iavf_fdir_conf *rule; >+ int ret; >+ >+ rule = rte_zmalloc("fdir_entry", sizeof(*rule), 0); >+ if (!rule) { >+ rte_flow_error_set(error, ENOMEM, >+ RTE_FLOW_ERROR_TYPE_HANDLE, NULL, >+ "Failed to allocate memory"); Better to be more specific, like "Failed to allocate memory for fdir rule." >+ return -rte_errno; >+ } >+ >+ ret = iavf_fdir_add(ad, filter); >+ if (ret) { >+ rte_flow_error_set(error, -ret, >+ RTE_FLOW_ERROR_TYPE_HANDLE, NULL, >+ "Add filter rule failed."); What about "Failed to add filter rule" to make it consistent with other error log. And same for other error logs below. Thanks, Xiaolong