Re: [dpdk-dev] [PATCH] net/ice: remove unnecessary variable

[Ye Xiaolong <xiaolong.ye@intel.com>]

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: stable@dpdk.org
>
>Signed-off-by: Qi Zhang <qi.z.zhang@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