[dpdk-stable] [dpdk-dev] [PATCH] net/ice: remove unnecessary variable
Zhang, Qi Z
qi.z.zhang at intel.com
Tue Mar 3 00:56:14 CET 2020
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye at intel.com>
> Sent: Monday, March 2, 2020 4:20 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>
> Cc: Xing, Beilei <beilei.xing at intel.com>; Cao, Yahui <yahui.cao at intel.com>;
> dev at dpdk.org; stable at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ice: remove unnecessary variable
>
> 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 at dpdk.org
> >
> >Signed-off-by: Qi Zhang <qi.z.zhang at 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?
Good point, meta release function could be implemented in low level engine and but should be directed by up layer, so I think the missing part is a new op like engine->meta_release.
Thanks
Qi
>
> Thanks,
> Xiaolong
More information about the stable
mailing list