From 31108c69ba5d55a7f35da1b650998be9a19e13dc Mon Sep 17 00:00:00 2001 From: Qi Zhang Date: Wed, 4 Mar 2020 10:05:49 +0800 Subject: [PATCH] net/ice: remove unnecessary variable Remove unnecessary variable "meta" in ice_flow_create and ice_flow_validate, it should be defined when really needed: its ice_parse_engine_create and ice_parse_engine_validate. A meta data 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, it is unnecessary to create a meta during parser, so NULL will passed 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 potential 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 Reviewed-by: Xiaolong Ye --- drivers/net/ice/ice_fdir_filter.c | 3 ++- drivers/net/ice/ice_generic_flow.c | 31 ++++++++++------------------- drivers/net/ice/ice_hash.c | 11 ++++++---- drivers/net/ice/ice_switch_filter.c | 15 ++++++++++---- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c index 5a791610f8..6342b560c9 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 38ac799d85..a1648eeb58 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,7 +1755,7 @@ 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; @@ -1779,7 +1771,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 +1805,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 +1813,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 +1836,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 +1853,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 +1863,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 0e9c3c4e50..0fdd4d68d4 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -434,14 +434,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 4a9356b317..66dc158efc 100644 --- a/drivers/net/ice/ice_switch_filter.c +++ b/drivers/net/ice/ice_switch_filter.c @@ -1088,10 +1088,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.20.1