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 AA41FA056F; Tue, 3 Mar 2020 00:56:23 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 895711BFF6; Tue, 3 Mar 2020 00:56:21 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 559223B5; Tue, 3 Mar 2020 00:56:18 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Mar 2020 15:56:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,509,1574150400"; d="scan'208";a="232062927" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga007.fm.intel.com with ESMTP; 02 Mar 2020 15:56:17 -0800 Received: from fmsmsx102.amr.corp.intel.com (10.18.124.200) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 2 Mar 2020 15:56:17 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX102.amr.corp.intel.com (10.18.124.200) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 2 Mar 2020 15:56:16 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.137]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.155]) with mapi id 14.03.0439.000; Tue, 3 Mar 2020 07:56:14 +0800 From: "Zhang, Qi Z" To: "Ye, Xiaolong" CC: "Xing, Beilei" , "Cao, Yahui" , "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] net/ice: remove unnecessary variable Thread-Index: AQHV690YwN5iPloKXU6Ix3advII+d6g0eM6AgAGJz/A= Date: Mon, 2 Mar 2020 23:56:14 +0000 Message-ID: <039ED4275CED7440929022BC67E70611547D8F44@SHSMSX103.ccr.corp.intel.com> References: <20200225131418.5750-1-qi.z.zhang@intel.com> <20200302081941.GC25927@intel.com> In-Reply-To: <20200302081941.GC25927@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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" > -----Original Message----- > From: Ye, Xiaolong > Sent: Monday, March 2, 2020 4:20 PM > To: Zhang, Qi Z > Cc: Xing, Beilei ; Cao, Yahui ; > dev@dpdk.org; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] net/ice: remove unnecessary variable >=20 > Hi, Qi >=20 > 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 >=20 > s/move/moves >=20 > >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 =3D NULL; > > struct ice_flow_parser_node *parser_node; > >+ void *meta =3D 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 =3D parser_node->parser->engine; @@ -1737,7 +1736,9 @@ > >ice_parse_engine_create(struct ice_adapter *ad, > > continue; > > } > > > >- ret =3D engine->create(ad, flow, *meta, error); > >+ ret =3D engine->create(ad, flow, meta, error); > >+ if (meta) > >+ rte_free(meta); >=20 > Maybe I miss something, I see meta in fdir's parse_pattern_action is assi= gned as > &pf->fdir.conf which is not dynamically allocated like sw_meta_ptr in swi= tch > 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 ne= w op like engine->meta_release. Thanks Qi >=20 > Thanks, > Xiaolong