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 <qi.z.zhang@intel.com>
Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
index 5a79161..6342b56 100644 (file)
--- 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 38ac799..a1648ee 100644 (file)
--- 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 0e9c3c4..0fdd4d6 100644 (file)
--- 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 4a9356b..66dc158 100644 (file)
--- 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;