Centralize flow memory management in ICE

ACL entry creation reserves a bitmap slot before programming hardware. If programming fails, the reserved slot is not restored, so repeated failures consume the available slots. The slot is also left reserved when it exceeds the supported entry limit.

Restore the bitmap slot on both failure paths. Keep it reserved only when entry creation succeeds.

Fixes 40d466fa9f76 ("net/ice: support ACL filter in DCF")

Cc [email protected]

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/ice_acl_filter.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/net/intel/ice/ice_acl_filter.c +2 -0

index a91bd558f3..eb5d88ccc5 100644
@@ -462,6 +462,7 @@ ice_acl_hw_set_conf(struct ice_pf *pf, struct ice_fdir_fltr *input,
 					 acts, act_cnt, &hw_entry);
 		if (ret) {
 			PMD_DRV_LOG(ERR, "Fail to add entry.");
+			rte_bitmap_set(pf->acl.slots, slot_id);
 			return ret;
 		}
 		rule->entry_id[entry_idx] = entry_id;

@@ -469,6 +470,7 @@ ice_acl_hw_set_conf(struct ice_pf *pf, struct ice_fdir_fltr *input,
 	} else {
 		PMD_DRV_LOG(ERR, "Exceed the maximum entry number(%d)"
 			    " HW supported!", MAX_ACL_NORMAL_ENTRIES);
+		rte_bitmap_set(pf->acl.slots, slot_id);
 		return -1;
 	}
 

Signature · 1 line

2.52.0

FDIR creation programs the hardware rule before inserting its software hash entry. If insertion fails, creation returns without removing the hardware rule or releasing its counter, and MARK parsing may remain enabled despite no flow handle being returned.

Reserve the hash entry before programming the rule, and release the counter if reservation fails. Remove the reserved entry if hardware programming fails. Enable MARK parsing only after both steps succeed.

Fixes 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")

Cc [email protected]

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/ice_fdir_filter.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

drivers/net/intel/ice/ice_fdir_filter.c +10 -8

index 517c32cecf..63a0d9fb1f 100644
@@ -1654,8 +1654,18 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		filter->input.cnt_index = filter->counter->hw_index;
 	}
 
+	/* Reserve the software entry before installing the hardware rule. */
+	ret = ice_fdir_entry_insert(pf, entry, &key);
+	if (ret) {
+		rte_flow_error_set(error, -ret,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Insert entry to table failed.");
+		goto free_counter;
+	}
+
 	ret = ice_fdir_add_del_filter(pf, filter, true);
 	if (ret) {
+		ice_fdir_entry_del(pf, &key);
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Add filter rule failed.");

@@ -1666,14 +1676,6 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		ice_fdir_rx_parsing_enable(ad, 1);
 
 	memcpy(entry, filter, sizeof(*entry));
-	ret = ice_fdir_entry_insert(pf, entry, &key);
-	if (ret) {
-		rte_flow_error_set(error, -ret,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "Insert entry to table failed.");
-		goto free_entry;
-	}
-
 	flow->rule = entry;
 	ice_fdir_cnt_update(pf, filter->input.flow_type, is_tun, true);
 

Signature · 1 line

2.52.0

VOID handling differs between pattern matching and engine parsing. Pattern matching relies on a temporary copy with VOID items removed, while engines handle them independently. This temporary copy makes memory management complicated, so we want to get rid of it.

However, before we can do that, we first need to standardize traversal of the pattern so that all engines can skip over VOID items. Add shared helpers to skip VOID items and iterate over non-VOID items, and use them in pattern matching and all engines; also use them for item counting, lookahead and RAW item lookup.

Signed-off-by Anatoly Burakov <[email protected]>

---

 drivers/net/intel/ice/ice_acl_filter.c    |  4 +---
 drivers/net/intel/ice/ice_fdir_filter.c   |  8 +++-----
 drivers/net/intel/ice/ice_generic_flow.c  | 11 +++++------
 drivers/net/intel/ice/ice_generic_flow.h  | 14 ++++++++++++++
 drivers/net/intel/ice/ice_hash.c          |  6 ++++--
 drivers/net/intel/ice/ice_switch_filter.c |  8 ++------
 6 files changed, 29 insertions(+), 22 deletions(-)

drivers/net/intel/ice/ice_acl_filter.c +1 -3

index eb5d88ccc5..91f684c9b9 100644
@@ -664,7 +664,7 @@ ice_acl_parse_pattern(__rte_unused struct ice_adapter *ad,
 	uint64_t input_set = ICE_INSET_NONE;
 	uint8_t flow_type = ICE_FLTR_PTYPE_NONF_NONE;
 
-	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
 		item_type = item->type;
 
 		switch (item_type) {

@@ -897,8 +897,6 @@ ice_acl_parse_pattern(__rte_unused struct ice_adapter *ad,
 				}
 			}
 			break;
-		case RTE_FLOW_ITEM_TYPE_VOID:
-			break;
 		default:
 			rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ITEM,

drivers/net/intel/ice/ice_fdir_filter.c +3 -5

index 63a0d9fb1f..b1a8753e90 100644
@@ -1904,7 +1904,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 	struct ice_parser_result rslt;
 	uint8_t item_num = 0;
 
-	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
 		if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN)
 			tunnel_type = ICE_FDIR_TUNNEL_TYPE_VXLAN;
 		if (item->type == RTE_FLOW_ITEM_TYPE_L2TPV2)

@@ -1922,7 +1922,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 	/* This loop parse flow pattern and distinguish Non-tunnel and tunnel
 	 * flow. input_set_i is used for inner part.
 	 */
-	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
 		item_type = item->type;
 
 		if (item->last && !(item_type == RTE_FLOW_ITEM_TYPE_IPV4 ||

@@ -2065,7 +2065,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 			if (!rte_is_zero_ether_addr(&eth_mask->hdr.src_addr))
 				*input_set |= ICE_INSET_SMAC;
 
-			next_type = (item + 1)->type;
+			next_type = ice_pattern_skip_void(item + 1)->type;
 			/* Ignore this field except for ICE_FLTR_PTYPE_NON_IP_L2 */
 			if (eth_mask->hdr.ether_type == RTE_BE16(0xffff) &&
 			    next_type == RTE_FLOW_ITEM_TYPE_END) {

@@ -2471,8 +2471,6 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 				p_v6->src_port = sctp_spec->hdr.src_port;
 			}
 			break;
-		case RTE_FLOW_ITEM_TYPE_VOID:
-			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
 			l3 = RTE_FLOW_ITEM_TYPE_END;
 			vxlan_spec = item->spec;

drivers/net/intel/ice/ice_generic_flow.c +5 -6

index 76b3800545..fee048e456 100644
@@ -2109,16 +2109,15 @@ static bool
 ice_match_pattern(enum rte_flow_item_type *item_array,
 		const struct rte_flow_item *pattern)
 {
-	const struct rte_flow_item *item = pattern;
+	const struct rte_flow_item *item;
 
-	while ((*item_array == item->type) &&
-	       (*item_array != RTE_FLOW_ITEM_TYPE_END)) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
+		if (*item_array != item->type)
+			return false;
 		item_array++;
-		item++;
 	}
 
-	return (*item_array == RTE_FLOW_ITEM_TYPE_END &&
-		item->type == RTE_FLOW_ITEM_TYPE_END);
+	return *item_array == RTE_FLOW_ITEM_TYPE_END;
 }
 
 struct ice_ptype_match {

drivers/net/intel/ice/ice_generic_flow.h +14 -0

index 650b43633f..983f86a5ab 100644
@@ -569,6 +569,20 @@ struct ice_flow_parser_node {
 void ice_register_flow_engine(struct ice_flow_engine *engine);
 int ice_flow_init(struct ice_adapter *ad);
 void ice_flow_uninit(struct ice_adapter *ad);
+
+static inline const struct rte_flow_item *
+ice_pattern_skip_void(const struct rte_flow_item *item)
+{
+	while (item->type == RTE_FLOW_ITEM_TYPE_VOID)
+		item++;
+	return item;
+}
+
+#define ICE_FLOW_FOREACH_ITEM(item, pattern) \
+	for ((item) = ice_pattern_skip_void(pattern); \
+	     (item)->type != RTE_FLOW_ITEM_TYPE_END; \
+	     (item) = ice_pattern_skip_void((item) + 1))
+
 struct ice_pattern_match_item *
 ice_search_pattern_match_item(struct ice_adapter *ad,
 			      const struct rte_flow_item pattern[],

drivers/net/intel/ice/ice_hash.c +4 -2

index 7a9a4f1a04..13bedfe90c 100644
@@ -624,7 +624,7 @@ ice_hash_parse_pattern(const struct rte_flow_item pattern[], uint64_t *phint,
 	const struct rte_flow_item *item = pattern;
 	const struct rte_flow_item_gtp_psc *psc;
 
-	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
 		if (item->last) {
 			rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ITEM, item,

@@ -1235,8 +1235,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 		goto error;
 
 	if (phint == ICE_PHINT_RAW) {
+		const struct rte_flow_item *raw_item = ice_pattern_skip_void(pattern);
+
 		rss_meta_ptr->raw.raw_ena = true;
-		ret = ice_hash_parse_raw_pattern(ad, pattern, error, rss_meta_ptr);
+		ret = ice_hash_parse_raw_pattern(ad, raw_item, error, rss_meta_ptr);
 		if (ret) {
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM, NULL,

drivers/net/intel/ice/ice_switch_filter.c +2 -6

index 6f5af03f6a..6bc1bdd09f 100644
@@ -489,8 +489,7 @@ ice_switch_parse_pattern(const struct rte_flow_item pattern[],
 	    *tun_type == ICE_NON_TUN_QINQ)
 		qinq_valid = 1;
 
-	for (item = pattern; item->type !=
-			RTE_FLOW_ITEM_TYPE_END; item++) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
 		if (item->last) {
 			rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ITEM,

@@ -1378,9 +1377,6 @@ ice_switch_parse_pattern(const struct rte_flow_item pattern[],
 			gtpu_psc_valid = 1;
 			break;
 
-		case RTE_FLOW_ITEM_TYPE_VOID:
-			break;
-
 		default:
 			rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_ITEM, pattern,

@@ -1805,7 +1801,7 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad,
 		return -rte_errno;
 	}
 
-	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+	ICE_FLOW_FOREACH_ITEM(item, pattern) {
 		item_num++;
 		if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
 			const struct rte_flow_item_eth *eth_mask;

Signature · 1 line

2.52.0

Pattern lookup allocates a temporary copy of the flow pattern with VOID items removed. Pattern matching and engine parsing now skip VOID items directly, so this copy is no longer needed.

Match against the original pattern and remove the temporary buffer, its allocation and cleanup paths, and the unused copying helpers.

Signed-off-by Anatoly Burakov <[email protected]>

---

Pattern lookup allocates a match item just to copy an entry from the engine's supported-pattern table. Each engine must then free this temporary object after parsing.

Make ice_search_pattern_match_item() fill caller-provided storage and return a status code. Use stack-local match items in all four engines, removing the allocation and corresponding cleanup paths.

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/ice_acl_filter.c    | 22 ++++++++------------
 drivers/net/intel/ice/ice_fdir_filter.c   | 18 ++++++++--------
 drivers/net/intel/ice/ice_generic_flow.c  | 25 +++++------------------
 drivers/net/intel/ice/ice_generic_flow.h  |  3 ++-
 drivers/net/intel/ice/ice_hash.c          | 13 +++++-------
 drivers/net/intel/ice/ice_switch_filter.c | 14 +++++--------
 6 files changed, 34 insertions(+), 61 deletions(-)

Quoted 8 lines · Anatoly Burakov

Pattern lookup allocates a match item just to copy an entry from the engine's supported-pattern table. Each engine must then free this temporary object after parsing.

Make ice_search_pattern_match_item() fill caller-provided storage and return a status code. Use stack-local match items in all four engines, removing the allocation and corresponding cleanup paths.

Quoted 1 line

Signed-off-by Anatoly Burakov <[email protected]>

Acked-by Bruce Richardson <[email protected]>

I'm sure there are still plenty more cleanups that can be done converting dynamic to stack storage. Even a quick review of this change shows that the "rss_meta_ptr" value in ice_hash_parse_pattern_action can be just put on stack to simplify the code.

Quoted 9 lines

---
  drivers/net/intel/ice/ice_acl_filter.c    | 22 ++++++++------------
  drivers/net/intel/ice/ice_fdir_filter.c   | 18 ++++++++--------
  drivers/net/intel/ice/ice_generic_flow.c  | 25 +++++------------------
  drivers/net/intel/ice/ice_generic_flow.h  |  3 ++-
  drivers/net/intel/ice/ice_hash.c          | 13 +++++-------
  drivers/net/intel/ice/ice_switch_filter.c | 14 +++++--------
  6 files changed, 34 insertions(+), 61 deletions(-)

Flow engines manage rule memory independently, with parsers returning either temporary metadata or shared scratch storage. This spreads ownership and cleanup across parsing, creation, destruction and uninitialization across the flow infrastructure, which is difficult to verify, reason about, and may have hidden bugs.

Make parsers allocate the final rule and transfer ownership to the generic flow layer on success. Free rules centrally after validation, failed creation, successful destruction and uninitialization, retaining them when destruction fails. Parsers clean up their own allocations on failure.

Some engines, in addition to the rule itself, will also allocate additional data for the flow (e.g. RAW pattern buffer in FDIR engine). We can make the flow memory management centralized in generic flow, but this additional data is engine-specific, so use the free callbacks to release only said additional flow data. Remove the RSS and ACL callbacks, the FDIR and ACL scratch storage, and the redundant allocations and copies during rule creation, and use stack storage where appropriate.

Signed-off-by Anatoly Burakov <[email protected]>

 drivers/net/intel/ice/ice_acl_filter.c    | 77 +++++++--------------
 drivers/net/intel/ice/ice_ethdev.h        |  2 -
 drivers/net/intel/ice/ice_fdir_filter.c   | 82 ++++++++---------------
 drivers/net/intel/ice/ice_generic_flow.c  | 29 ++++++--
 drivers/net/intel/ice/ice_generic_flow.h  |  8 ++-
 drivers/net/intel/ice/ice_hash.c          | 68 +++++--------------
 drivers/net/intel/ice/ice_switch_filter.c | 75 +++++----------------
 7 files changed, 117 insertions(+), 224 deletions(-)

drivers/net/intel/ice/ice_acl_filter.c +26 -51

index fe1d03556c..9414bb3f12 100644
@@ -46,6 +46,7 @@
 	ICE_INSET_SCTP_SRC_PORT | ICE_INSET_SCTP_DST_PORT)
 
 struct acl_rule {
+	struct ice_acl_conf conf;
 	enum ice_fltr_ptype flow_type;
 	uint64_t entry_id[4];
 };

@@ -504,42 +505,24 @@ ice_acl_hw_rem_conf(struct ice_pf *pf, struct acl_rule *rule, int32_t entry_idx)
 
 static int
 ice_acl_create_filter(struct ice_adapter *ad,
-		      struct rte_flow *flow,
+		      struct rte_flow *flow __rte_unused,
 		      void *meta,
 		      struct rte_flow_error *error)
 {
-	struct ice_acl_conf *filter = meta;
+	struct acl_rule *rule = meta;
+	struct ice_acl_conf *filter = &rule->conf;
 	enum ice_fltr_ptype flow_type = filter->input.flow_type;
 	struct ice_flow_action acts[1];
 	struct ice_pf *pf = &ad->pf;
-	struct ice_fdir_fltr *input;
-	struct acl_rule *rule;
+	struct ice_fdir_fltr input_buf = {0};
+	struct ice_fdir_fltr *input = &input_buf;
 	int ret;
 
-	rule = rte_zmalloc("acl_rule", sizeof(*rule), 0);
-	if (!rule) {
-		rte_flow_error_set(error, ENOMEM,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "Failed to allocate memory for acl rule");
-		return -rte_errno;
-	}
-
-	input = rte_zmalloc("acl_entry", sizeof(*input), 0);
-	if (!input) {
-		rte_flow_error_set(error, ENOMEM,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "Failed to allocate memory for acl input");
-		ret = -rte_errno;
-		goto err_acl_input_alloc;
-	}
-
 	ret = ice_acl_set_input_set(filter, input);
 	if (ret) {
-		rte_flow_error_set(error, -ret,
+		return rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "failed to set input set.");
-		ret = -rte_errno;
-		goto err_acl_set_input;
 	}
 
 	if (filter->input.dest_ctl == ICE_FLTR_PRGM_DESC_DEST_DROP_PKT) {

@@ -558,11 +541,9 @@ ice_acl_create_filter(struct ice_adapter *ad,
 	input->acl_fltr = true;
 	ret = ice_acl_hw_set_conf(pf, input, acts, rule, flow_type, 0);
 	if (ret) {
-		rte_flow_error_set(error, -ret,
+		return rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "failed to set hw configure.");
-		ret = -rte_errno;
-		return ret;
 	}
 
 	if (flow_type == ICE_FLTR_PTYPE_NONF_IPV4_OTHER ||

@@ -582,7 +563,6 @@ ice_acl_create_filter(struct ice_adapter *ad,
 	}
 
 	rule->flow_type = flow_type;
-	flow->rule = rule;
 	return 0;
 
 err_acl_hw_set_conf_sctp:

@@ -591,10 +571,6 @@ ice_acl_create_filter(struct ice_adapter *ad,
 	ice_acl_hw_rem_conf(pf, rule, 2);
 err_acl_hw_set_conf_udp:
 	ice_acl_hw_rem_conf(pf, rule, 1);
-err_acl_set_input:
-	rte_free(input);
-err_acl_input_alloc:
-	rte_free(rule);
 	return ret;
 }
 

@@ -635,18 +611,9 @@ ice_acl_destroy_filter(struct ice_adapter *ad,
 		break;
 	}
 
-	flow->rule = NULL;
-	rte_free(rule);
 	return ret;
 }
 
-static void
-ice_acl_filter_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-	flow->rule = NULL;
-}
-
 static int
 ice_acl_parse_pattern(__rte_unused struct ice_adapter *ad,
 		       const struct rte_flow_item pattern[],

@@ -996,14 +963,12 @@ ice_acl_parse(struct ice_adapter *ad,
 		.check = ice_acl_parse_action_check,
 		.driver_ctx = ad,
 	};
-	struct ice_pf *pf = &ad->pf;
-	struct ice_acl_conf *filter = &pf->acl.conf;
+	struct acl_rule *rule;
+	struct ice_acl_conf *filter;
 	struct ice_pattern_match_item item;
 	uint64_t input_set;
 	int ret;
 
-	memset(filter, 0, sizeof(*filter));
-
 	ret = ci_flow_check_attr(attr, NULL, error);
 	if (ret)
 		return ret;

@@ -1017,24 +982,35 @@ ice_acl_parse(struct ice_adapter *ad,
 	if (ret != 0)
 		return ret;
 
+	rule = rte_zmalloc("acl_rule", sizeof(*rule), 0);
+	if (rule == NULL) {
+		return rte_flow_error_set(error, ENOMEM,
+				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				"Failed to allocate memory for acl rule");
+	}
+	filter = &rule->conf;
+
 	ret = ice_acl_parse_pattern(ad, pattern, error, filter);
 	if (ret)
-		return ret;
+		goto error;
 	input_set = filter->input_set;
 	if (!input_set || input_set & ~item.input_set_mask_o) {
-		return rte_flow_error_set(error, EINVAL,
+		ret = rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
 				   pattern,
 				   "Invalid input set");
+		goto error;
 	}
 
 	ret = ice_acl_parse_action(&parsed_actions, filter, error);
 	if (ret)
-		return ret;
+		goto error;
 
-	if (meta)
-		*meta = filter;
+	*meta = rule;
+	return 0;
 
+error:
+	rte_free(rule);
 	return ret;
 }
 

@@ -1122,7 +1098,6 @@ ice_flow_engine ice_acl_engine = {
 	.uninit = ice_acl_uninit,
 	.create = ice_acl_create_filter,
 	.destroy = ice_acl_destroy_filter,
-	.free = ice_acl_filter_free,
 	.name = "acl",
 	.type = ICE_FLOW_ENGINE_ACL,
 	.rule_size = sizeof(struct acl_rule),

drivers/net/intel/ice/ice_ethdev.h +0 -2

index b802588b40..1baec0f97c 100644
@@ -465,7 +465,6 @@ struct ice_fdir_info {
 	void *prg_pkt;                 /* memory for fdir program packet */
 	uint64_t dma_addr;             /* physic address of packet memory*/
 	const struct rte_memzone *mz;
-	struct ice_fdir_filter_conf conf;
 
 	struct ice_fdir_filter_conf **hash_map;
 	struct rte_hash *hash_table;

@@ -502,7 +501,6 @@ struct ice_acl_conf {
  * A structure used to define fields of ACL related info.
  */
 struct ice_acl_info {
-	struct ice_acl_conf conf;
 	struct rte_bitmap *slots;
 	uint64_t hw_entry_id[MAX_ACL_NORMAL_ENTRIES];
 };

drivers/net/intel/ice/ice_fdir_filter.c +28 -54

index a4bb33264c..f4c886dded 100644
@@ -1537,14 +1537,14 @@ ice_fdir_entry_del(struct ice_pf *pf, struct ice_fdir_fltr_pattern *key)
 
 static int
 ice_fdir_create_filter(struct ice_adapter *ad,
-		       struct rte_flow *flow,
+		       struct rte_flow *flow __rte_unused,
 		       void *meta,
 		       struct rte_flow_error *error)
 {
 	struct ice_pf *pf = &ad->pf;
 	struct ice_fdir_filter_conf *filter = meta;
 	struct ice_fdir_info *fdir_info = &pf->fdir;
-	struct ice_fdir_filter_conf *entry, *node;
+	struct ice_fdir_filter_conf *node;
 	struct ice_fdir_fltr_pattern key;
 	bool is_tun;
 	int ret;

@@ -1579,12 +1579,12 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 			ret = ice_flow_set_hw_prof(hw, main_vsi, ctrl_vsi,
 						   &filter->prof, ICE_BLK_FD);
 			if (ret)
-				goto error;
+				return -rte_errno;
 		}
 
 		ret = ice_fdir_add_del_raw(pf, filter, true);
 		if (ret)
-			goto error;
+			return -rte_errno;
 
 		if (!fv_found) {
 			for (i = 0; i < filter->prof.fv_num; i++) {

@@ -1600,14 +1600,6 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		if (filter->mark_flag == 1)
 			ice_fdir_rx_parsing_enable(ad, 1);
 
-		entry = rte_zmalloc("fdir_entry", sizeof(*entry), 0);
-		if (!entry)
-			goto error;
-
-		memcpy(entry, filter, sizeof(*filter));
-
-		flow->rule = entry;
-
 		return 0;
 	}
 

@@ -1620,14 +1612,6 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		return -rte_errno;
 	}
 
-	entry = rte_zmalloc("fdir_entry", sizeof(*entry), 0);
-	if (!entry) {
-		rte_flow_error_set(error, ENOMEM,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "Failed to allocate memory");
-		return -rte_errno;
-	}
-
 	is_tun = ice_fdir_is_tunnel_profile(filter->tunnel_type);
 
 	ret = ice_fdir_input_set_conf(pf, filter->input.flow_type,

@@ -1637,7 +1621,7 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "Profile configure failed.");
-		goto free_entry;
+		return -rte_errno;
 	}
 
 	/* alloc counter for FDIR */

@@ -1649,13 +1633,13 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 			rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					"Failed to alloc FDIR counter.");
-			goto free_entry;
+			return -rte_errno;
 		}
 		filter->input.cnt_index = filter->counter->hw_index;
 	}
 
 	/* Reserve the software entry before installing the hardware rule. */
-	ret = ice_fdir_entry_insert(pf, entry, &key);
+	ret = ice_fdir_entry_insert(pf, filter, &key);
 	if (ret) {
 		rte_flow_error_set(error, -ret,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,

@@ -1675,8 +1659,6 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 	if (filter->mark_flag == 1)
 		ice_fdir_rx_parsing_enable(ad, 1);
 
-	memcpy(entry, filter, sizeof(*entry));
-	flow->rule = entry;
 	ice_fdir_cnt_update(pf, filter->input.flow_type, is_tun, true);
 
 	return 0;

@@ -1687,12 +1669,6 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		filter->counter = NULL;
 	}
 
-free_entry:
-	rte_free(entry);
-	return -rte_errno;
-
-error:
-	rte_free(filter->pkt_buf);
 	return -rte_errno;
 }
 

@@ -1739,11 +1715,6 @@ ice_fdir_destroy_filter(struct ice_adapter *ad,
 		if (filter->mark_flag == 1)
 			ice_fdir_rx_parsing_enable(ad, 0);
 
-		flow->rule = NULL;
-
-		rte_free(filter->pkt_buf);
-		rte_free(filter);
-
 		return 0;
 	}
 

@@ -1795,13 +1766,17 @@ ice_fdir_destroy_filter(struct ice_adapter *ad,
 	if (filter->mark_flag == 1)
 		ice_fdir_rx_parsing_enable(ad, 0);
 
-	flow->rule = NULL;
-
-	rte_free(filter);
-
 	return 0;
 }
 
+static void
+ice_fdir_filter_free(void *rule)
+{
+	struct ice_fdir_filter_conf *filter = rule;
+
+	rte_free(filter->pkt_buf);
+}
+
 static int
 ice_fdir_query_count(struct ice_adapter *ad,
 		      struct rte_flow *flow,

@@ -1850,6 +1825,7 @@ static struct ice_flow_engine ice_fdir_engine = {
 	.create = ice_fdir_create_filter,
 	.destroy = ice_fdir_destroy_filter,
 	.query_count = ice_fdir_query_count,
+	.free = ice_fdir_filter_free,
 	.name = "fdir",
 	.type = ICE_FLOW_ENGINE_FDIR,
 	.rule_size = sizeof(struct ice_fdir_filter_conf),

@@ -2814,12 +2790,10 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 }
 
 static int
-ice_fdir_parse_action(struct ice_adapter *ad,
+ice_fdir_parse_action(struct ice_fdir_filter_conf *filter,
 		const struct ci_flow_actions *actions,
 		struct rte_flow_error *error)
 {
-	struct ice_pf *pf = &ad->pf;
-	struct ice_fdir_filter_conf *filter = &pf->fdir.conf;
 	bool dest_set = false;
 	size_t i;
 

@@ -3021,8 +2995,7 @@ ice_fdir_parse(struct ice_adapter *ad,
 		.driver_ctx = ad,
 		.rss_queues_contig = true,
 	};
-	struct ice_pf *pf = &ad->pf;
-	struct ice_fdir_filter_conf *filter = &pf->fdir.conf;
+	struct ice_fdir_filter_conf *filter;
 	struct ice_pattern_match_item item;
 	uint64_t input_set;
 	bool raw = false;

@@ -3032,8 +3005,6 @@ ice_fdir_parse(struct ice_adapter *ad,
 	if (ret)
 		return ret;
 
-	memset(filter, 0, sizeof(*filter));
-
 	ret = ci_flow_check_actions(actions, &param, &parsed_actions, error);
 	if (ret)
 		return ret;

@@ -3044,6 +3015,13 @@ ice_fdir_parse(struct ice_adapter *ad,
 	if (ret != 0)
 		return ret;
 
+	filter = rte_zmalloc("ice_fdir_rule", sizeof(*filter), 0);
+	if (filter == NULL) {
+		return rte_flow_error_set(error, ENOMEM,
+				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				"Failed to allocate FDIR rule");
+	}
+
 	ret = ice_fdir_parse_pattern(ad, pattern, error, filter);
 	if (ret)
 		goto error;

@@ -3066,20 +3044,16 @@ ice_fdir_parse(struct ice_adapter *ad,
 		goto error;
 	}
 
-	ret = ice_fdir_parse_action(ad, &parsed_actions, error);
+	ret = ice_fdir_parse_action(filter, &parsed_actions, error);
 	if (ret)
 		goto error;
 
-	/* if meta is NULL we're validating so the flow won't be stored */
-	if (meta) {
-		*meta = filter;
-	} else if (filter->pkt_buf != NULL) {
-		rte_free(filter->pkt_buf);
-	}
+	*meta = filter;
 
 	return ret;
 error:
 	rte_free(filter->pkt_buf);
+	rte_free(filter);
 	return ret;
 }
 

drivers/net/intel/ice/ice_generic_flow.c +24 -5

index 1a30327293..25aa7aa41e 100644
@@ -2020,6 +2020,14 @@ ice_flow_init(struct ice_adapter *ad)
 	return 0;
 }
 
+static void
+ice_flow_rule_free(struct ice_flow_engine *engine, void *rule)
+{
+	if (engine->free != NULL)
+		engine->free(rule);
+	rte_free(rule);
+}
+
 void
 ice_flow_uninit(struct ice_adapter *ad)
 {

@@ -2041,8 +2049,7 @@ ice_flow_uninit(struct ice_adapter *ad)
 	/* Remove all flows */
 	while ((p_flow = TAILQ_FIRST(&pf->flow_list))) {
 		TAILQ_REMOVE(&pf->flow_list, p_flow, node);
-		if (p_flow->engine->free)
-			p_flow->engine->free(p_flow);
+		ice_flow_rule_free(p_flow->engine, p_flow->rule);
 		rte_free(p_flow);
 	}
 

@@ -2262,7 +2269,13 @@ ice_parse_engine_create(struct ice_adapter *ad,
 
 	RTE_ASSERT(parser->engine->create != NULL);
 
-	return parser->engine->create(ad, flow, meta, error) == 0;
+	if (parser->engine->create(ad, flow, meta, error) != 0) {
+		ice_flow_rule_free(parser->engine, meta);
+		return false;
+	}
+
+	flow->rule = meta;
+	return true;
 }
 
 static bool

@@ -2274,6 +2287,8 @@ ice_parse_engine_validate(struct ice_adapter *ad,
 		const struct rte_flow_action actions[],
 		struct rte_flow_error *error)
 {
+	void *meta = NULL;
+	int ret;
 
 	if (ICE_FLOW_ENGINE_DISABLED(ad->disabled_engine_mask,
 				     parser->engine->type)) {

@@ -2283,11 +2298,14 @@ ice_parse_engine_validate(struct ice_adapter *ad,
 		return false;
 	}
 
-	return parser->parse_pattern_action(ad,
+	ret = parser->parse_pattern_action(ad,
 					    parser->array,
 					    parser->array_len,
 					    pattern, actions, attr,
-					    NULL, error) >= 0;
+					    &meta, error);
+	if (ret >= 0)
+		ice_flow_rule_free(parser->engine, meta);
+	return ret >= 0;
 }
 
 static struct ice_flow_parser *get_flow_parser(uint32_t group)

@@ -2447,6 +2465,7 @@ ice_flow_destroy(struct rte_eth_dev *dev,
 	ret = flow->engine->destroy(ad, flow, error);
 	if (!ret) {
 		TAILQ_REMOVE(&pf->flow_list, flow, node);
+		ice_flow_rule_free(flow->engine, flow->rule);
 		rte_free(flow);
 	} else {
 		PMD_DRV_LOG(ERR, "Failed to destroy flow");

drivers/net/intel/ice/ice_generic_flow.h +7 -1

index a801e0c704..efb3ee9b69 100644
@@ -519,7 +519,13 @@ typedef int (*engine_query_t)(struct ice_adapter *ad,
 typedef int(*engine_redirect_t)(struct ice_adapter *ad,
 				struct rte_flow *flow,
 				struct ice_flow_redirect *redirect);
-typedef void (*engine_free_t) (struct rte_flow *flow);
+/* Free nested rule allocations only, without accessing hardware resources. */
+typedef void (*engine_free_t)(void *rule);
+/*
+ * Parsers return an allocated final rule through meta on success, and clean up
+ * their own allocations on failure. The generic layer owns successful results,
+ * including during validation; engine create/destroy callbacks must not free them.
+ */
 typedef int (*parse_pattern_action_t)(struct ice_adapter *ad,
 		struct ice_pattern_match_item *array,
 		uint32_t array_len,

drivers/net/intel/ice/ice_hash.c +15 -53

index 21545efefd..fe66b1dd5a 100644
@@ -72,12 +72,6 @@
 				 RTE_ETH_RSS_L3_PRE56	| \
 				 RTE_ETH_RSS_L3_PRE96)
 
-struct ice_rss_meta {
-	uint8_t hash_function;
-	struct ice_rss_hash_cfg cfg;
-	struct ice_rss_raw_cfg raw;
-};
-
 struct ice_hash_flow_cfg {
 	bool simple_xor;
 	struct ice_rss_cfg rss_cfg;

@@ -100,9 +94,6 @@ ice_hash_destroy(struct ice_adapter *ad,
 static void
 ice_hash_uninit(struct ice_adapter *ad);
 
-static void
-ice_hash_free(struct rte_flow *flow);
-
 static int
 ice_hash_parse_pattern_action(struct ice_adapter *ad,
 			struct ice_pattern_match_item *array,

@@ -590,7 +581,6 @@ static struct ice_flow_engine ice_hash_engine = {
 	.create = ice_hash_create,
 	.destroy = ice_hash_destroy,
 	.uninit = ice_hash_uninit,
-	.free = ice_hash_free,
 	.name = "hash",
 	.type = ICE_FLOW_ENGINE_HASH,
 	.rule_size = sizeof(struct ice_hash_flow_cfg),

@@ -667,7 +657,7 @@ static int
 ice_hash_parse_raw_pattern(struct ice_adapter *ad,
 				const struct rte_flow_item *item,
 				struct rte_flow_error *error,
-				struct ice_rss_meta *meta)
+				struct ice_hash_flow_cfg *meta)
 {
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
 	struct ice_parser_profile prof;

@@ -755,7 +745,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
 	if (ret)
 		goto free_mem;
 
-	memcpy(&meta->raw.prof, &prof, sizeof(prof));
+	memcpy(&meta->rss_cfg.raw.prof, &prof, sizeof(prof));
 
 free_mem:
 	free(pkt_buf);

@@ -1133,7 +1123,7 @@ ice_hash_parse_action_check(const struct ci_flow_actions *actions,
 static int
 ice_hash_parse_rss_action(struct ice_pattern_match_item *pattern_match_item,
 		const struct rte_flow_action_rss *rss,
-		uint64_t pattern_hint, struct ice_rss_meta *rss_meta,
+		uint64_t pattern_hint, struct ice_hash_flow_cfg *rss_meta,
 		struct rte_flow_error *error)
 {
 	struct ice_rss_hash_cfg *cfg = pattern_match_item->meta;

@@ -1146,18 +1136,17 @@ ice_hash_parse_rss_action(struct ice_pattern_match_item *pattern_match_item,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF, rss,
 					"XOR hash function is only supported for empty pattern");
 		}
-		rss_meta->hash_function = RTE_ETH_HASH_FUNCTION_SIMPLE_XOR;
+		rss_meta->simple_xor = true;
 		return 0;
 	}
 
 	if (rss->func == RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ) {
-		rss_meta->hash_function = RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ;
 		symm = true;
 	}
 
 	/* If pattern type is raw, no need to refine rss type */
 	if (pattern_hint == ICE_PHINT_RAW) {
-		rss_meta->raw.symm = symm;
+		rss_meta->rss_cfg.raw.symm = symm;
 		return 0;
 	}
 	cfg->symm = symm;

@@ -1174,8 +1163,8 @@ ice_hash_parse_rss_action(struct ice_pattern_match_item *pattern_match_item,
 				RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 				rss, "RSS type not supported");
 
-	rss_meta->cfg = *cfg;
-	ice_refine_hash_cfg(&rss_meta->cfg,
+	rss_meta->rss_cfg.hash = *cfg;
+	ice_refine_hash_cfg(&rss_meta->rss_cfg.hash,
 			    rss_type, pattern_hint);
 
 	return 0;

@@ -1202,7 +1191,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	};
 	const struct rte_flow_action_rss *rss;
 	struct ice_pattern_match_item pattern_match_item;
-	struct ice_rss_meta *rss_meta_ptr;
+	struct ice_hash_flow_cfg *rss_meta_ptr;
 	uint64_t phint = ICE_PHINT_NONE;
 	int ret = 0;
 

@@ -1235,7 +1224,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	if (phint == ICE_PHINT_RAW) {
 		const struct rte_flow_item *raw_item = ice_pattern_skip_void(pattern);
 
-		rss_meta_ptr->raw.raw_ena = true;
+		rss_meta_ptr->rss_cfg.raw.raw_ena = true;
 		ret = ice_hash_parse_raw_pattern(ad, raw_item, error, rss_meta_ptr);
 		if (ret) {
 			rte_flow_error_set(error, EINVAL,

@@ -1250,7 +1239,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 				rss_meta_ptr, error);
 
 error:
-	if (!ret && meta)
+	if (!ret)
 		*meta = rss_meta_ptr;
 	else
 		rte_free(rss_meta_ptr);

@@ -1325,7 +1314,7 @@ ice_hash_add_raw_cfg(struct ice_adapter *ad,
 
 static int
 ice_hash_create(struct ice_adapter *ad,
-		struct rte_flow *flow,
+		struct rte_flow *flow __rte_unused,
 		void *meta,
 		struct rte_flow_error *error)
 {

@@ -1334,33 +1323,19 @@ ice_hash_create(struct ice_adapter *ad,
 	struct ice_vsi *vsi = pf->main_vsi;
 	int ret;
 	uint32_t reg;
-	struct ice_hash_flow_cfg *filter_ptr;
-	struct ice_rss_meta *rss_meta = (struct ice_rss_meta *)meta;
-	uint8_t hash_function = rss_meta->hash_function;
+	struct ice_hash_flow_cfg *filter_ptr = meta;
 
-	filter_ptr = rte_zmalloc("ice_rss_filter",
-				sizeof(struct ice_hash_flow_cfg), 0);
-	if (!filter_ptr) {
-		rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				"No memory for filter_ptr");
-		return -ENOMEM;
-	}
-
-	if (hash_function == RTE_ETH_HASH_FUNCTION_SIMPLE_XOR) {
+	if (filter_ptr->simple_xor) {
 		/* Enable registers for simple_xor hash function. */
 		reg = ICE_READ_REG(hw, VSIQF_HASH_CTL(vsi->vsi_id));
 		reg = (reg & (~VSIQF_HASH_CTL_HASH_SCHEME_M)) |
 			(2 << VSIQF_HASH_CTL_HASH_SCHEME_S);
 		ICE_WRITE_REG(hw, VSIQF_HASH_CTL(vsi->vsi_id), reg);
 
-		filter_ptr->simple_xor = 1;
-
 		goto out;
 	} else {
-		if (rss_meta->raw.raw_ena) {
-			filter_ptr->rss_cfg.raw = rss_meta->raw;
-			ret = ice_hash_add_raw_cfg(ad, &rss_meta->raw,
+		if (filter_ptr->rss_cfg.raw.raw_ena) {
+			ret = ice_hash_add_raw_cfg(ad, &filter_ptr->rss_cfg.raw,
 						   pf->main_vsi->idx);
 			if (ret) {
 				rte_flow_error_set(error, EINVAL,

@@ -1370,7 +1345,6 @@ ice_hash_create(struct ice_adapter *ad,
 				goto error;
 			}
 		} else {
-			filter_ptr->rss_cfg.hash = rss_meta->cfg;
 			ret = ice_add_rss_cfg_wrap(pf, vsi->idx,
 						   &filter_ptr->rss_cfg.hash);
 			if (ret) {

@@ -1384,13 +1358,9 @@ ice_hash_create(struct ice_adapter *ad,
 	}
 
 out:
-	flow->rule = filter_ptr;
-	rte_free(meta);
 	return 0;
 
 error:
-	rte_free(filter_ptr);
-	rte_free(meta);
 	return -rte_errno;
 }
 

@@ -1483,11 +1453,9 @@ ice_hash_destroy(struct ice_adapter *ad,
 		}
 	}
 
-	rte_free(filter_ptr);
 	return 0;
 
 error:
-	rte_free(filter_ptr);
 	return -rte_errno;
 }
 

@@ -1495,9 +1463,3 @@ static void
 ice_hash_uninit(struct ice_adapter *ad __rte_unused)
 {
 }
-
-static void
-ice_hash_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-}

drivers/net/intel/ice/ice_switch_filter.c +17 -58

index 5c34089b40..4dd4707c40 100644
@@ -178,12 +178,6 @@
 	ICE_SW_INSET_GTPU_IPV6 | ICE_INSET_TCP_SRC_PORT | \
 	ICE_INSET_TCP_DST_PORT)
 
-struct sw_meta {
-	struct ice_adv_lkup_elem *list;
-	uint16_t lkups_num;
-	struct ice_adv_rule_info rule_info;
-};
-
 enum ice_sw_fltr_status {
 	ICE_SW_FLTR_ADDED,
 	ICE_SW_FLTR_RMV_FAILED_ON_RIDRECT,

@@ -291,7 +285,7 @@ ice_pattern_match_item ice_switch_supported_pattern[] = {
 
 static int
 ice_switch_create(struct ice_adapter *ad,
-		struct rte_flow *flow,
+		struct rte_flow *flow __rte_unused,
 		void *meta,
 		struct rte_flow_error *error)
 {

@@ -299,13 +293,10 @@ ice_switch_create(struct ice_adapter *ad,
 	struct ice_pf *pf = &ad->pf;
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
 	struct ice_rule_query_data rule_added = {0};
-	struct ice_switch_filter_conf *filter_conf_ptr;
-	struct ice_adv_lkup_elem *list =
-		((struct sw_meta *)meta)->list;
-	uint16_t lkups_cnt =
-		((struct sw_meta *)meta)->lkups_num;
-	struct ice_adv_rule_info *rule_info =
-		&((struct sw_meta *)meta)->rule_info;
+	struct ice_switch_filter_conf *filter_conf_ptr = meta;
+	struct ice_adv_lkup_elem *list = filter_conf_ptr->lkups;
+	uint16_t lkups_cnt = filter_conf_ptr->lkups_num;
+	struct ice_adv_rule_info *rule_info = &filter_conf_ptr->rule_info;
 
 	if (lkups_cnt > ICE_MAX_CHAIN_WORDS) {
 		rte_flow_error_set(error, EINVAL,

@@ -329,26 +320,11 @@ ice_switch_create(struct ice_adapter *ad,
 
 	ret = ice_add_adv_rule(hw, list, lkups_cnt, rule_info, &rule_added);
 	if (!ret) {
-		filter_conf_ptr = rte_zmalloc("ice_switch_filter",
-			sizeof(struct ice_switch_filter_conf), 0);
-		if (!filter_conf_ptr) {
-			rte_flow_error_set(error, EINVAL,
-				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for ice_switch_filter");
-			goto error;
-		}
-
 		filter_conf_ptr->sw_query_data = rule_added;
 
 		filter_conf_ptr->vsi_num =
 			ice_get_hw_vsi_num(hw, rule_info->sw_act.vsi_handle);
-		filter_conf_ptr->lkups = list;
-		filter_conf_ptr->lkups_num = lkups_cnt;
-		filter_conf_ptr->rule_info = *rule_info;
-
 		filter_conf_ptr->fltr_status = ICE_SW_FLTR_ADDED;
-
-		flow->rule = filter_conf_ptr;
 	} else {
 		if (ice_dcf_adminq_need_retry(ad))
 			ret = -EAGAIN;

@@ -361,26 +337,18 @@ ice_switch_create(struct ice_adapter *ad,
 		goto error;
 	}
 
-	rte_free(meta);
 	return 0;
 
 error:
-	rte_free(list);
-	rte_free(meta);
-
 	return -rte_errno;
 }
 
 static inline void
-ice_switch_filter_rule_free(struct rte_flow *flow)
+ice_switch_filter_rule_free(void *rule)
 {
-	struct ice_switch_filter_conf *filter_conf_ptr =
-		(struct ice_switch_filter_conf *)flow->rule;
+	struct ice_switch_filter_conf *filter_conf_ptr = rule;
 
-	if (filter_conf_ptr)
-		rte_free(filter_conf_ptr->lkups);
-
-	rte_free(filter_conf_ptr);
+	rte_free(filter_conf_ptr->lkups);
 }
 
 static int

@@ -402,8 +370,6 @@ ice_switch_destroy(struct ice_adapter *ad,
 			"no such flow"
 			" create by switch filter");
 
-		ice_switch_filter_rule_free(flow);
-
 		return -rte_errno;
 	}
 

@@ -427,7 +393,6 @@ ice_switch_destroy(struct ice_adapter *ad,
 		return -rte_errno;
 	}
 
-	ice_switch_filter_rule_free(flow);
 	return ret;
 }
 

@@ -1746,7 +1711,7 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad,
 {
 	struct ice_pf *pf = &ad->pf;
 	int ret = 0;
-	struct sw_meta *sw_meta_ptr = NULL;
+	struct ice_switch_filter_conf *filter = NULL;
 	struct ice_adv_rule_info rule_info;
 	struct ice_adv_lkup_elem *list = NULL;
 	uint16_t lkups_num = 0;

@@ -1839,12 +1804,11 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad,
 		return -rte_errno;
 	}
 
-	sw_meta_ptr =
-		rte_zmalloc(NULL, sizeof(*sw_meta_ptr), 0);
-	if (!sw_meta_ptr) {
+	filter = rte_zmalloc("ice_switch_filter", sizeof(*filter), 0);
+	if (filter == NULL) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-				   "No memory for sw_pattern_meta_ptr");
+				   "No memory for ice_switch_filter");
 		goto error;
 	}
 

@@ -1879,21 +1843,16 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad,
 	if (ret)
 		goto error;
 
-	if (meta) {
-		*meta = sw_meta_ptr;
-		((struct sw_meta *)*meta)->list = list;
-		((struct sw_meta *)*meta)->lkups_num = lkups_num;
-		((struct sw_meta *)*meta)->rule_info = rule_info;
-	} else {
-		rte_free(list);
-		rte_free(sw_meta_ptr);
-	}
+	filter->lkups = list;
+	filter->lkups_num = lkups_num;
+	filter->rule_info = rule_info;
+	*meta = filter;
 
 	return 0;
 
 error:
 	rte_free(list);
-	rte_free(sw_meta_ptr);
+	rte_free(filter);
 
 	return -rte_errno;
 }

Signature · 1 line

2.52.0

Quoted 20 lines · Anatoly Burakov

Flow engines manage rule memory independently, with parsers returning either temporary metadata or shared scratch storage. This spreads ownership and cleanup across parsing, creation, destruction and uninitialization across the flow infrastructure, which is difficult to verify, reason about, and may have hidden bugs.

Make parsers allocate the final rule and transfer ownership to the generic flow layer on success. Free rules centrally after validation, failed creation, successful destruction and uninitialization, retaining them when destruction fails. Parsers clean up their own allocations on failure.

Some engines, in addition to the rule itself, will also allocate additional data for the flow (e.g. RAW pattern buffer in FDIR engine). We can make the flow memory management centralized in generic flow, but this additional data is engine-specific, so use the free callbacks to release only said additional flow data. Remove the RSS and ACL callbacks, the FDIR and ACL scratch storage, and the redundant allocations and copies during rule creation, and use stack storage where appropriate.

Quoted 1 line

Signed-off-by Anatoly Burakov <[email protected]>

Acked-by Bruce Richardson <[email protected]>

After this cleanup, it looks like the flow parameter to each engine can be removed completely, since all four engines mark it as __rte_unused now.

This patchset rolls in a few fixes for flow engines, but the main point is to rework the flow engines to have more defined memory management.

Anatoly Burakov (6):
net/ice: release ACL slot on entry creation failure net/ice: fix inconsistent state on FDIR failure net/ice: standardize VOID handling in flow parsers net/ice: remove temporary flow pattern buffer net/ice: use caller-owned pattern match storage net/ice: centralize flow rule memory management

 drivers/net/intel/ice/ice_acl_filter.c    |  91 +++++---------
 drivers/net/intel/ice/ice_ethdev.h        |   2 -
 drivers/net/intel/ice/ice_fdir_filter.c   | 124 ++++++++-----------
 drivers/net/intel/ice/ice_generic_flow.c  | 141 ++++++----------------
 drivers/net/intel/ice/ice_generic_flow.h  |  25 +++-
 drivers/net/intel/ice/ice_hash.c          |  87 ++++---------
 drivers/net/intel/ice/ice_switch_filter.c |  97 ++++-----------
 7 files changed, 185 insertions(+), 382 deletions(-)