* [PATCH v1 0/6] Centralize flow memory management in ICE
@ 2026-10-20 12:11 Anatoly Burakov
2026-10-20 12:11 ` [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure Anatoly Burakov
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw)
To: dev
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(-)
--
2.52.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov @ 2026-10-20 12:11 ` Anatoly Burakov 2026-10-21 9:17 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 2/6] net/ice: fix inconsistent state on FDIR failure Anatoly Burakov ` (5 subsequent siblings) 6 siblings, 1 reply; 16+ messages in thread From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw) To: dev, Bruce Richardson, Qi Zhang, Xuan Ding, Simei Su 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: stable@dpdk.org Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> --- drivers/net/intel/ice/ice_acl_filter.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c index a91bd558f3..eb5d88ccc5 100644 --- a/drivers/net/intel/ice/ice_acl_filter.c +++ b/drivers/net/intel/ice/ice_acl_filter.c @@ -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; } -- 2.52.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure 2026-10-20 12:11 ` [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure Anatoly Burakov @ 2026-10-21 9:17 ` Bruce Richardson 0 siblings, 0 replies; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 9:17 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev, Qi Zhang, Xuan Ding, Simei Su On Tue, Oct 20, 2026 at 01:11:04PM +0100, Anatoly Burakov wrote: > 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: stable@dpdk.org > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/intel/ice/ice_acl_filter.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c > index a91bd558f3..eb5d88ccc5 100644 > --- a/drivers/net/intel/ice/ice_acl_filter.c > +++ b/drivers/net/intel/ice/ice_acl_filter.c > @@ -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; > } > > -- > 2.52.0 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 2/6] net/ice: fix inconsistent state on FDIR failure 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov 2026-10-20 12:11 ` [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure Anatoly Burakov @ 2026-10-20 12:11 ` Anatoly Burakov 2026-10-21 9:17 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 3/6] net/ice: standardize VOID handling in flow parsers Anatoly Burakov ` (4 subsequent siblings) 6 siblings, 1 reply; 16+ messages in thread From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw) To: dev, Bruce Richardson, Xiaolong Ye, Yahui Cao, Qi Zhang 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: stable@dpdk.org Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> --- drivers/net/intel/ice/ice_fdir_filter.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c index 517c32cecf..63a0d9fb1f 100644 --- a/drivers/net/intel/ice/ice_fdir_filter.c +++ b/drivers/net/intel/ice/ice_fdir_filter.c @@ -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); -- 2.52.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/6] net/ice: fix inconsistent state on FDIR failure 2026-10-20 12:11 ` [PATCH v1 2/6] net/ice: fix inconsistent state on FDIR failure Anatoly Burakov @ 2026-10-21 9:17 ` Bruce Richardson 0 siblings, 0 replies; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 9:17 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev, Xiaolong Ye, Yahui Cao, Qi Zhang On Tue, Oct 20, 2026 at 01:11:05PM +0100, Anatoly Burakov wrote: > 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: stable@dpdk.org > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/intel/ice/ice_fdir_filter.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c > index 517c32cecf..63a0d9fb1f 100644 > --- a/drivers/net/intel/ice/ice_fdir_filter.c > +++ b/drivers/net/intel/ice/ice_fdir_filter.c > @@ -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); > > -- > 2.52.0 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 3/6] net/ice: standardize VOID handling in flow parsers 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov 2026-10-20 12:11 ` [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure Anatoly Burakov 2026-10-20 12:11 ` [PATCH v1 2/6] net/ice: fix inconsistent state on FDIR failure Anatoly Burakov @ 2026-10-20 12:11 ` Anatoly Burakov 2026-10-21 9:18 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 4/6] net/ice: remove temporary flow pattern buffer Anatoly Burakov ` (3 subsequent siblings) 6 siblings, 1 reply; 16+ messages in thread From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw) To: dev, Bruce Richardson 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 <anatoly.burakov@intel.com> --- 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(-) diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c index eb5d88ccc5..91f684c9b9 100644 --- a/drivers/net/intel/ice/ice_acl_filter.c +++ b/drivers/net/intel/ice/ice_acl_filter.c @@ -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, diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c index 63a0d9fb1f..b1a8753e90 100644 --- a/drivers/net/intel/ice/ice_fdir_filter.c +++ b/drivers/net/intel/ice/ice_fdir_filter.c @@ -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(ð_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; diff --git a/drivers/net/intel/ice/ice_generic_flow.c b/drivers/net/intel/ice/ice_generic_flow.c index 76b3800545..fee048e456 100644 --- a/drivers/net/intel/ice/ice_generic_flow.c +++ b/drivers/net/intel/ice/ice_generic_flow.c @@ -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 { diff --git a/drivers/net/intel/ice/ice_generic_flow.h b/drivers/net/intel/ice/ice_generic_flow.h index 650b43633f..983f86a5ab 100644 --- a/drivers/net/intel/ice/ice_generic_flow.h +++ b/drivers/net/intel/ice/ice_generic_flow.h @@ -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[], diff --git a/drivers/net/intel/ice/ice_hash.c b/drivers/net/intel/ice/ice_hash.c index 7a9a4f1a04..13bedfe90c 100644 --- a/drivers/net/intel/ice/ice_hash.c +++ b/drivers/net/intel/ice/ice_hash.c @@ -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, diff --git a/drivers/net/intel/ice/ice_switch_filter.c b/drivers/net/intel/ice/ice_switch_filter.c index 6f5af03f6a..6bc1bdd09f 100644 --- a/drivers/net/intel/ice/ice_switch_filter.c +++ b/drivers/net/intel/ice/ice_switch_filter.c @@ -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; -- 2.52.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/6] net/ice: standardize VOID handling in flow parsers 2026-10-20 12:11 ` [PATCH v1 3/6] net/ice: standardize VOID handling in flow parsers Anatoly Burakov @ 2026-10-21 9:18 ` Bruce Richardson 0 siblings, 0 replies; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 9:18 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev On Tue, Oct 20, 2026 at 01:11:06PM +0100, Anatoly Burakov wrote: > 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 <anatoly.burakov@intel.com> > --- This loop macro looks a very nice cleanup. Acked-by: Bruce Richardson <bruce.richardson@intel.com> > 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(-) > > diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c > index eb5d88ccc5..91f684c9b9 100644 > --- a/drivers/net/intel/ice/ice_acl_filter.c > +++ b/drivers/net/intel/ice/ice_acl_filter.c > @@ -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, > diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c > index 63a0d9fb1f..b1a8753e90 100644 > --- a/drivers/net/intel/ice/ice_fdir_filter.c > +++ b/drivers/net/intel/ice/ice_fdir_filter.c > @@ -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(ð_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; > diff --git a/drivers/net/intel/ice/ice_generic_flow.c b/drivers/net/intel/ice/ice_generic_flow.c > index 76b3800545..fee048e456 100644 > --- a/drivers/net/intel/ice/ice_generic_flow.c > +++ b/drivers/net/intel/ice/ice_generic_flow.c > @@ -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 { > diff --git a/drivers/net/intel/ice/ice_generic_flow.h b/drivers/net/intel/ice/ice_generic_flow.h > index 650b43633f..983f86a5ab 100644 > --- a/drivers/net/intel/ice/ice_generic_flow.h > +++ b/drivers/net/intel/ice/ice_generic_flow.h > @@ -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[], > diff --git a/drivers/net/intel/ice/ice_hash.c b/drivers/net/intel/ice/ice_hash.c > index 7a9a4f1a04..13bedfe90c 100644 > --- a/drivers/net/intel/ice/ice_hash.c > +++ b/drivers/net/intel/ice/ice_hash.c > @@ -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, > diff --git a/drivers/net/intel/ice/ice_switch_filter.c b/drivers/net/intel/ice/ice_switch_filter.c > index 6f5af03f6a..6bc1bdd09f 100644 > --- a/drivers/net/intel/ice/ice_switch_filter.c > +++ b/drivers/net/intel/ice/ice_switch_filter.c > @@ -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; > -- > 2.52.0 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 4/6] net/ice: remove temporary flow pattern buffer 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov ` (2 preceding siblings ...) 2026-10-20 12:11 ` [PATCH v1 3/6] net/ice: standardize VOID handling in flow parsers Anatoly Burakov @ 2026-10-20 12:11 ` Anatoly Burakov 2026-10-21 9:19 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 5/6] net/ice: use caller-owned pattern match storage Anatoly Burakov ` (2 subsequent siblings) 6 siblings, 1 reply; 16+ messages in thread From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw) To: dev, Bruce Richardson 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 <anatoly.burakov@intel.com> --- drivers/net/intel/ice/ice_generic_flow.c | 76 +----------------------- 1 file changed, 1 insertion(+), 75 deletions(-) diff --git a/drivers/net/intel/ice/ice_generic_flow.c b/drivers/net/intel/ice/ice_generic_flow.c index fee048e456..ebe986c469 100644 --- a/drivers/net/intel/ice/ice_generic_flow.c +++ b/drivers/net/intel/ice/ice_generic_flow.c @@ -2052,58 +2052,6 @@ ice_flow_uninit(struct ice_adapter *ad) } } -/* Find the first VOID or non-VOID item pointer */ -static const struct rte_flow_item * -ice_find_first_item(const struct rte_flow_item *item, bool is_void) -{ - bool is_find; - - while (item->type != RTE_FLOW_ITEM_TYPE_END) { - if (is_void) - is_find = item->type == RTE_FLOW_ITEM_TYPE_VOID; - else - is_find = item->type != RTE_FLOW_ITEM_TYPE_VOID; - if (is_find) - break; - item++; - } - return item; -} - -/* Skip all VOID items of the pattern */ -static void -ice_pattern_skip_void_item(struct rte_flow_item *items, - const struct rte_flow_item *pattern) -{ - uint32_t cpy_count = 0; - const struct rte_flow_item *pb = pattern, *pe = pattern; - - for (;;) { - /* Find a non-void item first */ - pb = ice_find_first_item(pb, false); - if (pb->type == RTE_FLOW_ITEM_TYPE_END) { - pe = pb; - break; - } - - /* Find a void item */ - pe = ice_find_first_item(pb + 1, true); - - cpy_count = pe - pb; - memcpy(items, pb, sizeof(struct rte_flow_item) * cpy_count); - - items += cpy_count; - - if (pe->type == RTE_FLOW_ITEM_TYPE_END) { - break; - } - - pb = pe + 1; - } - /* Copy the END item. */ - memcpy(items, pe, sizeof(struct rte_flow_item)); -} - /* Check if the pattern matches a supported item type array */ static bool ice_match_pattern(enum rte_flow_item_type *item_array, @@ -2269,41 +2217,21 @@ ice_search_pattern_match_item(struct ice_adapter *ad, struct rte_flow_error *error) { struct ice_pattern_match_item *pattern_match_item; - /* need free by each filter */ - struct rte_flow_item *items; /* used for pattern without VOID items */ - uint32_t item_num = 0; /* non-void item number */ uint16_t i = 0; - /* Get the non-void item number of pattern */ - while ((pattern + i)->type != RTE_FLOW_ITEM_TYPE_END) { - if ((pattern + i)->type != RTE_FLOW_ITEM_TYPE_VOID) - item_num++; - i++; - } - item_num++; - - items = calloc(item_num, sizeof(struct rte_flow_item)); - if (!items) { - rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ITEM_NUM, - NULL, "No memory for PMD internal items."); - return NULL; - } pattern_match_item = calloc(1, sizeof(struct ice_pattern_match_item)); if (!pattern_match_item) { rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE, NULL, "Failed to allocate memory."); - free(items); return NULL; } - ice_pattern_skip_void_item(items, pattern); - if (!ice_pattern_is_supported(ad, pattern)) goto unsupported; for (i = 0; i < array_len; i++) { if (ice_match_pattern(array[i].pattern_list, - items)) { + pattern)) { pattern_match_item->input_set_mask_o = array[i].input_set_mask_o; pattern_match_item->input_set_mask_i = @@ -2311,7 +2239,6 @@ ice_search_pattern_match_item(struct ice_adapter *ad, pattern_match_item->pattern_list = array[i].pattern_list; pattern_match_item->meta = array[i].meta; - free(items); return pattern_match_item; } } @@ -2319,7 +2246,6 @@ ice_search_pattern_match_item(struct ice_adapter *ad, unsupported: rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, pattern, "Unsupported pattern"); - free(items); free(pattern_match_item); return NULL; } -- 2.52.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 4/6] net/ice: remove temporary flow pattern buffer 2026-10-20 12:11 ` [PATCH v1 4/6] net/ice: remove temporary flow pattern buffer Anatoly Burakov @ 2026-10-21 9:19 ` Bruce Richardson 0 siblings, 0 replies; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 9:19 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev On Tue, Oct 20, 2026 at 01:11:07PM +0100, Anatoly Burakov wrote: > 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 <anatoly.burakov@intel.com> > --- Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 5/6] net/ice: use caller-owned pattern match storage 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov ` (3 preceding siblings ...) 2026-10-20 12:11 ` [PATCH v1 4/6] net/ice: remove temporary flow pattern buffer Anatoly Burakov @ 2026-10-20 12:11 ` Anatoly Burakov 2026-10-21 9:25 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 6/6] net/ice: centralize flow rule memory management Anatoly Burakov 2026-10-21 10:02 ` [PATCH v1 0/6] Centralize flow memory management in ICE Bruce Richardson 6 siblings, 1 reply; 16+ messages in thread From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw) To: dev, Bruce Richardson 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 <anatoly.burakov@intel.com> --- 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(-) diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c index 91f684c9b9..fe1d03556c 100644 --- a/drivers/net/intel/ice/ice_acl_filter.c +++ b/drivers/net/intel/ice/ice_acl_filter.c @@ -998,7 +998,7 @@ ice_acl_parse(struct ice_adapter *ad, }; struct ice_pf *pf = &ad->pf; struct ice_acl_conf *filter = &pf->acl.conf; - struct ice_pattern_match_item *item = NULL; + struct ice_pattern_match_item item; uint64_t input_set; int ret; @@ -1012,33 +1012,29 @@ ice_acl_parse(struct ice_adapter *ad, if (ret) return ret; - item = ice_search_pattern_match_item(ad, pattern, array, array_len, - error); - if (!item) - return -rte_errno; + ret = ice_search_pattern_match_item(ad, pattern, array, array_len, + &item, error); + if (ret != 0) + return ret; ret = ice_acl_parse_pattern(ad, pattern, error, filter); if (ret) - goto error; + return ret; input_set = filter->input_set; - if (!input_set || input_set & ~item->input_set_mask_o) { - rte_flow_error_set(error, EINVAL, + if (!input_set || input_set & ~item.input_set_mask_o) { + return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_SPEC, pattern, "Invalid input set"); - ret = -rte_errno; - goto error; } ret = ice_acl_parse_action(&parsed_actions, filter, error); if (ret) - goto error; + return ret; if (meta) *meta = filter; -error: - free(item); return ret; } diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c index b1a8753e90..a4bb33264c 100644 --- a/drivers/net/intel/ice/ice_fdir_filter.c +++ b/drivers/net/intel/ice/ice_fdir_filter.c @@ -3023,7 +3023,7 @@ ice_fdir_parse(struct ice_adapter *ad, }; struct ice_pf *pf = &ad->pf; struct ice_fdir_filter_conf *filter = &pf->fdir.conf; - struct ice_pattern_match_item *item = NULL; + struct ice_pattern_match_item item; uint64_t input_set; bool raw = false; int ret; @@ -3038,17 +3038,17 @@ ice_fdir_parse(struct ice_adapter *ad, if (ret) return ret; - item = ice_search_pattern_match_item(ad, pattern, array, array_len, - error); + ret = ice_search_pattern_match_item(ad, pattern, array, array_len, + &item, error); - if (!item) - return -rte_errno; + if (ret != 0) + return ret; ret = ice_fdir_parse_pattern(ad, pattern, error, filter); if (ret) goto error; - if (item->pattern_list[0] == RTE_FLOW_ITEM_TYPE_RAW) + if (item.pattern_list[0] == RTE_FLOW_ITEM_TYPE_RAW) raw = true; input_set = filter->input_set_o | filter->input_set_i; @@ -3056,8 +3056,8 @@ ice_fdir_parse(struct ice_adapter *ad, if (!input_set || (filter->input_set_o & - ~(item->input_set_mask_o | ICE_INSET_ETHERTYPE)) || - (filter->input_set_i & ~item->input_set_mask_i)) { + ~(item.input_set_mask_o | ICE_INSET_ETHERTYPE)) || + (filter->input_set_i & ~item.input_set_mask_i)) { rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_SPEC, pattern, @@ -3077,11 +3077,9 @@ ice_fdir_parse(struct ice_adapter *ad, rte_free(filter->pkt_buf); } - free(item); return ret; error: rte_free(filter->pkt_buf); - free(item); return ret; } diff --git a/drivers/net/intel/ice/ice_generic_flow.c b/drivers/net/intel/ice/ice_generic_flow.c index ebe986c469..1a30327293 100644 --- a/drivers/net/intel/ice/ice_generic_flow.c +++ b/drivers/net/intel/ice/ice_generic_flow.c @@ -2209,45 +2209,30 @@ ice_pattern_is_supported(__rte_unused struct ice_adapter *ad, return false; } -struct ice_pattern_match_item * +int ice_search_pattern_match_item(struct ice_adapter *ad, const struct rte_flow_item pattern[], struct ice_pattern_match_item *array, uint32_t array_len, + struct ice_pattern_match_item *match, struct rte_flow_error *error) { - struct ice_pattern_match_item *pattern_match_item; uint16_t i = 0; - pattern_match_item = calloc(1, sizeof(struct ice_pattern_match_item)); - if (!pattern_match_item) { - rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE, - NULL, "Failed to allocate memory."); - return NULL; - } - if (!ice_pattern_is_supported(ad, pattern)) goto unsupported; for (i = 0; i < array_len; i++) { if (ice_match_pattern(array[i].pattern_list, pattern)) { - pattern_match_item->input_set_mask_o = - array[i].input_set_mask_o; - pattern_match_item->input_set_mask_i = - array[i].input_set_mask_i; - pattern_match_item->pattern_list = - array[i].pattern_list; - pattern_match_item->meta = array[i].meta; - return pattern_match_item; + *match = array[i]; + return 0; } } unsupported: - rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, + return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, pattern, "Unsupported pattern"); - free(pattern_match_item); - return NULL; } static bool diff --git a/drivers/net/intel/ice/ice_generic_flow.h b/drivers/net/intel/ice/ice_generic_flow.h index 983f86a5ab..a801e0c704 100644 --- a/drivers/net/intel/ice/ice_generic_flow.h +++ b/drivers/net/intel/ice/ice_generic_flow.h @@ -583,11 +583,12 @@ ice_pattern_skip_void(const struct rte_flow_item *item) (item)->type != RTE_FLOW_ITEM_TYPE_END; \ (item) = ice_pattern_skip_void((item) + 1)) -struct ice_pattern_match_item * +int ice_search_pattern_match_item(struct ice_adapter *ad, const struct rte_flow_item pattern[], struct ice_pattern_match_item *array, uint32_t array_len, + struct ice_pattern_match_item *match, struct rte_flow_error *error); int ice_flow_redirect(struct ice_adapter *ad, diff --git a/drivers/net/intel/ice/ice_hash.c b/drivers/net/intel/ice/ice_hash.c index 13bedfe90c..21545efefd 100644 --- a/drivers/net/intel/ice/ice_hash.c +++ b/drivers/net/intel/ice/ice_hash.c @@ -1201,7 +1201,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad, .check = ice_hash_parse_action_check, }; const struct rte_flow_action_rss *rss; - struct ice_pattern_match_item *pattern_match_item; + struct ice_pattern_match_item pattern_match_item; struct ice_rss_meta *rss_meta_ptr; uint64_t phint = ICE_PHINT_NONE; int ret = 0; @@ -1223,12 +1223,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad, } /* Check rss supported pattern and find matched pattern. */ - pattern_match_item = ice_search_pattern_match_item(ad, pattern, array, - array_len, error); - if (!pattern_match_item) { - ret = -rte_errno; + ret = ice_search_pattern_match_item(ad, pattern, array, array_len, + &pattern_match_item, error); + if (ret != 0) goto error; - } ret = ice_hash_parse_pattern(pattern, &phint, error); if (ret) @@ -1248,7 +1246,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad, } rss = parsed_actions.actions[0]->conf; - ret = ice_hash_parse_rss_action(pattern_match_item, rss, phint, + ret = ice_hash_parse_rss_action(&pattern_match_item, rss, phint, rss_meta_ptr, error); error: @@ -1256,7 +1254,6 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad, *meta = rss_meta_ptr; else rte_free(rss_meta_ptr); - free(pattern_match_item); return ret; } diff --git a/drivers/net/intel/ice/ice_switch_filter.c b/drivers/net/intel/ice/ice_switch_filter.c index 6bc1bdd09f..5c34089b40 100644 --- a/drivers/net/intel/ice/ice_switch_filter.c +++ b/drivers/net/intel/ice/ice_switch_filter.c @@ -1755,7 +1755,7 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad, uint16_t vlan_num = 0; enum ice_sw_tunnel_type tun_type = ICE_NON_TUN; - struct ice_pattern_match_item *pattern_match_item = NULL; + struct ice_pattern_match_item pattern_match_item; struct ci_flow_attr_check_param attr_param = { .allow_priority = true, }; @@ -1848,10 +1848,9 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad, goto error; } - pattern_match_item = - ice_search_pattern_match_item(ad, pattern, array, array_len, - error); - if (!pattern_match_item) { + ret = ice_search_pattern_match_item(ad, pattern, array, array_len, + &pattern_match_item, error); + if (ret != 0) { rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_HANDLE, NULL, "Invalid input pattern"); @@ -1859,7 +1858,7 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad, } if (!ice_switch_parse_pattern(pattern, error, list, &lkups_num, - &tun_type, pattern_match_item)) { + &tun_type, &pattern_match_item)) { rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM_SPEC, pattern, @@ -1890,14 +1889,11 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad, rte_free(sw_meta_ptr); } - free(pattern_match_item); - return 0; error: rte_free(list); rte_free(sw_meta_ptr); - free(pattern_match_item); return -rte_errno; } -- 2.52.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 5/6] net/ice: use caller-owned pattern match storage 2026-10-20 12:11 ` [PATCH v1 5/6] net/ice: use caller-owned pattern match storage Anatoly Burakov @ 2026-10-21 9:25 ` Bruce Richardson 2026-10-21 9:55 ` Burakov, Anatoly 0 siblings, 1 reply; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 9:25 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev On Tue, Oct 20, 2026 at 01:11:08PM +0100, Anatoly Burakov wrote: > 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 <anatoly.burakov@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> 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. > --- > 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(-) > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 5/6] net/ice: use caller-owned pattern match storage 2026-10-21 9:25 ` Bruce Richardson @ 2026-10-21 9:55 ` Burakov, Anatoly 0 siblings, 0 replies; 16+ messages in thread From: Burakov, Anatoly @ 2026-10-21 9:55 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev On 10/21/2026 11:25 AM, Bruce Richardson wrote: > On Tue, Oct 20, 2026 at 01:11:08PM +0100, Anatoly Burakov wrote: >> 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 <anatoly.burakov@intel.com> > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > > 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. If you mean allocate the meta on the stack, that particular fix wouldn't pan out because of the way engines manage memory (basically, rss_meta_ptr is the one that has to persist, so it has to be allocated on the heap). If you mean allocate the meta on the stack *in the caller*, that would work but require some re-plumbing because we'd need a union of all possible metadatas to pass into the parse function. So, doable, but a bit more work than just putting things on the stack. This suggestion is also nullified by patch 6, because it makes it so that the final flow object is allocated by the parse function, so there's no more "metadata" passing between pattern matcher and the engine - the heap-allocated object is the one we store in the flow list. However, you're right that there's plenty of things we could've done differently when it comes to memory management, and future work porting ice to flow_engine framework will fix all of these issues in flow our API (and will get rid of the nested allocations that are still kept in patch 6). >> --- >> 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(-) >> -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 6/6] net/ice: centralize flow rule memory management 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov ` (4 preceding siblings ...) 2026-10-20 12:11 ` [PATCH v1 5/6] net/ice: use caller-owned pattern match storage Anatoly Burakov @ 2026-10-20 12:11 ` Anatoly Burakov 2026-10-21 9:59 ` Bruce Richardson 2026-10-21 10:02 ` [PATCH v1 0/6] Centralize flow memory management in ICE Bruce Richardson 6 siblings, 1 reply; 16+ messages in thread From: Anatoly Burakov @ 2026-10-20 12:11 UTC (permalink / raw) To: dev, Bruce Richardson 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 <anatoly.burakov@intel.com> --- 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(-) diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c index fe1d03556c..9414bb3f12 100644 --- a/drivers/net/intel/ice/ice_acl_filter.c +++ b/drivers/net/intel/ice/ice_acl_filter.c @@ -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), diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h index b802588b40..1baec0f97c 100644 --- a/drivers/net/intel/ice/ice_ethdev.h +++ b/drivers/net/intel/ice/ice_ethdev.h @@ -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]; }; diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c index a4bb33264c..f4c886dded 100644 --- a/drivers/net/intel/ice/ice_fdir_filter.c +++ b/drivers/net/intel/ice/ice_fdir_filter.c @@ -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, ¶m, &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; } diff --git a/drivers/net/intel/ice/ice_generic_flow.c b/drivers/net/intel/ice/ice_generic_flow.c index 1a30327293..25aa7aa41e 100644 --- a/drivers/net/intel/ice/ice_generic_flow.c +++ b/drivers/net/intel/ice/ice_generic_flow.c @@ -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"); diff --git a/drivers/net/intel/ice/ice_generic_flow.h b/drivers/net/intel/ice/ice_generic_flow.h index a801e0c704..efb3ee9b69 100644 --- a/drivers/net/intel/ice/ice_generic_flow.h +++ b/drivers/net/intel/ice/ice_generic_flow.h @@ -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, diff --git a/drivers/net/intel/ice/ice_hash.c b/drivers/net/intel/ice/ice_hash.c index 21545efefd..fe66b1dd5a 100644 --- a/drivers/net/intel/ice/ice_hash.c +++ b/drivers/net/intel/ice/ice_hash.c @@ -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); -} diff --git a/drivers/net/intel/ice/ice_switch_filter.c b/drivers/net/intel/ice/ice_switch_filter.c index 5c34089b40..4dd4707c40 100644 --- a/drivers/net/intel/ice/ice_switch_filter.c +++ b/drivers/net/intel/ice/ice_switch_filter.c @@ -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; } -- 2.52.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 6/6] net/ice: centralize flow rule memory management 2026-10-20 12:11 ` [PATCH v1 6/6] net/ice: centralize flow rule memory management Anatoly Burakov @ 2026-10-21 9:59 ` Bruce Richardson 2026-10-21 10:07 ` Burakov, Anatoly 0 siblings, 1 reply; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 9:59 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev On Tue, Oct 20, 2026 at 01:11:09PM +0100, Anatoly Burakov wrote: > 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 <anatoly.burakov@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> 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. > 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(-) > > diff --git a/drivers/net/intel/ice/ice_acl_filter.c b/drivers/net/intel/ice/ice_acl_filter.c > index fe1d03556c..9414bb3f12 100644 > --- a/drivers/net/intel/ice/ice_acl_filter.c > +++ b/drivers/net/intel/ice/ice_acl_filter.c > @@ -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), > diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h > index b802588b40..1baec0f97c 100644 > --- a/drivers/net/intel/ice/ice_ethdev.h > +++ b/drivers/net/intel/ice/ice_ethdev.h > @@ -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]; > }; > diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c > index a4bb33264c..f4c886dded 100644 > --- a/drivers/net/intel/ice/ice_fdir_filter.c > +++ b/drivers/net/intel/ice/ice_fdir_filter.c > @@ -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, ¶m, &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; > } > > diff --git a/drivers/net/intel/ice/ice_generic_flow.c b/drivers/net/intel/ice/ice_generic_flow.c > index 1a30327293..25aa7aa41e 100644 > --- a/drivers/net/intel/ice/ice_generic_flow.c > +++ b/drivers/net/intel/ice/ice_generic_flow.c > @@ -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"); > diff --git a/drivers/net/intel/ice/ice_generic_flow.h b/drivers/net/intel/ice/ice_generic_flow.h > index a801e0c704..efb3ee9b69 100644 > --- a/drivers/net/intel/ice/ice_generic_flow.h > +++ b/drivers/net/intel/ice/ice_generic_flow.h > @@ -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, > diff --git a/drivers/net/intel/ice/ice_hash.c b/drivers/net/intel/ice/ice_hash.c > index 21545efefd..fe66b1dd5a 100644 > --- a/drivers/net/intel/ice/ice_hash.c > +++ b/drivers/net/intel/ice/ice_hash.c > @@ -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); > -} > diff --git a/drivers/net/intel/ice/ice_switch_filter.c b/drivers/net/intel/ice/ice_switch_filter.c > index 5c34089b40..4dd4707c40 100644 > --- a/drivers/net/intel/ice/ice_switch_filter.c > +++ b/drivers/net/intel/ice/ice_switch_filter.c > @@ -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; > } > -- > 2.52.0 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 6/6] net/ice: centralize flow rule memory management 2026-10-21 9:59 ` Bruce Richardson @ 2026-10-21 10:07 ` Burakov, Anatoly 0 siblings, 0 replies; 16+ messages in thread From: Burakov, Anatoly @ 2026-10-21 10:07 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev On 10/21/2026 11:59 AM, Bruce Richardson wrote: > On Tue, Oct 20, 2026 at 01:11:09PM +0100, Anatoly Burakov wrote: >> 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 <anatoly.burakov@intel.com> > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > > 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. > Yes, true. In ice, flows are basically two (or more) allocations - the flow header itself, and the actual rule it encodes (with whatever nested allocations the rule has - switch and FDIR RAW do that), but all of the latter happens inside the `flow->rule` pointer, so the `flow` itself is not needed. Arguably, `meta` also needs renaming into `rule`, because that's what it does now: it is meant to be a pointer to `flow->rule`. -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 0/6] Centralize flow memory management in ICE 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov ` (5 preceding siblings ...) 2026-10-20 12:11 ` [PATCH v1 6/6] net/ice: centralize flow rule memory management Anatoly Burakov @ 2026-10-21 10:02 ` Bruce Richardson 6 siblings, 0 replies; 16+ messages in thread From: Bruce Richardson @ 2026-10-21 10:02 UTC (permalink / raw) To: Anatoly Burakov; +Cc: dev On Tue, Oct 20, 2026 at 01:11:03PM +0100, Anatoly Burakov wrote: > 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(-) > Series applied to dpdk-next-net-intel. Thanks, /Bruce ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-10-21 10:08 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-20 12:11 [PATCH v1 0/6] Centralize flow memory management in ICE Anatoly Burakov 2026-10-20 12:11 ` [PATCH v1 1/6] net/ice: release ACL slot on entry creation failure Anatoly Burakov 2026-10-21 9:17 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 2/6] net/ice: fix inconsistent state on FDIR failure Anatoly Burakov 2026-10-21 9:17 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 3/6] net/ice: standardize VOID handling in flow parsers Anatoly Burakov 2026-10-21 9:18 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 4/6] net/ice: remove temporary flow pattern buffer Anatoly Burakov 2026-10-21 9:19 ` Bruce Richardson 2026-10-20 12:11 ` [PATCH v1 5/6] net/ice: use caller-owned pattern match storage Anatoly Burakov 2026-10-21 9:25 ` Bruce Richardson 2026-10-21 9:55 ` Burakov, Anatoly 2026-10-20 12:11 ` [PATCH v1 6/6] net/ice: centralize flow rule memory management Anatoly Burakov 2026-10-21 9:59 ` Bruce Richardson 2026-10-21 10:07 ` Burakov, Anatoly 2026-10-21 10:02 ` [PATCH v1 0/6] Centralize flow memory management in ICE Bruce Richardson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.