[dpdk-stable] patch 'net/ice: remove unnecessary variable' has been queued to stable release 19.11.3

[luca.boccassi at gmail.com]

Hi,

FYI, your patch has been queued to stable release 19.11.3

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 05/21/20. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Thanks.

Luca Boccassi

---
>From adf15e37f25ec94a1fb3dfbbc269181f79562018 Mon Sep 17 00:00:00 2001
From: Qi Zhang <qi.z.zhang at intel.com>
Date: Wed, 4 Mar 2020 10:05:49 +0800
Subject: [PATCH] net/ice: remove unnecessary variable

[ upstream commit 31108c69ba5d55a7f35da1b650998be9a19e13dc ]

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")

Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
Reviewed-by: Xiaolong Ye <xiaolong.ye at intel.com>
---
 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

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2020-05-19 13:56:19.794951365 +0100
+++ 0024-net-ice-remove-unnecessary-variable.patch	2020-05-19 13:56:18.207501603 +0100
@@ -1,8 +1,10 @@
-From 31108c69ba5d55a7f35da1b650998be9a19e13dc Mon Sep 17 00:00:00 2001
+From adf15e37f25ec94a1fb3dfbbc269181f79562018 Mon Sep 17 00:00:00 2001
 From: Qi Zhang <qi.z.zhang at intel.com>
 Date: Wed, 4 Mar 2020 10:05:49 +0800
 Subject: [PATCH] net/ice: remove unnecessary variable
 
+[ upstream commit 31108c69ba5d55a7f35da1b650998be9a19e13dc ]
+
 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.
@@ -24,7 +26,6 @@
 ice_parse_engine_create.
 
 Fixes: 4e27d3ed02bd ("net/ice: fix flow API framework")
-Cc: stable at dpdk.org
 
 Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
 Reviewed-by: Xiaolong Ye <xiaolong.ye at intel.com>