[PATCH v1 1/9] net/i40e: skip FDIR flow engine when FDIR is unavailable

[PATCH v1 1/9] net/i40e: skip FDIR flow engine when FDIR is unavailable

[Anatoly Burakov]

The FDIR flow engine was initialized even when flow director was not
usable. With no FDIR filter space, engine init failed and logged an
error. When FDIR was disabled because its queue could not be allocated,
the engine was initialized and FDIR flows only failed at install.

Return -ENOTSUP in both cases so that the engine is skipped.

Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_fdir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index 55997295edb..197b93f3746 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -2010,6 +2010,9 @@ i40e_fdir_flow_engine_init(const struct ci_flow_engine *engine,
 	uint32_t i;
 	int ret;
 
+	if ((pf->flags & I40E_FLAG_FDIR) == 0 || fdir_info->fdir_space_size == 0)
+		return -ENOTSUP;
+
 	snprintf(fdir_hash_name, sizeof(fdir_hash_name), "fdir_%s", dev_data->name);
 
 	struct rte_hash_parameters fdir_hash_params = {
-- 
2.52.0

[PATCH v1 2/9] net/i40e: reject VLAN with tunnel items in FDIR flows

[Anatoly Burakov]

The FDIR pattern graph allowed VLAN in front of ESP, L2TPv3 and GTP
items. These use customized pctypes whose input set does not include
the VLAN tag, so the VLAN match was silently ignored. Such patterns
were not accepted before the rework either.

Make VLAN lead to separate IPv4, IPv6 and UDP nodes that have no tunnel
successors.

Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_fdir.c | 65 +++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index 197b93f3746..276738907ad 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -45,7 +45,8 @@ struct i40e_fdir_engine_priv {
 
 /**
  * FDIR graph implementation (non-tunnel)
- * Pattern: START -> ETH -> [VLAN] -> (IPv4 | IPv6) -> [TCP | UDP | SCTP | ESP | L2TPv3 | GTP] -> END
+ * Pattern: START -> ETH -> (IPv4 | IPv6) -> [TCP | UDP | SCTP | ESP | L2TPv3 | GTP] -> END
+ *          START -> ETH -> VLAN -> (IPv4 | IPv6) -> [TCP | UDP | SCTP] -> END
  * With RAW flexible payload support:
  *   - L2: ETH/VLAN -> RAW -> RAW -> RAW -> END
  *   - L3: IPv4/IPv6 -> RAW -> RAW -> RAW -> END
@@ -53,6 +54,9 @@ struct i40e_fdir_engine_priv {
  * GTP tunnel support:
  *   - IPv4/IPv6 -> UDP -> GTP -> END (GTP-C, GTP-U outer)
  *   - IPv4/IPv6 -> UDP -> GTP -> IPv4/IPv6 -> END (GTP-U with inner IP)
+ * VLAN cannot be combined with ESP, L2TPv3 or GTP: these use customized
+ * pctypes whose input set does not include the VLAN tag, so VLAN is followed
+ * by separate IP/UDP nodes that do not lead to any of them.
  */
 
 enum i40e_fdir_node_id {
@@ -70,6 +74,9 @@ enum i40e_fdir_node_id {
 	I40E_FDIR_NODE_GTPU,
 	I40E_FDIR_NODE_INNER_IPV4,
 	I40E_FDIR_NODE_INNER_IPV6,
+	I40E_FDIR_NODE_VLAN_IPV4,
+	I40E_FDIR_NODE_VLAN_IPV6,
+	I40E_FDIR_NODE_VLAN_UDP,
 	I40E_FDIR_NODE_RAW,
 	I40E_FDIR_NODE_END,
 	I40E_FDIR_NODE_MAX,
@@ -1345,6 +1352,31 @@ static const struct flow_graph i40e_fdir_graph = {
 			.validate = i40e_fdir_node_ipv6_validate,
 			.process = i40e_fdir_node_inner_ipv6_process,
 		},
+		[I40E_FDIR_NODE_VLAN_IPV4] = {
+			.name = "VLAN_IPv4",
+			.type = RTE_FLOW_ITEM_TYPE_IPV4,
+			.constraints = FLOW_GRAPH_NODE_EXPECT_EMPTY |
+				       FLOW_GRAPH_NODE_EXPECT_SPEC |
+				       FLOW_GRAPH_NODE_EXPECT_RANGE,
+			.validate = i40e_fdir_node_ipv4_validate,
+			.process = i40e_fdir_node_ipv4_process,
+		},
+		[I40E_FDIR_NODE_VLAN_IPV6] = {
+			.name = "VLAN_IPv6",
+			.type = RTE_FLOW_ITEM_TYPE_IPV6,
+			.constraints = FLOW_GRAPH_NODE_EXPECT_EMPTY |
+				       FLOW_GRAPH_NODE_EXPECT_SPEC,
+			.validate = i40e_fdir_node_ipv6_validate,
+			.process = i40e_fdir_node_ipv6_process,
+		},
+		[I40E_FDIR_NODE_VLAN_UDP] = {
+			.name = "VLAN_UDP",
+			.type = RTE_FLOW_ITEM_TYPE_UDP,
+			.constraints = FLOW_GRAPH_NODE_EXPECT_EMPTY |
+				       FLOW_GRAPH_NODE_EXPECT_SPEC,
+			.validate = i40e_fdir_node_udp_validate,
+			.process = i40e_fdir_node_udp_process,
+		},
 		[I40E_FDIR_NODE_RAW] = {
 			.name = "RAW",
 			.type = RTE_FLOW_ITEM_TYPE_RAW,
@@ -1378,8 +1410,8 @@ static const struct flow_graph i40e_fdir_graph = {
 		},
 		[I40E_FDIR_NODE_VLAN] = {
 			.next = (size_t[]) {
-				I40E_FDIR_NODE_IPV4,
-				I40E_FDIR_NODE_IPV6,
+				I40E_FDIR_NODE_VLAN_IPV4,
+				I40E_FDIR_NODE_VLAN_IPV6,
 				I40E_FDIR_NODE_RAW,
 				I40E_FDIR_NODE_END,
 				FLOW_GRAPH_NODE_EDGE_END
@@ -1471,6 +1503,33 @@ static const struct flow_graph i40e_fdir_graph = {
 				FLOW_GRAPH_NODE_EDGE_END
 			}
 		},
+		[I40E_FDIR_NODE_VLAN_IPV4] = {
+			.next = (size_t[]) {
+				I40E_FDIR_NODE_TCP,
+				I40E_FDIR_NODE_VLAN_UDP,
+				I40E_FDIR_NODE_SCTP,
+				I40E_FDIR_NODE_RAW,
+				I40E_FDIR_NODE_END,
+				FLOW_GRAPH_NODE_EDGE_END
+			}
+		},
+		[I40E_FDIR_NODE_VLAN_IPV6] = {
+			.next = (size_t[]) {
+				I40E_FDIR_NODE_TCP,
+				I40E_FDIR_NODE_VLAN_UDP,
+				I40E_FDIR_NODE_SCTP,
+				I40E_FDIR_NODE_RAW,
+				I40E_FDIR_NODE_END,
+				FLOW_GRAPH_NODE_EDGE_END
+			}
+		},
+		[I40E_FDIR_NODE_VLAN_UDP] = {
+			.next = (size_t[]) {
+				I40E_FDIR_NODE_RAW,
+				I40E_FDIR_NODE_END,
+				FLOW_GRAPH_NODE_EDGE_END
+			}
+		},
 		[I40E_FDIR_NODE_RAW] = {
 			.next = (size_t[]) {
 				I40E_FDIR_NODE_RAW,
-- 
2.52.0

[PATCH v1 3/9] net/i40e: move FDIR flow state out of the flow engine

[Anatoly Burakov]

The FDIR flow pool, its free bitmap, and the duplicate filter hash table
are currently owned by the FDIR flow engine. We will be adding another
FDIR-based engine, so this shared state needs to move into the driver.

The pool and bitmap could be moved as is, but they are built around the
FDIR engine's flow structure, which is private to the engine, so the
driver would become tightly coupled to the engine's flow definitions.
The custom allocation scheme gives us nothing over a regular mempool
other than slightly lower memory usage. Rather than introduce that
coupling, replace the pool and bitmap with a mempool sized from the
FDIR engine flow size.

Allocate the hash table and the mempool at device init, and free them
at device close.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_ethdev.c    |   6 +-
 drivers/net/intel/i40e/i40e_ethdev.h    |  12 +-
 drivers/net/intel/i40e/i40e_fdir.c      |  59 +++++++++-
 drivers/net/intel/i40e/i40e_flow_fdir.c | 147 +++---------------------
 4 files changed, 91 insertions(+), 133 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
index cc000ec2a8b..4e66959c6a2 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.c
+++ b/drivers/net/intel/i40e/i40e_ethdev.c
@@ -1660,7 +1660,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	/* Initialize the filter invalidation configuration */
 	i40e_init_filter_invalidation(pf);
 
-	i40e_fdir_flow_store_init(dev);
+	ret = i40e_fdir_flow_store_init(dev);
+	if (ret < 0)
+		goto err_fdir_flow_store_init;
 	/* after device init, tunnel configuration is in its default state */
 	pf->tunnel_state = (struct i40e_tunnel_state){0};
 
@@ -1679,6 +1681,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	return 0;
 
 err_flow_engine_conf_init:
+	i40e_fdir_flow_store_free(&pf->fdir);
+err_fdir_flow_store_init:
 	rte_intr_callback_unregister(intr_handle,
 		i40e_dev_interrupt_handler, dev);
 	rte_free(dev->data->mac_addrs);
diff --git a/drivers/net/intel/i40e/i40e_ethdev.h b/drivers/net/intel/i40e/i40e_ethdev.h
index 77f79fc7da7..e5e136f5540 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.h
+++ b/drivers/net/intel/i40e/i40e_ethdev.h
@@ -780,6 +780,14 @@ struct i40e_fdir_tmpl_store {
 	struct i40e_fdir_tmpl_list list;
 };
 
+/* tracking for rte_flow filters; shared by all FDIR flow engines */
+struct i40e_fdir_flow_store {
+	/* preallocated flows, one per FDIR filter */
+	struct rte_mempool *pool;
+	/* used to reject duplicate filters */
+	struct rte_hash *hash_table;
+};
+
 /*
  *  A structure used to define fields of a FDIR related info.
  */
@@ -799,6 +807,7 @@ struct i40e_fdir_info {
 
 	struct i40e_fdir_state fdir_state;
 	struct i40e_fdir_tmpl_store tmpls;
+	struct i40e_fdir_flow_store flow_store;
 
 	/*
 	 * Priority ordering at filter invalidation(destroying a flow) between
@@ -1425,7 +1434,8 @@ bool i40e_fdir_filter_needs_status_wait(const struct i40e_pf *pf,
 					uint32_t filter_count);
 enum i40e_filter_pctype i40e_fdir_filter_pctype(const struct i40e_fdir_filter *filter);
 void i40e_fdir_tmpl_store_free(struct i40e_fdir_info *fdir_info);
-void i40e_fdir_flow_store_init(struct rte_eth_dev *dev);
+int i40e_fdir_flow_store_init(struct rte_eth_dev *dev);
+void i40e_fdir_flow_store_free(struct i40e_fdir_info *fdir_info);
 int i40e_fdir_inset_check(struct i40e_pf *pf,
 		enum i40e_filter_pctype pctype,
 		uint64_t input_set);
diff --git a/drivers/net/intel/i40e/i40e_fdir.c b/drivers/net/intel/i40e/i40e_fdir.c
index 2464eb47c22..c520baf619f 100644
--- a/drivers/net/intel/i40e/i40e_fdir.c
+++ b/drivers/net/intel/i40e/i40e_fdir.c
@@ -15,6 +15,7 @@
 #include <rte_log.h>
 #include <rte_memzone.h>
 #include <rte_malloc.h>
+#include <rte_mempool.h>
 #include <rte_arp.h>
 #include <rte_ip.h>
 #include <rte_udp.h>
@@ -28,6 +29,7 @@
 #include "base/i40e_type.h"
 #include "base/i40e_prototype.h"
 #include "i40e_ethdev.h"
+#include "i40e_flow.h"
 #include "i40e_rxtx.h"
 
 #define I40E_FDIR_MZ_NAME          "FDIR_MEMZONE"
@@ -960,14 +962,26 @@ i40e_fdir_programming_status_cleanup(struct ci_rx_queue *rxq)
 		PMD_DRV_LOG(INFO, "error report captured.");
 }
 
-void
+int
 i40e_fdir_flow_store_init(struct rte_eth_dev *dev)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	struct i40e_fdir_info *fdir_info = &pf->fdir;
+	struct i40e_fdir_flow_store *store = &fdir_info->flow_store;
 	uint32_t alloc = hw->func_caps.fd_filters_guaranteed;
 	uint32_t best = hw->func_caps.fd_filters_best_effort;
+	char fdir_hash_name[RTE_HASH_NAMESIZE];
+	struct rte_hash_parameters fdir_hash_params = {
+		.name = fdir_hash_name,
+		.entries = I40E_MAX_FDIR_FILTER_NUM,
+		.key_len = sizeof(struct i40e_fdir_input),
+		.hash_func = rte_hash_crc,
+		.hash_func_init_val = 0,
+		.socket_id = rte_socket_id(),
+	};
+	char fdir_pool_name[RTE_MEMPOOL_NAMESIZE];
+	int ret;
 
 	fdir_info->fdir_space_size = alloc + best;
 	fdir_info->fdir_rule_count = 0;
@@ -975,6 +989,48 @@ i40e_fdir_flow_store_init(struct rte_eth_dev *dev)
 	fdir_info->fdir_guarantee_total_space = alloc;
 
 	PMD_DRV_LOG(INFO, "FDIR guarantee space: %u, best_effort space %u.", alloc, best);
+
+	if ((pf->flags & I40E_FLAG_FDIR) == 0 || fdir_info->fdir_space_size == 0)
+		return 0;
+
+	snprintf(fdir_hash_name, sizeof(fdir_hash_name), "fdir_%s", dev->data->name);
+	store->hash_table = rte_hash_create(&fdir_hash_params);
+	if (store->hash_table == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to create fdir hash table: %s", rte_strerror(rte_errno));
+		ret = -rte_errno;
+		goto err;
+	}
+
+	snprintf(fdir_pool_name, sizeof(fdir_pool_name), "fdir_%s", dev->data->name);
+	store->pool = rte_mempool_create(fdir_pool_name, fdir_info->fdir_space_size,
+			i40e_flow_engine_fdir.flow_size, 0, 0, NULL, NULL, NULL, NULL,
+			rte_socket_id(),
+			/* pack flows tightly */
+			RTE_MEMPOOL_F_NO_SPREAD | RTE_MEMPOOL_F_NO_CACHE_ALIGN |
+			RTE_MEMPOOL_F_NO_IOVA_CONTIG |
+			/* flows are only accessed under the flow engine lock */
+			RTE_MEMPOOL_F_SP_PUT | RTE_MEMPOOL_F_SC_GET);
+	if (store->pool == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to create fdir flow pool: %s", rte_strerror(rte_errno));
+		ret = -rte_errno;
+		goto err;
+	}
+
+	return 0;
+err:
+	i40e_fdir_flow_store_free(fdir_info);
+	return ret;
+}
+
+void
+i40e_fdir_flow_store_free(struct i40e_fdir_info *fdir_info)
+{
+	struct i40e_fdir_flow_store *store = &fdir_info->flow_store;
+
+	rte_mempool_free(store->pool);
+	rte_hash_free(store->hash_table);
+	store->pool = NULL;
+	store->hash_table = NULL;
 }
 
 static uint32_t
@@ -1726,6 +1782,7 @@ i40e_fdir_cleanup(struct rte_eth_dev *dev)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 
 	i40e_fdir_tmpl_store_free(&pf->fdir);
+	i40e_fdir_flow_store_free(&pf->fdir);
 
 	if (pf->fdir.hw_configured) {
 		i40e_fdir_teardown(pf);
diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index 276738907ad..45a93defa7a 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -5,9 +5,7 @@
 #include "i40e_ethdev.h"
 #include "i40e_flow.h"
 
-#include <rte_bitmap.h>
-#include <rte_malloc.h>
-#include <rte_hash_crc.h>
+#include <rte_mempool.h>
 
 #include "../common/flow_engine.h"
 #include "../common/flow_check.h"
@@ -29,20 +27,6 @@ struct i40e_flow_engine_fdir_flow {
 	bool installed;
 };
 
-struct i40e_fdir_flow_pool_entry {
-	struct i40e_flow_engine_fdir_flow flow;
-	uint32_t idx;
-};
-
-struct i40e_fdir_engine_priv {
-	struct i40e_fdir_flow_pool_entry *pool;
-	struct rte_bitmap *bitmap;
-	struct rte_hash *hash_table;
-};
-
-#define I40E_FDIR_FLOW_ENTRY(flow_ptr) \
-	container_of((flow_ptr), struct i40e_fdir_flow_pool_entry, flow)
-
 /**
  * FDIR graph implementation (non-tunnel)
  * Pattern: START -> ETH -> (IPv4 | IPv6) -> [TCP | UDP | SCTP | ESP | L2TPv3 | GTP] -> END
@@ -1698,19 +1682,19 @@ i40e_fdir_ctx_to_flow(const struct ci_flow_engine_ctx *ctx,
 
 /* Add a flow director filter into the SW hash table */
 static int
-i40e_fdir_filter_hash_add(struct i40e_fdir_engine_priv *priv,
+i40e_fdir_filter_hash_add(struct i40e_fdir_flow_store *store,
 		const struct i40e_fdir_input *filter_key)
 {
 	int ret;
 
 	/* try to find the filter in the hash table first */
-	ret = rte_hash_lookup(priv->hash_table, filter_key);
+	ret = rte_hash_lookup(store->hash_table, filter_key);
 	if (ret >= 0) {
 		PMD_DRV_LOG(ERR, "Filter already exists!");
 		return -EEXIST;
 	}
 
-	ret = rte_hash_add_key(priv->hash_table, filter_key);
+	ret = rte_hash_add_key(store->hash_table, filter_key);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "Failed to insert fdir filter: %s", rte_strerror(-ret));
 		return ret;
@@ -1721,12 +1705,12 @@ i40e_fdir_filter_hash_add(struct i40e_fdir_engine_priv *priv,
 
 /* Delete a flow director filter from the SW hash table */
 static int
-i40e_fdir_filter_hash_del(struct i40e_fdir_engine_priv *priv,
+i40e_fdir_filter_hash_del(struct i40e_fdir_flow_store *store,
 		const struct i40e_fdir_input *filter_key)
 {
 	int ret;
 
-	ret = rte_hash_del_key(priv->hash_table, filter_key);
+	ret = rte_hash_del_key(store->hash_table, filter_key);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR,
 			    "Failed to delete fdir filter: %s", rte_strerror(-ret));
@@ -1742,7 +1726,6 @@ i40e_fdir_filter_hash_del(struct i40e_fdir_engine_priv *priv,
 static int
 i40e_fdir_flow_register(struct ci_flow *flow, struct rte_flow_error *error)
 {
-	struct i40e_fdir_engine_priv *priv = flow->engine_priv;
 	struct i40e_flow_engine_fdir_flow *fdir_flow = (struct i40e_flow_engine_fdir_flow *)flow;
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(flow->dev_data->dev_private);
 	const struct i40e_fdir_filter *filter = &fdir_flow->fdir_filter;
@@ -1791,7 +1774,7 @@ i40e_fdir_flow_register(struct ci_flow *flow, struct rte_flow_error *error)
 	}
 
 	/* register the flow with the hash table */
-	ret = i40e_fdir_filter_hash_add(priv, &filter->input);
+	ret = i40e_fdir_filter_hash_add(&pf->fdir.flow_store, &filter->input);
 	if (ret < 0) {
 		return rte_flow_error_set(error, -ret,
 				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
@@ -1833,7 +1816,6 @@ i40e_fdir_flow_register(struct ci_flow *flow, struct rte_flow_error *error)
 static int
 i40e_fdir_flow_unregister(struct ci_flow *flow, struct rte_flow_error *error)
 {
-	struct i40e_fdir_engine_priv *priv = flow->engine_priv;
 	struct i40e_flow_engine_fdir_flow *fdir_flow = (struct i40e_flow_engine_fdir_flow *)flow;
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(flow->dev_data->dev_private);
 	const struct i40e_fdir_filter *filter = &fdir_flow->fdir_filter;
@@ -1844,7 +1826,7 @@ i40e_fdir_flow_unregister(struct ci_flow *flow, struct rte_flow_error *error)
 	struct i40e_fdir_pctype_state *pctype_state = &fdir->pctype[pctype];
 	int ret;
 
-	ret = i40e_fdir_filter_hash_del(priv, &filter->input);
+	ret = i40e_fdir_filter_hash_del(&pf->fdir.flow_store, &filter->input);
 	if (ret < 0) {
 		return rte_flow_error_set(error, -ret,
 				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
@@ -2053,124 +2035,34 @@ i40e_fdir_flow_uninstall(struct ci_flow *flow, struct rte_flow_error *error)
 }
 
 static int
-i40e_fdir_flow_engine_init(const struct ci_flow_engine *engine,
+i40e_fdir_flow_engine_init(const struct ci_flow_engine *engine __rte_unused,
 		struct rte_eth_dev_data *dev_data,
-		void *priv_data)
+		void *priv_data __rte_unused)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev_data->dev_private);
-	struct i40e_fdir_info *fdir_info = &pf->fdir;
-	struct i40e_fdir_engine_priv *priv = priv_data;
-	char fdir_hash_name[RTE_HASH_NAMESIZE];
-	struct i40e_fdir_flow_pool_entry *pool;
-	struct rte_hash *hash_table;
-	struct rte_bitmap *bmp;
-	uint32_t bmp_size;
-	void *bmp_mem;
-	uint32_t i;
-	int ret;
 
-	if ((pf->flags & I40E_FLAG_FDIR) == 0 || fdir_info->fdir_space_size == 0)
+	if ((pf->flags & I40E_FLAG_FDIR) == 0 || pf->fdir.fdir_space_size == 0)
 		return -ENOTSUP;
 
-	snprintf(fdir_hash_name, sizeof(fdir_hash_name), "fdir_%s", dev_data->name);
-
-	struct rte_hash_parameters fdir_hash_params = {
-		.name = fdir_hash_name,
-		.entries = I40E_MAX_FDIR_FILTER_NUM,
-		.key_len = sizeof(struct i40e_fdir_input),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-	};
-
-	hash_table = rte_hash_create(&fdir_hash_params);
-	if (hash_table == NULL) {
-		PMD_INIT_LOG(ERR, "Failed to create fdir hash table: %s", rte_strerror(rte_errno));
-		ret = -rte_errno;
-		goto err_hash;
-	}
-
-	pool = rte_zmalloc(engine->name,
-			fdir_info->fdir_space_size * sizeof(*pool), 0);
-	if (pool == NULL) {
-		PMD_INIT_LOG(ERR, "Failed to allocate fdir pool");
-		ret = -ENOMEM;
-		goto err_pool;
-	}
-
-	bmp_size = rte_bitmap_get_memory_footprint(fdir_info->fdir_space_size);
-	bmp_mem = rte_zmalloc("fdir_bmap", bmp_size, RTE_CACHE_LINE_SIZE);
-	if (bmp_mem == NULL) {
-		PMD_INIT_LOG(ERR, "Failed to allocate fdir bitmap");
-		ret = -ENOMEM;
-		goto err_bitmap_mem;
-	}
-
-	bmp = rte_bitmap_init(fdir_info->fdir_space_size, bmp_mem, bmp_size);
-	if (bmp == NULL) {
-		PMD_INIT_LOG(ERR, "Failed to initialize fdir bitmap: %s", rte_strerror(rte_errno));
-		ret = -rte_errno;
-		goto err_bitmap;
-	}
-
-	for (i = 0; i < fdir_info->fdir_space_size; i++) {
-		pool[i].idx = i;
-		rte_bitmap_set(bmp, i);
-	}
-
-	priv->pool = pool;
-	priv->bitmap = bmp;
-	priv->hash_table = hash_table;
-
 	return 0;
-err_bitmap:
-	rte_free(bmp_mem);
-err_bitmap_mem:
-	rte_free(pool);
-err_pool:
-	rte_hash_free(hash_table);
-err_hash:
-	return ret;
-}
-
-static void
-i40e_fdir_flow_engine_uninit(const struct ci_flow_engine *engine __rte_unused,
-		void *priv_data)
-{
-	struct i40e_fdir_engine_priv *priv = priv_data;
-
-	rte_free(priv->bitmap);
-	rte_free(priv->pool);
-	rte_hash_free(priv->hash_table);
 }
 
 static struct ci_flow *
 i40e_fdir_flow_alloc(const struct ci_flow_engine *engine __rte_unused,
 		struct rte_eth_dev_data *dev_data,
-		void *priv_data)
+		void *priv_data __rte_unused)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev_data->dev_private);
 	struct i40e_fdir_info *fdir_info = &pf->fdir;
-	struct i40e_fdir_engine_priv *priv = priv_data;
 	struct i40e_flow_engine_fdir_flow *flow;
-	uint64_t slab = 0;
-	uint32_t pos = 0;
-	uint32_t bit;
 	size_t mem_sz;
-	int ret;
 
 	if (fdir_info->fdir_rule_count >= fdir_info->fdir_space_size)
 		return NULL;
 
-	ret = rte_bitmap_scan(priv->bitmap, &pos, &slab);
-	if (ret == 0)
+	if (rte_mempool_get(fdir_info->flow_store.pool, (void **)&flow) < 0)
 		return NULL;
 
-	bit = rte_bsf64(slab);
-	pos += bit;
-	rte_bitmap_clear(priv->bitmap, pos);
-
-	flow = &priv->pool[pos].flow;
 	/* do not touch ci_flow members, they are initialized by the caller */
 	mem_sz = sizeof(*flow) - sizeof(struct ci_flow);
 
@@ -2180,20 +2072,16 @@ i40e_fdir_flow_alloc(const struct ci_flow_engine *engine __rte_unused,
 
 static void
 i40e_fdir_flow_free(struct ci_flow *flow,
-		struct rte_eth_dev_data *dev_data __rte_unused,
-		void *priv_data)
+		struct rte_eth_dev_data *dev_data,
+		void *priv_data __rte_unused)
 {
-	struct i40e_fdir_engine_priv *priv = priv_data;
-	struct i40e_fdir_flow_pool_entry *entry;
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev_data->dev_private);
 
-	entry = I40E_FDIR_FLOW_ENTRY((struct i40e_flow_engine_fdir_flow *)flow);
-	/* idx is not set at alloc, it's set at init */
-	rte_bitmap_set(priv->bitmap, entry->idx);
+	rte_mempool_put(pf->fdir.flow_store.pool, flow);
 }
 
 static const struct ci_flow_engine_ops i40e_flow_engine_fdir_ops = {
 	.engine_init = i40e_fdir_flow_engine_init,
-	.engine_uninit = i40e_fdir_flow_engine_uninit,
 	.flow_alloc = i40e_fdir_flow_alloc,
 	.flow_free = i40e_fdir_flow_free,
 	.ctx_init = i40e_fdir_ctx_init,
@@ -2209,6 +2097,5 @@ const struct ci_flow_engine i40e_flow_engine_fdir = {
 	.ops = &i40e_flow_engine_fdir_ops,
 	.ctx_size = sizeof(struct i40e_fdir_ctx),
 	.flow_size = sizeof(struct i40e_flow_engine_fdir_flow),
-	.priv_size = sizeof(struct i40e_fdir_engine_priv),
 	.graph = &i40e_fdir_graph,
 };
-- 
2.52.0

[PATCH v1 4/9] net/i40e: fix FDIR ethertype match with other items

[Anatoly Burakov]

An FDIR filter matching the ETH ethertype can only match the ethertype,
but the FDIR parser accepted it together with other pattern items.

Reject the ethertype in the FDIR graph, and move ethertype matching to
a separate FDIR engine that only accepts an ETH item followed by END.

Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_fdir.c      |   4 +-
 drivers/net/intel/i40e/i40e_flow.c      |   1 +
 drivers/net/intel/i40e/i40e_flow.h      |   1 +
 drivers/net/intel/i40e/i40e_flow_fdir.c | 178 ++++++++++++++++--------
 4 files changed, 126 insertions(+), 58 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_fdir.c b/drivers/net/intel/i40e/i40e_fdir.c
index c520baf619f..69b3b763bae 100644
--- a/drivers/net/intel/i40e/i40e_fdir.c
+++ b/drivers/net/intel/i40e/i40e_fdir.c
@@ -1003,7 +1003,9 @@ i40e_fdir_flow_store_init(struct rte_eth_dev *dev)
 
 	snprintf(fdir_pool_name, sizeof(fdir_pool_name), "fdir_%s", dev->data->name);
 	store->pool = rte_mempool_create(fdir_pool_name, fdir_info->fdir_space_size,
-			i40e_flow_engine_fdir.flow_size, 0, 0, NULL, NULL, NULL, NULL,
+			RTE_MAX(i40e_flow_engine_fdir.flow_size,
+				i40e_flow_engine_fdir_ethertype.flow_size),
+			0, 0, NULL, NULL, NULL, NULL,
 			rte_socket_id(),
 			/* pack flows tightly */
 			RTE_MEMPOOL_F_NO_SPREAD | RTE_MEMPOOL_F_NO_CACHE_ALIGN |
diff --git a/drivers/net/intel/i40e/i40e_flow.c b/drivers/net/intel/i40e/i40e_flow.c
index 82a77064f06..0c73f0f13a7 100644
--- a/drivers/net/intel/i40e/i40e_flow.c
+++ b/drivers/net/intel/i40e/i40e_flow.c
@@ -34,6 +34,7 @@ const struct ci_flow_engine_list i40e_flow_engine_list = {
 	{
 		&i40e_flow_engine_ethertype,
 		&i40e_flow_engine_fdir,
+		&i40e_flow_engine_fdir_ethertype,
 		&i40e_flow_engine_tunnel_qinq,
 		&i40e_flow_engine_tunnel_vxlan,
 		&i40e_flow_engine_tunnel_nvgre,
diff --git a/drivers/net/intel/i40e/i40e_flow.h b/drivers/net/intel/i40e/i40e_flow.h
index 31269d4d93a..1577df4993e 100644
--- a/drivers/net/intel/i40e/i40e_flow.h
+++ b/drivers/net/intel/i40e/i40e_flow.h
@@ -17,6 +17,7 @@ extern const struct ci_flow_engine_list i40e_flow_engine_list;
 
 extern const struct ci_flow_engine i40e_flow_engine_ethertype;
 extern const struct ci_flow_engine i40e_flow_engine_fdir;
+extern const struct ci_flow_engine i40e_flow_engine_fdir_ethertype;
 extern const struct ci_flow_engine i40e_flow_engine_tunnel_qinq;
 extern const struct ci_flow_engine i40e_flow_engine_tunnel_vxlan;
 extern const struct ci_flow_engine i40e_flow_engine_tunnel_nvgre;
diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index 45a93defa7a..ba8c18250c8 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -70,9 +70,8 @@ static int
 i40e_fdir_node_eth_validate(const void *ctx __rte_unused, const struct rte_flow_item *item,
 		struct rte_flow_error *error)
 {
-	const struct rte_flow_item_eth *eth_spec = item->spec;
 	const struct rte_flow_item_eth *eth_mask = ci_flow_item_mask(item);
-	bool no_src_mac, no_dst_mac, src_mac, dst_mac, no_etype;
+	bool no_src_mac, no_dst_mac, src_mac, dst_mac;
 
 	/* may be empty */
 	if (eth_mask == NULL)
@@ -81,12 +80,15 @@ i40e_fdir_node_eth_validate(const void *ctx __rte_unused, const struct rte_flow_
 	/* source and destination masks may be all zero or all one */
 	no_src_mac = CI_FIELD_IS_ZERO(&eth_mask->hdr.src_addr);
 	no_dst_mac = CI_FIELD_IS_ZERO(&eth_mask->hdr.dst_addr);
-	no_etype = CI_FIELD_IS_ZERO(&eth_mask->hdr.ether_type);
 	src_mac = CI_FIELD_IS_MASKED(&eth_mask->hdr.src_addr);
 	dst_mac = CI_FIELD_IS_MASKED(&eth_mask->hdr.dst_addr);
 
+	if (!CI_FIELD_IS_ZERO(&eth_mask->hdr.ether_type)) {
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item, "Ethertype match not supported");
+	}
 	/* can't be all zero */
-	if (no_src_mac && no_dst_mac && no_etype) {
+	if (no_src_mac && no_dst_mac) {
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ITEM, item, "Invalid eth mask");
 	}
@@ -97,38 +99,18 @@ i40e_fdir_node_eth_validate(const void *ctx __rte_unused, const struct rte_flow_
 				RTE_FLOW_ERROR_TYPE_ITEM, item, "Invalid eth mask");
 	}
 
-	/* ethertype can either be unmasked or fully masked */
-	if (no_etype)
-		return 0;
-
-	if (!CI_FIELD_IS_MASKED(&eth_mask->hdr.ether_type)) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM, item, "Invalid ethertype mask");
-	}
-
-	/* Check for valid ethertype (not IPv4/IPv6) */
-	uint16_t ether_type = rte_be_to_cpu_16(eth_spec->hdr.ether_type);
-	if (ether_type == RTE_ETHER_TYPE_IPV4 ||
-	    ether_type == RTE_ETHER_TYPE_IPV6) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM, item,
-				"IPv4/IPv6 not supported by ethertype filter");
-	}
-
 	return 0;
 }
 
 static int
 i40e_fdir_node_eth_process(void *ctx, const struct rte_flow_item *item,
-		struct rte_flow_error *error)
+		struct rte_flow_error *error __rte_unused)
 {
 	struct i40e_fdir_ctx *fdir_ctx = ctx;
 	struct i40e_fdir_filter *fdir_filter = &fdir_ctx->fdir_filter;
 	const struct rte_flow_item_eth *eth_spec = item->spec;
 	const struct rte_flow_item_eth *eth_mask = ci_flow_item_mask(item);
-	uint16_t tpid, ether_type;
 	uint64_t input_set = 0;
-	int ret;
 
 	/* Set layer index for L2 flexible payload (after ETH/VLAN) */
 	fdir_filter->input.flow_ext.layer_idx = I40E_FLXPLD_L2_IDX;
@@ -151,40 +133,17 @@ i40e_fdir_node_eth_process(void *ctx, const struct rte_flow_item *item,
 		input_set |= I40E_INSET_DMAC;
 	}
 
-	/* do we care for ethertype? */
-	if (eth_mask->hdr.ether_type) {
-		struct i40e_pf *pf =
-				I40E_DEV_PRIVATE_TO_PF(fdir_ctx->base.dev_data->dev_private);
-
-		ether_type = rte_be_to_cpu_16(eth_spec->hdr.ether_type);
-		ret = i40e_get_outer_vlan(pf, &tpid);
-		if (ret != 0) {
-			return rte_flow_error_set(error, EIO,
-					RTE_FLOW_ERROR_TYPE_ITEM, item,
-					"Can not get the Ethertype identifying the L2 tag");
-		}
-		if (ether_type == tpid) {
-			return rte_flow_error_set(error, EINVAL,
-						RTE_FLOW_ERROR_TYPE_ITEM, item,
-						"Unsupported ether_type in control packet filter.");
-		}
-		fdir_filter->input.flow.l2_flow.ether_type = eth_spec->hdr.ether_type;
-		input_set |= I40E_INSET_LAST_ETHER_TYPE;
-	}
-
 	fdir_filter->input.flow_ext.input_set = input_set;
 
 	return 0;
 }
 
 static int
-i40e_fdir_node_vlan_validate(const void *ctx, const struct rte_flow_item *item,
+i40e_fdir_node_vlan_validate(const void *ctx __rte_unused, const struct rte_flow_item *item,
 		struct rte_flow_error *error)
 {
 	const struct rte_flow_item_vlan *vlan_spec = item->spec;
 	const struct rte_flow_item_vlan *vlan_mask = ci_flow_item_mask(item);
-	const struct i40e_fdir_ctx *fdir_ctx = ctx;
-	const struct i40e_fdir_filter *fdir_filter = &fdir_ctx->fdir_filter;
 	uint16_t ether_type;
 
 	if (vlan_mask == NULL)
@@ -206,13 +165,6 @@ i40e_fdir_node_vlan_validate(const void *ctx, const struct rte_flow_item *item,
 				"Invalid VLAN header mask");
 	}
 
-	/* can't match on eth_proto as we're already matching on ethertype */
-	if (fdir_filter->input.flow_ext.input_set & I40E_INSET_LAST_ETHER_TYPE) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM, item,
-				"Cannot set two ethertype filters");
-	}
-
 	ether_type = rte_be_to_cpu_16(vlan_spec->hdr.eth_proto);
 	if (ether_type == RTE_ETHER_TYPE_IPV4 ||
 	    ether_type == RTE_ETHER_TYPE_IPV6) {
@@ -1388,7 +1340,6 @@ static const struct flow_graph i40e_fdir_graph = {
 				I40E_FDIR_NODE_IPV4,
 				I40E_FDIR_NODE_IPV6,
 				I40E_FDIR_NODE_RAW,
-				I40E_FDIR_NODE_END,
 				FLOW_GRAPH_NODE_EDGE_END
 			}
 		},
@@ -1524,6 +1475,111 @@ static const struct flow_graph i40e_fdir_graph = {
 	},
 };
 
+/**
+ * FDIR ethertype graph implementation
+ * Pattern: START -> ETH -> END
+ */
+
+enum i40e_fdir_ethertype_node_id {
+	I40E_FDIR_ETHERTYPE_NODE_START = FLOW_GRAPH_NODE_FIRST,
+	I40E_FDIR_ETHERTYPE_NODE_ETH,
+	I40E_FDIR_ETHERTYPE_NODE_END,
+	I40E_FDIR_ETHERTYPE_NODE_MAX,
+};
+
+static int
+i40e_fdir_ethertype_node_eth_validate(const void *ctx __rte_unused,
+		const struct rte_flow_item *item, struct rte_flow_error *error)
+{
+	const struct rte_flow_item_eth *eth_spec = item->spec;
+	const struct rte_flow_item_eth *eth_mask = ci_flow_item_mask(item);
+	uint16_t ether_type;
+
+	if (!CI_FIELD_IS_ZERO(&eth_mask->hdr.src_addr) ||
+	    !CI_FIELD_IS_ZERO(&eth_mask->hdr.dst_addr)) {
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item, "Invalid eth mask");
+	}
+	if (!CI_FIELD_IS_MASKED(&eth_mask->hdr.ether_type)) {
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item, "Invalid ethertype mask");
+	}
+
+	ether_type = rte_be_to_cpu_16(eth_spec->hdr.ether_type);
+	if (ether_type == RTE_ETHER_TYPE_IPV4 ||
+	    ether_type == RTE_ETHER_TYPE_IPV6) {
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item,
+				"IPv4/IPv6 not supported by ethertype filter");
+	}
+
+	return 0;
+}
+
+static int
+i40e_fdir_ethertype_node_eth_process(void *ctx, const struct rte_flow_item *item,
+		struct rte_flow_error *error)
+{
+	struct i40e_fdir_ctx *fdir_ctx = ctx;
+	struct i40e_fdir_filter *fdir_filter = &fdir_ctx->fdir_filter;
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(fdir_ctx->base.dev_data->dev_private);
+	const struct rte_flow_item_eth *eth_spec = item->spec;
+	uint16_t tpid;
+	int ret;
+
+	ret = i40e_get_outer_vlan(pf, &tpid);
+	if (ret != 0) {
+		return rte_flow_error_set(error, EIO,
+				RTE_FLOW_ERROR_TYPE_ITEM, item,
+				"Can not get the Ethertype identifying the L2 tag");
+	}
+	if (rte_be_to_cpu_16(eth_spec->hdr.ether_type) == tpid) {
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item,
+				"Unsupported ether_type in control packet filter.");
+	}
+
+	fdir_filter->input.flow_ext.layer_idx = I40E_FLXPLD_L2_IDX;
+	fdir_filter->input.pctype = I40E_FILTER_PCTYPE_L2_PAYLOAD;
+	fdir_filter->input.flow.l2_flow.ether_type = eth_spec->hdr.ether_type;
+	fdir_filter->input.flow_ext.input_set = I40E_INSET_LAST_ETHER_TYPE;
+
+	return 0;
+}
+
+static const struct flow_graph i40e_fdir_ethertype_graph = {
+	.nodes = (struct flow_graph_node[]) {
+		[I40E_FDIR_ETHERTYPE_NODE_START] = { .name = "START" },
+		[I40E_FDIR_ETHERTYPE_NODE_ETH] = {
+			.name = "ETH",
+			.type = RTE_FLOW_ITEM_TYPE_ETH,
+			.constraints = FLOW_GRAPH_NODE_EXPECT_SPEC,
+			.validate = i40e_fdir_ethertype_node_eth_validate,
+			.process = i40e_fdir_ethertype_node_eth_process,
+		},
+		[I40E_FDIR_ETHERTYPE_NODE_END] = {
+			.name = "END",
+			.type = RTE_FLOW_ITEM_TYPE_END,
+			.validate = i40e_fdir_node_end_validate,
+			.process = i40e_fdir_node_end_process
+		},
+	},
+	.edges = (struct flow_graph_edge[]) {
+		[I40E_FDIR_ETHERTYPE_NODE_START] = {
+			.next = (size_t[]) {
+				I40E_FDIR_ETHERTYPE_NODE_ETH,
+				FLOW_GRAPH_NODE_EDGE_END
+			}
+		},
+		[I40E_FDIR_ETHERTYPE_NODE_ETH] = {
+			.next = (size_t[]) {
+				I40E_FDIR_ETHERTYPE_NODE_END,
+				FLOW_GRAPH_NODE_EDGE_END
+			}
+		},
+	},
+};
+
 static int
 i40e_fdir_action_check(const struct ci_flow_actions *actions,
 		const struct ci_flow_actions_check_param *param,
@@ -2099,3 +2155,11 @@ const struct ci_flow_engine i40e_flow_engine_fdir = {
 	.flow_size = sizeof(struct i40e_flow_engine_fdir_flow),
 	.graph = &i40e_fdir_graph,
 };
+
+const struct ci_flow_engine i40e_flow_engine_fdir_ethertype = {
+	.name = "i40e_fdir_ethertype",
+	.ops = &i40e_flow_engine_fdir_ops,
+	.ctx_size = sizeof(struct i40e_fdir_ctx),
+	.flow_size = sizeof(struct i40e_flow_engine_fdir_flow),
+	.graph = &i40e_fdir_ethertype_graph,
+};
-- 
2.52.0

[PATCH v1 5/9] net/i40e: validate IPv4 fragment offset without range

[Anatoly Burakov]

The FDIR IPv4 node only checked fragment_offset when the item had a
range. Without one, any fragment_offset mask and spec were accepted and
then ignored, so the rule was installed as a non-fragmented IPv4 rule.
The old parser rejected these.

Check fragment_offset whether or not a range is given.

Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_fdir.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index ba8c18250c8..0c9dce6fd9d 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -257,11 +257,9 @@ i40e_fdir_node_ipv4_validate(const void *ctx __rte_unused, const struct rte_flow
 				"Invalid IPv4 header mask");
 	}
 
-	if (ipv4_last == NULL)
-		return 0;
-
 	/* Only fragment_offset supports range */
-	if (ipv4_last->hdr.version_ihl ||
+	if (ipv4_last != NULL &&
+	    (ipv4_last->hdr.version_ihl ||
 	    ipv4_last->hdr.type_of_service ||
 	    ipv4_last->hdr.total_length ||
 	    ipv4_last->hdr.packet_id ||
@@ -269,16 +267,25 @@ i40e_fdir_node_ipv4_validate(const void *ctx __rte_unused, const struct rte_flow
 	    ipv4_last->hdr.next_proto_id ||
 	    ipv4_last->hdr.hdr_checksum ||
 	    ipv4_last->hdr.src_addr ||
-	    ipv4_last->hdr.dst_addr) {
+	    ipv4_last->hdr.dst_addr)) {
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ITEM, item,
 				"IPv4 range only supported for fragment_offset");
 	}
 
-	/* Validate fragment_offset range values */
+	/* Validate fragment_offset values */
 	uint16_t frag_mask = rte_be_to_cpu_16(ipv4_mask->hdr.fragment_offset);
 	uint16_t frag_spec = rte_be_to_cpu_16(ipv4_spec->hdr.fragment_offset);
-	uint16_t frag_last = rte_be_to_cpu_16(ipv4_last->hdr.fragment_offset);
+	uint16_t frag_last = ipv4_last != NULL ?
+			rte_be_to_cpu_16(ipv4_last->hdr.fragment_offset) : 0;
+
+	if (frag_mask == 0) {
+		if (ipv4_last == NULL)
+			return 0; /* fragment_offset not matched */
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item,
+				"IPv4 fragment_offset range requires a mask");
+	}
 
 	/* Mask must be 0x3fff (fragment offset + MF flag) */
 	if (frag_mask != (RTE_IPV4_HDR_OFFSET_MASK | RTE_IPV4_HDR_MF_FLAG)) {
-- 
2.52.0

[PATCH v1 6/9] net/i40e: fix SCTP verification tag truncation in FDIR

[Anatoly Burakov]

The SCTP verification tag is a 32-bit field, but the FDIR SCTP node
stored its spec and mask in 16-bit variables, truncating the tag
programmed into the filter.

Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_fdir.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index 0c9dce6fd9d..b1dcc4b6fad 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -684,7 +684,8 @@ i40e_fdir_node_sctp_process(void *ctx, const struct rte_flow_item *item,
 	struct i40e_fdir_filter *fdir_filter = &fdir_ctx->fdir_filter;
 	const struct rte_flow_item_sctp *sctp_spec = item->spec;
 	const struct rte_flow_item_sctp *sctp_mask = ci_flow_item_mask(item);
-	rte_be16_t src_spec, dst_spec, src_mask, dst_mask, tag_spec, tag_mask;
+	rte_be16_t src_spec, dst_spec, src_mask, dst_mask;
+	rte_be32_t tag_spec, tag_mask;
 	bool is_ipv4;
 
 	/* Set layer index for L4 flexible payload */
-- 
2.52.0

[PATCH v1 7/9] net/i40e: validate raw FDIR offsets and lengths

[Anatoly Burakov]

The FDIR RAW node only rejected absolute offsets when the relative
field was masked, and otherwise treated the offset as relative. It also
accepted odd lengths, but flexible payload is extracted in 2-byte
words, so the last byte was dropped from the match and the data of any
following RAW item was misaligned.

Reject absolute offsets regardless of the mask, and reject odd lengths.

Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_fdir.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
index b1dcc4b6fad..41298508582 100644
--- a/drivers/net/intel/i40e/i40e_flow_fdir.c
+++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
@@ -754,7 +754,7 @@ i40e_fdir_node_raw_validate(const void *ctx, const struct rte_flow_item *item,
 	size_t spec_size, spec_offset;
 	size_t total_size, i;
 	size_t new_src_offset;
-	bool wants_offset, wants_search, wants_limit, wants_absolute;
+	bool wants_offset, wants_search, wants_limit;
 
 	if (raw_mask == NULL) {
 		return rte_flow_error_set(error, EINVAL,
@@ -788,6 +788,11 @@ i40e_fdir_node_raw_validate(const void *ctx, const struct rte_flow_item *item,
 				RTE_FLOW_ERROR_TYPE_ITEM, item,
 				"RAW mask length must match spec length or be 0xffff");
 	}
+	if (raw_spec->length % sizeof(uint16_t) != 0) {
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM, item,
+				"RAW length must be a multiple of 2 bytes");
+	}
 
 	/* control field masks must be all zero or all one */
 	if (!CI_FIELD_IS_ZERO_OR_MASKED(&raw_mask->offset) ||
@@ -799,7 +804,6 @@ i40e_fdir_node_raw_validate(const void *ctx, const struct rte_flow_item *item,
 	wants_offset = raw_mask->offset != 0;
 	wants_search = raw_mask->search != 0 && raw_spec->search != 0;
 	wants_limit = raw_mask->limit != 0 && raw_spec->limit != 0;
-	wants_absolute = raw_mask->relative != 0 && raw_spec->relative == 0;
 
 	/* Offset must be 16-bit aligned */
 	if (wants_offset && (raw_spec->offset % sizeof(uint16_t) != 0)) {
@@ -823,7 +827,7 @@ i40e_fdir_node_raw_validate(const void *ctx, const struct rte_flow_item *item,
 	}
 
 	/* only relative offsets are supported */
-	if (wants_absolute) {
+	if (raw_spec->relative == 0) {
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ITEM, item,
 				"RAW absolute offset not supported");
-- 
2.52.0

[PATCH v1 8/9] net/i40e: disable tunnel replacement in multi-driver mode

[Anatoly Burakov]

Cloud filter replacement is not supported in multi-driver mode, so
tunnel flows that need it (MPLS, GTP, QinQ, L4 port) always failed to
install. The failure also marked the tunnel state as needing a device
reset, which blocked all other tunnel flows.

Do not initialize the engines for these tunnel types in multi-driver
mode.

Fixes: 28a3d1c89167 ("net/i40e: reimplement tunnel parsers")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_tunnel.c | 34 ++++++++++++++++++++---
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_flow_tunnel.c b/drivers/net/intel/i40e/i40e_flow_tunnel.c
index 237678ab8c2..d74f46705f7 100644
--- a/drivers/net/intel/i40e/i40e_flow_tunnel.c
+++ b/drivers/net/intel/i40e/i40e_flow_tunnel.c
@@ -1883,6 +1883,20 @@ i40e_tunnel_flow_engine_init(const struct ci_flow_engine *engine __rte_unused,
 	return tunnel_priv->state == NULL ? -ENOMEM : 0;
 }
 
+static int
+i40e_tunnel_replace_flow_engine_init(const struct ci_flow_engine *engine,
+		struct rte_eth_dev_data *dev_data,
+		void *priv)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev_data->dev_private);
+
+	/* tunnel filter replacement is not supported in multi-driver mode */
+	if (pf->support_multi_driver)
+		return -ENOTSUP;
+
+	return i40e_tunnel_flow_engine_init(engine, dev_data, priv);
+}
+
 static void
 i40e_tunnel_flow_engine_uninit(const struct ci_flow_engine *engine __rte_unused,
 		void *priv)
@@ -1903,6 +1917,18 @@ static const struct ci_flow_engine_ops i40e_flow_engine_tunnel_ops = {
 	.flow_uninstall = i40e_tunnel_flow_uninstall,
 };
 
+/* for tunnel types that require tunnel filter replacement */
+static const struct ci_flow_engine_ops i40e_flow_engine_tunnel_replace_ops = {
+	.engine_init = i40e_tunnel_replace_flow_engine_init,
+	.engine_uninit = i40e_tunnel_flow_engine_uninit,
+	.ctx_init = i40e_tunnel_ctx_init,
+	.ctx_to_flow = i40e_tunnel_ctx_to_flow,
+	.flow_register = i40e_tunnel_flow_register,
+	.flow_unregister = i40e_tunnel_flow_unregister,
+	.flow_install = i40e_tunnel_flow_install,
+	.flow_uninstall = i40e_tunnel_flow_uninstall,
+};
+
 const struct ci_flow_engine i40e_flow_engine_tunnel_nvgre = {
 	.name = "i40e_tunnel_nvgre",
 	.ops = &i40e_flow_engine_tunnel_ops,
@@ -1923,7 +1949,7 @@ const struct ci_flow_engine i40e_flow_engine_tunnel_vxlan = {
 
 const struct ci_flow_engine i40e_flow_engine_tunnel_mpls = {
 	.name = "i40e_tunnel_mpls",
-	.ops = &i40e_flow_engine_tunnel_ops,
+	.ops = &i40e_flow_engine_tunnel_replace_ops,
 	.ctx_size = sizeof(struct i40e_tunnel_ctx),
 	.flow_size = sizeof(struct i40e_tunnel_flow),
 	.priv_size = sizeof(struct i40e_tunnel_priv),
@@ -1932,7 +1958,7 @@ const struct ci_flow_engine i40e_flow_engine_tunnel_mpls = {
 
 const struct ci_flow_engine i40e_flow_engine_tunnel_gtp = {
 	.name = "i40e_tunnel_gtp",
-	.ops = &i40e_flow_engine_tunnel_ops,
+	.ops = &i40e_flow_engine_tunnel_replace_ops,
 	.ctx_size = sizeof(struct i40e_tunnel_ctx),
 	.flow_size = sizeof(struct i40e_tunnel_flow),
 	.priv_size = sizeof(struct i40e_tunnel_priv),
@@ -1941,7 +1967,7 @@ const struct ci_flow_engine i40e_flow_engine_tunnel_gtp = {
 
 const struct ci_flow_engine i40e_flow_engine_tunnel_l4 = {
 	.name = "i40e_tunnel_l4",
-	.ops = &i40e_flow_engine_tunnel_ops,
+	.ops = &i40e_flow_engine_tunnel_replace_ops,
 	.ctx_size = sizeof(struct i40e_tunnel_ctx),
 	.flow_size = sizeof(struct i40e_tunnel_flow),
 	.priv_size = sizeof(struct i40e_tunnel_priv),
@@ -1950,7 +1976,7 @@ const struct ci_flow_engine i40e_flow_engine_tunnel_l4 = {
 
 const struct ci_flow_engine i40e_flow_engine_tunnel_qinq = {
 	.name = "i40e_tunnel_qinq",
-	.ops = &i40e_flow_engine_tunnel_ops,
+	.ops = &i40e_flow_engine_tunnel_replace_ops,
 	.ctx_size = sizeof(struct i40e_tunnel_ctx),
 	.flow_size = sizeof(struct i40e_tunnel_flow),
 	.priv_size = sizeof(struct i40e_tunnel_priv),
-- 
2.52.0

[PATCH v1 9/9] net/i40e: allow L2 and L3 only hash patterns

[Anatoly Burakov]

The hash pattern graph had no END edge after ETH, IPv4 and IPv6, so RSS
rules with patterns ending in one of those items were rejected. They
were supported before the rework.

Add the missing END edges.

Fixes: 3332d0afdb61 ("net/i40e: reimplement hash parser")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/i40e/i40e_flow_hash.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/i40e/i40e_flow_hash.c b/drivers/net/intel/i40e/i40e_flow_hash.c
index 78b0544be63..28f2dd0e708 100644
--- a/drivers/net/intel/i40e/i40e_flow_hash.c
+++ b/drivers/net/intel/i40e/i40e_flow_hash.c
@@ -168,8 +168,10 @@ i40e_hash_get_inset(uint64_t rss_types, bool symmetric_enable)
 
 /*
  * Hash pattern graph implementation
- * Pattern: START -> ETH -> [VLAN] -> [VLAN] -> (IPv4|IPv6) -> (TCP|UDP|SCTP|ESP|L2TPV3OIP|AH)
- *          START -> ETH -> [VLAN] -> [VLAN] -> (IPv4|IPv6) frag
+ * Pattern: START -> ETH
+ *          START -> ETH -> [VLAN] -> [VLAN] -> (IPv4|IPv6)
+ *          START -> ETH -> [VLAN] -> [VLAN] -> (IPv4|IPv6) -> (TCP|UDP|SCTP|ESP|L2TPV3OIP|AH)
+ *          START -> ETH -> [VLAN] -> [VLAN] -> IPv6 -> IPV6_FRAG_EXT
  *          START -> ETH -> [VLAN] -> [VLAN] -> (IPv4|IPv6) -> UDP -> (GTPC|ESP|GTPU)
  *          START -> ETH -> [VLAN] -> [VLAN] -> (IPv4|IPv6) -> UDP -> GTPU -> (IPv4|IPv6)
  */
@@ -491,6 +493,7 @@ static const struct flow_graph i40e_hash_pattern_graph = {
 				I40E_HASH_PATTERN_NODE_OUTER_VLAN,
 				I40E_HASH_PATTERN_NODE_IPV4,
 				I40E_HASH_PATTERN_NODE_IPV6,
+				I40E_HASH_PATTERN_NODE_END,
 				FLOW_GRAPH_NODE_EDGE_END
 			}
 		},
@@ -517,6 +520,7 @@ static const struct flow_graph i40e_hash_pattern_graph = {
 				I40E_HASH_PATTERN_NODE_ESP,
 				I40E_HASH_PATTERN_NODE_L2TPV3OIP,
 				I40E_HASH_PATTERN_NODE_AH,
+				I40E_HASH_PATTERN_NODE_END,
 				FLOW_GRAPH_NODE_EDGE_END
 			}
 		},
@@ -529,6 +533,7 @@ static const struct flow_graph i40e_hash_pattern_graph = {
 				I40E_HASH_PATTERN_NODE_ESP,
 				I40E_HASH_PATTERN_NODE_L2TPV3OIP,
 				I40E_HASH_PATTERN_NODE_AH,
+				I40E_HASH_PATTERN_NODE_END,
 				FLOW_GRAPH_NODE_EDGE_END
 			}
 		},
-- 
2.52.0

Re: [PATCH v1 1/9] net/i40e: skip FDIR flow engine when FDIR is unavailable

[Bruce Richardson]

On Fri, Oct 30, 2026 at 09:40:14AM +0000, Anatoly Burakov wrote:
> The FDIR flow engine was initialized even when flow director was not
> usable. With no FDIR filter space, engine init failed and logged an
> error. When FDIR was disabled because its queue could not be allocated,
> the engine was initialized and FDIR flows only failed at install.
> 

This commit log is not entirely clear, for example, when you talk about cases
where the engine init already fails - given that you are adding code to
make the engine init fail.

> Return -ENOTSUP in both cases so that the engine is skipped.

You actually refer to 3 cases, I think: fdir not usable, no filter space,
and no queue allocation. From the patch, I don't see the third case being
relevant, so maybe drop the mention of it?

> 
> Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/i40e/i40e_flow_fdir.c | 3 +++
>  1 file changed, 3 insertions(+)
> 

Code change looks reasonable, though, so with commit log cleanup:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


> diff --git a/drivers/net/intel/i40e/i40e_flow_fdir.c b/drivers/net/intel/i40e/i40e_flow_fdir.c
> index 55997295edb..197b93f3746 100644
> --- a/drivers/net/intel/i40e/i40e_flow_fdir.c
> +++ b/drivers/net/intel/i40e/i40e_flow_fdir.c
> @@ -2010,6 +2010,9 @@ i40e_fdir_flow_engine_init(const struct ci_flow_engine *engine,
>  	uint32_t i;
>  	int ret;
>  
> +	if ((pf->flags & I40E_FLAG_FDIR) == 0 || fdir_info->fdir_space_size == 0)
> +		return -ENOTSUP;
> +
>  	snprintf(fdir_hash_name, sizeof(fdir_hash_name), "fdir_%s", dev_data->name);
>  
>  	struct rte_hash_parameters fdir_hash_params = {
> -- 
> 2.52.0
> 

Re: [PATCH v1 2/9] net/i40e: reject VLAN with tunnel items in FDIR flows

[Bruce Richardson]

On Fri, Oct 30, 2026 at 09:40:15AM +0000, Anatoly Burakov wrote:
> The FDIR pattern graph allowed VLAN in front of ESP, L2TPv3 and GTP
> items. These use customized pctypes whose input set does not include
> the VLAN tag, so the VLAN match was silently ignored. Such patterns
> were not accepted before the rework either.
> 
> Make VLAN lead to separate IPv4, IPv6 and UDP nodes that have no tunnel
> successors.
> 
> Fixes: 05432a6e4676 ("net/i40e: reimplement FDIR parser")
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/i40e/i40e_flow_fdir.c | 65 +++++++++++++++++++++++--
>  1 file changed, 62 insertions(+), 3 deletions(-)
Acked-by: Bruce Richardson <bruce.richardson@intel.com>