[PATCH v1 0/7] IAVF Fixes

[PATCH v1 0/7] IAVF Fixes

[Anatoly Burakov]

An assortment of IAVF flow-related memory leak fixes and cleanups.

Anatoly Burakov (7):
  net/iavf: fix memory leak in flow teardown
  net/iavf: fix null engine for egress security flows
  net/iavf: fix generic flow error-path leaks
  net/iavf: fix flow subscription metadata leaks
  net/iavf: fix hash flow metadata leaks
  net/iavf: fix IPsec flow metadata leaks
  net/iavf: do not store ethdev pointer

 drivers/net/intel/iavf/iavf.h              |   9 +-
 drivers/net/intel/iavf/iavf_ethdev.c       |  13 +-
 drivers/net/intel/iavf/iavf_fdir.c         |   3 -
 drivers/net/intel/iavf/iavf_fsub.c         |   7 +-
 drivers/net/intel/iavf/iavf_generic_flow.c |  58 +++----
 drivers/net/intel/iavf/iavf_generic_flow.h |   4 +-
 drivers/net/intel/iavf/iavf_hash.c         |  24 ++-
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 191 +++++++++++++++++----
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |   4 +-
 drivers/net/intel/iavf/iavf_vchnl.c        |   7 +-
 10 files changed, 222 insertions(+), 98 deletions(-)

-- 
2.52.0

[PATCH v1 1/7] net/iavf: fix memory leak in flow teardown

[Anatoly Burakov]

Flow uninit skips hardware teardown during a PF-initiated reset, but
only the hash engine provides a callback to free its rule. FDIR, flow
subscription and IPsec rules are therefore leaked. Conversely, hash
destroy removes its rule from hardware but frees only the flow handle.

Free the rule alongside the flow handle in the generic destroy and
uninit paths. Remove the engine-specific rule frees and the redundant
free callback, so both paths have the same software ownership.

Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")
Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_fdir.c         | 3 ---
 drivers/net/intel/iavf/iavf_fsub.c         | 3 ---
 drivers/net/intel/iavf/iavf_generic_flow.c | 4 ++--
 drivers/net/intel/iavf/iavf_generic_flow.h | 2 --
 drivers/net/intel/iavf/iavf_hash.c         | 9 ---------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 1 -
 6 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_fdir.c b/drivers/net/intel/iavf/iavf_fdir.c
index 67003183bbd..c3babed947c 100644
--- a/drivers/net/intel/iavf/iavf_fdir.c
+++ b/drivers/net/intel/iavf/iavf_fdir.c
@@ -406,9 +406,6 @@ iavf_fdir_destroy(struct iavf_adapter *ad,
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 0);
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return 0;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_fsub.c b/drivers/net/intel/iavf/iavf_fsub.c
index 19ae0e2dc2e..4b2e40af3c0 100644
--- a/drivers/net/intel/iavf/iavf_fsub.c
+++ b/drivers/net/intel/iavf/iavf_fsub.c
@@ -120,9 +120,6 @@ iavf_fsub_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		return -rte_errno;
 	}
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 92ca20031c0..53a16d13827 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1851,8 +1851,7 @@ iavf_flow_uninit(struct iavf_adapter *ad)
 	/* Remove all flows */
 	while ((p_flow = TAILQ_FIRST(&vf->flow_list))) {
 		TAILQ_REMOVE(&vf->flow_list, p_flow, node);
-		if (p_flow->engine->free)
-			p_flow->engine->free(p_flow);
+		rte_free(p_flow->rule);
 		rte_free(p_flow);
 	}
 
@@ -2301,6 +2300,7 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
+		rte_free(flow->rule);
 		rte_free(flow);
 	} else {
 		PMD_DRV_LOG(ERR, "Failed to destroy flow");
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 6665d688183..3a8f5f92e8b 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -465,7 +465,6 @@ typedef int (*engine_query_t)(struct iavf_adapter *ad,
 		struct rte_flow *flow,
 		struct rte_flow_query_count *count,
 		struct rte_flow_error *error);
-typedef void (*engine_free_t) (struct rte_flow *flow);
 typedef int (*parse_pattern_action_t)(struct iavf_adapter *ad,
 		struct iavf_pattern_match_item *array,
 		uint32_t array_len,
@@ -494,7 +493,6 @@ struct iavf_flow_engine {
 	engine_create_t create;
 	engine_destroy_t destroy;
 	engine_query_t query_count;
-	engine_free_t free;
 	const char *name;
 	enum iavf_flow_engine_type type;
 	size_t rule_size;
diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index 95150944025..cef30cd08e1 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -79,8 +79,6 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		  struct rte_flow_error *error);
 static void
 iavf_hash_uninit_parser(struct iavf_adapter *ad);
-static void
-iavf_hash_free(struct rte_flow *flow);
 static int
 iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 			       struct iavf_pattern_match_item *array,
@@ -682,7 +680,6 @@ static struct iavf_flow_engine iavf_hash_engine = {
 	.create = iavf_hash_create,
 	.destroy = iavf_hash_destroy,
 	.uninit = iavf_hash_uninit_parser,
-	.free = iavf_hash_free,
 	.name = "hash",
 	.type = IAVF_FLOW_ENGINE_HASH,
 	.rule_size = sizeof(struct virtchnl_rss_cfg),
@@ -1670,9 +1667,3 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 }
-
-static void
-iavf_hash_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-}
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index 06b51b5e622..2478bdc245d 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1919,7 +1919,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 
 	iavf_ipsec_crypto_security_policy_delete(ad,
 			ipsec_flow->is_ipv4, ipsec_flow->id);
-	rte_free(ipsec_flow);
 	return 0;
 }
 
-- 
2.52.0

[PATCH v1 2/7] net/iavf: fix null engine for egress security flows

[Anatoly Burakov]

Egress security flows bypass the flow parsers and are inserted with a
NULL `engine` pointer. The create path then dereferences that pointer
when logging the new flow, and other engine-based operations cannot
handle the flow consistently.

Register an egress security engine and route these flows through the
normal parser and lifecycle callbacks. Egress flows own no rule and
require no hardware teardown.

Fixes: e783da175250 ("net/iavf: fix memory leak on egress IPsec flows")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c |  44 ++-----
 drivers/net/intel/iavf/iavf_generic_flow.h |   2 +-
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 130 +++++++++++++++++++--
 3 files changed, 133 insertions(+), 43 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 53a16d13827..5759381b8ff 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1903,7 +1903,8 @@ iavf_register_parser(struct iavf_flow_parser *parser,
 			}
 		}
 		TAILQ_INSERT_HEAD(list, parser_node, node);
-	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO) {
+	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		   parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS) {
 		list = &vf->ipsec_crypto_parser_list;
 		TAILQ_INSERT_HEAD(list, parser_node, node);
 	} else if (parser->engine->type == IAVF_FLOW_ENGINE_FSUB) {
@@ -1932,7 +1933,8 @@ iavf_unregister_parser(struct iavf_flow_parser *parser,
 	else if ((parser->engine->type == IAVF_FLOW_ENGINE_FDIR) ||
 		 (parser->engine->type == IAVF_FLOW_ENGINE_FSUB))
 		list = &vf->dist_parser_list;
-	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO)
+	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		 parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS)
 		list = &vf->ipsec_crypto_parser_list;
 
 	if (list == NULL)
@@ -2220,12 +2222,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 		return flow;
 	}
 
-	/* Special case for inline crypto egress flows */
-	if (attr->egress && actions[0].type == RTE_FLOW_ACTION_TYPE_SECURITY) {
-		flow->is_ipsec_egress_flow = true;
-		goto tailq_insert;
-	}
-
 	ret = iavf_flow_process_filter(dev, flow, attr, pattern, actions,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
@@ -2236,7 +2232,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 	}
 
 	flow->engine = engine;
-tailq_insert:
 	rte_spinlock_lock(&vf->flow_ops_lock);
 	TAILQ_INSERT_TAIL(&vf->flow_list, flow, node);
 	rte_spinlock_unlock(&vf->flow_ops_lock);
@@ -2252,14 +2247,7 @@ iavf_flow_is_valid(struct rte_flow *flow)
 	struct iavf_flow_engine *engine;
 	void *temp;
 
-	if (flow == NULL)
-		return false;
-
-	/* these flows don't use engines */
-	if (flow->is_ipsec_egress_flow)
-		return true;
-
-	if (flow->engine) {
+	if (flow != NULL && flow->engine != NULL) {
 		RTE_TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
 			if (engine == flow->engine)
 				return true;
@@ -2277,26 +2265,17 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 	struct iavf_adapter *ad =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	bool need_destroy;
 	int ret = 0;
 
-	if (!iavf_flow_is_valid(flow)) {
+	if (!iavf_flow_is_valid(flow) || flow->engine->destroy == NULL) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Invalid flow destroy");
 		return -rte_errno;
 	}
-	need_destroy = !flow->is_ipsec_egress_flow;
-
-	if (need_destroy && flow->engine->destroy == NULL) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Invalid flow destroy");
-	}
 
 	rte_spinlock_lock(&vf->flow_ops_lock);
-	if (need_destroy)
-		ret = flow->engine->destroy(ad, flow, error);
+	ret = flow->engine->destroy(ad, flow, error);
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
@@ -2415,16 +2394,17 @@ iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		if (flow != NULL && f != flow)
 			continue;
 
-		/* this should not happen */
-		if (f->engine == NULL || f->rule == NULL) {
+		if (!iavf_flow_is_valid(f)) {
 			PMD_DRV_LOG(DEBUG, "Invalid flow");
 			continue;
 		}
 
 		found = true;
 
-		rule_size = f->engine->rule_size;
-		rule_data = f->rule;
+		if (f->rule != NULL) {
+			rule_size = f->engine->rule_size;
+			rule_data = f->rule;
+		}
 
 		iavf_flow_dump_blob(file,
 			f->engine->name != NULL ?
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 3a8f5f92e8b..75bb92900b8 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -481,6 +481,7 @@ enum iavf_flow_engine_type {
 	IAVF_FLOW_ENGINE_FDIR,
 	IAVF_FLOW_ENGINE_HASH,
 	IAVF_FLOW_ENGINE_FSUB,
+	IAVF_FLOW_ENGINE_IPSEC_EGRESS,
 	IAVF_FLOW_ENGINE_MAX,
 };
 
@@ -503,7 +504,6 @@ TAILQ_HEAD(iavf_engine_list, iavf_flow_engine);
 /* Struct to store flow created. */
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) node;
-	bool is_ipsec_egress_flow;
 	struct iavf_flow_engine *engine;
 	void *rule;
 };
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index 2478bdc245d..9fcdc311377 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1832,6 +1832,7 @@ iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
 
 
 static struct iavf_flow_parser iavf_ipsec_flow_parser;
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser;
 
 static int
 iavf_ipsec_flow_init(struct iavf_adapter *ad)
@@ -1922,16 +1923,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 	return 0;
 }
 
-static struct iavf_flow_engine iavf_ipsec_flow_engine = {
-	.init = iavf_ipsec_flow_init,
-	.uninit = iavf_ipsec_flow_uninit,
-	.create = iavf_ipsec_flow_create,
-	.destroy = iavf_ipsec_flow_destroy,
-	.name = "ipsec_crypto",
-	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
-	.rule_size = sizeof(struct iavf_ipsec_flow_item),
-};
-
 static int
 iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		      struct iavf_pattern_match_item *array,
@@ -1975,6 +1966,115 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 	return ret;
 }
 
+/*
+ * Inline crypto egress flows only associate Tx traffic with a security
+ * session; nothing is programmed in hardware, so the engine owns no rule.
+ */
+static int
+iavf_ipsec_egress_flow_init(struct iavf_adapter *ad)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+
+	if (!vf->vf_res)
+		return -EINVAL;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_INLINE_IPSEC_CRYPTO))
+		return -ENOTSUP;
+
+	return iavf_register_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static void
+iavf_ipsec_egress_flow_uninit(struct iavf_adapter *ad)
+{
+	iavf_unregister_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static int
+iavf_ipsec_egress_flow_create(__rte_unused struct iavf_adapter *ad,
+		struct rte_flow *flow,
+		__rte_unused void *meta,
+		__rte_unused struct rte_flow_error *error)
+{
+	flow->rule = NULL;
+	return 0;
+}
+
+static int
+iavf_ipsec_egress_flow_destroy(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct rte_flow *flow,
+		__rte_unused struct rte_flow_error *error)
+{
+	return 0;
+}
+
+static int
+iavf_ipsec_egress_flow_validation(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct rte_flow *flow,
+		__rte_unused void *meta,
+		__rte_unused struct rte_flow_error *error)
+{
+	return 0;
+}
+
+/* The pattern is not used, as there is nothing to match in hardware. */
+static int
+iavf_ipsec_egress_flow_parse(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct iavf_pattern_match_item *array,
+		__rte_unused uint32_t array_len,
+		__rte_unused const struct rte_flow_item pattern[],
+		const struct rte_flow_action actions[],
+		const struct rte_flow_attr *attr,
+		void **meta,
+		struct rte_flow_error *error)
+{
+	struct ci_flow_actions parsed_actions = {0};
+	struct ci_flow_actions_check_param param = {
+		.allowed_types = (enum rte_flow_action_type[]){
+			RTE_FLOW_ACTION_TYPE_SECURITY,
+			RTE_FLOW_ACTION_TYPE_END,
+		},
+		.max_actions = 1,
+	};
+	struct ci_flow_attr_check_param attr_param = {
+		.require_egress = true,
+	};
+	int ret;
+
+	ret = ci_flow_check_attr(attr, &attr_param, error);
+	if (ret)
+		return ret;
+
+	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
+	if (ret < 0)
+		return ret;
+
+	if (meta != NULL)
+		*meta = NULL;
+
+	return 0;
+}
+
+static struct iavf_flow_engine iavf_ipsec_flow_engine = {
+	.init = iavf_ipsec_flow_init,
+	.uninit = iavf_ipsec_flow_uninit,
+	.create = iavf_ipsec_flow_create,
+	.destroy = iavf_ipsec_flow_destroy,
+	.name = "ipsec_crypto",
+	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
+	.rule_size = sizeof(struct iavf_ipsec_flow_item),
+};
+
+static struct iavf_flow_engine iavf_ipsec_egress_flow_engine = {
+	.init = iavf_ipsec_egress_flow_init,
+	.uninit = iavf_ipsec_egress_flow_uninit,
+	.create = iavf_ipsec_egress_flow_create,
+	.destroy = iavf_ipsec_egress_flow_destroy,
+	.validation = iavf_ipsec_egress_flow_validation,
+	.name = "ipsec_egress",
+	.type = IAVF_FLOW_ENGINE_IPSEC_EGRESS,
+};
+
 static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.engine = &iavf_ipsec_flow_engine,
 	.array = iavf_ipsec_flow_pattern,
@@ -1982,7 +2082,17 @@ static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.parse_pattern_action = iavf_ipsec_flow_parse,
 };
 
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser = {
+	.engine = &iavf_ipsec_egress_flow_engine,
+	.parse_pattern_action = iavf_ipsec_egress_flow_parse,
+};
+
 RTE_INIT(iavf_ipsec_flow_engine_register)
 {
 	iavf_register_flow_engine(&iavf_ipsec_flow_engine);
 }
+
+RTE_INIT(iavf_ipsec_egress_flow_engine_register)
+{
+	iavf_register_flow_engine(&iavf_ipsec_egress_flow_engine);
+}
-- 
2.52.0

[PATCH v1 3/7] net/iavf: fix generic flow error-path leaks

[Anatoly Burakov]

If a flow engine fails to initialize, parsers registered by earlier
engines remain allocated. Pattern matching also leaks its item array
when allocation of the match result fails.

Unwind registered parsers on flow initialization failure and free the
item array on match-result allocation failure.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 5759381b8ff..15ed41db065 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1821,17 +1821,22 @@ iavf_flow_init(struct iavf_adapter *ad)
 		if (engine->init == NULL) {
 			PMD_INIT_LOG(ERR, "Invalid engine type (%d)",
 				     engine->type);
-			return -ENOTSUP;
+			ret = -ENOTSUP;
+			goto err;
 		}
 
 		ret = engine->init(ad);
 		if (ret && ret != -ENOTSUP) {
 			PMD_INIT_LOG(ERR, "Failed to initialize engine %d",
 				     engine->type);
-			return ret;
+			goto err;
 		}
 	}
 	return 0;
+
+err:
+	iavf_flow_uninit(ad);
+	return ret;
 }
 
 void
@@ -2048,6 +2053,7 @@ iavf_search_pattern_match_item(const struct rte_flow_item pattern[],
 	if (!pattern_match_item) {
 		rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Failed to allocate memory.");
+		rte_free(items);
 		return NULL;
 	}
 
-- 
2.52.0

[PATCH v1 4/7] net/iavf: fix flow subscription metadata leaks

[Anatoly Burakov]

Flow subscription parsing allocates metadata for create and validate.
The create callback leaks it when rule allocation or subscription fails,
and the validate callback never frees it.

Release the metadata on both create failure paths and after validation.

Fixes: 7b902af49947 ("net/iavf: support flow subscription rule")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_fsub.c b/drivers/net/intel/iavf/iavf_fsub.c
index 4b2e40af3c0..8afbb698141 100644
--- a/drivers/net/intel/iavf/iavf_fsub.c
+++ b/drivers/net/intel/iavf/iavf_fsub.c
@@ -81,7 +81,7 @@ iavf_fsub_create(struct iavf_adapter *ad, struct rte_flow *flow,
 		rte_flow_error_set(error, ENOMEM,
 				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				"Failed to allocate memory for fsub rule");
-		return -rte_errno;
+		goto free_entry;
 	}
 
 	ret = iavf_flow_sub(ad, filter);
@@ -99,6 +99,7 @@ iavf_fsub_create(struct iavf_adapter *ad, struct rte_flow *flow,
 	return ret;
 
 free_entry:
+	rte_free(meta);
 	rte_free(rule);
 	return -rte_errno;
 }
@@ -133,6 +134,7 @@ iavf_fsub_validation(struct iavf_adapter *ad,
 	int ret;
 
 	ret = iavf_flow_sub_check(ad, filter);
+	rte_free(filter);
 	if (ret) {
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-- 
2.52.0

[PATCH v1 5/7] net/iavf: fix hash flow metadata leaks

[Anatoly Burakov]

Hash flow parsing allocates RSS metadata, but validation has no callback
to release it. Creation also leaks the metadata if rule allocation or
the request to add the RSS rule fails.

Add a validation callback to release the parsed metadata, and free it
on both creation failure paths. Parsing already checks the RSS inputs;
validation does not add or remove a rule.

Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")
Fixes: 215a247b5f33 ("net/iavf: refactor hash flow")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index cef30cd08e1..6787a380f8e 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -80,6 +80,9 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 static void
 iavf_hash_uninit_parser(struct iavf_adapter *ad);
 static int
+iavf_hash_validation(struct iavf_adapter *ad, struct rte_flow *flow,
+		     void *meta, struct rte_flow_error *error);
+static int
 iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 			       struct iavf_pattern_match_item *array,
 			       uint32_t array_len,
@@ -680,6 +683,7 @@ static struct iavf_flow_engine iavf_hash_engine = {
 	.create = iavf_hash_create,
 	.destroy = iavf_hash_destroy,
 	.uninit = iavf_hash_uninit_parser,
+	.validation = iavf_hash_validation,
 	.name = "hash",
 	.type = IAVF_FLOW_ENGINE_HASH,
 	.rule_size = sizeof(struct virtchnl_rss_cfg),
@@ -1579,6 +1583,15 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	return ret;
 }
 
+static int
+iavf_hash_validation(__rte_unused struct iavf_adapter *ad,
+		     __rte_unused struct rte_flow *flow, void *meta,
+		     __rte_unused struct rte_flow_error *error)
+{
+	rte_free(meta);
+	return 0;
+}
+
 static int
 iavf_hash_create(__rte_unused struct iavf_adapter *ad,
 		 __rte_unused struct rte_flow *flow, void *meta,
@@ -1594,6 +1607,7 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "No memory for rss rule");
+		rte_free(meta);
 		return -ENOMEM;
 	}
 
@@ -1609,6 +1623,7 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to add rss rule.");
 		rte_free(rss_cfg);
+		rte_free(meta);
 		return -rte_errno;
 	}
 
-- 
2.52.0

RE: [PATCH v1 5/7] net/iavf: fix hash flow metadata leaks

[Mandal, Anurag]

> -----Original Message-----
> From: Anatoly Burakov <anatoly.burakov@intel.com>
> Sent: 14 October 2026 20:49
> To: dev@dpdk.org
> Subject: [PATCH v1 5/7] net/iavf: fix hash flow metadata leaks
> 
> Hash flow parsing allocates RSS metadata, but validation has no callback to
> release it. Creation also leaks the metadata if rule allocation or the request to
> add the RSS rule fails.
> 
> Add a validation callback to release the parsed metadata, and free it on both
> creation failure paths. Parsing already checks the RSS inputs; validation does
> not add or remove a rule.
> 
> Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")
> Fixes: 215a247b5f33 ("net/iavf: refactor hash flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_hash.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/intel/iavf/iavf_hash.c
> b/drivers/net/intel/iavf/iavf_hash.c
> index cef30cd08e1..6787a380f8e 100644
> --- a/drivers/net/intel/iavf/iavf_hash.c
> +++ b/drivers/net/intel/iavf/iavf_hash.c
> @@ -80,6 +80,9 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct
> rte_flow *flow,  static void  iavf_hash_uninit_parser(struct iavf_adapter *ad);
> static int
> +iavf_hash_validation(struct iavf_adapter *ad, struct rte_flow *flow,
> +		     void *meta, struct rte_flow_error *error); static int
>  iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
>  			       struct iavf_pattern_match_item *array,
>  			       uint32_t array_len,
> @@ -680,6 +683,7 @@ static struct iavf_flow_engine iavf_hash_engine = {
>  	.create = iavf_hash_create,
>  	.destroy = iavf_hash_destroy,
>  	.uninit = iavf_hash_uninit_parser,
> +	.validation = iavf_hash_validation,
>  	.name = "hash",
>  	.type = IAVF_FLOW_ENGINE_HASH,
>  	.rule_size = sizeof(struct virtchnl_rss_cfg), @@ -1579,6 +1583,15 @@
> iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
>  	return ret;
>  }
> 
> +static int
> +iavf_hash_validation(__rte_unused struct iavf_adapter *ad,
> +		     __rte_unused struct rte_flow *flow, void *meta,
> +		     __rte_unused struct rte_flow_error *error) {
> +	rte_free(meta);
> +	return 0;
> +}
> +
>  static int
>  iavf_hash_create(__rte_unused struct iavf_adapter *ad,
>  		 __rte_unused struct rte_flow *flow, void *meta, @@ -1594,6
> +1607,7 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
>  		rte_flow_error_set(error, EINVAL,
>  				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>  				   "No memory for rss rule");
> +		rte_free(meta);
>  		return -ENOMEM;
>  	}
> 
> @@ -1609,6 +1623,7 @@ iavf_hash_create(__rte_unused struct iavf_adapter
> *ad,
>  				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>  				   "Failed to add rss rule.");
>  		rte_free(rss_cfg);
> +		rte_free(meta);
>  		return -rte_errno;
>  	}
> 
> --
> 2.52.0

Hi Anatoly, 

I had submitted similar fix  in this patch earlier.
https://patches.dpdk.org/project/dpdk/patch/20261010121255.528329-1-anurag.mandal@intel.com/

Thanks,
Anurag M

Re: [PATCH v1 5/7] net/iavf: fix hash flow metadata leaks

[Burakov, Anatoly]

On 10/16/2026 5:12 AM, Mandal, Anurag wrote:
> 
>> -----Original Message-----
>> From: Anatoly Burakov <anatoly.burakov@intel.com>
>> Sent: 14 October 2026 20:49
>> To: dev@dpdk.org
>> Subject: [PATCH v1 5/7] net/iavf: fix hash flow metadata leaks
>>
>> Hash flow parsing allocates RSS metadata, but validation has no callback to
>> release it. Creation also leaks the metadata if rule allocation or the request to
>> add the RSS rule fails.
>>
>> Add a validation callback to release the parsed metadata, and free it on both
>> creation failure paths. Parsing already checks the RSS inputs; validation does
>> not add or remove a rule.
>>
>> Fixes: 7be10c3004be ("net/iavf: add RSS configuration for VF")
>> Fixes: 215a247b5f33 ("net/iavf: refactor hash flow")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   drivers/net/intel/iavf/iavf_hash.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/intel/iavf/iavf_hash.c
>> b/drivers/net/intel/iavf/iavf_hash.c
>> index cef30cd08e1..6787a380f8e 100644
>> --- a/drivers/net/intel/iavf/iavf_hash.c
>> +++ b/drivers/net/intel/iavf/iavf_hash.c
>> @@ -80,6 +80,9 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct
>> rte_flow *flow,  static void  iavf_hash_uninit_parser(struct iavf_adapter *ad);
>> static int
>> +iavf_hash_validation(struct iavf_adapter *ad, struct rte_flow *flow,
>> +		     void *meta, struct rte_flow_error *error); static int
>>   iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
>>   			       struct iavf_pattern_match_item *array,
>>   			       uint32_t array_len,
>> @@ -680,6 +683,7 @@ static struct iavf_flow_engine iavf_hash_engine = {
>>   	.create = iavf_hash_create,
>>   	.destroy = iavf_hash_destroy,
>>   	.uninit = iavf_hash_uninit_parser,
>> +	.validation = iavf_hash_validation,
>>   	.name = "hash",
>>   	.type = IAVF_FLOW_ENGINE_HASH,
>>   	.rule_size = sizeof(struct virtchnl_rss_cfg), @@ -1579,6 +1583,15 @@
>> iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
>>   	return ret;
>>   }
>>
>> +static int
>> +iavf_hash_validation(__rte_unused struct iavf_adapter *ad,
>> +		     __rte_unused struct rte_flow *flow, void *meta,
>> +		     __rte_unused struct rte_flow_error *error) {
>> +	rte_free(meta);
>> +	return 0;
>> +}
>> +
>>   static int
>>   iavf_hash_create(__rte_unused struct iavf_adapter *ad,
>>   		 __rte_unused struct rte_flow *flow, void *meta, @@ -1594,6
>> +1607,7 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
>>   		rte_flow_error_set(error, EINVAL,
>>   				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>>   				   "No memory for rss rule");
>> +		rte_free(meta);
>>   		return -ENOMEM;
>>   	}
>>
>> @@ -1609,6 +1623,7 @@ iavf_hash_create(__rte_unused struct iavf_adapter
>> *ad,
>>   				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>>   				   "Failed to add rss rule.");
>>   		rte_free(rss_cfg);
>> +		rte_free(meta);
>>   		return -rte_errno;
>>   	}
>>
>> --
>> 2.52.0
> 
> Hi Anatoly,
> 
> I had submitted similar fix  in this patch earlier.
> https://patches.dpdk.org/project/dpdk/patch/20261010121255.528329-1-anurag.mandal@intel.com/
> 
> Thanks,
> Anurag M

Thanks, I didn't see it before! Your patch fixes some things mine 
doesn't, and vice versa, so what I'll do (if you don't mind!) is I'll 
respin my patchset and include your patch with my additions.

-- 
Thanks,
Anatoly

[PATCH v1 6/7] net/iavf: fix IPsec flow metadata leaks

[Anatoly Burakov]

IPsec flow parsing leaves the pattern-match item allocated. Validation
has no callback to release the parsed flow item, and creation leaks it
when adding the security policy fails.

Free the pattern-match item after parsing, add a validation callback
to release the parsed flow item, and free it if policy creation fails.
Report a parsing error when no flow item was produced.

Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 40 +++++++++++++++++-----
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index 9fcdc311377..cd13e14ae74 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1893,6 +1893,7 @@ iavf_ipsec_flow_create(struct iavf_adapter *ad,
 	}
 
 	if (flow_id < 1) {
+		rte_free(ipsec_flow);
 		rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				"Failed to add SA.");
@@ -1923,6 +1924,16 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 	return 0;
 }
 
+static int
+iavf_ipsec_flow_validation(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct rte_flow *flow,
+		void *meta,
+		__rte_unused struct rte_flow_error *error)
+{
+	rte_free(meta);
+	return 0;
+}
+
 static int
 iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		      struct iavf_pattern_match_item *array,
@@ -1942,6 +1953,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		.max_actions = 1,
 	};
 	struct iavf_pattern_match_item *item = NULL;
+	struct iavf_ipsec_flow_item *fi = NULL;
 	int ret = -1;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
@@ -1953,17 +1965,28 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		return ret;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (item && item->meta) {
+	if (item == NULL)
+		return -rte_errno;
+
+	if (item->meta != NULL) {
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
-		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad->vf.eth_dev, pattern, session, type);
-		if (fi && meta) {
-			*meta = fi;
-			ret = 0;
-		}
+
+		fi = iavf_ipsec_flow_item_parse(ad->vf.eth_dev, pattern, session, type);
 	}
-	return ret;
+	rte_free(item);
+
+	if (fi == NULL)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+				"Invalid IPsec flow.");
+
+	if (meta != NULL)
+		*meta = fi;
+	else
+		rte_free(fi);
+
+	return 0;
 }
 
 /*
@@ -2060,6 +2083,7 @@ static struct iavf_flow_engine iavf_ipsec_flow_engine = {
 	.uninit = iavf_ipsec_flow_uninit,
 	.create = iavf_ipsec_flow_create,
 	.destroy = iavf_ipsec_flow_destroy,
+	.validation = iavf_ipsec_flow_validation,
 	.name = "ipsec_crypto",
 	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
 	.rule_size = sizeof(struct iavf_ipsec_flow_item),
-- 
2.52.0

[PATCH v1 7/7] net/iavf: do not store ethdev pointer

[Anatoly Burakov]

We should not be storing rte_eth_dev pointers in shared memory, because
rte_eth_dev pointers are process local. To address that, remove the pointer
from iavf_info, replace it with `rte_eth_devices`-based lookup, and rework
the call paths to either use that lookup, or not require ethdev pointer
in the first place.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  9 ++++++--
 drivers/net/intel/iavf/iavf_ethdev.c       | 13 ++++++------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 24 +++++++++++-----------
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |  4 ++--
 drivers/net/intel/iavf/iavf_vchnl.c        |  7 ++++---
 5 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index c40b97444b5..1f277eeee29 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -7,6 +7,7 @@
 
 #include <sys/queue.h>
 
+#include <ethdev_driver.h>
 #include <rte_kvargs.h>
 #include <rte_tm_driver.h>
 
@@ -293,8 +294,6 @@ struct iavf_info {
 	struct iavf_qtc_map *qtc_map;
 	struct iavf_tm_conf tm_conf;
 
-	struct rte_eth_dev *eth_dev;
-
 	RTE_ATOMIC(bool)in_reset_recovery;
 	bool reset_pending;
 	bool pf_reset_in_progress;
@@ -408,6 +407,12 @@ struct iavf_adapter {
 	uint16_t tpid;  /* VLAN tag identifier */
 };
 
+static inline struct rte_eth_dev *
+iavf_get_eth_dev(struct iavf_adapter *adapter)
+{
+	return &rte_eth_devices[adapter->dev_data->port_id];
+}
+
 /* IAVF_DEV_PRIVATE_TO */
 #define IAVF_DEV_PRIVATE_TO_ADAPTER(adapter) \
 	((struct iavf_adapter *)adapter)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 6cf12ec34f1..76c51e7c25b 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -326,6 +326,7 @@ static void
 iavf_dev_watchdog(void *cb_arg)
 {
 	struct iavf_adapter *adapter = cb_arg;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
 	int vfr_inprogress = 0, rc = 0;
 
@@ -339,7 +340,7 @@ iavf_dev_watchdog(void *cb_arg)
 
 		if (!vfr_inprogress) {
 			PMD_DRV_LOG(INFO, "VF \"%s\" reset has completed",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 			adapter->vf.vf_reset = false;
 			iavf_set_no_poll(adapter, false);
 		}
@@ -350,14 +351,14 @@ iavf_dev_watchdog(void *cb_arg)
 		if (vfr_inprogress) {
 			PMD_DRV_LOG(INFO,
 				"VF \"%s\" reset event detected by watchdog",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
 			iavf_set_no_poll(adapter, false);
 			adapter->vf.link_up = false;
 
-			iavf_dev_event_post(adapter->vf.eth_dev, RTE_ETH_EVENT_INTR_RESET,
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
 				NULL, 0);
 		}
 	}
@@ -365,11 +366,11 @@ iavf_dev_watchdog(void *cb_arg)
 	if (adapter->devargs.watchdog_period) {
 		/* re-alarm watchdog */
 		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
-					&iavf_dev_watchdog, cb_arg);
+					&iavf_dev_watchdog, dev);
 
 		if (rc)
 			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 	}
 }
 
@@ -2674,8 +2675,6 @@ iavf_init_vf(struct rte_eth_dev *dev)
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 
-	vf->eth_dev = dev;
-
 	err = iavf_parse_devargs(dev);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Failed to parse devargs");
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index cd13e14ae74..8056c889cb1 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -689,11 +689,9 @@ iavf_ipsec_crypto_session_create(void *device,
  * returns: 1 if valid
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(ethdev->data->dev_private);
 	const struct iavf_security_session *sess = (const void *)session->driver_priv_data;
 
 	/* verify we have a valid session and that it belong to this adapter */
@@ -1502,24 +1500,25 @@ static struct rte_security_ops iavf_ipsec_crypto_ops = {
 int
 iavf_security_ctx_create(struct iavf_adapter *adapter)
 {
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct rte_security_ctx *sctx;
 
 	sctx = rte_malloc("security_ctx", sizeof(struct rte_security_ctx), 0);
 	if (sctx == NULL)
 		return -ENOMEM;
 
-	sctx->device = adapter->vf.eth_dev;
+	sctx->device = dev;
 	sctx->ops = &iavf_ipsec_crypto_ops;
 	sctx->sess_cnt = 0;
 
-	adapter->vf.eth_dev->security_ctx = sctx;
+	dev->security_ctx = sctx;
 
 	if (adapter->security_ctx == NULL) {
 		adapter->security_ctx = rte_malloc("iavf_security_ctx",
 				sizeof(struct iavf_security_ctx), 0);
 		if (adapter->security_ctx == NULL) {
-			rte_free(adapter->vf.eth_dev->security_ctx);
-			adapter->vf.eth_dev->security_ctx = NULL;
+			rte_free(dev->security_ctx);
+			dev->security_ctx = NULL;
 			return -ENOMEM;
 		}
 	}
@@ -1565,7 +1564,8 @@ iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter)
 int
 iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 {
-	struct rte_security_ctx *sctx  = adapter->vf.eth_dev->security_ctx;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
+	struct rte_security_ctx *sctx  = dev->security_ctx;
 	struct iavf_security_ctx *iavf_sctx = adapter->security_ctx;
 
 	if (iavf_sctx == NULL)
@@ -1578,7 +1578,7 @@ iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 	rte_free(sctx);
 
 	adapter->security_ctx = NULL;
-	adapter->vf.eth_dev->security_ctx = NULL;
+	dev->security_ctx = NULL;
 
 	return 0;
 }
@@ -1748,7 +1748,7 @@ parse_udp_item(const struct rte_flow_item_udp *item, struct rte_udp_hdr *udp)
 }
 
 static struct iavf_ipsec_flow_item *
-iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
+iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 		const struct rte_flow_item pattern[],
 		const struct rte_security_session *session,
 		uint32_t type)
@@ -1819,7 +1819,7 @@ iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
 		goto flow_cleanup;
 	}
 
-	if (!iavf_ipsec_crypto_action_valid(ethdev, session,
+	if (!iavf_ipsec_crypto_action_valid(adapter, session,
 			ipsec_flow->spi))
 		goto flow_cleanup;
 
@@ -1972,7 +1972,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
 
-		fi = iavf_ipsec_flow_item_parse(ad->vf.eth_dev, pattern, session, type);
+		fi = iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 	}
 	rte_free(item);
 
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.h b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
index 795b75a624d..3d4b1da127a 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.h
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
@@ -127,13 +127,13 @@ int iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter);
 /**
  * Destroy security context
  */
-int iavf_security_ctx_destroy(struct iavf_adapter *adapterv);
+int iavf_security_ctx_destroy(struct iavf_adapter *adapter);
 
 /**
  * Verify that the inline IPsec Crypto action is valid for this device
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi);
 
 /**
diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 57488512f43..26dae423a48 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -314,13 +314,14 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 		    opcode, vf->cmd_retval);
 
 	if (opcode == VIRTCHNL_OP_EVENT) {
+		struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 		struct virtchnl_pf_event *vpe =
 			(struct virtchnl_pf_event *)event.msg_buf;
 
 		result = IAVF_MSG_SYS;
 		switch (vpe->event) {
 		case VIRTCHNL_EVENT_LINK_CHANGE:
-			iavf_handle_link_change_event(vf->eth_dev, vpe);
+			iavf_handle_link_change_event(dev, vpe);
 			break;
 		case VIRTCHNL_EVENT_RESET_IMPENDING:
 			/*
@@ -335,8 +336,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				iavf_set_no_poll(adapter, false);
 				iavf_dev_watchdog_enable(adapter);
 				if (adapter->devargs.no_poll_on_link_down)
-					iavf_dev_tx_drain(vf->eth_dev);
-				iavf_dev_event_post(vf->eth_dev,
+					iavf_dev_tx_drain(dev);
+				iavf_dev_event_post(dev,
 					RTE_ETH_EVENT_INTR_RESET,
 					NULL, 0);
 			}
-- 
2.52.0

[PATCH v2 0/9] IAVF Fixes

[Anatoly Burakov]

An assortment of IAVF flow-related memory leak fixes and cleanups.

v1 -> v2:
- Integrated Anurag's earlier fix [1] into the patchset
- Reworked generic flow to have centralized memory management
- Reordered some patches
- Integrated the IRQ leaks patchset [2] [3]
- Reworked IPsec fix

[1] https://patches.dpdk.org/project/dpdk/patch/20261010121255.528329-1-anurag.mandal@intel.com/
[2] https://patches.dpdk.org/project/dpdk/patch/809e0c5b2ab57f1b14c68be8ac34e70cbbe82233.1792053236.git.anatoly.burakov@intel.com/
[3] https://patches.dpdk.org/project/dpdk/patch/5488d85c2ded08838becaee6de8328694ee00234.1792053236.git.anatoly.burakov@intel.com/

Anatoly Burakov (8):
  net/iavf: keep device stopped if start fails
  net/iavf: do not store ethdev pointer
  net/iavf: fix false success on flow validation
  net/iavf: fix memory leaks around IRQ resources
  net/iavf: fix generic flow error-path leaks
  net/iavf: centralize memory management for flows
  net/iavf: fix null engine for egress security flows
  net/iavf: fix IPsec flow metadata leaks

Anurag Mandal (1):
  net/iavf: fix memory leak in raw pattern parsing

 drivers/net/intel/iavf/iavf.h              |  14 +-
 drivers/net/intel/iavf/iavf_ethdev.c       |  42 +++--
 drivers/net/intel/iavf/iavf_fdir.c         |  68 ++++----
 drivers/net/intel/iavf/iavf_fsub.c         |  23 +--
 drivers/net/intel/iavf/iavf_generic_flow.c |  95 ++++++------
 drivers/net/intel/iavf/iavf_generic_flow.h |   4 +-
 drivers/net/intel/iavf/iavf_hash.c         |  80 +++-------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 171 +++++++++++++++++----
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |   4 +-
 drivers/net/intel/iavf/iavf_vchnl.c        |   7 +-
 10 files changed, 296 insertions(+), 212 deletions(-)

-- 
2.52.0

[PATCH v2 1/9] net/iavf: keep device stopped if start fails

[Anatoly Burakov]

Currently, `iavf_dev_start()` clears `adapter->stopped` before several
operations that can fail. A subsequent `iavf_dev_stop()` then disables
interrupts and stops and resets queues even though startup did not
complete.

Mark the adapter running only after startup succeeds, and return early
from stop when it is already stopped.

Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 7f1ad3d88c..68aced0e31 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1039,8 +1039,6 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	if (adapter->closed)
 		return -1;
 
-	adapter->stopped = 0;
-
 	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
 	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				      dev->data->nb_tx_queues);
@@ -1112,6 +1110,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 
 	iavf_phc_sync_alarm_start(dev);
+	adapter->stopped = 0;
 
 	/* An explicit start supersedes any pending deferred start */
 	vf->start_pending = false;
@@ -1135,13 +1134,13 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 	if (adapter->closed)
 		return -1;
 
+	if (adapter->stopped == 1)
+		return 0;
+
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) &&
 	    dev->data->dev_conf.intr_conf.rxq != 0)
 		rte_intr_disable(intr_handle);
 
-	if (adapter->stopped == 1)
-		return 0;
-
 	iavf_phc_sync_alarm_stop(dev);
 
 	/* Disable the interrupt for Rx */
-- 
2.52.0

[PATCH v2 2/9] net/iavf: do not store ethdev pointer

[Anatoly Burakov]

We should not be storing rte_eth_dev pointers in shared memory, because
rte_eth_dev pointers are process local. To address that, remove the pointer
from iavf_info, replace it with `rte_eth_devices`-based lookup, and rework
the call paths to either use that lookup, or not require ethdev pointer
in the first place.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  9 ++++++--
 drivers/net/intel/iavf/iavf_ethdev.c       | 13 ++++++------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 24 +++++++++++-----------
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |  4 ++--
 drivers/net/intel/iavf/iavf_vchnl.c        |  7 ++++---
 5 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index c40b97444b..1f277eeee2 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -7,6 +7,7 @@
 
 #include <sys/queue.h>
 
+#include <ethdev_driver.h>
 #include <rte_kvargs.h>
 #include <rte_tm_driver.h>
 
@@ -293,8 +294,6 @@ struct iavf_info {
 	struct iavf_qtc_map *qtc_map;
 	struct iavf_tm_conf tm_conf;
 
-	struct rte_eth_dev *eth_dev;
-
 	RTE_ATOMIC(bool)in_reset_recovery;
 	bool reset_pending;
 	bool pf_reset_in_progress;
@@ -408,6 +407,12 @@ struct iavf_adapter {
 	uint16_t tpid;  /* VLAN tag identifier */
 };
 
+static inline struct rte_eth_dev *
+iavf_get_eth_dev(struct iavf_adapter *adapter)
+{
+	return &rte_eth_devices[adapter->dev_data->port_id];
+}
+
 /* IAVF_DEV_PRIVATE_TO */
 #define IAVF_DEV_PRIVATE_TO_ADAPTER(adapter) \
 	((struct iavf_adapter *)adapter)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 68aced0e31..befee1d3eb 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -326,6 +326,7 @@ static void
 iavf_dev_watchdog(void *cb_arg)
 {
 	struct iavf_adapter *adapter = cb_arg;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
 	int vfr_inprogress = 0, rc = 0;
 
@@ -339,7 +340,7 @@ iavf_dev_watchdog(void *cb_arg)
 
 		if (!vfr_inprogress) {
 			PMD_DRV_LOG(INFO, "VF \"%s\" reset has completed",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 			adapter->vf.vf_reset = false;
 			iavf_set_no_poll(adapter, false);
 		}
@@ -350,14 +351,14 @@ iavf_dev_watchdog(void *cb_arg)
 		if (vfr_inprogress) {
 			PMD_DRV_LOG(INFO,
 				"VF \"%s\" reset event detected by watchdog",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
 			iavf_set_no_poll(adapter, false);
 			adapter->vf.link_up = false;
 
-			iavf_dev_event_post(adapter->vf.eth_dev, RTE_ETH_EVENT_INTR_RESET,
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
 				NULL, 0);
 		}
 	}
@@ -365,11 +366,11 @@ iavf_dev_watchdog(void *cb_arg)
 	if (adapter->devargs.watchdog_period) {
 		/* re-alarm watchdog */
 		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
-					&iavf_dev_watchdog, cb_arg);
+					&iavf_dev_watchdog, dev);
 
 		if (rc)
 			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 	}
 }
 
@@ -2669,8 +2670,6 @@ iavf_init_vf(struct rte_eth_dev *dev)
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 
-	vf->eth_dev = dev;
-
 	err = iavf_parse_devargs(dev);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Failed to parse devargs");
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index 06b51b5e62..b481330346 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -689,11 +689,9 @@ iavf_ipsec_crypto_session_create(void *device,
  * returns: 1 if valid
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(ethdev->data->dev_private);
 	const struct iavf_security_session *sess = (const void *)session->driver_priv_data;
 
 	/* verify we have a valid session and that it belong to this adapter */
@@ -1502,24 +1500,25 @@ static struct rte_security_ops iavf_ipsec_crypto_ops = {
 int
 iavf_security_ctx_create(struct iavf_adapter *adapter)
 {
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct rte_security_ctx *sctx;
 
 	sctx = rte_malloc("security_ctx", sizeof(struct rte_security_ctx), 0);
 	if (sctx == NULL)
 		return -ENOMEM;
 
-	sctx->device = adapter->vf.eth_dev;
+	sctx->device = dev;
 	sctx->ops = &iavf_ipsec_crypto_ops;
 	sctx->sess_cnt = 0;
 
-	adapter->vf.eth_dev->security_ctx = sctx;
+	dev->security_ctx = sctx;
 
 	if (adapter->security_ctx == NULL) {
 		adapter->security_ctx = rte_malloc("iavf_security_ctx",
 				sizeof(struct iavf_security_ctx), 0);
 		if (adapter->security_ctx == NULL) {
-			rte_free(adapter->vf.eth_dev->security_ctx);
-			adapter->vf.eth_dev->security_ctx = NULL;
+			rte_free(dev->security_ctx);
+			dev->security_ctx = NULL;
 			return -ENOMEM;
 		}
 	}
@@ -1565,7 +1564,8 @@ iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter)
 int
 iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 {
-	struct rte_security_ctx *sctx  = adapter->vf.eth_dev->security_ctx;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
+	struct rte_security_ctx *sctx  = dev->security_ctx;
 	struct iavf_security_ctx *iavf_sctx = adapter->security_ctx;
 
 	if (iavf_sctx == NULL)
@@ -1578,7 +1578,7 @@ iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 	rte_free(sctx);
 
 	adapter->security_ctx = NULL;
-	adapter->vf.eth_dev->security_ctx = NULL;
+	dev->security_ctx = NULL;
 
 	return 0;
 }
@@ -1748,7 +1748,7 @@ parse_udp_item(const struct rte_flow_item_udp *item, struct rte_udp_hdr *udp)
 }
 
 static struct iavf_ipsec_flow_item *
-iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
+iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 		const struct rte_flow_item pattern[],
 		const struct rte_security_session *session,
 		uint32_t type)
@@ -1819,7 +1819,7 @@ iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
 		goto flow_cleanup;
 	}
 
-	if (!iavf_ipsec_crypto_action_valid(ethdev, session,
+	if (!iavf_ipsec_crypto_action_valid(adapter, session,
 			ipsec_flow->spi))
 		goto flow_cleanup;
 
@@ -1967,7 +1967,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
 		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad->vf.eth_dev, pattern, session, type);
+				iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 		if (fi && meta) {
 			*meta = fi;
 			ret = 0;
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.h b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
index 795b75a624..3d4b1da127 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.h
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
@@ -127,13 +127,13 @@ int iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter);
 /**
  * Destroy security context
  */
-int iavf_security_ctx_destroy(struct iavf_adapter *adapterv);
+int iavf_security_ctx_destroy(struct iavf_adapter *adapter);
 
 /**
  * Verify that the inline IPsec Crypto action is valid for this device
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi);
 
 /**
diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 57488512f4..26dae423a4 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -314,13 +314,14 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 		    opcode, vf->cmd_retval);
 
 	if (opcode == VIRTCHNL_OP_EVENT) {
+		struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 		struct virtchnl_pf_event *vpe =
 			(struct virtchnl_pf_event *)event.msg_buf;
 
 		result = IAVF_MSG_SYS;
 		switch (vpe->event) {
 		case VIRTCHNL_EVENT_LINK_CHANGE:
-			iavf_handle_link_change_event(vf->eth_dev, vpe);
+			iavf_handle_link_change_event(dev, vpe);
 			break;
 		case VIRTCHNL_EVENT_RESET_IMPENDING:
 			/*
@@ -335,8 +336,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				iavf_set_no_poll(adapter, false);
 				iavf_dev_watchdog_enable(adapter);
 				if (adapter->devargs.no_poll_on_link_down)
-					iavf_dev_tx_drain(vf->eth_dev);
-				iavf_dev_event_post(vf->eth_dev,
+					iavf_dev_tx_drain(dev);
+				iavf_dev_event_post(dev,
 					RTE_ETH_EVENT_INTR_RESET,
 					NULL, 0);
 			}
-- 
2.52.0

[PATCH v2 3/9] net/iavf: fix false success on flow validation

[Anatoly Burakov]

`iavf_parse_engine_validate()` sets an error when validation fails, but
then returns the engine pointer. `iavf_flow_process_filter()` treats a
non-NULL engine as success, so `rte_flow_validate()` can accept an
invalid flow.

Return `NULL` on validation failure and return the engine only after
validation succeeds.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 92ca20031c..14242e6ebb 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -2134,10 +2134,11 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation failed");
-			break;
+			return NULL;
 		}
+		return engine;
 	}
-	return engine;
+	return NULL;
 }
 
 
-- 
2.52.0

[PATCH v2 4/9] net/iavf: fix memory leak in raw pattern parsing

[Anatoly Burakov]

From: Anurag Mandal <anurag.mandal@intel.com>

If mask buffer allocation fails in `iavf_hash_parse_raw_pattern()`,
the previously allocated packet buffer is leaked.

Free the packet buffer before returning the allocation error.

Fixes: bc0e85586eaf ("net/iavf: support VF RSS flow rule with raw pattern")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    The API was removed in 92ef4b8f1688 ("ethdev: remove deprecated shared counter attribute")
    
    However, it may not be reasonable to pin a Fixline on that commit because that
    would imply that it *should have* updated ice driver's plubmbing to not support
    shared counters any more, which I do not believe is reasonable. I'm leaving
    this note for posterity and in case it is determined that the Fixline is needed
    after all.

 drivers/net/intel/iavf/iavf_hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index 9515094402..a240c73614 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -918,8 +918,10 @@ iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 		return -ENOMEM;
 
 	msk_buf = rte_zmalloc(NULL, pkt_len, 0);
-	if (!msk_buf)
+	if (!msk_buf) {
+		rte_free(pkt_buf);
 		return -ENOMEM;
+	}
 
 	/* convert string to int array */
 	for (i = 0, j = 0; i < spec_len; i += 2, j++) {
-- 
2.52.0

Re: [PATCH v2 4/9] net/iavf: fix memory leak in raw pattern parsing

[Burakov, Anatoly]

On 10/19/2026 5:06 PM, Anatoly Burakov wrote:
> The API was removed in 92ef4b8f1688 ("ethdev: remove deprecated shared 
> counter attribute") However, it may not be reasonable to pin a Fixline 
> on that commit because that would imply that it *should have*updated ice 
> driver's plubmbing to not support shared counters any more, which I do 
> not believe is reasonable. I'm leaving this note for posterity and in 
> case it is determined that the Fixline is needed after all.

This was accidentally included. It belongs in a different patch in a 
different driver, but `git notes` doesn't handle rebases very well...

-- 
Thanks,
Anatoly

[PATCH v2 5/9] net/iavf: fix memory leaks around IRQ resources

[Anatoly Burakov]

`iavf_config_rx_queues_irqs()` allocates a queue-vector map and may
enable eventfds and allocate an interrupt vector list. If queue startup
then fails, those resources remain allocated. The queue-vector map is
also retained after a normal stop, leaking it across start/stop cycles.

Release the resources on failed startup and free the queue-vector map
on stop. Clean up eventfds when interrupt setup fails as well.

Fixes: f593944fc988 ("net/iavf: enable IRQ mapping configuration for large VF")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index befee1d3eb..bb97a89414 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -863,7 +863,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 	if (rte_intr_cap_multiple(intr_handle) &&
 	    dev->data->dev_conf.intr_conf.rxq) {
 		if (rte_intr_efd_enable(intr_handle, dev->data->nb_rx_queues))
-			return -1;
+			goto efd_alloc_err;
 	}
 
 	if (rte_intr_dp_is_en(intr_handle)) {
@@ -871,7 +871,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 						    dev->data->nb_rx_queues)) {
 			PMD_DRV_LOG(ERR, "Failed to allocate %d rx intr_vec",
 				    dev->data->nb_rx_queues);
-			return -1;
+			goto qv_map_alloc_err;
 		}
 	}
 
@@ -981,6 +981,8 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 
 qv_map_alloc_err:
 	rte_intr_vec_list_free(intr_handle);
+efd_alloc_err:
+	rte_intr_efd_disable(intr_handle);
 
 	return -1;
 }
@@ -1034,12 +1036,15 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	struct rte_intr_handle *intr_handle = dev->intr_handle;
 	uint16_t num_queue_pairs;
 	uint16_t index = 0;
+	bool wb_on_itr;
 
 	PMD_INIT_FUNC_TRACE();
 
 	if (adapter->closed)
 		return -1;
 
+	wb_on_itr = (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) != 0;
+
 	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
 	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				      dev->data->nb_tx_queues);
@@ -1089,7 +1094,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 	/* re-enable intr again, because efd assign may change */
 	if (dev->data->dev_conf.intr_conf.rxq != 0) {
-		if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
+		if (wb_on_itr)
 			rte_intr_disable(intr_handle);
 		rte_intr_enable(intr_handle);
 	}
@@ -1107,7 +1112,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 
 	if (iavf_start_queues(dev) != 0) {
 		PMD_DRV_LOG(ERR, "enable queues failed");
-		goto error;
+		goto irq_error;
 	}
 
 	iavf_phc_sync_alarm_start(dev);
@@ -1118,6 +1123,13 @@ iavf_dev_start(struct rte_eth_dev *dev)
 
 	return 0;
 
+irq_error:
+	if (!wb_on_itr && dev->data->dev_conf.intr_conf.rxq != 0)
+		rte_intr_disable(intr_handle);
+	rte_intr_efd_disable(intr_handle);
+	rte_intr_vec_list_free(intr_handle);
+	rte_free(vf->qv_map);
+	vf->qv_map = NULL;
 error:
 	return -1;
 }
@@ -1150,6 +1162,8 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 	rte_intr_vec_list_free(intr_handle);
 
 	iavf_stop_queues(dev);
+	rte_free(vf->qv_map);
+	vf->qv_map = NULL;
 
 	adapter->stopped = 1;
 	dev->data->dev_started = 0;
-- 
2.52.0

[PATCH v2 6/9] net/iavf: fix generic flow error-path leaks

[Anatoly Burakov]

If a flow engine fails to initialize, parsers registered by earlier
engines remain allocated. Pattern matching also leaks its item array
when allocation of the match result fails.

Unwind registered parsers on flow initialization failure and free the
item array on match-result allocation failure.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 14242e6ebb..cd4d4a012b 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1821,17 +1821,22 @@ iavf_flow_init(struct iavf_adapter *ad)
 		if (engine->init == NULL) {
 			PMD_INIT_LOG(ERR, "Invalid engine type (%d)",
 				     engine->type);
-			return -ENOTSUP;
+			ret = -ENOTSUP;
+			goto err;
 		}
 
 		ret = engine->init(ad);
 		if (ret && ret != -ENOTSUP) {
 			PMD_INIT_LOG(ERR, "Failed to initialize engine %d",
 				     engine->type);
-			return ret;
+			goto err;
 		}
 	}
 	return 0;
+
+err:
+	iavf_flow_uninit(ad);
+	return ret;
 }
 
 void
@@ -2047,6 +2052,7 @@ iavf_search_pattern_match_item(const struct rte_flow_item pattern[],
 	if (!pattern_match_item) {
 		rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Failed to allocate memory.");
+		rte_free(items);
 		return NULL;
 	}
 
-- 
2.52.0

[PATCH v2 7/9] net/iavf: centralize memory management for flows

[Anatoly Burakov]

Flow engines currently distribute rule allocation and cleanup between the
parser, engine create and destroy callbacks, and generic flow code. This
leads to a lot of potential for memory leaks and bugs. In particular, one
of the issues currently present is that when PF initiates reset, we are
meant to just drop all flows without sending any virtchnl commands, but
because not all engines implement the `free` callback, some flows will be
leaked.

To address that specific leak as well as all other potential leaks due to
misuse of memory by engines, use parser-allocated rule objects as the
persistent rules and centralize their lifetime in the generic flow layer.
This requires fixing all engines to not manage their own memory:

- Allocate the final FDIR rule in the parser, use it for action checks,
  and remove the per-VF scratch configuration.
- Have FSUB create adopt its parser-produced rule instead of allocating
  and copying a second object.
- Build the final RSS configuration in the parser and remove the
  intermediate RSS metadata, including its unused raw-pattern flag.
- Let generic flow code free rules on validate, create failure,
  successful destroy, and uninit; remove engine-specific free callbacks
  and rule frees from destroy callbacks.

Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  5 --
 drivers/net/intel/iavf/iavf_fdir.c         | 68 +++++++++----------
 drivers/net/intel/iavf/iavf_fsub.c         | 23 +------
 drivers/net/intel/iavf/iavf_generic_flow.c | 36 ++++++++--
 drivers/net/intel/iavf/iavf_generic_flow.h |  2 -
 drivers/net/intel/iavf/iavf_hash.c         | 76 +++++++---------------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c |  1 -
 7 files changed, 88 insertions(+), 123 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index 1f277eeee2..8b826662c6 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -155,10 +155,6 @@ struct iavf_fdir_conf {
 	uint32_t mark_flag;
 };
 
-struct iavf_fdir_info {
-	struct iavf_fdir_conf conf;
-};
-
 struct iavf_fsub_conf {
 	struct virtchnl_flow_sub sub_fltr;
 	struct virtchnl_flow_unsub unsub_fltr;
@@ -286,7 +282,6 @@ struct iavf_info {
 	struct iavf_parser_list dist_parser_list;
 	struct iavf_parser_list ipsec_crypto_parser_list;
 
-	struct iavf_fdir_info fdir; /* flow director info */
 	/* indicate large VF support enabled or not */
 	bool lv_enabled;
 
diff --git a/drivers/net/intel/iavf/iavf_fdir.c b/drivers/net/intel/iavf/iavf_fdir.c
index 67003183bb..7b9d3aac7f 100644
--- a/drivers/net/intel/iavf/iavf_fdir.c
+++ b/drivers/net/intel/iavf/iavf_fdir.c
@@ -353,36 +353,20 @@ iavf_fdir_create(struct iavf_adapter *ad,
 		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 for fdir rule");
-		return -rte_errno;
-	}
-
 	ret = iavf_fdir_add(ad, filter);
 	if (ret) {
-		rte_flow_error_set(error, -ret,
+		return rte_flow_error_set(error, -ret,
 				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				"Failed to add filter rule.");
-		goto free_entry;
 	}
 
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 1);
 
-	memcpy(rule, filter, sizeof(*rule));
-	flow->rule = rule;
-
+	flow->rule = filter;
 	return 0;
-
-free_entry:
-	rte_free(rule);
-	return -rte_errno;
 }
 
 static int
@@ -406,9 +390,6 @@ iavf_fdir_destroy(struct iavf_adapter *ad,
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 0);
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return 0;
 }
 
@@ -1444,14 +1425,19 @@ iavf_fdir_action_check_qregion(struct iavf_adapter *ad,
 	return 0;
 }
 
+struct iavf_fdir_parse_ctx {
+	struct iavf_adapter *ad;
+	struct iavf_fdir_conf *filter;
+};
+
 static int
 iavf_fdir_parse_action_check(const struct ci_flow_actions *actions,
 		const struct ci_flow_actions_check_param *param,
 		struct rte_flow_error *error)
 {
-	struct iavf_adapter *ad = param->driver_ctx;
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	struct iavf_fdir_conf *filter = &vf->fdir.conf;
+	struct iavf_fdir_parse_ctx *ctx = param->driver_ctx;
+	struct iavf_adapter *ad = ctx->ad;
+	struct iavf_fdir_conf *filter = ctx->filter;
 	uint32_t dest_num = 0, mark_num = 0;
 	size_t i, number = 0;
 	bool has_drop = false;
@@ -1557,6 +1543,8 @@ iavf_fdir_parse(struct iavf_adapter *ad,
 		struct rte_flow_error *error)
 {
 	struct ci_flow_actions parsed_actions = {0};
+	struct iavf_fdir_conf *filter;
+	struct iavf_fdir_parse_ctx ctx;
 	struct ci_flow_actions_check_param param = {
 		.allowed_types = (enum rte_flow_action_type[]){
 			RTE_FLOW_ACTION_TYPE_PASSTHRU,
@@ -1568,38 +1556,52 @@ iavf_fdir_parse(struct iavf_adapter *ad,
 		},
 		.max_actions = 2,
 		.check = iavf_fdir_parse_action_check,
-		.driver_ctx = ad,
 		.rss_queues_contig = true,
 	};
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	struct iavf_fdir_conf *filter = &vf->fdir.conf;
 	struct iavf_pattern_match_item *item = NULL;
 	int ret;
 
-	memset(filter, 0, sizeof(*filter));
+	filter = rte_zmalloc("iavf_fdir_meta", sizeof(*filter), 0);
+	if (filter == NULL) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Failed to allocate FDIR metadata.");
+		return -rte_errno;
+	}
+
+	ctx.ad = ad;
+	ctx.filter = filter;
+	param.driver_ctx = &ctx;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (!item)
-		return -rte_errno;
+	if (item == NULL) {
+		ret = -rte_errno;
+		goto error;
+	}
 
 	ret = iavf_fdir_parse_pattern(ad, pattern, item->input_set_mask,
 				      error, filter);
 	if (ret)
 		goto error;
 
-	if (meta)
+	rte_free(item);
+	if (meta != NULL)
 		*meta = filter;
+	else
+		rte_free(filter);
+	return ret;
 
 error:
 	rte_free(item);
+	rte_free(filter);
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_fsub.c b/drivers/net/intel/iavf/iavf_fsub.c
index 19ae0e2dc2..e4e0e20c4f 100644
--- a/drivers/net/intel/iavf/iavf_fsub.c
+++ b/drivers/net/intel/iavf/iavf_fsub.c
@@ -73,34 +73,18 @@ iavf_fsub_create(struct iavf_adapter *ad, struct rte_flow *flow,
 		 void *meta, struct rte_flow_error *error)
 {
 	struct iavf_fsub_conf *filter = meta;
-	struct iavf_fsub_conf *rule;
 	int ret;
 
-	rule = rte_zmalloc("fsub_entry", sizeof(*rule), 0);
-	if (!rule) {
-		rte_flow_error_set(error, ENOMEM,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Failed to allocate memory for fsub rule");
-		return -rte_errno;
-	}
-
 	ret = iavf_flow_sub(ad, filter);
 	if (ret) {
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to subscribe flow rule.");
-		goto free_entry;
+		return -rte_errno;
 	}
 
-	memcpy(rule, filter, sizeof(*rule));
-	flow->rule = rule;
-
-	rte_free(meta);
+	flow->rule = filter;
 	return ret;
-
-free_entry:
-	rte_free(rule);
-	return -rte_errno;
 }
 
 static int
@@ -120,9 +104,6 @@ iavf_fsub_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		return -rte_errno;
 	}
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index cd4d4a012b..1593ea297c 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -46,6 +46,13 @@ static int iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		FILE *file,
 		struct rte_flow_error *error);
 
+static void
+iavf_flow_free(struct rte_flow *flow)
+{
+	rte_free(flow->rule);
+	rte_free(flow);
+}
+
 const struct rte_flow_ops iavf_flow_ops = {
 	.validate = iavf_flow_validate,
 	.create = iavf_flow_create,
@@ -1856,9 +1863,7 @@ iavf_flow_uninit(struct iavf_adapter *ad)
 	/* Remove all flows */
 	while ((p_flow = TAILQ_FIRST(&vf->flow_list))) {
 		TAILQ_REMOVE(&vf->flow_list, p_flow, node);
-		if (p_flow->engine->free)
-			p_flow->engine->free(p_flow);
-		rte_free(p_flow);
+		iavf_flow_free(p_flow);
 	}
 
 	/* Cleanup parser list */
@@ -2092,6 +2097,7 @@ iavf_parse_engine_create(struct iavf_adapter *ad,
 	void *meta = NULL;
 
 	RTE_TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
+		meta = NULL;
 		if (parser_node->parser->parse_pattern_action(ad,
 				parser_node->parser->array,
 				parser_node->parser->array_len,
@@ -2101,8 +2107,17 @@ iavf_parse_engine_create(struct iavf_adapter *ad,
 		engine = parser_node->parser->engine;
 
 		RTE_ASSERT(engine->create != NULL);
-		if (!(engine->create(ad, flow, meta, error)))
+		if (engine->create(ad, flow, meta, error) == 0) {
+			if (flow->rule != meta)
+				rte_free(meta);
 			return engine;
+		}
+
+		if (flow->rule != NULL && flow->rule != meta)
+			rte_free(flow->rule);
+		rte_free(meta);
+		flow->rule = NULL;
+		meta = NULL;
 	}
 	return NULL;
 }
@@ -2120,8 +2135,10 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 	struct iavf_flow_parser_node *parser_node;
 	void *temp;
 	void *meta = NULL;
+	int ret;
 
 	RTE_TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
+		meta = NULL;
 		if (parser_node->parser->parse_pattern_action(ad,
 				parser_node->parser->array,
 				parser_node->parser->array_len,
@@ -2130,13 +2147,18 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 
 		engine = parser_node->parser->engine;
 		if (engine->validation == NULL) {
+			rte_free(meta);
+			meta = NULL;
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation not support");
 			continue;
 		}
 
-		if (engine->validation(ad, flow, meta, error)) {
+		ret = engine->validation(ad, flow, meta, error);
+		rte_free(meta);
+		meta = NULL;
+		if (ret != 0) {
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation failed");
@@ -2238,7 +2260,7 @@ iavf_flow_create(struct rte_eth_dev *dev,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "Failed to create flow");
-		rte_free(flow);
+		iavf_flow_free(flow);
 		flow = NULL;
 		goto free_flow;
 	}
@@ -2308,7 +2330,7 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
-		rte_free(flow);
+		iavf_flow_free(flow);
 	} else {
 		PMD_DRV_LOG(ERR, "Failed to destroy flow");
 	}
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 6665d68818..3a8f5f92e8 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -465,7 +465,6 @@ typedef int (*engine_query_t)(struct iavf_adapter *ad,
 		struct rte_flow *flow,
 		struct rte_flow_query_count *count,
 		struct rte_flow_error *error);
-typedef void (*engine_free_t) (struct rte_flow *flow);
 typedef int (*parse_pattern_action_t)(struct iavf_adapter *ad,
 		struct iavf_pattern_match_item *array,
 		uint32_t array_len,
@@ -494,7 +493,6 @@ struct iavf_flow_engine {
 	engine_create_t create;
 	engine_destroy_t destroy;
 	engine_query_t query_count;
-	engine_free_t free;
 	const char *name;
 	enum iavf_flow_engine_type type;
 	size_t rule_size;
diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index a240c73614..6352bf28f9 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -58,12 +58,6 @@ struct iavf_hash_match_type {
 	uint64_t pattern_hint;
 };
 
-struct iavf_rss_meta {
-	struct virtchnl_proto_hdrs proto_hdrs;
-	enum virtchnl_rss_algorithm rss_algorithm;
-	bool raw_ena;
-};
-
 struct iavf_hash_flow_cfg {
 	struct virtchnl_rss_cfg *rss_cfg;
 	bool simple_xor;
@@ -79,8 +73,6 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		  struct rte_flow_error *error);
 static void
 iavf_hash_uninit_parser(struct iavf_adapter *ad);
-static void
-iavf_hash_free(struct rte_flow *flow);
 static int
 iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 			       struct iavf_pattern_match_item *array,
@@ -682,7 +674,6 @@ static struct iavf_flow_engine iavf_hash_engine = {
 	.create = iavf_hash_create,
 	.destroy = iavf_hash_destroy,
 	.uninit = iavf_hash_uninit_parser,
-	.free = iavf_hash_free,
 	.name = "hash",
 	.type = IAVF_FLOW_ENGINE_HASH,
 	.rule_size = sizeof(struct virtchnl_rss_cfg),
@@ -886,7 +877,7 @@ iavf_hash_parse_pattern(const struct rte_flow_item pattern[], uint64_t *phint,
 static int
 iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			struct rte_flow_error *error,
-			struct iavf_rss_meta *meta)
+			struct virtchnl_rss_cfg *rss_cfg)
 {
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
 	uint16_t spec_len, pkt_len;
@@ -958,9 +949,9 @@ iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			msk_buf[j] = tmp_val * 16 + tmp_c - '0';
 	}
 
-	memcpy(meta->proto_hdrs.raw.spec, pkt_buf, pkt_len);
-	memcpy(meta->proto_hdrs.raw.mask, msk_buf, pkt_len);
-	meta->proto_hdrs.raw.pkt_len = pkt_len;
+	memcpy(rss_cfg->proto_hdrs.raw.spec, pkt_buf, pkt_len);
+	memcpy(rss_cfg->proto_hdrs.raw.mask, msk_buf, pkt_len);
+	rss_cfg->proto_hdrs.raw.pkt_len = pkt_len;
 
 	rte_free(pkt_buf);
 	rte_free(msk_buf);
@@ -1433,12 +1424,12 @@ iavf_any_invalid_rss_type(enum rte_eth_hash_function rss_func,
 static int
 iavf_hash_parse_rss_type(struct iavf_pattern_match_item *match_item,
 		const struct rte_flow_action_rss *rss,
-		uint64_t pattern_hint, struct iavf_rss_meta *rss_meta,
+		uint64_t pattern_hint, struct virtchnl_rss_cfg *rss_cfg,
 		struct rte_flow_error *error)
 {
 	uint64_t rss_type;
 
-	rss_meta->rss_algorithm = rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ ?
+	rss_cfg->rss_algorithm = rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ ?
 		VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC :
 		VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 
@@ -1458,9 +1449,9 @@ iavf_hash_parse_rss_type(struct iavf_pattern_match_item *match_item,
 				"RSS type not supported");
 	}
 
-	memcpy(&rss_meta->proto_hdrs, match_item->meta, sizeof(struct virtchnl_proto_hdrs));
+	memcpy(&rss_cfg->proto_hdrs, match_item->meta, sizeof(rss_cfg->proto_hdrs));
 
-	iavf_refine_proto_hdrs(&rss_meta->proto_hdrs, rss_type, pattern_hint);
+	iavf_refine_proto_hdrs(&rss_cfg->proto_hdrs, rss_type, pattern_hint);
 
 	return 0;
 }
@@ -1526,7 +1517,7 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	};
 	const struct rte_flow_action_rss *rss;
 	struct iavf_pattern_match_item *pattern_match_item;
-	struct iavf_rss_meta *rss_meta_ptr;
+	struct virtchnl_rss_cfg *rss_cfg;
 	uint64_t phint = IAVF_PHINT_NONE;
 	int ret = 0;
 
@@ -1538,12 +1529,12 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	if (ret)
 		return ret;
 
-	rss_meta_ptr = rte_zmalloc(NULL, sizeof(*rss_meta_ptr), 0);
-	if (!rss_meta_ptr) {
-		rte_flow_error_set(error, EINVAL,
+	rss_cfg = rte_zmalloc("iavf_rss_rule", sizeof(*rss_cfg), 0);
+	if (rss_cfg == NULL) {
+		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for rss_meta_ptr");
-		return -ENOMEM;
+				   "No memory for RSS rule.");
+		return -rte_errno;
 	}
 
 	/* Check rss supported pattern and find matched pattern. */
@@ -1560,8 +1551,7 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 		goto error;
 
 	if (phint == IAVF_PHINT_RAW) {
-		rss_meta_ptr->raw_ena = true;
-		ret = iavf_hash_parse_raw_pattern(pattern, error, rss_meta_ptr);
+		ret = iavf_hash_parse_raw_pattern(pattern, error, rss_cfg);
 		if (ret) {
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM, NULL,
@@ -1571,13 +1561,13 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	}
 
 	rss = parsed_actions.actions[0]->conf;
-	ret = iavf_hash_parse_rss_type(pattern_match_item, rss, phint, rss_meta_ptr, error);
+	ret = iavf_hash_parse_rss_type(pattern_match_item, rss, phint, rss_cfg, error);
 
 error:
 	if (!ret && meta)
-		*meta = rss_meta_ptr;
+		*meta = rss_cfg;
 	else
-		rte_free(rss_meta_ptr);
+		rte_free(rss_cfg);
 
 	rte_free(pattern_match_item);
 
@@ -1585,26 +1575,13 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 }
 
 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)
+iavf_hash_create(struct iavf_adapter *ad,
+		 struct rte_flow *flow, void *meta,
+		 struct rte_flow_error *error)
 {
-	struct iavf_rss_meta *rss_meta = (struct iavf_rss_meta *)meta;
-	struct virtchnl_rss_cfg *rss_cfg;
+	struct virtchnl_rss_cfg *rss_cfg = meta;
 	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->rss_algorithm;
-
 	ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
 	if (!ret) {
 		flow->rule = rss_cfg;
@@ -1613,12 +1590,9 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to add rss rule.");
-		rte_free(rss_cfg);
 		return -rte_errno;
 	}
 
-	rte_free(meta);
-
 	return ret;
 }
 
@@ -1672,9 +1646,3 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 }
-
-static void
-iavf_hash_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-}
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index b481330346..e9acd1bb1d 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1919,7 +1919,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 
 	iavf_ipsec_crypto_security_policy_delete(ad,
 			ipsec_flow->is_ipv4, ipsec_flow->id);
-	rte_free(ipsec_flow);
 	return 0;
 }
 
-- 
2.52.0

[PATCH v2 8/9] net/iavf: fix null engine for egress security flows

[Anatoly Burakov]

Egress security flows bypass the flow parsers and are inserted with a
NULL `engine` pointer. The create path then dereferences that pointer
when logging the new flow, and other engine-based operations cannot
handle the flow consistently.

Register an egress security engine and route these flows through the
normal parser and lifecycle callbacks. Egress flows own no rule and
require no hardware teardown.

Fixes: e783da175250 ("net/iavf: fix memory leak on egress IPsec flows")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c |  44 +++-----
 drivers/net/intel/iavf/iavf_generic_flow.h |   2 +-
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 120 +++++++++++++++++++--
 3 files changed, 123 insertions(+), 43 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 1593ea297c..be7ddf5db6 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1914,7 +1914,8 @@ iavf_register_parser(struct iavf_flow_parser *parser,
 			}
 		}
 		TAILQ_INSERT_HEAD(list, parser_node, node);
-	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO) {
+	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		   parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS) {
 		list = &vf->ipsec_crypto_parser_list;
 		TAILQ_INSERT_HEAD(list, parser_node, node);
 	} else if (parser->engine->type == IAVF_FLOW_ENGINE_FSUB) {
@@ -1943,7 +1944,8 @@ iavf_unregister_parser(struct iavf_flow_parser *parser,
 	else if ((parser->engine->type == IAVF_FLOW_ENGINE_FDIR) ||
 		 (parser->engine->type == IAVF_FLOW_ENGINE_FSUB))
 		list = &vf->dist_parser_list;
-	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO)
+	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		 parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS)
 		list = &vf->ipsec_crypto_parser_list;
 
 	if (list == NULL)
@@ -2250,12 +2252,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 		return flow;
 	}
 
-	/* Special case for inline crypto egress flows */
-	if (attr->egress && actions[0].type == RTE_FLOW_ACTION_TYPE_SECURITY) {
-		flow->is_ipsec_egress_flow = true;
-		goto tailq_insert;
-	}
-
 	ret = iavf_flow_process_filter(dev, flow, attr, pattern, actions,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
@@ -2266,7 +2262,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 	}
 
 	flow->engine = engine;
-tailq_insert:
 	rte_spinlock_lock(&vf->flow_ops_lock);
 	TAILQ_INSERT_TAIL(&vf->flow_list, flow, node);
 	rte_spinlock_unlock(&vf->flow_ops_lock);
@@ -2282,14 +2277,7 @@ iavf_flow_is_valid(struct rte_flow *flow)
 	struct iavf_flow_engine *engine;
 	void *temp;
 
-	if (flow == NULL)
-		return false;
-
-	/* these flows don't use engines */
-	if (flow->is_ipsec_egress_flow)
-		return true;
-
-	if (flow->engine) {
+	if (flow != NULL && flow->engine != NULL) {
 		RTE_TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
 			if (engine == flow->engine)
 				return true;
@@ -2307,26 +2295,17 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 	struct iavf_adapter *ad =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	bool need_destroy;
 	int ret = 0;
 
-	if (!iavf_flow_is_valid(flow)) {
+	if (!iavf_flow_is_valid(flow) || flow->engine->destroy == NULL) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Invalid flow destroy");
 		return -rte_errno;
 	}
-	need_destroy = !flow->is_ipsec_egress_flow;
-
-	if (need_destroy && flow->engine->destroy == NULL) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Invalid flow destroy");
-	}
 
 	rte_spinlock_lock(&vf->flow_ops_lock);
-	if (need_destroy)
-		ret = flow->engine->destroy(ad, flow, error);
+	ret = flow->engine->destroy(ad, flow, error);
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
@@ -2444,16 +2423,17 @@ iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		if (flow != NULL && f != flow)
 			continue;
 
-		/* this should not happen */
-		if (f->engine == NULL || f->rule == NULL) {
+		if (!iavf_flow_is_valid(f)) {
 			PMD_DRV_LOG(DEBUG, "Invalid flow");
 			continue;
 		}
 
 		found = true;
 
-		rule_size = f->engine->rule_size;
-		rule_data = f->rule;
+		if (f->rule != NULL) {
+			rule_size = f->engine->rule_size;
+			rule_data = f->rule;
+		}
 
 		iavf_flow_dump_blob(file,
 			f->engine->name != NULL ?
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 3a8f5f92e8..75bb92900b 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -481,6 +481,7 @@ enum iavf_flow_engine_type {
 	IAVF_FLOW_ENGINE_FDIR,
 	IAVF_FLOW_ENGINE_HASH,
 	IAVF_FLOW_ENGINE_FSUB,
+	IAVF_FLOW_ENGINE_IPSEC_EGRESS,
 	IAVF_FLOW_ENGINE_MAX,
 };
 
@@ -503,7 +504,6 @@ TAILQ_HEAD(iavf_engine_list, iavf_flow_engine);
 /* Struct to store flow created. */
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) node;
-	bool is_ipsec_egress_flow;
 	struct iavf_flow_engine *engine;
 	void *rule;
 };
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index e9acd1bb1d..f7e391a74a 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1832,6 +1832,7 @@ iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 
 
 static struct iavf_flow_parser iavf_ipsec_flow_parser;
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser;
 
 static int
 iavf_ipsec_flow_init(struct iavf_adapter *ad)
@@ -1922,16 +1923,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 	return 0;
 }
 
-static struct iavf_flow_engine iavf_ipsec_flow_engine = {
-	.init = iavf_ipsec_flow_init,
-	.uninit = iavf_ipsec_flow_uninit,
-	.create = iavf_ipsec_flow_create,
-	.destroy = iavf_ipsec_flow_destroy,
-	.name = "ipsec_crypto",
-	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
-	.rule_size = sizeof(struct iavf_ipsec_flow_item),
-};
-
 static int
 iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		      struct iavf_pattern_match_item *array,
@@ -1975,6 +1966,105 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 	return ret;
 }
 
+/*
+ * Inline crypto egress flows only associate Tx traffic with a security
+ * session; nothing is programmed in hardware, so the engine owns no rule.
+ */
+static int
+iavf_ipsec_egress_flow_init(struct iavf_adapter *ad)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+
+	if (!vf->vf_res)
+		return -EINVAL;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_INLINE_IPSEC_CRYPTO))
+		return -ENOTSUP;
+
+	return iavf_register_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static void
+iavf_ipsec_egress_flow_uninit(struct iavf_adapter *ad)
+{
+	iavf_unregister_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static int
+iavf_ipsec_egress_flow_create(__rte_unused struct iavf_adapter *ad,
+		struct rte_flow *flow,
+		__rte_unused void *meta,
+		__rte_unused struct rte_flow_error *error)
+{
+	flow->rule = NULL;
+	return 0;
+}
+
+static int
+iavf_ipsec_egress_flow_destroy(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct rte_flow *flow,
+		__rte_unused struct rte_flow_error *error)
+{
+	return 0;
+}
+
+/* The pattern is not used, as there is nothing to match in hardware. */
+static int
+iavf_ipsec_egress_flow_parse(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct iavf_pattern_match_item *array,
+		__rte_unused uint32_t array_len,
+		__rte_unused const struct rte_flow_item pattern[],
+		const struct rte_flow_action actions[],
+		const struct rte_flow_attr *attr,
+		void **meta,
+		struct rte_flow_error *error)
+{
+	struct ci_flow_actions parsed_actions = {0};
+	struct ci_flow_actions_check_param param = {
+		.allowed_types = (enum rte_flow_action_type[]){
+			RTE_FLOW_ACTION_TYPE_SECURITY,
+			RTE_FLOW_ACTION_TYPE_END,
+		},
+		.max_actions = 1,
+	};
+	struct ci_flow_attr_check_param attr_param = {
+		.require_egress = true,
+	};
+	int ret;
+
+	ret = ci_flow_check_attr(attr, &attr_param, error);
+	if (ret)
+		return ret;
+
+	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
+	if (ret < 0)
+		return ret;
+
+	if (meta != NULL)
+		*meta = NULL;
+
+	return 0;
+}
+
+static struct iavf_flow_engine iavf_ipsec_flow_engine = {
+	.init = iavf_ipsec_flow_init,
+	.uninit = iavf_ipsec_flow_uninit,
+	.create = iavf_ipsec_flow_create,
+	.destroy = iavf_ipsec_flow_destroy,
+	.name = "ipsec_crypto",
+	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
+	.rule_size = sizeof(struct iavf_ipsec_flow_item),
+};
+
+static struct iavf_flow_engine iavf_ipsec_egress_flow_engine = {
+	.init = iavf_ipsec_egress_flow_init,
+	.uninit = iavf_ipsec_egress_flow_uninit,
+	.create = iavf_ipsec_egress_flow_create,
+	.destroy = iavf_ipsec_egress_flow_destroy,
+	.name = "ipsec_egress",
+	.type = IAVF_FLOW_ENGINE_IPSEC_EGRESS,
+};
+
 static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.engine = &iavf_ipsec_flow_engine,
 	.array = iavf_ipsec_flow_pattern,
@@ -1982,7 +2072,17 @@ static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.parse_pattern_action = iavf_ipsec_flow_parse,
 };
 
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser = {
+	.engine = &iavf_ipsec_egress_flow_engine,
+	.parse_pattern_action = iavf_ipsec_egress_flow_parse,
+};
+
 RTE_INIT(iavf_ipsec_flow_engine_register)
 {
 	iavf_register_flow_engine(&iavf_ipsec_flow_engine);
 }
+
+RTE_INIT(iavf_ipsec_egress_flow_engine_register)
+{
+	iavf_register_flow_engine(&iavf_ipsec_egress_flow_engine);
+}
-- 
2.52.0

[PATCH v2 9/9] net/iavf: fix IPsec flow metadata leaks

[Anatoly Burakov]

IPsec flow parsing leaves the pattern-match item allocated after
extracting its metadata. It also leaves the parsed flow item allocated
when no metadata output is requested.

Free the pattern-match item after parsing and free the parsed flow item
when it cannot be returned to the caller. Propagate pattern-match failure
and report a parsing error when no flow item was produced.

Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 28 +++++++++++++++-------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index f7e391a74a..be0a0d0df6 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1942,6 +1942,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		.max_actions = 1,
 	};
 	struct iavf_pattern_match_item *item = NULL;
+	struct iavf_ipsec_flow_item *fi = NULL;
 	int ret = -1;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
@@ -1953,17 +1954,28 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		return ret;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (item && item->meta) {
+	if (item == NULL)
+		return -rte_errno;
+
+	if (item->meta != NULL) {
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
-		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad, pattern, session, type);
-		if (fi && meta) {
-			*meta = fi;
-			ret = 0;
-		}
+
+		fi = iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 	}
-	return ret;
+	rte_free(item);
+
+	if (fi == NULL)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+				"Invalid IPsec flow.");
+
+	if (meta != NULL)
+		*meta = fi;
+	else
+		rte_free(fi);
+
+	return 0;
 }
 
 /*
-- 
2.52.0

[PATCH v3 00/10] IAVF Fixes

[Anatoly Burakov]

An assortment of IAVF flow-related memory leak fixes and cleanups.

v2 -> v3:
- Rebase on top of latest next-net-intel including Anurag's fix [1]
- Dropped remnants of Anurag's patch [1] as it's now in the tree
- Fixed PF reset path still tearing down RSS when it shouldn't
- Fixed hash engine checking for VF reset when that's no longer necessary

v1 -> v2:
- Integrated Anurag's earlier fix [1] into the patchset
- Reworked generic flow to have centralized memory management
- Reordered some patches
- Integrated the IRQ leaks patchset [2] [3]
- Reworked IPsec fix

[1] https://patches.dpdk.org/project/dpdk/patch/20261010121255.528329-1-anurag.mandal@intel.com/
[2] https://patches.dpdk.org/project/dpdk/patch/809e0c5b2ab57f1b14c68be8ac34e70cbbe82233.1792053236.git.anatoly.burakov@intel.com/
[3] https://patches.dpdk.org/project/dpdk/patch/5488d85c2ded08838becaee6de8328694ee00234.1792053236.git.anatoly.burakov@intel.com/

Anatoly Burakov (10):
  net/iavf: keep device stopped if start fails
  net/iavf: do not store ethdev pointer
  net/iavf: fix false success on flow validation
  net/iavf: fix memory leaks around IRQ resources
  net/iavf: fix generic flow error-path leaks
  net/iavf: centralize memory management for flows
  net/iavf: skip RSS teardown during PF-initiated reset
  net/iavf: remove redundant reset checks from RSS engine
  net/iavf: fix null engine for egress security flows
  net/iavf: fix IPsec flow metadata leaks

 drivers/net/intel/iavf/iavf.h              |  14 +-
 drivers/net/intel/iavf/iavf_ethdev.c       |  50 +++---
 drivers/net/intel/iavf/iavf_fdir.c         |  68 ++++----
 drivers/net/intel/iavf/iavf_fsub.c         |  23 +--
 drivers/net/intel/iavf/iavf_generic_flow.c |  95 ++++++------
 drivers/net/intel/iavf/iavf_generic_flow.h |   4 +-
 drivers/net/intel/iavf/iavf_hash.c         | 103 ++++---------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 171 +++++++++++++++++----
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |   4 +-
 drivers/net/intel/iavf/iavf_vchnl.c        |   7 +-
 10 files changed, 303 insertions(+), 236 deletions(-)

-- 
2.52.0

[PATCH v3 01/10] net/iavf: keep device stopped if start fails

[Anatoly Burakov]

Currently, `iavf_dev_start()` clears `adapter->stopped` before several
operations that can fail. A subsequent `iavf_dev_stop()` then disables
interrupts and stops and resets queues even though startup did not
complete.

Mark the adapter running only after startup succeeds, and return early
from stop when it is already stopped.

Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 7f1ad3d88c..68aced0e31 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1039,8 +1039,6 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	if (adapter->closed)
 		return -1;
 
-	adapter->stopped = 0;
-
 	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
 	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				      dev->data->nb_tx_queues);
@@ -1112,6 +1110,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 
 	iavf_phc_sync_alarm_start(dev);
+	adapter->stopped = 0;
 
 	/* An explicit start supersedes any pending deferred start */
 	vf->start_pending = false;
@@ -1135,13 +1134,13 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 	if (adapter->closed)
 		return -1;
 
+	if (adapter->stopped == 1)
+		return 0;
+
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) &&
 	    dev->data->dev_conf.intr_conf.rxq != 0)
 		rte_intr_disable(intr_handle);
 
-	if (adapter->stopped == 1)
-		return 0;
-
 	iavf_phc_sync_alarm_stop(dev);
 
 	/* Disable the interrupt for Rx */
-- 
2.52.0

[PATCH v3 02/10] net/iavf: do not store ethdev pointer

[Anatoly Burakov]

We should not be storing rte_eth_dev pointers in shared memory, because
rte_eth_dev pointers are process local. To address that, remove the pointer
from iavf_info, replace it with `rte_eth_devices`-based lookup, and rework
the call paths to either use that lookup, or not require ethdev pointer
in the first place.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  9 ++++++--
 drivers/net/intel/iavf/iavf_ethdev.c       | 13 ++++++------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 24 +++++++++++-----------
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |  4 ++--
 drivers/net/intel/iavf/iavf_vchnl.c        |  7 ++++---
 5 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index c40b97444b..1f277eeee2 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -7,6 +7,7 @@
 
 #include <sys/queue.h>
 
+#include <ethdev_driver.h>
 #include <rte_kvargs.h>
 #include <rte_tm_driver.h>
 
@@ -293,8 +294,6 @@ struct iavf_info {
 	struct iavf_qtc_map *qtc_map;
 	struct iavf_tm_conf tm_conf;
 
-	struct rte_eth_dev *eth_dev;
-
 	RTE_ATOMIC(bool)in_reset_recovery;
 	bool reset_pending;
 	bool pf_reset_in_progress;
@@ -408,6 +407,12 @@ struct iavf_adapter {
 	uint16_t tpid;  /* VLAN tag identifier */
 };
 
+static inline struct rte_eth_dev *
+iavf_get_eth_dev(struct iavf_adapter *adapter)
+{
+	return &rte_eth_devices[adapter->dev_data->port_id];
+}
+
 /* IAVF_DEV_PRIVATE_TO */
 #define IAVF_DEV_PRIVATE_TO_ADAPTER(adapter) \
 	((struct iavf_adapter *)adapter)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 68aced0e31..befee1d3eb 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -326,6 +326,7 @@ static void
 iavf_dev_watchdog(void *cb_arg)
 {
 	struct iavf_adapter *adapter = cb_arg;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
 	int vfr_inprogress = 0, rc = 0;
 
@@ -339,7 +340,7 @@ iavf_dev_watchdog(void *cb_arg)
 
 		if (!vfr_inprogress) {
 			PMD_DRV_LOG(INFO, "VF \"%s\" reset has completed",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 			adapter->vf.vf_reset = false;
 			iavf_set_no_poll(adapter, false);
 		}
@@ -350,14 +351,14 @@ iavf_dev_watchdog(void *cb_arg)
 		if (vfr_inprogress) {
 			PMD_DRV_LOG(INFO,
 				"VF \"%s\" reset event detected by watchdog",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
 			iavf_set_no_poll(adapter, false);
 			adapter->vf.link_up = false;
 
-			iavf_dev_event_post(adapter->vf.eth_dev, RTE_ETH_EVENT_INTR_RESET,
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
 				NULL, 0);
 		}
 	}
@@ -365,11 +366,11 @@ iavf_dev_watchdog(void *cb_arg)
 	if (adapter->devargs.watchdog_period) {
 		/* re-alarm watchdog */
 		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
-					&iavf_dev_watchdog, cb_arg);
+					&iavf_dev_watchdog, dev);
 
 		if (rc)
 			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 	}
 }
 
@@ -2669,8 +2670,6 @@ iavf_init_vf(struct rte_eth_dev *dev)
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 
-	vf->eth_dev = dev;
-
 	err = iavf_parse_devargs(dev);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Failed to parse devargs");
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index 06b51b5e62..b481330346 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -689,11 +689,9 @@ iavf_ipsec_crypto_session_create(void *device,
  * returns: 1 if valid
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(ethdev->data->dev_private);
 	const struct iavf_security_session *sess = (const void *)session->driver_priv_data;
 
 	/* verify we have a valid session and that it belong to this adapter */
@@ -1502,24 +1500,25 @@ static struct rte_security_ops iavf_ipsec_crypto_ops = {
 int
 iavf_security_ctx_create(struct iavf_adapter *adapter)
 {
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct rte_security_ctx *sctx;
 
 	sctx = rte_malloc("security_ctx", sizeof(struct rte_security_ctx), 0);
 	if (sctx == NULL)
 		return -ENOMEM;
 
-	sctx->device = adapter->vf.eth_dev;
+	sctx->device = dev;
 	sctx->ops = &iavf_ipsec_crypto_ops;
 	sctx->sess_cnt = 0;
 
-	adapter->vf.eth_dev->security_ctx = sctx;
+	dev->security_ctx = sctx;
 
 	if (adapter->security_ctx == NULL) {
 		adapter->security_ctx = rte_malloc("iavf_security_ctx",
 				sizeof(struct iavf_security_ctx), 0);
 		if (adapter->security_ctx == NULL) {
-			rte_free(adapter->vf.eth_dev->security_ctx);
-			adapter->vf.eth_dev->security_ctx = NULL;
+			rte_free(dev->security_ctx);
+			dev->security_ctx = NULL;
 			return -ENOMEM;
 		}
 	}
@@ -1565,7 +1564,8 @@ iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter)
 int
 iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 {
-	struct rte_security_ctx *sctx  = adapter->vf.eth_dev->security_ctx;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
+	struct rte_security_ctx *sctx  = dev->security_ctx;
 	struct iavf_security_ctx *iavf_sctx = adapter->security_ctx;
 
 	if (iavf_sctx == NULL)
@@ -1578,7 +1578,7 @@ iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 	rte_free(sctx);
 
 	adapter->security_ctx = NULL;
-	adapter->vf.eth_dev->security_ctx = NULL;
+	dev->security_ctx = NULL;
 
 	return 0;
 }
@@ -1748,7 +1748,7 @@ parse_udp_item(const struct rte_flow_item_udp *item, struct rte_udp_hdr *udp)
 }
 
 static struct iavf_ipsec_flow_item *
-iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
+iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 		const struct rte_flow_item pattern[],
 		const struct rte_security_session *session,
 		uint32_t type)
@@ -1819,7 +1819,7 @@ iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
 		goto flow_cleanup;
 	}
 
-	if (!iavf_ipsec_crypto_action_valid(ethdev, session,
+	if (!iavf_ipsec_crypto_action_valid(adapter, session,
 			ipsec_flow->spi))
 		goto flow_cleanup;
 
@@ -1967,7 +1967,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
 		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad->vf.eth_dev, pattern, session, type);
+				iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 		if (fi && meta) {
 			*meta = fi;
 			ret = 0;
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.h b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
index 795b75a624..3d4b1da127 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.h
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
@@ -127,13 +127,13 @@ int iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter);
 /**
  * Destroy security context
  */
-int iavf_security_ctx_destroy(struct iavf_adapter *adapterv);
+int iavf_security_ctx_destroy(struct iavf_adapter *adapter);
 
 /**
  * Verify that the inline IPsec Crypto action is valid for this device
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi);
 
 /**
diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 57488512f4..26dae423a4 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -314,13 +314,14 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 		    opcode, vf->cmd_retval);
 
 	if (opcode == VIRTCHNL_OP_EVENT) {
+		struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 		struct virtchnl_pf_event *vpe =
 			(struct virtchnl_pf_event *)event.msg_buf;
 
 		result = IAVF_MSG_SYS;
 		switch (vpe->event) {
 		case VIRTCHNL_EVENT_LINK_CHANGE:
-			iavf_handle_link_change_event(vf->eth_dev, vpe);
+			iavf_handle_link_change_event(dev, vpe);
 			break;
 		case VIRTCHNL_EVENT_RESET_IMPENDING:
 			/*
@@ -335,8 +336,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				iavf_set_no_poll(adapter, false);
 				iavf_dev_watchdog_enable(adapter);
 				if (adapter->devargs.no_poll_on_link_down)
-					iavf_dev_tx_drain(vf->eth_dev);
-				iavf_dev_event_post(vf->eth_dev,
+					iavf_dev_tx_drain(dev);
+				iavf_dev_event_post(dev,
 					RTE_ETH_EVENT_INTR_RESET,
 					NULL, 0);
 			}
-- 
2.52.0

[PATCH v3 03/10] net/iavf: fix false success on flow validation

[Anatoly Burakov]

`iavf_parse_engine_validate()` sets an error when validation fails, but
then returns the engine pointer. `iavf_flow_process_filter()` treats a
non-NULL engine as success, so `rte_flow_validate()` can accept an
invalid flow.

Return `NULL` on validation failure and return the engine only after
validation succeeds.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 92ca20031c..14242e6ebb 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -2134,10 +2134,11 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation failed");
-			break;
+			return NULL;
 		}
+		return engine;
 	}
-	return engine;
+	return NULL;
 }
 
 
-- 
2.52.0

[PATCH v3 04/10] net/iavf: fix memory leaks around IRQ resources

[Anatoly Burakov]

`iavf_config_rx_queues_irqs()` allocates a queue-vector map and may
enable eventfds and allocate an interrupt vector list. If queue startup
then fails, those resources remain allocated. The queue-vector map is
also retained after a normal stop, leaking it across start/stop cycles.

Release the resources on failed startup and free the queue-vector map
on stop. Clean up eventfds when interrupt setup fails as well.

Fixes: f593944fc988 ("net/iavf: enable IRQ mapping configuration for large VF")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index befee1d3eb..bb97a89414 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -863,7 +863,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 	if (rte_intr_cap_multiple(intr_handle) &&
 	    dev->data->dev_conf.intr_conf.rxq) {
 		if (rte_intr_efd_enable(intr_handle, dev->data->nb_rx_queues))
-			return -1;
+			goto efd_alloc_err;
 	}
 
 	if (rte_intr_dp_is_en(intr_handle)) {
@@ -871,7 +871,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 						    dev->data->nb_rx_queues)) {
 			PMD_DRV_LOG(ERR, "Failed to allocate %d rx intr_vec",
 				    dev->data->nb_rx_queues);
-			return -1;
+			goto qv_map_alloc_err;
 		}
 	}
 
@@ -981,6 +981,8 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 
 qv_map_alloc_err:
 	rte_intr_vec_list_free(intr_handle);
+efd_alloc_err:
+	rte_intr_efd_disable(intr_handle);
 
 	return -1;
 }
@@ -1034,12 +1036,15 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	struct rte_intr_handle *intr_handle = dev->intr_handle;
 	uint16_t num_queue_pairs;
 	uint16_t index = 0;
+	bool wb_on_itr;
 
 	PMD_INIT_FUNC_TRACE();
 
 	if (adapter->closed)
 		return -1;
 
+	wb_on_itr = (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) != 0;
+
 	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
 	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				      dev->data->nb_tx_queues);
@@ -1089,7 +1094,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 	/* re-enable intr again, because efd assign may change */
 	if (dev->data->dev_conf.intr_conf.rxq != 0) {
-		if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
+		if (wb_on_itr)
 			rte_intr_disable(intr_handle);
 		rte_intr_enable(intr_handle);
 	}
@@ -1107,7 +1112,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 
 	if (iavf_start_queues(dev) != 0) {
 		PMD_DRV_LOG(ERR, "enable queues failed");
-		goto error;
+		goto irq_error;
 	}
 
 	iavf_phc_sync_alarm_start(dev);
@@ -1118,6 +1123,13 @@ iavf_dev_start(struct rte_eth_dev *dev)
 
 	return 0;
 
+irq_error:
+	if (!wb_on_itr && dev->data->dev_conf.intr_conf.rxq != 0)
+		rte_intr_disable(intr_handle);
+	rte_intr_efd_disable(intr_handle);
+	rte_intr_vec_list_free(intr_handle);
+	rte_free(vf->qv_map);
+	vf->qv_map = NULL;
 error:
 	return -1;
 }
@@ -1150,6 +1162,8 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 	rte_intr_vec_list_free(intr_handle);
 
 	iavf_stop_queues(dev);
+	rte_free(vf->qv_map);
+	vf->qv_map = NULL;
 
 	adapter->stopped = 1;
 	dev->data->dev_started = 0;
-- 
2.52.0

[PATCH v3 05/10] net/iavf: fix generic flow error-path leaks

[Anatoly Burakov]

If a flow engine fails to initialize, parsers registered by earlier
engines remain allocated. Pattern matching also leaks its item array
when allocation of the match result fails.

Unwind registered parsers on flow initialization failure and free the
item array on match-result allocation failure.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 14242e6ebb..cd4d4a012b 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1821,17 +1821,22 @@ iavf_flow_init(struct iavf_adapter *ad)
 		if (engine->init == NULL) {
 			PMD_INIT_LOG(ERR, "Invalid engine type (%d)",
 				     engine->type);
-			return -ENOTSUP;
+			ret = -ENOTSUP;
+			goto err;
 		}
 
 		ret = engine->init(ad);
 		if (ret && ret != -ENOTSUP) {
 			PMD_INIT_LOG(ERR, "Failed to initialize engine %d",
 				     engine->type);
-			return ret;
+			goto err;
 		}
 	}
 	return 0;
+
+err:
+	iavf_flow_uninit(ad);
+	return ret;
 }
 
 void
@@ -2047,6 +2052,7 @@ iavf_search_pattern_match_item(const struct rte_flow_item pattern[],
 	if (!pattern_match_item) {
 		rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Failed to allocate memory.");
+		rte_free(items);
 		return NULL;
 	}
 
-- 
2.52.0

[PATCH v3 06/10] net/iavf: centralize memory management for flows

[Anatoly Burakov]

Flow engines currently distribute rule allocation and cleanup between the
parser, engine create and destroy callbacks, and generic flow code. This
leads to a lot of potential for memory leaks and bugs. In particular, one
of the issues currently present is that when PF initiates reset, we are
meant to just drop all flows without sending any virtchnl commands, but
because not all engines implement the `free` callback, some flows will be
leaked.

To address that specific leak as well as all other potential leaks due to
misuse of memory by engines, use parser-allocated rule objects as the
persistent rules and centralize their lifetime in the generic flow layer.
This requires fixing all engines to not manage their own memory:

- Allocate the final FDIR rule in the parser, use it for action checks,
  and remove the per-VF scratch configuration.
- Have FSUB create adopt its parser-produced rule instead of allocating
  and copying a second object.
- Build the final RSS configuration in the parser and remove the
  intermediate RSS metadata, including its unused raw-pattern flag.
- Let generic flow code free rules on validate, create failure,
  successful destroy, and uninit; remove engine-specific free callbacks
  and rule frees from destroy callbacks.

Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  5 --
 drivers/net/intel/iavf/iavf_fdir.c         | 68 +++++++--------
 drivers/net/intel/iavf/iavf_fsub.c         | 23 +----
 drivers/net/intel/iavf/iavf_generic_flow.c | 36 ++++++--
 drivers/net/intel/iavf/iavf_generic_flow.h |  2 -
 drivers/net/intel/iavf/iavf_hash.c         | 99 +++++++---------------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c |  1 -
 7 files changed, 97 insertions(+), 137 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index 1f277eeee2..8b826662c6 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -155,10 +155,6 @@ struct iavf_fdir_conf {
 	uint32_t mark_flag;
 };
 
-struct iavf_fdir_info {
-	struct iavf_fdir_conf conf;
-};
-
 struct iavf_fsub_conf {
 	struct virtchnl_flow_sub sub_fltr;
 	struct virtchnl_flow_unsub unsub_fltr;
@@ -286,7 +282,6 @@ struct iavf_info {
 	struct iavf_parser_list dist_parser_list;
 	struct iavf_parser_list ipsec_crypto_parser_list;
 
-	struct iavf_fdir_info fdir; /* flow director info */
 	/* indicate large VF support enabled or not */
 	bool lv_enabled;
 
diff --git a/drivers/net/intel/iavf/iavf_fdir.c b/drivers/net/intel/iavf/iavf_fdir.c
index 67003183bb..7b9d3aac7f 100644
--- a/drivers/net/intel/iavf/iavf_fdir.c
+++ b/drivers/net/intel/iavf/iavf_fdir.c
@@ -353,36 +353,20 @@ iavf_fdir_create(struct iavf_adapter *ad,
 		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 for fdir rule");
-		return -rte_errno;
-	}
-
 	ret = iavf_fdir_add(ad, filter);
 	if (ret) {
-		rte_flow_error_set(error, -ret,
+		return rte_flow_error_set(error, -ret,
 				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				"Failed to add filter rule.");
-		goto free_entry;
 	}
 
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 1);
 
-	memcpy(rule, filter, sizeof(*rule));
-	flow->rule = rule;
-
+	flow->rule = filter;
 	return 0;
-
-free_entry:
-	rte_free(rule);
-	return -rte_errno;
 }
 
 static int
@@ -406,9 +390,6 @@ iavf_fdir_destroy(struct iavf_adapter *ad,
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 0);
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return 0;
 }
 
@@ -1444,14 +1425,19 @@ iavf_fdir_action_check_qregion(struct iavf_adapter *ad,
 	return 0;
 }
 
+struct iavf_fdir_parse_ctx {
+	struct iavf_adapter *ad;
+	struct iavf_fdir_conf *filter;
+};
+
 static int
 iavf_fdir_parse_action_check(const struct ci_flow_actions *actions,
 		const struct ci_flow_actions_check_param *param,
 		struct rte_flow_error *error)
 {
-	struct iavf_adapter *ad = param->driver_ctx;
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	struct iavf_fdir_conf *filter = &vf->fdir.conf;
+	struct iavf_fdir_parse_ctx *ctx = param->driver_ctx;
+	struct iavf_adapter *ad = ctx->ad;
+	struct iavf_fdir_conf *filter = ctx->filter;
 	uint32_t dest_num = 0, mark_num = 0;
 	size_t i, number = 0;
 	bool has_drop = false;
@@ -1557,6 +1543,8 @@ iavf_fdir_parse(struct iavf_adapter *ad,
 		struct rte_flow_error *error)
 {
 	struct ci_flow_actions parsed_actions = {0};
+	struct iavf_fdir_conf *filter;
+	struct iavf_fdir_parse_ctx ctx;
 	struct ci_flow_actions_check_param param = {
 		.allowed_types = (enum rte_flow_action_type[]){
 			RTE_FLOW_ACTION_TYPE_PASSTHRU,
@@ -1568,38 +1556,52 @@ iavf_fdir_parse(struct iavf_adapter *ad,
 		},
 		.max_actions = 2,
 		.check = iavf_fdir_parse_action_check,
-		.driver_ctx = ad,
 		.rss_queues_contig = true,
 	};
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	struct iavf_fdir_conf *filter = &vf->fdir.conf;
 	struct iavf_pattern_match_item *item = NULL;
 	int ret;
 
-	memset(filter, 0, sizeof(*filter));
+	filter = rte_zmalloc("iavf_fdir_meta", sizeof(*filter), 0);
+	if (filter == NULL) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Failed to allocate FDIR metadata.");
+		return -rte_errno;
+	}
+
+	ctx.ad = ad;
+	ctx.filter = filter;
+	param.driver_ctx = &ctx;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (!item)
-		return -rte_errno;
+	if (item == NULL) {
+		ret = -rte_errno;
+		goto error;
+	}
 
 	ret = iavf_fdir_parse_pattern(ad, pattern, item->input_set_mask,
 				      error, filter);
 	if (ret)
 		goto error;
 
-	if (meta)
+	rte_free(item);
+	if (meta != NULL)
 		*meta = filter;
+	else
+		rte_free(filter);
+	return ret;
 
 error:
 	rte_free(item);
+	rte_free(filter);
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_fsub.c b/drivers/net/intel/iavf/iavf_fsub.c
index 19ae0e2dc2..e4e0e20c4f 100644
--- a/drivers/net/intel/iavf/iavf_fsub.c
+++ b/drivers/net/intel/iavf/iavf_fsub.c
@@ -73,34 +73,18 @@ iavf_fsub_create(struct iavf_adapter *ad, struct rte_flow *flow,
 		 void *meta, struct rte_flow_error *error)
 {
 	struct iavf_fsub_conf *filter = meta;
-	struct iavf_fsub_conf *rule;
 	int ret;
 
-	rule = rte_zmalloc("fsub_entry", sizeof(*rule), 0);
-	if (!rule) {
-		rte_flow_error_set(error, ENOMEM,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Failed to allocate memory for fsub rule");
-		return -rte_errno;
-	}
-
 	ret = iavf_flow_sub(ad, filter);
 	if (ret) {
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to subscribe flow rule.");
-		goto free_entry;
+		return -rte_errno;
 	}
 
-	memcpy(rule, filter, sizeof(*rule));
-	flow->rule = rule;
-
-	rte_free(meta);
+	flow->rule = filter;
 	return ret;
-
-free_entry:
-	rte_free(rule);
-	return -rte_errno;
 }
 
 static int
@@ -120,9 +104,6 @@ iavf_fsub_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		return -rte_errno;
 	}
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index cd4d4a012b..1593ea297c 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -46,6 +46,13 @@ static int iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		FILE *file,
 		struct rte_flow_error *error);
 
+static void
+iavf_flow_free(struct rte_flow *flow)
+{
+	rte_free(flow->rule);
+	rte_free(flow);
+}
+
 const struct rte_flow_ops iavf_flow_ops = {
 	.validate = iavf_flow_validate,
 	.create = iavf_flow_create,
@@ -1856,9 +1863,7 @@ iavf_flow_uninit(struct iavf_adapter *ad)
 	/* Remove all flows */
 	while ((p_flow = TAILQ_FIRST(&vf->flow_list))) {
 		TAILQ_REMOVE(&vf->flow_list, p_flow, node);
-		if (p_flow->engine->free)
-			p_flow->engine->free(p_flow);
-		rte_free(p_flow);
+		iavf_flow_free(p_flow);
 	}
 
 	/* Cleanup parser list */
@@ -2092,6 +2097,7 @@ iavf_parse_engine_create(struct iavf_adapter *ad,
 	void *meta = NULL;
 
 	RTE_TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
+		meta = NULL;
 		if (parser_node->parser->parse_pattern_action(ad,
 				parser_node->parser->array,
 				parser_node->parser->array_len,
@@ -2101,8 +2107,17 @@ iavf_parse_engine_create(struct iavf_adapter *ad,
 		engine = parser_node->parser->engine;
 
 		RTE_ASSERT(engine->create != NULL);
-		if (!(engine->create(ad, flow, meta, error)))
+		if (engine->create(ad, flow, meta, error) == 0) {
+			if (flow->rule != meta)
+				rte_free(meta);
 			return engine;
+		}
+
+		if (flow->rule != NULL && flow->rule != meta)
+			rte_free(flow->rule);
+		rte_free(meta);
+		flow->rule = NULL;
+		meta = NULL;
 	}
 	return NULL;
 }
@@ -2120,8 +2135,10 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 	struct iavf_flow_parser_node *parser_node;
 	void *temp;
 	void *meta = NULL;
+	int ret;
 
 	RTE_TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
+		meta = NULL;
 		if (parser_node->parser->parse_pattern_action(ad,
 				parser_node->parser->array,
 				parser_node->parser->array_len,
@@ -2130,13 +2147,18 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 
 		engine = parser_node->parser->engine;
 		if (engine->validation == NULL) {
+			rte_free(meta);
+			meta = NULL;
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation not support");
 			continue;
 		}
 
-		if (engine->validation(ad, flow, meta, error)) {
+		ret = engine->validation(ad, flow, meta, error);
+		rte_free(meta);
+		meta = NULL;
+		if (ret != 0) {
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation failed");
@@ -2238,7 +2260,7 @@ iavf_flow_create(struct rte_eth_dev *dev,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "Failed to create flow");
-		rte_free(flow);
+		iavf_flow_free(flow);
 		flow = NULL;
 		goto free_flow;
 	}
@@ -2308,7 +2330,7 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
-		rte_free(flow);
+		iavf_flow_free(flow);
 	} else {
 		PMD_DRV_LOG(ERR, "Failed to destroy flow");
 	}
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 6665d68818..3a8f5f92e8 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -465,7 +465,6 @@ typedef int (*engine_query_t)(struct iavf_adapter *ad,
 		struct rte_flow *flow,
 		struct rte_flow_query_count *count,
 		struct rte_flow_error *error);
-typedef void (*engine_free_t) (struct rte_flow *flow);
 typedef int (*parse_pattern_action_t)(struct iavf_adapter *ad,
 		struct iavf_pattern_match_item *array,
 		uint32_t array_len,
@@ -494,7 +493,6 @@ struct iavf_flow_engine {
 	engine_create_t create;
 	engine_destroy_t destroy;
 	engine_query_t query_count;
-	engine_free_t free;
 	const char *name;
 	enum iavf_flow_engine_type type;
 	size_t rule_size;
diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index 00b616f34b..6352bf28f9 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -58,12 +58,6 @@ struct iavf_hash_match_type {
 	uint64_t pattern_hint;
 };
 
-struct iavf_rss_meta {
-	struct virtchnl_proto_hdrs proto_hdrs;
-	enum virtchnl_rss_algorithm rss_algorithm;
-	bool raw_ena;
-};
-
 struct iavf_hash_flow_cfg {
 	struct virtchnl_rss_cfg *rss_cfg;
 	bool simple_xor;
@@ -79,8 +73,6 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		  struct rte_flow_error *error);
 static void
 iavf_hash_uninit_parser(struct iavf_adapter *ad);
-static void
-iavf_hash_free(struct rte_flow *flow);
 static int
 iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 			       struct iavf_pattern_match_item *array,
@@ -682,7 +674,6 @@ static struct iavf_flow_engine iavf_hash_engine = {
 	.create = iavf_hash_create,
 	.destroy = iavf_hash_destroy,
 	.uninit = iavf_hash_uninit_parser,
-	.free = iavf_hash_free,
 	.name = "hash",
 	.type = IAVF_FLOW_ENGINE_HASH,
 	.rule_size = sizeof(struct virtchnl_rss_cfg),
@@ -886,7 +877,7 @@ iavf_hash_parse_pattern(const struct rte_flow_item pattern[], uint64_t *phint,
 static int
 iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			struct rte_flow_error *error,
-			struct iavf_rss_meta *meta)
+			struct virtchnl_rss_cfg *rss_cfg)
 {
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
 	uint16_t spec_len, pkt_len;
@@ -958,9 +949,9 @@ iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			msk_buf[j] = tmp_val * 16 + tmp_c - '0';
 	}
 
-	memcpy(meta->proto_hdrs.raw.spec, pkt_buf, pkt_len);
-	memcpy(meta->proto_hdrs.raw.mask, msk_buf, pkt_len);
-	meta->proto_hdrs.raw.pkt_len = pkt_len;
+	memcpy(rss_cfg->proto_hdrs.raw.spec, pkt_buf, pkt_len);
+	memcpy(rss_cfg->proto_hdrs.raw.mask, msk_buf, pkt_len);
+	rss_cfg->proto_hdrs.raw.pkt_len = pkt_len;
 
 	rte_free(pkt_buf);
 	rte_free(msk_buf);
@@ -1433,12 +1424,12 @@ iavf_any_invalid_rss_type(enum rte_eth_hash_function rss_func,
 static int
 iavf_hash_parse_rss_type(struct iavf_pattern_match_item *match_item,
 		const struct rte_flow_action_rss *rss,
-		uint64_t pattern_hint, struct iavf_rss_meta *rss_meta,
+		uint64_t pattern_hint, struct virtchnl_rss_cfg *rss_cfg,
 		struct rte_flow_error *error)
 {
 	uint64_t rss_type;
 
-	rss_meta->rss_algorithm = rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ ?
+	rss_cfg->rss_algorithm = rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ ?
 		VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC :
 		VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 
@@ -1458,9 +1449,9 @@ iavf_hash_parse_rss_type(struct iavf_pattern_match_item *match_item,
 				"RSS type not supported");
 	}
 
-	memcpy(&rss_meta->proto_hdrs, match_item->meta, sizeof(struct virtchnl_proto_hdrs));
+	memcpy(&rss_cfg->proto_hdrs, match_item->meta, sizeof(rss_cfg->proto_hdrs));
 
-	iavf_refine_proto_hdrs(&rss_meta->proto_hdrs, rss_type, pattern_hint);
+	iavf_refine_proto_hdrs(&rss_cfg->proto_hdrs, rss_type, pattern_hint);
 
 	return 0;
 }
@@ -1526,7 +1517,7 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	};
 	const struct rte_flow_action_rss *rss;
 	struct iavf_pattern_match_item *pattern_match_item;
-	struct iavf_rss_meta *rss_meta_ptr;
+	struct virtchnl_rss_cfg *rss_cfg;
 	uint64_t phint = IAVF_PHINT_NONE;
 	int ret = 0;
 
@@ -1538,12 +1529,12 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	if (ret)
 		return ret;
 
-	rss_meta_ptr = rte_zmalloc(NULL, sizeof(*rss_meta_ptr), 0);
-	if (!rss_meta_ptr) {
+	rss_cfg = rte_zmalloc("iavf_rss_rule", sizeof(*rss_cfg), 0);
+	if (rss_cfg == NULL) {
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for rss_meta_ptr");
-		return -ENOMEM;
+				   "No memory for RSS rule.");
+		return -rte_errno;
 	}
 
 	/* Check rss supported pattern and find matched pattern. */
@@ -1560,8 +1551,7 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 		goto error;
 
 	if (phint == IAVF_PHINT_RAW) {
-		rss_meta_ptr->raw_ena = true;
-		ret = iavf_hash_parse_raw_pattern(pattern, error, rss_meta_ptr);
+		ret = iavf_hash_parse_raw_pattern(pattern, error, rss_cfg);
 		if (ret) {
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM, NULL,
@@ -1571,13 +1561,13 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	}
 
 	rss = parsed_actions.actions[0]->conf;
-	ret = iavf_hash_parse_rss_type(pattern_match_item, rss, phint, rss_meta_ptr, error);
+	ret = iavf_hash_parse_rss_type(pattern_match_item, rss, phint, rss_cfg, error);
 
 error:
 	if (!ret && meta)
-		*meta = rss_meta_ptr;
+		*meta = rss_cfg;
 	else
-		rte_free(rss_meta_ptr);
+		rte_free(rss_cfg);
 
 	rte_free(pattern_match_item);
 
@@ -1585,27 +1575,13 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 }
 
 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)
+iavf_hash_create(struct iavf_adapter *ad,
+		 struct rte_flow *flow, void *meta,
+		 struct rte_flow_error *error)
 {
-	struct iavf_rss_meta *rss_meta = (struct iavf_rss_meta *)meta;
-	struct virtchnl_rss_cfg *rss_cfg;
+	struct virtchnl_rss_cfg *rss_cfg = meta;
 	int ret = 0;
 
-	rss_cfg = rte_zmalloc("iavf rss rule",
-			      sizeof(struct virtchnl_rss_cfg), 0);
-	if (!rss_cfg) {
-		rte_flow_error_set(error, ENOMEM,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for rss rule");
-		rte_free(meta);
-		return -ENOMEM;
-	}
-
-	rss_cfg->proto_hdrs = rss_meta->proto_hdrs;
-	rss_cfg->rss_algorithm = rss_meta->rss_algorithm;
-
 	ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
 	if (!ret) {
 		flow->rule = rss_cfg;
@@ -1614,13 +1590,9 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to add rss rule.");
-		rte_free(rss_cfg);
-		rte_free(meta);
 		return -rte_errno;
 	}
 
-	rte_free(meta);
-
 	return ret;
 }
 
@@ -1633,22 +1605,19 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
 	struct virtchnl_rss_cfg *rss_cfg;
 	int ret = 0;
 
+	if (vf->vf_reset)
+		return 0;
+
 	rss_cfg = (struct virtchnl_rss_cfg *)flow->rule;
 
-	if (!vf->vf_reset) {
-		ret = iavf_add_del_rss_cfg(ad, rss_cfg, false);
-		if (ret) {
-			PMD_DRV_LOG(ERR, "fail to del RSS configure");
-			rte_flow_error_set(error, -ret,
-					   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-					   "Failed to delete rss rule.");
-			return -rte_errno;
-		}
+	ret = iavf_add_del_rss_cfg(ad, rss_cfg, false);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to del RSS configure");
+		rte_flow_error_set(error, -ret,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Failed to delete rss rule.");
+		return -rte_errno;
 	}
-
-	flow->rule = NULL;
-	rte_free(rss_cfg);
-
 	return ret;
 }
 
@@ -1677,9 +1646,3 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 }
-
-static void
-iavf_hash_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-}
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index b481330346..e9acd1bb1d 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1919,7 +1919,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 
 	iavf_ipsec_crypto_security_policy_delete(ad,
 			ipsec_flow->is_ipv4, ipsec_flow->id);
-	rte_free(ipsec_flow);
 	return 0;
 }
 
-- 
2.52.0

[PATCH v3 07/10] net/iavf: skip RSS teardown during PF-initiated reset

[Anatoly Burakov]

During PF-initiated reset recovery, `iavf_dev_close()` skips flow flush
to avoid sending virtchnl messages to the PF. However, `iavf_hash_uninit()`
is still called unconditionally, even though any existing RSS configuration
is now invalid because of PF-initiated reset.

Move the RSS teardown under the same `pf_reset_in_progress` check as flow
flush, so that no RSS configuration messages are sent during PF-initiated
reset recovery.

Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index bb97a89414..cfe5c45e72 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -3254,12 +3254,12 @@ iavf_dev_close(struct rte_eth_dev *dev)
 	/* free iAVF security device context all related resources */
 	iavf_security_ctx_destroy(adapter);
 
-	/* remove RSS configuration */
-	iavf_hash_uninit(adapter);
-
 	/* Skip the virtchnl-emitting teardown on a PF-initiated reset */
-	if (!vf->pf_reset_in_progress)
+	if (!vf->pf_reset_in_progress) {
+		/* remove RSS configuration */
+		iavf_hash_uninit(adapter);
 		iavf_flow_flush(dev, NULL);
+	}
 	iavf_flow_uninit(adapter);
 
 	/*
-- 
2.52.0

[PATCH v3 08/10] net/iavf: remove redundant reset checks from RSS engine

[Anatoly Burakov]

The RSS engine checks `vf_reset` during init, destroy and uninit to avoid
sending virtchnl commands while the VF is resetting. These checks are no
longer needed:

- `iavf_hash_init()` only registers the parser and sends no commands.
- `iavf_dev_close()` skips both flow flush and RSS teardown during
  PF-initiated reset recovery.
- the generic flow layer frees rule memory without relying on engine
  callbacks.
- `iavf_execute_vf_cmd()` already refuses to send commands while
  `vf_reset` is set.

Remove these checks as they are no longer needed.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_hash.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index 6352bf28f9..fe5d725064 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -762,9 +762,6 @@ iavf_hash_init(struct iavf_adapter *ad)
 	struct iavf_flow_parser *parser;
 	int ret;
 
-	if (vf->vf_reset)
-		return -EIO;
-
 	if (!vf->vf_res)
 		return -EINVAL;
 
@@ -1601,13 +1598,9 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
 		  struct rte_flow *flow,
 		  __rte_unused struct rte_flow_error *error)
 {
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
 	struct virtchnl_rss_cfg *rss_cfg;
 	int ret = 0;
 
-	if (vf->vf_reset)
-		return 0;
-
 	rss_cfg = (struct virtchnl_rss_cfg *)flow->rule;
 
 	ret = iavf_add_del_rss_cfg(ad, rss_cfg, false);
@@ -1633,9 +1626,6 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
 	struct rte_eth_rss_conf *rss_conf;
 
-	if (vf->vf_reset)
-		return;
-
 	if (!vf->vf_res)
 		return;
 
-- 
2.52.0

[PATCH v3 09/10] net/iavf: fix null engine for egress security flows

[Anatoly Burakov]

Egress security flows bypass the flow parsers and are inserted with a
NULL `engine` pointer. The create path then dereferences that pointer
when logging the new flow, and other engine-based operations cannot
handle the flow consistently.

Register an egress security engine and route these flows through the
normal parser and lifecycle callbacks. Egress flows own no rule and
require no hardware teardown.

Fixes: e783da175250 ("net/iavf: fix memory leak on egress IPsec flows")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c |  44 +++-----
 drivers/net/intel/iavf/iavf_generic_flow.h |   2 +-
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 120 +++++++++++++++++++--
 3 files changed, 123 insertions(+), 43 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 1593ea297c..be7ddf5db6 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1914,7 +1914,8 @@ iavf_register_parser(struct iavf_flow_parser *parser,
 			}
 		}
 		TAILQ_INSERT_HEAD(list, parser_node, node);
-	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO) {
+	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		   parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS) {
 		list = &vf->ipsec_crypto_parser_list;
 		TAILQ_INSERT_HEAD(list, parser_node, node);
 	} else if (parser->engine->type == IAVF_FLOW_ENGINE_FSUB) {
@@ -1943,7 +1944,8 @@ iavf_unregister_parser(struct iavf_flow_parser *parser,
 	else if ((parser->engine->type == IAVF_FLOW_ENGINE_FDIR) ||
 		 (parser->engine->type == IAVF_FLOW_ENGINE_FSUB))
 		list = &vf->dist_parser_list;
-	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO)
+	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		 parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS)
 		list = &vf->ipsec_crypto_parser_list;
 
 	if (list == NULL)
@@ -2250,12 +2252,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 		return flow;
 	}
 
-	/* Special case for inline crypto egress flows */
-	if (attr->egress && actions[0].type == RTE_FLOW_ACTION_TYPE_SECURITY) {
-		flow->is_ipsec_egress_flow = true;
-		goto tailq_insert;
-	}
-
 	ret = iavf_flow_process_filter(dev, flow, attr, pattern, actions,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
@@ -2266,7 +2262,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 	}
 
 	flow->engine = engine;
-tailq_insert:
 	rte_spinlock_lock(&vf->flow_ops_lock);
 	TAILQ_INSERT_TAIL(&vf->flow_list, flow, node);
 	rte_spinlock_unlock(&vf->flow_ops_lock);
@@ -2282,14 +2277,7 @@ iavf_flow_is_valid(struct rte_flow *flow)
 	struct iavf_flow_engine *engine;
 	void *temp;
 
-	if (flow == NULL)
-		return false;
-
-	/* these flows don't use engines */
-	if (flow->is_ipsec_egress_flow)
-		return true;
-
-	if (flow->engine) {
+	if (flow != NULL && flow->engine != NULL) {
 		RTE_TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
 			if (engine == flow->engine)
 				return true;
@@ -2307,26 +2295,17 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 	struct iavf_adapter *ad =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	bool need_destroy;
 	int ret = 0;
 
-	if (!iavf_flow_is_valid(flow)) {
+	if (!iavf_flow_is_valid(flow) || flow->engine->destroy == NULL) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Invalid flow destroy");
 		return -rte_errno;
 	}
-	need_destroy = !flow->is_ipsec_egress_flow;
-
-	if (need_destroy && flow->engine->destroy == NULL) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Invalid flow destroy");
-	}
 
 	rte_spinlock_lock(&vf->flow_ops_lock);
-	if (need_destroy)
-		ret = flow->engine->destroy(ad, flow, error);
+	ret = flow->engine->destroy(ad, flow, error);
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
@@ -2444,16 +2423,17 @@ iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		if (flow != NULL && f != flow)
 			continue;
 
-		/* this should not happen */
-		if (f->engine == NULL || f->rule == NULL) {
+		if (!iavf_flow_is_valid(f)) {
 			PMD_DRV_LOG(DEBUG, "Invalid flow");
 			continue;
 		}
 
 		found = true;
 
-		rule_size = f->engine->rule_size;
-		rule_data = f->rule;
+		if (f->rule != NULL) {
+			rule_size = f->engine->rule_size;
+			rule_data = f->rule;
+		}
 
 		iavf_flow_dump_blob(file,
 			f->engine->name != NULL ?
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 3a8f5f92e8..75bb92900b 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -481,6 +481,7 @@ enum iavf_flow_engine_type {
 	IAVF_FLOW_ENGINE_FDIR,
 	IAVF_FLOW_ENGINE_HASH,
 	IAVF_FLOW_ENGINE_FSUB,
+	IAVF_FLOW_ENGINE_IPSEC_EGRESS,
 	IAVF_FLOW_ENGINE_MAX,
 };
 
@@ -503,7 +504,6 @@ TAILQ_HEAD(iavf_engine_list, iavf_flow_engine);
 /* Struct to store flow created. */
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) node;
-	bool is_ipsec_egress_flow;
 	struct iavf_flow_engine *engine;
 	void *rule;
 };
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index e9acd1bb1d..f7e391a74a 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1832,6 +1832,7 @@ iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 
 
 static struct iavf_flow_parser iavf_ipsec_flow_parser;
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser;
 
 static int
 iavf_ipsec_flow_init(struct iavf_adapter *ad)
@@ -1922,16 +1923,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 	return 0;
 }
 
-static struct iavf_flow_engine iavf_ipsec_flow_engine = {
-	.init = iavf_ipsec_flow_init,
-	.uninit = iavf_ipsec_flow_uninit,
-	.create = iavf_ipsec_flow_create,
-	.destroy = iavf_ipsec_flow_destroy,
-	.name = "ipsec_crypto",
-	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
-	.rule_size = sizeof(struct iavf_ipsec_flow_item),
-};
-
 static int
 iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		      struct iavf_pattern_match_item *array,
@@ -1975,6 +1966,105 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 	return ret;
 }
 
+/*
+ * Inline crypto egress flows only associate Tx traffic with a security
+ * session; nothing is programmed in hardware, so the engine owns no rule.
+ */
+static int
+iavf_ipsec_egress_flow_init(struct iavf_adapter *ad)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+
+	if (!vf->vf_res)
+		return -EINVAL;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_INLINE_IPSEC_CRYPTO))
+		return -ENOTSUP;
+
+	return iavf_register_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static void
+iavf_ipsec_egress_flow_uninit(struct iavf_adapter *ad)
+{
+	iavf_unregister_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static int
+iavf_ipsec_egress_flow_create(__rte_unused struct iavf_adapter *ad,
+		struct rte_flow *flow,
+		__rte_unused void *meta,
+		__rte_unused struct rte_flow_error *error)
+{
+	flow->rule = NULL;
+	return 0;
+}
+
+static int
+iavf_ipsec_egress_flow_destroy(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct rte_flow *flow,
+		__rte_unused struct rte_flow_error *error)
+{
+	return 0;
+}
+
+/* The pattern is not used, as there is nothing to match in hardware. */
+static int
+iavf_ipsec_egress_flow_parse(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct iavf_pattern_match_item *array,
+		__rte_unused uint32_t array_len,
+		__rte_unused const struct rte_flow_item pattern[],
+		const struct rte_flow_action actions[],
+		const struct rte_flow_attr *attr,
+		void **meta,
+		struct rte_flow_error *error)
+{
+	struct ci_flow_actions parsed_actions = {0};
+	struct ci_flow_actions_check_param param = {
+		.allowed_types = (enum rte_flow_action_type[]){
+			RTE_FLOW_ACTION_TYPE_SECURITY,
+			RTE_FLOW_ACTION_TYPE_END,
+		},
+		.max_actions = 1,
+	};
+	struct ci_flow_attr_check_param attr_param = {
+		.require_egress = true,
+	};
+	int ret;
+
+	ret = ci_flow_check_attr(attr, &attr_param, error);
+	if (ret)
+		return ret;
+
+	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
+	if (ret < 0)
+		return ret;
+
+	if (meta != NULL)
+		*meta = NULL;
+
+	return 0;
+}
+
+static struct iavf_flow_engine iavf_ipsec_flow_engine = {
+	.init = iavf_ipsec_flow_init,
+	.uninit = iavf_ipsec_flow_uninit,
+	.create = iavf_ipsec_flow_create,
+	.destroy = iavf_ipsec_flow_destroy,
+	.name = "ipsec_crypto",
+	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
+	.rule_size = sizeof(struct iavf_ipsec_flow_item),
+};
+
+static struct iavf_flow_engine iavf_ipsec_egress_flow_engine = {
+	.init = iavf_ipsec_egress_flow_init,
+	.uninit = iavf_ipsec_egress_flow_uninit,
+	.create = iavf_ipsec_egress_flow_create,
+	.destroy = iavf_ipsec_egress_flow_destroy,
+	.name = "ipsec_egress",
+	.type = IAVF_FLOW_ENGINE_IPSEC_EGRESS,
+};
+
 static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.engine = &iavf_ipsec_flow_engine,
 	.array = iavf_ipsec_flow_pattern,
@@ -1982,7 +2072,17 @@ static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.parse_pattern_action = iavf_ipsec_flow_parse,
 };
 
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser = {
+	.engine = &iavf_ipsec_egress_flow_engine,
+	.parse_pattern_action = iavf_ipsec_egress_flow_parse,
+};
+
 RTE_INIT(iavf_ipsec_flow_engine_register)
 {
 	iavf_register_flow_engine(&iavf_ipsec_flow_engine);
 }
+
+RTE_INIT(iavf_ipsec_egress_flow_engine_register)
+{
+	iavf_register_flow_engine(&iavf_ipsec_egress_flow_engine);
+}
-- 
2.52.0

Re: [PATCH v3 09/10] net/iavf: fix null engine for egress security flows

[Radu Nicolau]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]


On 20-Oct-26 10:11 AM, Anatoly Burakov wrote:
> Egress security flows bypass the flow parsers and are inserted with a
> NULL `engine` pointer. The create path then dereferences that pointer
> when logging the new flow, and other engine-based operations cannot
> handle the flow consistently.
>
> Register an egress security engine and route these flows through the
> normal parser and lifecycle callbacks. Egress flows own no rule and
> require no hardware teardown.
>
> Fixes: e783da175250 ("net/iavf: fix memory leak on egress IPsec flows")
> Cc:stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov<anatoly.burakov@intel.com>
> ---
Acked-by: Radu Nicolau <radu.nicolau@intel.com>

[-- Attachment #2: Type: text/html, Size: 37101 bytes --]

[PATCH v3 10/10] net/iavf: fix IPsec flow metadata leaks

[Anatoly Burakov]

IPsec flow parsing leaves the pattern-match item allocated after
extracting its metadata. It also leaves the parsed flow item allocated
when no metadata output is requested.

Free the pattern-match item after parsing and free the parsed flow item
when it cannot be returned to the caller. Propagate pattern-match failure
and report a parsing error when no flow item was produced.

Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 28 +++++++++++++++-------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index f7e391a74a..be0a0d0df6 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1942,6 +1942,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		.max_actions = 1,
 	};
 	struct iavf_pattern_match_item *item = NULL;
+	struct iavf_ipsec_flow_item *fi = NULL;
 	int ret = -1;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
@@ -1953,17 +1954,28 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		return ret;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (item && item->meta) {
+	if (item == NULL)
+		return -rte_errno;
+
+	if (item->meta != NULL) {
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
-		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad, pattern, session, type);
-		if (fi && meta) {
-			*meta = fi;
-			ret = 0;
-		}
+
+		fi = iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 	}
-	return ret;
+	rte_free(item);
+
+	if (fi == NULL)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+				"Invalid IPsec flow.");
+
+	if (meta != NULL)
+		*meta = fi;
+	else
+		rte_free(fi);
+
+	return 0;
 }
 
 /*
-- 
2.52.0

Re: [PATCH v3 10/10] net/iavf: fix IPsec flow metadata leaks

[Radu Nicolau]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]


On 20-Oct-26 10:11 AM, Anatoly Burakov wrote:
> IPsec flow parsing leaves the pattern-match item allocated after
> extracting its metadata. It also leaves the parsed flow item allocated
> when no metadata output is requested.
>
> Free the pattern-match item after parsing and free the parsed flow item
> when it cannot be returned to the caller. Propagate pattern-match failure
> and report a parsing error when no flow item was produced.
>
> Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
> Cc:stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov<anatoly.burakov@intel.com>
> ---

Acked-by: Radu Nicolau <radu.nicolau@intel.com>

[-- Attachment #2: Type: text/html, Size: 37077 bytes --]

[PATCH v4 00/10] IAVF Fixes

[Anatoly Burakov]

An assortment of IAVF flow-related memory leak fixes and cleanups.

v3 -> v4:
- Fixed watchdog crash in commit 2

v2 -> v3:
- Rebase on top of latest next-net-intel including Anurag's fix [1]
- Dropped remnants of Anurag's patch [1] as it's now in the tree
- Fixed PF reset path still tearing down RSS when it shouldn't
- Fixed hash engine checking for VF reset when that's no longer necessary

v1 -> v2:
- Integrated Anurag's earlier fix [1] into the patchset
- Reworked generic flow to have centralized memory management
- Reordered some patches
- Integrated the IRQ leaks patchset [2] [3]
- Reworked IPsec fix

[1] https://patches.dpdk.org/project/dpdk/patch/20261010121255.528329-1-anurag.mandal@intel.com/
[2] https://patches.dpdk.org/project/dpdk/patch/809e0c5b2ab57f1b14c68be8ac34e70cbbe82233.1792053236.git.anatoly.burakov@intel.com/
[3] https://patches.dpdk.org/project/dpdk/patch/5488d85c2ded08838becaee6de8328694ee00234.1792053236.git.anatoly.burakov@intel.com/

Anatoly Burakov (10):
  net/iavf: keep device stopped if start fails
  net/iavf: do not store ethdev pointer
  net/iavf: fix false success on flow validation
  net/iavf: fix memory leaks around IRQ resources
  net/iavf: fix generic flow error-path leaks
  net/iavf: centralize memory management for flows
  net/iavf: skip RSS teardown during PF-initiated reset
  net/iavf: remove redundant reset checks from RSS engine
  net/iavf: fix null engine for egress security flows
  net/iavf: fix IPsec flow metadata leaks

 drivers/net/intel/iavf/iavf.h              |  14 +-
 drivers/net/intel/iavf/iavf_ethdev.c       |  48 +++---
 drivers/net/intel/iavf/iavf_fdir.c         |  68 ++++----
 drivers/net/intel/iavf/iavf_fsub.c         |  23 +--
 drivers/net/intel/iavf/iavf_generic_flow.c |  95 ++++++------
 drivers/net/intel/iavf/iavf_generic_flow.h |   4 +-
 drivers/net/intel/iavf/iavf_hash.c         | 103 ++++---------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 171 +++++++++++++++++----
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |   4 +-
 drivers/net/intel/iavf/iavf_vchnl.c        |   7 +-
 10 files changed, 302 insertions(+), 235 deletions(-)

-- 
2.52.0

[PATCH v4 01/10] net/iavf: keep device stopped if start fails

[Anatoly Burakov]

Currently, `iavf_dev_start()` clears `adapter->stopped` before several
operations that can fail. A subsequent `iavf_dev_stop()` then disables
interrupts and stops and resets queues even though startup did not
complete.

Mark the adapter running only after startup succeeds, and return early
from stop when it is already stopped.

Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 9e0e13d4992..1ec9e946fc4 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1039,8 +1039,6 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	if (adapter->closed)
 		return -1;
 
-	adapter->stopped = 0;
-
 	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
 	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				      dev->data->nb_tx_queues);
@@ -1112,6 +1110,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 
 	iavf_phc_sync_alarm_start(dev);
+	adapter->stopped = 0;
 
 	/* An explicit start supersedes any pending deferred start */
 	vf->start_pending = false;
@@ -1135,13 +1134,13 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 	if (adapter->closed)
 		return -1;
 
+	if (adapter->stopped == 1)
+		return 0;
+
 	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) &&
 	    dev->data->dev_conf.intr_conf.rxq != 0)
 		rte_intr_disable(intr_handle);
 
-	if (adapter->stopped == 1)
-		return 0;
-
 	iavf_phc_sync_alarm_stop(dev);
 
 	/* Disable the interrupt for Rx */
-- 
2.52.0

Re: [PATCH v4 01/10] net/iavf: keep device stopped if start fails

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:41PM +0100, Anatoly Burakov wrote:
> Currently, `iavf_dev_start()` clears `adapter->stopped` before several
> operations that can fail. A subsequent `iavf_dev_stop()` then disables
> interrupts and stops and resets queues even though startup did not
> complete.
> 
> Mark the adapter running only after startup succeeds, and return early
> from stop when it is already stopped.
> 
> Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_ethdev.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 02/10] net/iavf: do not store ethdev pointer

[Anatoly Burakov]

We should not be storing rte_eth_dev pointers in shared memory, because
rte_eth_dev pointers are process local. To address that, remove the pointer
from iavf_info, replace it with `rte_eth_devices`-based lookup, and rework
the call paths to either use that lookup, or not require ethdev pointer
in the first place.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  9 ++++++--
 drivers/net/intel/iavf/iavf_ethdev.c       | 11 +++++-----
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 24 +++++++++++-----------
 drivers/net/intel/iavf/iavf_ipsec_crypto.h |  4 ++--
 drivers/net/intel/iavf/iavf_vchnl.c        |  7 ++++---
 5 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index c40b97444b5..1f277eeee29 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -7,6 +7,7 @@
 
 #include <sys/queue.h>
 
+#include <ethdev_driver.h>
 #include <rte_kvargs.h>
 #include <rte_tm_driver.h>
 
@@ -293,8 +294,6 @@ struct iavf_info {
 	struct iavf_qtc_map *qtc_map;
 	struct iavf_tm_conf tm_conf;
 
-	struct rte_eth_dev *eth_dev;
-
 	RTE_ATOMIC(bool)in_reset_recovery;
 	bool reset_pending;
 	bool pf_reset_in_progress;
@@ -408,6 +407,12 @@ struct iavf_adapter {
 	uint16_t tpid;  /* VLAN tag identifier */
 };
 
+static inline struct rte_eth_dev *
+iavf_get_eth_dev(struct iavf_adapter *adapter)
+{
+	return &rte_eth_devices[adapter->dev_data->port_id];
+}
+
 /* IAVF_DEV_PRIVATE_TO */
 #define IAVF_DEV_PRIVATE_TO_ADAPTER(adapter) \
 	((struct iavf_adapter *)adapter)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 1ec9e946fc4..b5beaee8604 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -326,6 +326,7 @@ static void
 iavf_dev_watchdog(void *cb_arg)
 {
 	struct iavf_adapter *adapter = cb_arg;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
 	int vfr_inprogress = 0, rc = 0;
 
@@ -339,7 +340,7 @@ iavf_dev_watchdog(void *cb_arg)
 
 		if (!vfr_inprogress) {
 			PMD_DRV_LOG(INFO, "VF \"%s\" reset has completed",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 			adapter->vf.vf_reset = false;
 			iavf_set_no_poll(adapter, false);
 		}
@@ -350,14 +351,14 @@ iavf_dev_watchdog(void *cb_arg)
 		if (vfr_inprogress) {
 			PMD_DRV_LOG(INFO,
 				"VF \"%s\" reset event detected by watchdog",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
 			iavf_set_no_poll(adapter, false);
 			adapter->vf.link_up = false;
 
-			iavf_dev_event_post(adapter->vf.eth_dev, RTE_ETH_EVENT_INTR_RESET,
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
 				NULL, 0);
 		}
 	}
@@ -369,7 +370,7 @@ iavf_dev_watchdog(void *cb_arg)
 
 		if (rc)
 			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-				adapter->vf.eth_dev->data->name);
+				dev->data->name);
 	}
 }
 
@@ -2669,8 +2670,6 @@ iavf_init_vf(struct rte_eth_dev *dev)
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 
-	vf->eth_dev = dev;
-
 	err = iavf_parse_devargs(dev);
 	if (err) {
 		PMD_INIT_LOG(ERR, "Failed to parse devargs");
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index 06b51b5e622..b4813303464 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -689,11 +689,9 @@ iavf_ipsec_crypto_session_create(void *device,
  * returns: 1 if valid
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi)
 {
-	struct iavf_adapter *adapter =
-		IAVF_DEV_PRIVATE_TO_ADAPTER(ethdev->data->dev_private);
 	const struct iavf_security_session *sess = (const void *)session->driver_priv_data;
 
 	/* verify we have a valid session and that it belong to this adapter */
@@ -1502,24 +1500,25 @@ static struct rte_security_ops iavf_ipsec_crypto_ops = {
 int
 iavf_security_ctx_create(struct iavf_adapter *adapter)
 {
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 	struct rte_security_ctx *sctx;
 
 	sctx = rte_malloc("security_ctx", sizeof(struct rte_security_ctx), 0);
 	if (sctx == NULL)
 		return -ENOMEM;
 
-	sctx->device = adapter->vf.eth_dev;
+	sctx->device = dev;
 	sctx->ops = &iavf_ipsec_crypto_ops;
 	sctx->sess_cnt = 0;
 
-	adapter->vf.eth_dev->security_ctx = sctx;
+	dev->security_ctx = sctx;
 
 	if (adapter->security_ctx == NULL) {
 		adapter->security_ctx = rte_malloc("iavf_security_ctx",
 				sizeof(struct iavf_security_ctx), 0);
 		if (adapter->security_ctx == NULL) {
-			rte_free(adapter->vf.eth_dev->security_ctx);
-			adapter->vf.eth_dev->security_ctx = NULL;
+			rte_free(dev->security_ctx);
+			dev->security_ctx = NULL;
 			return -ENOMEM;
 		}
 	}
@@ -1565,7 +1564,8 @@ iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter)
 int
 iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 {
-	struct rte_security_ctx *sctx  = adapter->vf.eth_dev->security_ctx;
+	struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
+	struct rte_security_ctx *sctx  = dev->security_ctx;
 	struct iavf_security_ctx *iavf_sctx = adapter->security_ctx;
 
 	if (iavf_sctx == NULL)
@@ -1578,7 +1578,7 @@ iavf_security_ctx_destroy(struct iavf_adapter *adapter)
 	rte_free(sctx);
 
 	adapter->security_ctx = NULL;
-	adapter->vf.eth_dev->security_ctx = NULL;
+	dev->security_ctx = NULL;
 
 	return 0;
 }
@@ -1748,7 +1748,7 @@ parse_udp_item(const struct rte_flow_item_udp *item, struct rte_udp_hdr *udp)
 }
 
 static struct iavf_ipsec_flow_item *
-iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
+iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 		const struct rte_flow_item pattern[],
 		const struct rte_security_session *session,
 		uint32_t type)
@@ -1819,7 +1819,7 @@ iavf_ipsec_flow_item_parse(struct rte_eth_dev *ethdev,
 		goto flow_cleanup;
 	}
 
-	if (!iavf_ipsec_crypto_action_valid(ethdev, session,
+	if (!iavf_ipsec_crypto_action_valid(adapter, session,
 			ipsec_flow->spi))
 		goto flow_cleanup;
 
@@ -1967,7 +1967,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
 		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad->vf.eth_dev, pattern, session, type);
+				iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 		if (fi && meta) {
 			*meta = fi;
 			ret = 0;
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.h b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
index 795b75a624d..3d4b1da127a 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.h
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.h
@@ -127,13 +127,13 @@ int iavf_security_get_pkt_md_offset(struct iavf_adapter *adapter);
 /**
  * Destroy security context
  */
-int iavf_security_ctx_destroy(struct iavf_adapter *adapterv);
+int iavf_security_ctx_destroy(struct iavf_adapter *adapter);
 
 /**
  * Verify that the inline IPsec Crypto action is valid for this device
  */
 uint32_t
-iavf_ipsec_crypto_action_valid(struct rte_eth_dev *ethdev,
+iavf_ipsec_crypto_action_valid(struct iavf_adapter *adapter,
 	const struct rte_security_session *session, uint32_t spi);
 
 /**
diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 57488512f43..26dae423a48 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -314,13 +314,14 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 		    opcode, vf->cmd_retval);
 
 	if (opcode == VIRTCHNL_OP_EVENT) {
+		struct rte_eth_dev *dev = iavf_get_eth_dev(adapter);
 		struct virtchnl_pf_event *vpe =
 			(struct virtchnl_pf_event *)event.msg_buf;
 
 		result = IAVF_MSG_SYS;
 		switch (vpe->event) {
 		case VIRTCHNL_EVENT_LINK_CHANGE:
-			iavf_handle_link_change_event(vf->eth_dev, vpe);
+			iavf_handle_link_change_event(dev, vpe);
 			break;
 		case VIRTCHNL_EVENT_RESET_IMPENDING:
 			/*
@@ -335,8 +336,8 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				iavf_set_no_poll(adapter, false);
 				iavf_dev_watchdog_enable(adapter);
 				if (adapter->devargs.no_poll_on_link_down)
-					iavf_dev_tx_drain(vf->eth_dev);
-				iavf_dev_event_post(vf->eth_dev,
+					iavf_dev_tx_drain(dev);
+				iavf_dev_event_post(dev,
 					RTE_ETH_EVENT_INTR_RESET,
 					NULL, 0);
 			}
-- 
2.52.0

Re: [PATCH v4 02/10] net/iavf: do not store ethdev pointer

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:42PM +0100, Anatoly Burakov wrote:
> We should not be storing rte_eth_dev pointers in shared memory, because
> rte_eth_dev pointers are process local. To address that, remove the pointer
> from iavf_info, replace it with `rte_eth_devices`-based lookup, and rework
> the call paths to either use that lookup, or not require ethdev pointer
> in the first place.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf.h              |  9 ++++++--
>  drivers/net/intel/iavf/iavf_ethdev.c       | 11 +++++-----
>  drivers/net/intel/iavf/iavf_ipsec_crypto.c | 24 +++++++++++-----------
>  drivers/net/intel/iavf/iavf_ipsec_crypto.h |  4 ++--
>  drivers/net/intel/iavf/iavf_vchnl.c        |  7 ++++---
>  5 files changed, 30 insertions(+), 25 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 03/10] net/iavf: fix false success on flow validation

[Anatoly Burakov]

`iavf_parse_engine_validate()` sets an error when validation fails, but
then returns the engine pointer. `iavf_flow_process_filter()` treats a
non-NULL engine as success, so `rte_flow_validate()` can accept an
invalid flow.

Return `NULL` on validation failure and return the engine only after
validation succeeds.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 92ca20031c0..14242e6ebbb 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -2134,10 +2134,11 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation failed");
-			break;
+			return NULL;
 		}
+		return engine;
 	}
-	return engine;
+	return NULL;
 }
 
 
-- 
2.52.0

Re: [PATCH v4 03/10] net/iavf: fix false success on flow validation

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:43PM +0100, Anatoly Burakov wrote:
> `iavf_parse_engine_validate()` sets an error when validation fails, but
> then returns the engine pointer. `iavf_flow_process_filter()` treats a
> non-NULL engine as success, so `rte_flow_validate()` can accept an
> invalid flow.
> 
> Return `NULL` on validation failure and return the engine only after
> validation succeeds.
> 
> Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_generic_flow.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 04/10] net/iavf: fix memory leaks around IRQ resources

[Anatoly Burakov]

`iavf_config_rx_queues_irqs()` allocates a queue-vector map and may
enable eventfds and allocate an interrupt vector list. If queue startup
then fails, those resources remain allocated. The queue-vector map is
also retained after a normal stop, leaking it across start/stop cycles.

Release the resources on failed startup and free the queue-vector map
on stop. Clean up eventfds when interrupt setup fails as well.

Fixes: f593944fc988 ("net/iavf: enable IRQ mapping configuration for large VF")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_ethdev.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index b5beaee8604..891880cf6e6 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -863,7 +863,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 	if (rte_intr_cap_multiple(intr_handle) &&
 	    dev->data->dev_conf.intr_conf.rxq) {
 		if (rte_intr_efd_enable(intr_handle, dev->data->nb_rx_queues))
-			return -1;
+			goto efd_alloc_err;
 	}
 
 	if (rte_intr_dp_is_en(intr_handle)) {
@@ -871,7 +871,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 						    dev->data->nb_rx_queues)) {
 			PMD_DRV_LOG(ERR, "Failed to allocate %d rx intr_vec",
 				    dev->data->nb_rx_queues);
-			return -1;
+			goto qv_map_alloc_err;
 		}
 	}
 
@@ -981,6 +981,8 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 
 qv_map_alloc_err:
 	rte_intr_vec_list_free(intr_handle);
+efd_alloc_err:
+	rte_intr_efd_disable(intr_handle);
 
 	return -1;
 }
@@ -1034,12 +1036,15 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	struct rte_intr_handle *intr_handle = dev->intr_handle;
 	uint16_t num_queue_pairs;
 	uint16_t index = 0;
+	bool wb_on_itr;
 
 	PMD_INIT_FUNC_TRACE();
 
 	if (adapter->closed)
 		return -1;
 
+	wb_on_itr = (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) != 0;
+
 	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
 	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
 				      dev->data->nb_tx_queues);
@@ -1089,7 +1094,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 	/* re-enable intr again, because efd assign may change */
 	if (dev->data->dev_conf.intr_conf.rxq != 0) {
-		if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
+		if (wb_on_itr)
 			rte_intr_disable(intr_handle);
 		rte_intr_enable(intr_handle);
 	}
@@ -1107,7 +1112,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
 
 	if (iavf_start_queues(dev) != 0) {
 		PMD_DRV_LOG(ERR, "enable queues failed");
-		goto error;
+		goto irq_error;
 	}
 
 	iavf_phc_sync_alarm_start(dev);
@@ -1118,6 +1123,13 @@ iavf_dev_start(struct rte_eth_dev *dev)
 
 	return 0;
 
+irq_error:
+	if (!wb_on_itr && dev->data->dev_conf.intr_conf.rxq != 0)
+		rte_intr_disable(intr_handle);
+	rte_intr_efd_disable(intr_handle);
+	rte_intr_vec_list_free(intr_handle);
+	rte_free(vf->qv_map);
+	vf->qv_map = NULL;
 error:
 	return -1;
 }
@@ -1150,6 +1162,8 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 	rte_intr_vec_list_free(intr_handle);
 
 	iavf_stop_queues(dev);
+	rte_free(vf->qv_map);
+	vf->qv_map = NULL;
 
 	adapter->stopped = 1;
 	dev->data->dev_started = 0;
-- 
2.52.0

Re: [PATCH v4 04/10] net/iavf: fix memory leaks around IRQ resources

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:44PM +0100, Anatoly Burakov wrote:
> `iavf_config_rx_queues_irqs()` allocates a queue-vector map and may
> enable eventfds and allocate an interrupt vector list. If queue startup
> then fails, those resources remain allocated. The queue-vector map is
> also retained after a normal stop, leaking it across start/stop cycles.
> 
> Release the resources on failed startup and free the queue-vector map
> on stop. Clean up eventfds when interrupt setup fails as well.
> 
> Fixes: f593944fc988 ("net/iavf: enable IRQ mapping configuration for large VF")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_ethdev.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
> index b5beaee8604..891880cf6e6 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -863,7 +863,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
>  	if (rte_intr_cap_multiple(intr_handle) &&
>  	    dev->data->dev_conf.intr_conf.rxq) {
>  		if (rte_intr_efd_enable(intr_handle, dev->data->nb_rx_queues))
> -			return -1;
> +			goto efd_alloc_err;
>  	}

This change doesn't make sense to me. If the enable fails, we should not
need to call disable when handling that error.

Independently AI review flags this that you might want to consider:

Info
Pre-existing and not introduced by this commit: the new failure branch at iavf_ethdev.c:866 calls cleanup after rte_intr_efd_enable() fails, but that helper can create eventfds before returning an error and before recording the completed count at eal_interrupts.c:1580. The cleanup closes only the recorded eventfds at eal_interrupts.c:1623, so a partial allocation failure can still leak descriptors. Track and release partially created eventfds in the helper or cleanup path. Confidence: 9/10.

>  
>  	if (rte_intr_dp_is_en(intr_handle)) {
> @@ -871,7 +871,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
>  						    dev->data->nb_rx_queues)) {
>  			PMD_DRV_LOG(ERR, "Failed to allocate %d rx intr_vec",
>  				    dev->data->nb_rx_queues);
> -			return -1;
> +			goto qv_map_alloc_err;
>  		}
>  	}
>  
> @@ -981,6 +981,8 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
>  
>  qv_map_alloc_err:
>  	rte_intr_vec_list_free(intr_handle);
> +efd_alloc_err:
> +	rte_intr_efd_disable(intr_handle);
>  
>  	return -1;
>  }
> @@ -1034,12 +1036,15 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  	struct rte_intr_handle *intr_handle = dev->intr_handle;
>  	uint16_t num_queue_pairs;
>  	uint16_t index = 0;
> +	bool wb_on_itr;
>  
>  	PMD_INIT_FUNC_TRACE();
>  
>  	if (adapter->closed)
>  		return -1;
>  
> +	wb_on_itr = (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) != 0;
> +
>  	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
>  	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
>  				      dev->data->nb_tx_queues);
> @@ -1089,7 +1094,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  	}
>  	/* re-enable intr again, because efd assign may change */
>  	if (dev->data->dev_conf.intr_conf.rxq != 0) {
> -		if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
> +		if (wb_on_itr)
>  			rte_intr_disable(intr_handle);
>  		rte_intr_enable(intr_handle);
>  	}
> @@ -1107,7 +1112,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  
>  	if (iavf_start_queues(dev) != 0) {
>  		PMD_DRV_LOG(ERR, "enable queues failed");
> -		goto error;
> +		goto irq_error;
>  	}
>  
>  	iavf_phc_sync_alarm_start(dev);
> @@ -1118,6 +1123,13 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  
>  	return 0;
>  
> +irq_error:
> +	if (!wb_on_itr && dev->data->dev_conf.intr_conf.rxq != 0)
> +		rte_intr_disable(intr_handle);
> +	rte_intr_efd_disable(intr_handle);
> +	rte_intr_vec_list_free(intr_handle);
> +	rte_free(vf->qv_map);
> +	vf->qv_map = NULL;
>  error:
>  	return -1;
>  }
> @@ -1150,6 +1162,8 @@ iavf_dev_stop(struct rte_eth_dev *dev)
>  	rte_intr_vec_list_free(intr_handle);
>  
>  	iavf_stop_queues(dev);
> +	rte_free(vf->qv_map);
> +	vf->qv_map = NULL;
>  
>  	adapter->stopped = 1;
>  	dev->data->dev_started = 0;
> -- 
> 2.52.0
> 

[PATCH v4 05/10] net/iavf: fix generic flow error-path leaks

[Anatoly Burakov]

If a flow engine fails to initialize, parsers registered by earlier
engines remain allocated. Pattern matching also leaks its item array
when allocation of the match result fails.

Unwind registered parsers on flow initialization failure and free the
item array on match-result allocation failure.

Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 14242e6ebbb..cd4d4a012b1 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1821,17 +1821,22 @@ iavf_flow_init(struct iavf_adapter *ad)
 		if (engine->init == NULL) {
 			PMD_INIT_LOG(ERR, "Invalid engine type (%d)",
 				     engine->type);
-			return -ENOTSUP;
+			ret = -ENOTSUP;
+			goto err;
 		}
 
 		ret = engine->init(ad);
 		if (ret && ret != -ENOTSUP) {
 			PMD_INIT_LOG(ERR, "Failed to initialize engine %d",
 				     engine->type);
-			return ret;
+			goto err;
 		}
 	}
 	return 0;
+
+err:
+	iavf_flow_uninit(ad);
+	return ret;
 }
 
 void
@@ -2047,6 +2052,7 @@ iavf_search_pattern_match_item(const struct rte_flow_item pattern[],
 	if (!pattern_match_item) {
 		rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Failed to allocate memory.");
+		rte_free(items);
 		return NULL;
 	}
 
-- 
2.52.0

Re: [PATCH v4 05/10] net/iavf: fix generic flow error-path leaks

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:45PM +0100, Anatoly Burakov wrote:
> If a flow engine fails to initialize, parsers registered by earlier
> engines remain allocated. Pattern matching also leaks its item array
> when allocation of the match result fails.
> 
> Unwind registered parsers on flow initialization failure and free the
> item array on match-result allocation failure.
> 
> Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_generic_flow.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 06/10] net/iavf: centralize memory management for flows

[Anatoly Burakov]

Flow engines currently distribute rule allocation and cleanup between the
parser, engine create and destroy callbacks, and generic flow code. This
leads to a lot of potential for memory leaks and bugs. In particular, one
of the issues currently present is that when PF initiates reset, we are
meant to just drop all flows without sending any virtchnl commands, but
because not all engines implement the `free` callback, some flows will be
leaked.

To address that specific leak as well as all other potential leaks due to
misuse of memory by engines, use parser-allocated rule objects as the
persistent rules and centralize their lifetime in the generic flow layer.
This requires fixing all engines to not manage their own memory:

- Allocate the final FDIR rule in the parser, use it for action checks,
  and remove the per-VF scratch configuration.
- Have FSUB create adopt its parser-produced rule instead of allocating
  and copying a second object.
- Build the final RSS configuration in the parser and remove the
  intermediate RSS metadata, including its unused raw-pattern flag.
- Let generic flow code free rules on validate, create failure,
  successful destroy, and uninit; remove engine-specific free callbacks
  and rule frees from destroy callbacks.

Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf.h              |  5 --
 drivers/net/intel/iavf/iavf_fdir.c         | 68 +++++++--------
 drivers/net/intel/iavf/iavf_fsub.c         | 23 +----
 drivers/net/intel/iavf/iavf_generic_flow.c | 36 ++++++--
 drivers/net/intel/iavf/iavf_generic_flow.h |  2 -
 drivers/net/intel/iavf/iavf_hash.c         | 99 +++++++---------------
 drivers/net/intel/iavf/iavf_ipsec_crypto.c |  1 -
 7 files changed, 97 insertions(+), 137 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h
index 1f277eeee29..8b826662c62 100644
--- a/drivers/net/intel/iavf/iavf.h
+++ b/drivers/net/intel/iavf/iavf.h
@@ -155,10 +155,6 @@ struct iavf_fdir_conf {
 	uint32_t mark_flag;
 };
 
-struct iavf_fdir_info {
-	struct iavf_fdir_conf conf;
-};
-
 struct iavf_fsub_conf {
 	struct virtchnl_flow_sub sub_fltr;
 	struct virtchnl_flow_unsub unsub_fltr;
@@ -286,7 +282,6 @@ struct iavf_info {
 	struct iavf_parser_list dist_parser_list;
 	struct iavf_parser_list ipsec_crypto_parser_list;
 
-	struct iavf_fdir_info fdir; /* flow director info */
 	/* indicate large VF support enabled or not */
 	bool lv_enabled;
 
diff --git a/drivers/net/intel/iavf/iavf_fdir.c b/drivers/net/intel/iavf/iavf_fdir.c
index 67003183bbd..7b9d3aac7fa 100644
--- a/drivers/net/intel/iavf/iavf_fdir.c
+++ b/drivers/net/intel/iavf/iavf_fdir.c
@@ -353,36 +353,20 @@ iavf_fdir_create(struct iavf_adapter *ad,
 		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 for fdir rule");
-		return -rte_errno;
-	}
-
 	ret = iavf_fdir_add(ad, filter);
 	if (ret) {
-		rte_flow_error_set(error, -ret,
+		return rte_flow_error_set(error, -ret,
 				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				"Failed to add filter rule.");
-		goto free_entry;
 	}
 
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 1);
 
-	memcpy(rule, filter, sizeof(*rule));
-	flow->rule = rule;
-
+	flow->rule = filter;
 	return 0;
-
-free_entry:
-	rte_free(rule);
-	return -rte_errno;
 }
 
 static int
@@ -406,9 +390,6 @@ iavf_fdir_destroy(struct iavf_adapter *ad,
 	if (filter->mark_flag == 1)
 		iavf_fdir_rx_proc_enable(ad, 0);
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return 0;
 }
 
@@ -1444,14 +1425,19 @@ iavf_fdir_action_check_qregion(struct iavf_adapter *ad,
 	return 0;
 }
 
+struct iavf_fdir_parse_ctx {
+	struct iavf_adapter *ad;
+	struct iavf_fdir_conf *filter;
+};
+
 static int
 iavf_fdir_parse_action_check(const struct ci_flow_actions *actions,
 		const struct ci_flow_actions_check_param *param,
 		struct rte_flow_error *error)
 {
-	struct iavf_adapter *ad = param->driver_ctx;
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	struct iavf_fdir_conf *filter = &vf->fdir.conf;
+	struct iavf_fdir_parse_ctx *ctx = param->driver_ctx;
+	struct iavf_adapter *ad = ctx->ad;
+	struct iavf_fdir_conf *filter = ctx->filter;
 	uint32_t dest_num = 0, mark_num = 0;
 	size_t i, number = 0;
 	bool has_drop = false;
@@ -1557,6 +1543,8 @@ iavf_fdir_parse(struct iavf_adapter *ad,
 		struct rte_flow_error *error)
 {
 	struct ci_flow_actions parsed_actions = {0};
+	struct iavf_fdir_conf *filter;
+	struct iavf_fdir_parse_ctx ctx;
 	struct ci_flow_actions_check_param param = {
 		.allowed_types = (enum rte_flow_action_type[]){
 			RTE_FLOW_ACTION_TYPE_PASSTHRU,
@@ -1568,38 +1556,52 @@ iavf_fdir_parse(struct iavf_adapter *ad,
 		},
 		.max_actions = 2,
 		.check = iavf_fdir_parse_action_check,
-		.driver_ctx = ad,
 		.rss_queues_contig = true,
 	};
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	struct iavf_fdir_conf *filter = &vf->fdir.conf;
 	struct iavf_pattern_match_item *item = NULL;
 	int ret;
 
-	memset(filter, 0, sizeof(*filter));
+	filter = rte_zmalloc("iavf_fdir_meta", sizeof(*filter), 0);
+	if (filter == NULL) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Failed to allocate FDIR metadata.");
+		return -rte_errno;
+	}
+
+	ctx.ad = ad;
+	ctx.filter = filter;
+	param.driver_ctx = &ctx;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (!item)
-		return -rte_errno;
+	if (item == NULL) {
+		ret = -rte_errno;
+		goto error;
+	}
 
 	ret = iavf_fdir_parse_pattern(ad, pattern, item->input_set_mask,
 				      error, filter);
 	if (ret)
 		goto error;
 
-	if (meta)
+	rte_free(item);
+	if (meta != NULL)
 		*meta = filter;
+	else
+		rte_free(filter);
+	return ret;
 
 error:
 	rte_free(item);
+	rte_free(filter);
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_fsub.c b/drivers/net/intel/iavf/iavf_fsub.c
index 19ae0e2dc2e..e4e0e20c4fa 100644
--- a/drivers/net/intel/iavf/iavf_fsub.c
+++ b/drivers/net/intel/iavf/iavf_fsub.c
@@ -73,34 +73,18 @@ iavf_fsub_create(struct iavf_adapter *ad, struct rte_flow *flow,
 		 void *meta, struct rte_flow_error *error)
 {
 	struct iavf_fsub_conf *filter = meta;
-	struct iavf_fsub_conf *rule;
 	int ret;
 
-	rule = rte_zmalloc("fsub_entry", sizeof(*rule), 0);
-	if (!rule) {
-		rte_flow_error_set(error, ENOMEM,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Failed to allocate memory for fsub rule");
-		return -rte_errno;
-	}
-
 	ret = iavf_flow_sub(ad, filter);
 	if (ret) {
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to subscribe flow rule.");
-		goto free_entry;
+		return -rte_errno;
 	}
 
-	memcpy(rule, filter, sizeof(*rule));
-	flow->rule = rule;
-
-	rte_free(meta);
+	flow->rule = filter;
 	return ret;
-
-free_entry:
-	rte_free(rule);
-	return -rte_errno;
 }
 
 static int
@@ -120,9 +104,6 @@ iavf_fsub_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		return -rte_errno;
 	}
 
-	flow->rule = NULL;
-	rte_free(filter);
-
 	return ret;
 }
 
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index cd4d4a012b1..1593ea297c2 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -46,6 +46,13 @@ static int iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		FILE *file,
 		struct rte_flow_error *error);
 
+static void
+iavf_flow_free(struct rte_flow *flow)
+{
+	rte_free(flow->rule);
+	rte_free(flow);
+}
+
 const struct rte_flow_ops iavf_flow_ops = {
 	.validate = iavf_flow_validate,
 	.create = iavf_flow_create,
@@ -1856,9 +1863,7 @@ iavf_flow_uninit(struct iavf_adapter *ad)
 	/* Remove all flows */
 	while ((p_flow = TAILQ_FIRST(&vf->flow_list))) {
 		TAILQ_REMOVE(&vf->flow_list, p_flow, node);
-		if (p_flow->engine->free)
-			p_flow->engine->free(p_flow);
-		rte_free(p_flow);
+		iavf_flow_free(p_flow);
 	}
 
 	/* Cleanup parser list */
@@ -2092,6 +2097,7 @@ iavf_parse_engine_create(struct iavf_adapter *ad,
 	void *meta = NULL;
 
 	RTE_TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
+		meta = NULL;
 		if (parser_node->parser->parse_pattern_action(ad,
 				parser_node->parser->array,
 				parser_node->parser->array_len,
@@ -2101,8 +2107,17 @@ iavf_parse_engine_create(struct iavf_adapter *ad,
 		engine = parser_node->parser->engine;
 
 		RTE_ASSERT(engine->create != NULL);
-		if (!(engine->create(ad, flow, meta, error)))
+		if (engine->create(ad, flow, meta, error) == 0) {
+			if (flow->rule != meta)
+				rte_free(meta);
 			return engine;
+		}
+
+		if (flow->rule != NULL && flow->rule != meta)
+			rte_free(flow->rule);
+		rte_free(meta);
+		flow->rule = NULL;
+		meta = NULL;
 	}
 	return NULL;
 }
@@ -2120,8 +2135,10 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 	struct iavf_flow_parser_node *parser_node;
 	void *temp;
 	void *meta = NULL;
+	int ret;
 
 	RTE_TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
+		meta = NULL;
 		if (parser_node->parser->parse_pattern_action(ad,
 				parser_node->parser->array,
 				parser_node->parser->array_len,
@@ -2130,13 +2147,18 @@ iavf_parse_engine_validate(struct iavf_adapter *ad,
 
 		engine = parser_node->parser->engine;
 		if (engine->validation == NULL) {
+			rte_free(meta);
+			meta = NULL;
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation not support");
 			continue;
 		}
 
-		if (engine->validation(ad, flow, meta, error)) {
+		ret = engine->validation(ad, flow, meta, error);
+		rte_free(meta);
+		meta = NULL;
+		if (ret != 0) {
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL, "Validation failed");
@@ -2238,7 +2260,7 @@ iavf_flow_create(struct rte_eth_dev *dev,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "Failed to create flow");
-		rte_free(flow);
+		iavf_flow_free(flow);
 		flow = NULL;
 		goto free_flow;
 	}
@@ -2308,7 +2330,7 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
-		rte_free(flow);
+		iavf_flow_free(flow);
 	} else {
 		PMD_DRV_LOG(ERR, "Failed to destroy flow");
 	}
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 6665d688183..3a8f5f92e8b 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -465,7 +465,6 @@ typedef int (*engine_query_t)(struct iavf_adapter *ad,
 		struct rte_flow *flow,
 		struct rte_flow_query_count *count,
 		struct rte_flow_error *error);
-typedef void (*engine_free_t) (struct rte_flow *flow);
 typedef int (*parse_pattern_action_t)(struct iavf_adapter *ad,
 		struct iavf_pattern_match_item *array,
 		uint32_t array_len,
@@ -494,7 +493,6 @@ struct iavf_flow_engine {
 	engine_create_t create;
 	engine_destroy_t destroy;
 	engine_query_t query_count;
-	engine_free_t free;
 	const char *name;
 	enum iavf_flow_engine_type type;
 	size_t rule_size;
diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index 00b616f34ba..6352bf28f93 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -58,12 +58,6 @@ struct iavf_hash_match_type {
 	uint64_t pattern_hint;
 };
 
-struct iavf_rss_meta {
-	struct virtchnl_proto_hdrs proto_hdrs;
-	enum virtchnl_rss_algorithm rss_algorithm;
-	bool raw_ena;
-};
-
 struct iavf_hash_flow_cfg {
 	struct virtchnl_rss_cfg *rss_cfg;
 	bool simple_xor;
@@ -79,8 +73,6 @@ iavf_hash_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
 		  struct rte_flow_error *error);
 static void
 iavf_hash_uninit_parser(struct iavf_adapter *ad);
-static void
-iavf_hash_free(struct rte_flow *flow);
 static int
 iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 			       struct iavf_pattern_match_item *array,
@@ -682,7 +674,6 @@ static struct iavf_flow_engine iavf_hash_engine = {
 	.create = iavf_hash_create,
 	.destroy = iavf_hash_destroy,
 	.uninit = iavf_hash_uninit_parser,
-	.free = iavf_hash_free,
 	.name = "hash",
 	.type = IAVF_FLOW_ENGINE_HASH,
 	.rule_size = sizeof(struct virtchnl_rss_cfg),
@@ -886,7 +877,7 @@ iavf_hash_parse_pattern(const struct rte_flow_item pattern[], uint64_t *phint,
 static int
 iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			struct rte_flow_error *error,
-			struct iavf_rss_meta *meta)
+			struct virtchnl_rss_cfg *rss_cfg)
 {
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
 	uint16_t spec_len, pkt_len;
@@ -958,9 +949,9 @@ iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			msk_buf[j] = tmp_val * 16 + tmp_c - '0';
 	}
 
-	memcpy(meta->proto_hdrs.raw.spec, pkt_buf, pkt_len);
-	memcpy(meta->proto_hdrs.raw.mask, msk_buf, pkt_len);
-	meta->proto_hdrs.raw.pkt_len = pkt_len;
+	memcpy(rss_cfg->proto_hdrs.raw.spec, pkt_buf, pkt_len);
+	memcpy(rss_cfg->proto_hdrs.raw.mask, msk_buf, pkt_len);
+	rss_cfg->proto_hdrs.raw.pkt_len = pkt_len;
 
 	rte_free(pkt_buf);
 	rte_free(msk_buf);
@@ -1433,12 +1424,12 @@ iavf_any_invalid_rss_type(enum rte_eth_hash_function rss_func,
 static int
 iavf_hash_parse_rss_type(struct iavf_pattern_match_item *match_item,
 		const struct rte_flow_action_rss *rss,
-		uint64_t pattern_hint, struct iavf_rss_meta *rss_meta,
+		uint64_t pattern_hint, struct virtchnl_rss_cfg *rss_cfg,
 		struct rte_flow_error *error)
 {
 	uint64_t rss_type;
 
-	rss_meta->rss_algorithm = rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ ?
+	rss_cfg->rss_algorithm = rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ ?
 		VIRTCHNL_RSS_ALG_TOEPLITZ_SYMMETRIC :
 		VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 
@@ -1458,9 +1449,9 @@ iavf_hash_parse_rss_type(struct iavf_pattern_match_item *match_item,
 				"RSS type not supported");
 	}
 
-	memcpy(&rss_meta->proto_hdrs, match_item->meta, sizeof(struct virtchnl_proto_hdrs));
+	memcpy(&rss_cfg->proto_hdrs, match_item->meta, sizeof(rss_cfg->proto_hdrs));
 
-	iavf_refine_proto_hdrs(&rss_meta->proto_hdrs, rss_type, pattern_hint);
+	iavf_refine_proto_hdrs(&rss_cfg->proto_hdrs, rss_type, pattern_hint);
 
 	return 0;
 }
@@ -1526,7 +1517,7 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	};
 	const struct rte_flow_action_rss *rss;
 	struct iavf_pattern_match_item *pattern_match_item;
-	struct iavf_rss_meta *rss_meta_ptr;
+	struct virtchnl_rss_cfg *rss_cfg;
 	uint64_t phint = IAVF_PHINT_NONE;
 	int ret = 0;
 
@@ -1538,12 +1529,12 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	if (ret)
 		return ret;
 
-	rss_meta_ptr = rte_zmalloc(NULL, sizeof(*rss_meta_ptr), 0);
-	if (!rss_meta_ptr) {
+	rss_cfg = rte_zmalloc("iavf_rss_rule", sizeof(*rss_cfg), 0);
+	if (rss_cfg == NULL) {
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for rss_meta_ptr");
-		return -ENOMEM;
+				   "No memory for RSS rule.");
+		return -rte_errno;
 	}
 
 	/* Check rss supported pattern and find matched pattern. */
@@ -1560,8 +1551,7 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 		goto error;
 
 	if (phint == IAVF_PHINT_RAW) {
-		rss_meta_ptr->raw_ena = true;
-		ret = iavf_hash_parse_raw_pattern(pattern, error, rss_meta_ptr);
+		ret = iavf_hash_parse_raw_pattern(pattern, error, rss_cfg);
 		if (ret) {
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM, NULL,
@@ -1571,13 +1561,13 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 	}
 
 	rss = parsed_actions.actions[0]->conf;
-	ret = iavf_hash_parse_rss_type(pattern_match_item, rss, phint, rss_meta_ptr, error);
+	ret = iavf_hash_parse_rss_type(pattern_match_item, rss, phint, rss_cfg, error);
 
 error:
 	if (!ret && meta)
-		*meta = rss_meta_ptr;
+		*meta = rss_cfg;
 	else
-		rte_free(rss_meta_ptr);
+		rte_free(rss_cfg);
 
 	rte_free(pattern_match_item);
 
@@ -1585,27 +1575,13 @@ iavf_hash_parse_pattern_action(struct iavf_adapter *ad,
 }
 
 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)
+iavf_hash_create(struct iavf_adapter *ad,
+		 struct rte_flow *flow, void *meta,
+		 struct rte_flow_error *error)
 {
-	struct iavf_rss_meta *rss_meta = (struct iavf_rss_meta *)meta;
-	struct virtchnl_rss_cfg *rss_cfg;
+	struct virtchnl_rss_cfg *rss_cfg = meta;
 	int ret = 0;
 
-	rss_cfg = rte_zmalloc("iavf rss rule",
-			      sizeof(struct virtchnl_rss_cfg), 0);
-	if (!rss_cfg) {
-		rte_flow_error_set(error, ENOMEM,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for rss rule");
-		rte_free(meta);
-		return -ENOMEM;
-	}
-
-	rss_cfg->proto_hdrs = rss_meta->proto_hdrs;
-	rss_cfg->rss_algorithm = rss_meta->rss_algorithm;
-
 	ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
 	if (!ret) {
 		flow->rule = rss_cfg;
@@ -1614,13 +1590,9 @@ iavf_hash_create(__rte_unused struct iavf_adapter *ad,
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Failed to add rss rule.");
-		rte_free(rss_cfg);
-		rte_free(meta);
 		return -rte_errno;
 	}
 
-	rte_free(meta);
-
 	return ret;
 }
 
@@ -1633,22 +1605,19 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
 	struct virtchnl_rss_cfg *rss_cfg;
 	int ret = 0;
 
+	if (vf->vf_reset)
+		return 0;
+
 	rss_cfg = (struct virtchnl_rss_cfg *)flow->rule;
 
-	if (!vf->vf_reset) {
-		ret = iavf_add_del_rss_cfg(ad, rss_cfg, false);
-		if (ret) {
-			PMD_DRV_LOG(ERR, "fail to del RSS configure");
-			rte_flow_error_set(error, -ret,
-					   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-					   "Failed to delete rss rule.");
-			return -rte_errno;
-		}
+	ret = iavf_add_del_rss_cfg(ad, rss_cfg, false);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "fail to del RSS configure");
+		rte_flow_error_set(error, -ret,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Failed to delete rss rule.");
+		return -rte_errno;
 	}
-
-	flow->rule = NULL;
-	rte_free(rss_cfg);
-
 	return ret;
 }
 
@@ -1677,9 +1646,3 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
 		PMD_DRV_LOG(ERR, "fail to delete default RSS");
 }
-
-static void
-iavf_hash_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-}
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index b4813303464..e9acd1bb1dd 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1919,7 +1919,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 
 	iavf_ipsec_crypto_security_policy_delete(ad,
 			ipsec_flow->is_ipv4, ipsec_flow->id);
-	rte_free(ipsec_flow);
 	return 0;
 }
 
-- 
2.52.0

Re: [PATCH v4 06/10] net/iavf: centralize memory management for flows

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:46PM +0100, Anatoly Burakov wrote:
> Flow engines currently distribute rule allocation and cleanup between the
> parser, engine create and destroy callbacks, and generic flow code. This
> leads to a lot of potential for memory leaks and bugs. In particular, one
> of the issues currently present is that when PF initiates reset, we are
> meant to just drop all flows without sending any virtchnl commands, but
> because not all engines implement the `free` callback, some flows will be
> leaked.
> 
> To address that specific leak as well as all other potential leaks due to
> misuse of memory by engines, use parser-allocated rule objects as the
> persistent rules and centralize their lifetime in the generic flow layer.
> This requires fixing all engines to not manage their own memory:
> 
> - Allocate the final FDIR rule in the parser, use it for action checks,
>   and remove the per-VF scratch configuration.
> - Have FSUB create adopt its parser-produced rule instead of allocating
>   and copying a second object.
> - Build the final RSS configuration in the parser and remove the
>   intermediate RSS metadata, including its unused raw-pattern flag.
> - Let generic flow code free rules on validate, create failure,
>   successful destroy, and uninit; remove engine-specific free callbacks
>   and rule frees from destroy callbacks.
> 
> Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
> Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf.h              |  5 --
>  drivers/net/intel/iavf/iavf_fdir.c         | 68 +++++++--------
>  drivers/net/intel/iavf/iavf_fsub.c         | 23 +----
>  drivers/net/intel/iavf/iavf_generic_flow.c | 36 ++++++--
>  drivers/net/intel/iavf/iavf_generic_flow.h |  2 -
>  drivers/net/intel/iavf/iavf_hash.c         | 99 +++++++---------------
>  drivers/net/intel/iavf/iavf_ipsec_crypto.c |  1 -
>  7 files changed, 97 insertions(+), 137 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 07/10] net/iavf: skip RSS teardown during PF-initiated reset

[Anatoly Burakov]

During PF-initiated reset recovery, `iavf_dev_close()` skips flow flush
to avoid sending virtchnl messages to the PF. However, `iavf_hash_uninit()`
is still called unconditionally, even though any existing RSS configuration
is now invalid because of PF-initiated reset.

Move the RSS teardown under the same `pf_reset_in_progress` check as flow
flush, so that no RSS configuration messages are sent during PF-initiated
reset recovery.

Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 891880cf6e6..26f891515ec 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -3246,12 +3246,12 @@ iavf_dev_close(struct rte_eth_dev *dev)
 	/* free iAVF security device context all related resources */
 	iavf_security_ctx_destroy(adapter);
 
-	/* remove RSS configuration */
-	iavf_hash_uninit(adapter);
-
 	/* Skip the virtchnl-emitting teardown on a PF-initiated reset */
-	if (!vf->pf_reset_in_progress)
+	if (!vf->pf_reset_in_progress) {
+		/* remove RSS configuration */
+		iavf_hash_uninit(adapter);
 		iavf_flow_flush(dev, NULL);
+	}
 	iavf_flow_uninit(adapter);
 
 	/*
-- 
2.52.0

Re: [PATCH v4 07/10] net/iavf: skip RSS teardown during PF-initiated reset

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:47PM +0100, Anatoly Burakov wrote:
> During PF-initiated reset recovery, `iavf_dev_close()` skips flow flush
> to avoid sending virtchnl messages to the PF. However, `iavf_hash_uninit()`
> is still called unconditionally, even though any existing RSS configuration
> is now invalid because of PF-initiated reset.
> 
> Move the RSS teardown under the same `pf_reset_in_progress` check as flow
> flush, so that no RSS configuration messages are sent during PF-initiated
> reset recovery.
> 
> Fixes: 4dad1a6815d6 ("net/iavf: skip flow flush during PF-initiated reset")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_ethdev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 08/10] net/iavf: remove redundant reset checks from RSS engine

[Anatoly Burakov]

The RSS engine checks `vf_reset` during init, destroy and uninit to avoid
sending virtchnl commands while the VF is resetting. These checks are no
longer needed:

- `iavf_hash_init()` only registers the parser and sends no commands.
- `iavf_dev_close()` skips both flow flush and RSS teardown during
  PF-initiated reset recovery.
- the generic flow layer frees rule memory without relying on engine
  callbacks.
- `iavf_execute_vf_cmd()` already refuses to send commands while
  `vf_reset` is set.

Remove these checks as they are no longer needed.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/iavf/iavf_hash.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
index 6352bf28f93..fe5d725064b 100644
--- a/drivers/net/intel/iavf/iavf_hash.c
+++ b/drivers/net/intel/iavf/iavf_hash.c
@@ -762,9 +762,6 @@ iavf_hash_init(struct iavf_adapter *ad)
 	struct iavf_flow_parser *parser;
 	int ret;
 
-	if (vf->vf_reset)
-		return -EIO;
-
 	if (!vf->vf_res)
 		return -EINVAL;
 
@@ -1601,13 +1598,9 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
 		  struct rte_flow *flow,
 		  __rte_unused struct rte_flow_error *error)
 {
-	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
 	struct virtchnl_rss_cfg *rss_cfg;
 	int ret = 0;
 
-	if (vf->vf_reset)
-		return 0;
-
 	rss_cfg = (struct virtchnl_rss_cfg *)flow->rule;
 
 	ret = iavf_add_del_rss_cfg(ad, rss_cfg, false);
@@ -1633,9 +1626,6 @@ iavf_hash_uninit(struct iavf_adapter *ad)
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
 	struct rte_eth_rss_conf *rss_conf;
 
-	if (vf->vf_reset)
-		return;
-
 	if (!vf->vf_res)
 		return;
 
-- 
2.52.0

Re: [PATCH v4 08/10] net/iavf: remove redundant reset checks from RSS engine

[Bruce Richardson]

On Wed, Oct 21, 2026 at 03:25:48PM +0100, Anatoly Burakov wrote:
> The RSS engine checks `vf_reset` during init, destroy and uninit to avoid
> sending virtchnl commands while the VF is resetting. These checks are no
> longer needed:
> 
> - `iavf_hash_init()` only registers the parser and sends no commands.
> - `iavf_dev_close()` skips both flow flush and RSS teardown during
>   PF-initiated reset recovery.
> - the generic flow layer frees rule memory without relying on engine
>   callbacks.
> - `iavf_execute_vf_cmd()` already refuses to send commands while
>   `vf_reset` is set.
> 
> Remove these checks as they are no longer needed.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_hash.c | 10 ----------
>  1 file changed, 10 deletions(-)
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v4 09/10] net/iavf: fix null engine for egress security flows

[Anatoly Burakov]

Egress security flows bypass the flow parsers and are inserted with a
NULL `engine` pointer. The create path then dereferences that pointer
when logging the new flow, and other engine-based operations cannot
handle the flow consistently.

Register an egress security engine and route these flows through the
normal parser and lifecycle callbacks. Egress flows own no rule and
require no hardware teardown.

Fixes: e783da175250 ("net/iavf: fix memory leak on egress IPsec flows")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/intel/iavf/iavf_generic_flow.c |  44 +++-----
 drivers/net/intel/iavf/iavf_generic_flow.h |   2 +-
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 120 +++++++++++++++++++--
 3 files changed, 123 insertions(+), 43 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 1593ea297c2..be7ddf5db63 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1914,7 +1914,8 @@ iavf_register_parser(struct iavf_flow_parser *parser,
 			}
 		}
 		TAILQ_INSERT_HEAD(list, parser_node, node);
-	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO) {
+	} else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		   parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS) {
 		list = &vf->ipsec_crypto_parser_list;
 		TAILQ_INSERT_HEAD(list, parser_node, node);
 	} else if (parser->engine->type == IAVF_FLOW_ENGINE_FSUB) {
@@ -1943,7 +1944,8 @@ iavf_unregister_parser(struct iavf_flow_parser *parser,
 	else if ((parser->engine->type == IAVF_FLOW_ENGINE_FDIR) ||
 		 (parser->engine->type == IAVF_FLOW_ENGINE_FSUB))
 		list = &vf->dist_parser_list;
-	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO)
+	else if (parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_CRYPTO ||
+		 parser->engine->type == IAVF_FLOW_ENGINE_IPSEC_EGRESS)
 		list = &vf->ipsec_crypto_parser_list;
 
 	if (list == NULL)
@@ -2250,12 +2252,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 		return flow;
 	}
 
-	/* Special case for inline crypto egress flows */
-	if (attr->egress && actions[0].type == RTE_FLOW_ACTION_TYPE_SECURITY) {
-		flow->is_ipsec_egress_flow = true;
-		goto tailq_insert;
-	}
-
 	ret = iavf_flow_process_filter(dev, flow, attr, pattern, actions,
 			&engine, iavf_parse_engine_create, error);
 	if (ret < 0) {
@@ -2266,7 +2262,6 @@ iavf_flow_create(struct rte_eth_dev *dev,
 	}
 
 	flow->engine = engine;
-tailq_insert:
 	rte_spinlock_lock(&vf->flow_ops_lock);
 	TAILQ_INSERT_TAIL(&vf->flow_list, flow, node);
 	rte_spinlock_unlock(&vf->flow_ops_lock);
@@ -2282,14 +2277,7 @@ iavf_flow_is_valid(struct rte_flow *flow)
 	struct iavf_flow_engine *engine;
 	void *temp;
 
-	if (flow == NULL)
-		return false;
-
-	/* these flows don't use engines */
-	if (flow->is_ipsec_egress_flow)
-		return true;
-
-	if (flow->engine) {
+	if (flow != NULL && flow->engine != NULL) {
 		RTE_TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
 			if (engine == flow->engine)
 				return true;
@@ -2307,26 +2295,17 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 	struct iavf_adapter *ad =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
-	bool need_destroy;
 	int ret = 0;
 
-	if (!iavf_flow_is_valid(flow)) {
+	if (!iavf_flow_is_valid(flow) || flow->engine->destroy == NULL) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "Invalid flow destroy");
 		return -rte_errno;
 	}
-	need_destroy = !flow->is_ipsec_egress_flow;
-
-	if (need_destroy && flow->engine->destroy == NULL) {
-		return rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"Invalid flow destroy");
-	}
 
 	rte_spinlock_lock(&vf->flow_ops_lock);
-	if (need_destroy)
-		ret = flow->engine->destroy(ad, flow, error);
+	ret = flow->engine->destroy(ad, flow, error);
 
 	if (!ret) {
 		TAILQ_REMOVE(&vf->flow_list, flow, node);
@@ -2444,16 +2423,17 @@ iavf_flow_dev_dump(struct rte_eth_dev *dev,
 		if (flow != NULL && f != flow)
 			continue;
 
-		/* this should not happen */
-		if (f->engine == NULL || f->rule == NULL) {
+		if (!iavf_flow_is_valid(f)) {
 			PMD_DRV_LOG(DEBUG, "Invalid flow");
 			continue;
 		}
 
 		found = true;
 
-		rule_size = f->engine->rule_size;
-		rule_data = f->rule;
+		if (f->rule != NULL) {
+			rule_size = f->engine->rule_size;
+			rule_data = f->rule;
+		}
 
 		iavf_flow_dump_blob(file,
 			f->engine->name != NULL ?
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.h b/drivers/net/intel/iavf/iavf_generic_flow.h
index 3a8f5f92e8b..75bb92900b8 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.h
+++ b/drivers/net/intel/iavf/iavf_generic_flow.h
@@ -481,6 +481,7 @@ enum iavf_flow_engine_type {
 	IAVF_FLOW_ENGINE_FDIR,
 	IAVF_FLOW_ENGINE_HASH,
 	IAVF_FLOW_ENGINE_FSUB,
+	IAVF_FLOW_ENGINE_IPSEC_EGRESS,
 	IAVF_FLOW_ENGINE_MAX,
 };
 
@@ -503,7 +504,6 @@ TAILQ_HEAD(iavf_engine_list, iavf_flow_engine);
 /* Struct to store flow created. */
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) node;
-	bool is_ipsec_egress_flow;
 	struct iavf_flow_engine *engine;
 	void *rule;
 };
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index e9acd1bb1dd..f7e391a74a0 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1832,6 +1832,7 @@ iavf_ipsec_flow_item_parse(struct iavf_adapter *adapter,
 
 
 static struct iavf_flow_parser iavf_ipsec_flow_parser;
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser;
 
 static int
 iavf_ipsec_flow_init(struct iavf_adapter *ad)
@@ -1922,16 +1923,6 @@ iavf_ipsec_flow_destroy(struct iavf_adapter *ad,
 	return 0;
 }
 
-static struct iavf_flow_engine iavf_ipsec_flow_engine = {
-	.init = iavf_ipsec_flow_init,
-	.uninit = iavf_ipsec_flow_uninit,
-	.create = iavf_ipsec_flow_create,
-	.destroy = iavf_ipsec_flow_destroy,
-	.name = "ipsec_crypto",
-	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
-	.rule_size = sizeof(struct iavf_ipsec_flow_item),
-};
-
 static int
 iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		      struct iavf_pattern_match_item *array,
@@ -1975,6 +1966,105 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 	return ret;
 }
 
+/*
+ * Inline crypto egress flows only associate Tx traffic with a security
+ * session; nothing is programmed in hardware, so the engine owns no rule.
+ */
+static int
+iavf_ipsec_egress_flow_init(struct iavf_adapter *ad)
+{
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
+
+	if (!vf->vf_res)
+		return -EINVAL;
+
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_INLINE_IPSEC_CRYPTO))
+		return -ENOTSUP;
+
+	return iavf_register_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static void
+iavf_ipsec_egress_flow_uninit(struct iavf_adapter *ad)
+{
+	iavf_unregister_parser(&iavf_ipsec_egress_flow_parser, ad);
+}
+
+static int
+iavf_ipsec_egress_flow_create(__rte_unused struct iavf_adapter *ad,
+		struct rte_flow *flow,
+		__rte_unused void *meta,
+		__rte_unused struct rte_flow_error *error)
+{
+	flow->rule = NULL;
+	return 0;
+}
+
+static int
+iavf_ipsec_egress_flow_destroy(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct rte_flow *flow,
+		__rte_unused struct rte_flow_error *error)
+{
+	return 0;
+}
+
+/* The pattern is not used, as there is nothing to match in hardware. */
+static int
+iavf_ipsec_egress_flow_parse(__rte_unused struct iavf_adapter *ad,
+		__rte_unused struct iavf_pattern_match_item *array,
+		__rte_unused uint32_t array_len,
+		__rte_unused const struct rte_flow_item pattern[],
+		const struct rte_flow_action actions[],
+		const struct rte_flow_attr *attr,
+		void **meta,
+		struct rte_flow_error *error)
+{
+	struct ci_flow_actions parsed_actions = {0};
+	struct ci_flow_actions_check_param param = {
+		.allowed_types = (enum rte_flow_action_type[]){
+			RTE_FLOW_ACTION_TYPE_SECURITY,
+			RTE_FLOW_ACTION_TYPE_END,
+		},
+		.max_actions = 1,
+	};
+	struct ci_flow_attr_check_param attr_param = {
+		.require_egress = true,
+	};
+	int ret;
+
+	ret = ci_flow_check_attr(attr, &attr_param, error);
+	if (ret)
+		return ret;
+
+	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
+	if (ret < 0)
+		return ret;
+
+	if (meta != NULL)
+		*meta = NULL;
+
+	return 0;
+}
+
+static struct iavf_flow_engine iavf_ipsec_flow_engine = {
+	.init = iavf_ipsec_flow_init,
+	.uninit = iavf_ipsec_flow_uninit,
+	.create = iavf_ipsec_flow_create,
+	.destroy = iavf_ipsec_flow_destroy,
+	.name = "ipsec_crypto",
+	.type = IAVF_FLOW_ENGINE_IPSEC_CRYPTO,
+	.rule_size = sizeof(struct iavf_ipsec_flow_item),
+};
+
+static struct iavf_flow_engine iavf_ipsec_egress_flow_engine = {
+	.init = iavf_ipsec_egress_flow_init,
+	.uninit = iavf_ipsec_egress_flow_uninit,
+	.create = iavf_ipsec_egress_flow_create,
+	.destroy = iavf_ipsec_egress_flow_destroy,
+	.name = "ipsec_egress",
+	.type = IAVF_FLOW_ENGINE_IPSEC_EGRESS,
+};
+
 static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.engine = &iavf_ipsec_flow_engine,
 	.array = iavf_ipsec_flow_pattern,
@@ -1982,7 +2072,17 @@ static struct iavf_flow_parser iavf_ipsec_flow_parser = {
 	.parse_pattern_action = iavf_ipsec_flow_parse,
 };
 
+static struct iavf_flow_parser iavf_ipsec_egress_flow_parser = {
+	.engine = &iavf_ipsec_egress_flow_engine,
+	.parse_pattern_action = iavf_ipsec_egress_flow_parse,
+};
+
 RTE_INIT(iavf_ipsec_flow_engine_register)
 {
 	iavf_register_flow_engine(&iavf_ipsec_flow_engine);
 }
+
+RTE_INIT(iavf_ipsec_egress_flow_engine_register)
+{
+	iavf_register_flow_engine(&iavf_ipsec_egress_flow_engine);
+}
-- 
2.52.0

[PATCH v4 10/10] net/iavf: fix IPsec flow metadata leaks

[Anatoly Burakov]

IPsec flow parsing leaves the pattern-match item allocated after
extracting its metadata. It also leaves the parsed flow item allocated
when no metadata output is requested.

Free the pattern-match item after parsing and free the parsed flow item
when it cannot be returned to the caller. Propagate pattern-match failure
and report a parsing error when no flow item was produced.

Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/intel/iavf/iavf_ipsec_crypto.c | 28 +++++++++++++++-------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index f7e391a74a0..be0a0d0df6e 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1942,6 +1942,7 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		.max_actions = 1,
 	};
 	struct iavf_pattern_match_item *item = NULL;
+	struct iavf_ipsec_flow_item *fi = NULL;
 	int ret = -1;
 
 	ret = ci_flow_check_attr(attr, NULL, error);
@@ -1953,17 +1954,28 @@ iavf_ipsec_flow_parse(struct iavf_adapter *ad,
 		return ret;
 
 	item = iavf_search_pattern_match_item(pattern, array, array_len, error);
-	if (item && item->meta) {
+	if (item == NULL)
+		return -rte_errno;
+
+	if (item->meta != NULL) {
 		const struct rte_security_session *session = parsed_actions.actions[0]->conf;
 		uint32_t type = (uint64_t)(item->meta);
-		struct iavf_ipsec_flow_item *fi =
-				iavf_ipsec_flow_item_parse(ad, pattern, session, type);
-		if (fi && meta) {
-			*meta = fi;
-			ret = 0;
-		}
+
+		fi = iavf_ipsec_flow_item_parse(ad, pattern, session, type);
 	}
-	return ret;
+	rte_free(item);
+
+	if (fi == NULL)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+				"Invalid IPsec flow.");
+
+	if (meta != NULL)
+		*meta = fi;
+	else
+		rte_free(fi);
+
+	return 0;
 }
 
 /*
-- 
2.52.0