From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id D122EA0568; Mon, 2 Mar 2020 09:21:55 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3CE6F1C001; Mon, 2 Mar 2020 09:21:55 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 4D7EA1BFF9; Mon, 2 Mar 2020 09:21:53 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Mar 2020 00:21:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,506,1574150400"; d="scan'208";a="273631904" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by fmsmga002.fm.intel.com with ESMTP; 02 Mar 2020 00:21:50 -0800 Date: Mon, 2 Mar 2020 16:19:41 +0800 From: Ye Xiaolong To: Qi Zhang Cc: beilei.xing@intel.com, yahui.cao@intel.com, dev@dpdk.org, stable@dpdk.org Message-ID: <20200302081941.GC25927@intel.com> References: <20200225131418.5750-1-qi.z.zhang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200225131418.5750-1-qi.z.zhang@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH] net/ice: remove unnecessary variable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >--- > 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