Hi, Qi
On 02/25, Qi Zhang wrote:
>Remove unnecessary variable "meta" in ice_flow_create and
>ice_flow_validate, it should be defined when really be needed:
>its ice_parse_engine_create and ice_parse_engine_validate.
>
>The patch also move the meta's memory free from each filter
s/move/moves
>engine->create to upper layer, the memory leakage when create
>a fdir rule is also be fixed.
>
>Fixes: f5cafa961fae ("net/ice: add flow director create and destroy")
>Cc: sta...@dpdk.org
>
>Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
>---
> drivers/net/ice/ice_generic_flow.c | 28 ++++++++++++++--------------
> drivers/net/ice/ice_hash.c | 2 --
> drivers/net/ice/ice_switch_filter.c | 3 ---
> 3 files changed, 14 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/net/ice/ice_generic_flow.c
>b/drivers/net/ice/ice_generic_flow.c
>index 38ac799d8..f22538758 100644
>--- a/drivers/net/ice/ice_generic_flow.c
>+++ b/drivers/net/ice/ice_generic_flow.c
>@@ -1375,7 +1375,6 @@ typedef struct ice_flow_engine *
>(*parse_engine_t)(struct ice_adapter *ad,
> struct ice_parser_list *parser_list,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
>- void **meta,
> struct rte_flow_error *error);
>
> void
>@@ -1713,11 +1712,11 @@ ice_parse_engine_create(struct ice_adapter *ad,
> struct ice_parser_list *parser_list,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
>- void **meta,
> struct rte_flow_error *error)
> {
> struct ice_flow_engine *engine = NULL;
> struct ice_flow_parser_node *parser_node;
>+ void *meta = NULL;
> void *temp;
>
> TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
>@@ -1726,7 +1725,7 @@ ice_parse_engine_create(struct ice_adapter *ad,
> if (parser_node->parser->parse_pattern_action(ad,
> parser_node->parser->array,
> parser_node->parser->array_len,
>- pattern, actions, meta, error) < 0)
>+ pattern, actions, &meta, error) < 0)
> continue;
>
> engine = parser_node->parser->engine;
>@@ -1737,7 +1736,9 @@ ice_parse_engine_create(struct ice_adapter *ad,
> continue;
> }
>
>- ret = engine->create(ad, flow, *meta, error);
>+ ret = engine->create(ad, flow, meta, error);
>+ if (meta)
>+ rte_free(meta);
Maybe I miss something, I see meta in fdir's parse_pattern_action is assigned
as
&pf->fdir.conf which is not dynamically allocated like sw_meta_ptr in switch
and
rss_meta_ptr in hash, is it valid to call rte_free(meta) for fdir?
And theoretically, if engine->create is NULL, is there a memory leak for meta?
Thanks,
Xiaolong
