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 <simei...@intel.com>
>---
> 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 <sys/queue.h>
>+#include <stdio.h>
>+#include <errno.h>
>+#include <stdint.h>
>+#include <string.h>
>+#include <unistd.h>
>+#include <stdarg.h>
>+
>+#include <rte_ether.h>
>+#include <rte_ethdev_driver.h>
>+#include <rte_malloc.h>
>+#include <rte_tailq.h>
>+
>+#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
