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 2CD7DA056C; Wed, 4 Mar 2020 03:02:32 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D6E672C02; Wed, 4 Mar 2020 03:02:30 +0100 (CET) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id A420E2BA8; Wed, 4 Mar 2020 03:02:28 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Mar 2020 18:02:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,511,1574150400"; d="scan'208";a="229154620" Received: from dpdk51.sh.intel.com ([10.67.110.245]) by orsmga007.jf.intel.com with ESMTP; 03 Mar 2020 18:02:21 -0800 From: Qi Zhang To: xiaolong.ye@intel.com, yahui.cao@intel.com, simei.su@intel.com, wei.zhao@intel.com Cc: dev@dpdk.org, Qi Zhang , stable@dpdk.org Date: Wed, 4 Mar 2020 10:05:49 +0800 Message-Id: <20200304020549.13304-1-qi.z.zhang@intel.com> X-Mailer: git-send-email 2.13.6 Subject: [dpdk-dev] [PATCH v3] 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" 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. A meta data be created by parser->parse_pattern_action should be freed inside parser->engine->create. During a validate operation, since parser->engine_>create will not be invoked, a meta is not necessary be created during parser, so NULL will parsed to engine->parse_pattern_action and all parser's parse_pattern_action need to be modified to handle meta = NULL properly. With above implementation, the patch also fixes a potentional memory leak in ice_parse_engine_validate, since meta may not be freed. BTW, an engine without a create op should be regarded as a bug. So use RTE_ASSERT to replace runtime engine->create == NULL check in ice_parse_engine_create. Fixes: 4e27d3ed02bd ("net/ice: fix flow API framework") Cc: stable@dpdk.org Signed-off-by: Qi Zhang --- v3: - rollback meta free in ice_switch_create and ice_hash_create, since we assume parser->parse_pattern_action and parser->engine->create should be paired for meta memory allocation / free. - free sw_meta_ptr and list if meta == NULL in ice_switch_parse_pattern_action - use RTE_ASSERT to replace runtime check for engine->create == NULL; v2: - remove meta free from low level engine. - alloc parse NULL to parser->parse_pattern_action. drivers/net/ice/ice_fdir_filter.c | 3 ++- drivers/net/ice/ice_generic_flow.c | 32 +++++++++++--------------------- drivers/net/ice/ice_hash.c | 11 +++++++---- drivers/net/ice/ice_switch_filter.c | 16 +++++++++++----- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c index 5a791610f..6342b560c 100644 --- a/drivers/net/ice/ice_fdir_filter.c +++ b/drivers/net/ice/ice_fdir_filter.c @@ -1966,7 +1966,8 @@ ice_fdir_parse(struct ice_adapter *ad, if (ret) goto error; - *meta = filter; + if (meta) + *meta = filter; error: rte_free(item); return ret; diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c index 38ac799d8..64c0543c0 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,18 +1725,12 @@ 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; - if (engine->create == NULL) { - rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_HANDLE, - NULL, "Invalid engine"); - continue; - } - - ret = engine->create(ad, flow, *meta, error); + RTE_ASSERT(engine->create != NULL); + ret = engine->create(ad, flow, meta, error); if (ret == 0) return engine; else if (ret == -EEXIST) @@ -1752,7 +1745,6 @@ ice_parse_engine_validate(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; @@ -1763,12 +1755,13 @@ ice_parse_engine_validate(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, NULL, error) < 0) continue; engine = parser_node->parser->engine; break; } + return engine; } @@ -1779,7 +1772,6 @@ ice_flow_process_filter(struct rte_eth_dev *dev, const struct rte_flow_item pattern[], const struct rte_flow_action actions[], struct ice_flow_engine **engine, - void **meta, parse_engine_t ice_parse_engine, struct rte_flow_error *error) { @@ -1814,7 +1806,7 @@ ice_flow_process_filter(struct rte_eth_dev *dev, return ret; *engine = ice_parse_engine(ad, flow, &pf->rss_parser_list, - pattern, actions, meta, error); + pattern, actions, error); if (*engine != NULL) return 0; @@ -1822,11 +1814,11 @@ ice_flow_process_filter(struct rte_eth_dev *dev, case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR_ONLY: case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR: *engine = ice_parse_engine(ad, flow, &pf->dist_parser_list, - pattern, actions, meta, error); + pattern, actions, error); break; case ICE_FLOW_CLASSIFY_STAGE_PERMISSION: *engine = ice_parse_engine(ad, flow, &pf->perm_parser_list, - pattern, actions, meta, error); + pattern, actions, error); break; default: return -EINVAL; @@ -1845,11 +1837,10 @@ ice_flow_validate(struct rte_eth_dev *dev, const struct rte_flow_action actions[], struct rte_flow_error *error) { - void *meta; struct ice_flow_engine *engine; return ice_flow_process_filter(dev, NULL, attr, pattern, actions, - &engine, &meta, ice_parse_engine_validate, error); + &engine, ice_parse_engine_validate, error); } static struct rte_flow * @@ -1863,7 +1854,6 @@ ice_flow_create(struct rte_eth_dev *dev, struct rte_flow *flow = NULL; int ret; struct ice_flow_engine *engine = NULL; - void *meta; flow = rte_zmalloc("ice_flow", sizeof(struct rte_flow), 0); if (!flow) { @@ -1874,7 +1864,7 @@ ice_flow_create(struct rte_eth_dev *dev, } ret = ice_flow_process_filter(dev, flow, attr, pattern, actions, - &engine, &meta, ice_parse_engine_create, error); + &engine, ice_parse_engine_create, error); if (ret < 0) goto free_flow; flow->engine = engine; diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index d891538bd..c692130e7 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -432,14 +432,17 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad, goto error; /* Save protocol header to rss_meta. */ - *meta = rss_meta_ptr; - ((struct rss_meta *)*meta)->pkt_hdr = ((struct rss_type_match_hdr *) + rss_meta_ptr->pkt_hdr = ((struct rss_type_match_hdr *) (pattern_match_item->meta))->hdr_mask; /* Check rss action. */ - ret = ice_hash_parse_action(pattern_match_item, actions, meta, error); + ret = ice_hash_parse_action(pattern_match_item, actions, + (void **)&rss_meta_ptr, error); + error: - if (ret) + if (!ret && meta) + *meta = rss_meta_ptr; + else rte_free(rss_meta_ptr); rte_free(pattern_match_item); diff --git a/drivers/net/ice/ice_switch_filter.c b/drivers/net/ice/ice_switch_filter.c index 4a9356b31..87d484b75 100644 --- a/drivers/net/ice/ice_switch_filter.c +++ b/drivers/net/ice/ice_switch_filter.c @@ -255,7 +255,6 @@ ice_switch_create(struct ice_adapter *ad, error: rte_free(list); rte_free(meta); - return -rte_errno; } @@ -1088,10 +1087,17 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad, "Invalid input action"); goto error; } - *meta = sw_meta_ptr; - ((struct sw_meta *)*meta)->list = list; - ((struct sw_meta *)*meta)->lkups_num = lkups_num; - ((struct sw_meta *)*meta)->rule_info = rule_info; + + if (meta) { + *meta = sw_meta_ptr; + ((struct sw_meta *)*meta)->list = list; + ((struct sw_meta *)*meta)->lkups_num = lkups_num; + ((struct sw_meta *)*meta)->rule_info = rule_info; + } else { + rte_free(list); + rte_free(sw_meta_ptr); + } + rte_free(pattern_match_item); return 0; -- 2.13.6