diff mbox series

[1/2] net/ice: save meta on switch filter creation

Message ID 20211101084506.513407-1-dapengx.yu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [1/2] net/ice: save meta on switch filter creation |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Yu, DapengX Nov. 1, 2021, 8:45 a.m. UTC 
  From: Dapeng Yu <dapengx.yu@intel.com>

The meta is abandoned when switch filter is created in original
implementation.

This patch saved the meta in RTE flow for future use.

Fixes: 47d460d63233 ("net/ice: rework switch filter")
Cc: stable@dpdk.org

Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
---
 drivers/net/ice/ice_switch_filter.c | 69 ++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 20 deletions(-)

Comments

Qi Zhang Nov. 2, 2021, 12:06 a.m. UTC | #1 
> -----Original Message-----
> From: Yu, DapengX <dapengx.yu@intel.com>
> Sent: Monday, November 1, 2021 4:45 PM
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX
> <dapengx.yu@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/2] net/ice: save meta on switch filter creation
> 
> From: Dapeng Yu <dapengx.yu@intel.com>
> 
> The meta is abandoned when switch filter is created in original
> implementation.
> 
> This patch saved the meta in RTE flow for future use.

Can be more specific, it is used for flow replay when handling exception at flow redirect.

Will update during merge.

> 
> Fixes: 47d460d63233 ("net/ice: rework switch filter")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> ---
>  drivers/net/ice/ice_switch_filter.c | 69 ++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_switch_filter.c
> b/drivers/net/ice/ice_switch_filter.c
> index 6b0c1bff1e..804cf9b812 100644
> --- a/drivers/net/ice/ice_switch_filter.c
> +++ b/drivers/net/ice/ice_switch_filter.c
> @@ -180,6 +180,21 @@ struct sw_meta {
>  	struct ice_adv_rule_info rule_info;
>  };
> 
> +struct ice_switch_filter_conf {
> +	bool added;
> +
> +	/* Only when added flag is true, the query data is valid */
> +	struct ice_rule_query_data sw_query_data;
> +
> +	/*
> +	 * The lookup elements and rule info are saved here when filter creation
> +	 * succeeds.
> +	 */
> +	uint16_t lkups_num;
> +	struct ice_adv_lkup_elem *lkups;
> +	struct ice_adv_rule_info rule_info;
> +};
> +
>  static struct ice_flow_parser ice_switch_dist_parser;  static struct
> ice_flow_parser ice_switch_perm_parser;
> 
> @@ -359,7 +374,7 @@ ice_switch_create(struct ice_adapter *ad,
>  	struct ice_pf *pf = &ad->pf;
>  	struct ice_hw *hw = ICE_PF_TO_HW(pf);
>  	struct ice_rule_query_data rule_added = {0};
> -	struct ice_rule_query_data *filter_ptr;
> +	struct ice_switch_filter_conf *filter_conf_ptr;
>  	struct ice_adv_lkup_elem *list =
>  		((struct sw_meta *)meta)->list;
>  	uint16_t lkups_cnt =
> @@ -381,18 +396,24 @@ ice_switch_create(struct ice_adapter *ad,
>  	}
>  	ret = ice_add_adv_rule(hw, list, lkups_cnt, rule_info, &rule_added);
>  	if (!ret) {
> -		filter_ptr = rte_zmalloc("ice_switch_filter",
> -			sizeof(struct ice_rule_query_data), 0);
> -		if (!filter_ptr) {
> +		filter_conf_ptr = rte_zmalloc("ice_switch_filter",
> +			sizeof(struct ice_switch_filter_conf), 0);
> +		if (!filter_conf_ptr) {
>  			rte_flow_error_set(error, EINVAL,
>  				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>  				   "No memory for ice_switch_filter");
>  			goto error;
>  		}
> -		flow->rule = filter_ptr;
> -		rte_memcpy(filter_ptr,
> -			&rule_added,
> -			sizeof(struct ice_rule_query_data));
> +
> +		filter_conf_ptr->sw_query_data = rule_added;
> +
> +		filter_conf_ptr->lkups = list;
> +		filter_conf_ptr->lkups_num = lkups_cnt;
> +		filter_conf_ptr->rule_info = *rule_info;
> +
> +		filter_conf_ptr->added = true;
> +
> +		flow->rule = filter_conf_ptr;
>  	} else {
>  		rte_flow_error_set(error, EINVAL,
>  			RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> @@ -400,7 +421,6 @@ ice_switch_create(struct ice_adapter *ad,
>  		goto error;
>  	}
> 
> -	rte_free(list);
>  	rte_free(meta);
>  	return 0;
> 
> @@ -411,6 +431,18 @@ ice_switch_create(struct ice_adapter *ad,
>  	return -rte_errno;
>  }
> 
> +static inline void
> +ice_switch_filter_rule_free(struct rte_flow *flow) {
> +	struct ice_switch_filter_conf *filter_conf_ptr =
> +		(struct ice_switch_filter_conf *)flow->rule;
> +
> +	if (filter_conf_ptr)
> +		rte_free(filter_conf_ptr->lkups);
> +
> +	rte_free(filter_conf_ptr);
> +}
> +
>  static int
>  ice_switch_destroy(struct ice_adapter *ad,
>  		struct rte_flow *flow,
> @@ -418,20 +450,23 @@ ice_switch_destroy(struct ice_adapter *ad,  {
>  	struct ice_hw *hw = &ad->hw;
>  	int ret;
> -	struct ice_rule_query_data *filter_ptr;
> +	struct ice_switch_filter_conf *filter_conf_ptr;
> 
> -	filter_ptr = (struct ice_rule_query_data *)
> +	filter_conf_ptr = (struct ice_switch_filter_conf *)
>  		flow->rule;
> 
> -	if (!filter_ptr) {
> +	if (!filter_conf_ptr || !filter_conf_ptr->added) {
>  		rte_flow_error_set(error, EINVAL,
>  			RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>  			"no such flow"
>  			" create by switch filter");
> +
> +		ice_switch_filter_rule_free(flow);
> +
>  		return -rte_errno;
>  	}
> 
> -	ret = ice_rem_adv_rule_by_id(hw, filter_ptr);
> +	ret = ice_rem_adv_rule_by_id(hw, &filter_conf_ptr->sw_query_data);
>  	if (ret) {
>  		rte_flow_error_set(error, EINVAL,
>  			RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> @@ -439,16 +474,10 @@ ice_switch_destroy(struct ice_adapter *ad,
>  		return -rte_errno;
>  	}
> 
> -	rte_free(filter_ptr);
> +	ice_switch_filter_rule_free(flow);
>  	return ret;
>  }
> 
> -static void
> -ice_switch_filter_rule_free(struct rte_flow *flow) -{
> -	rte_free(flow->rule);
> -}
> -
>  static bool
>  ice_switch_parse_pattern(const struct rte_flow_item pattern[],
>  		struct rte_flow_error *error,
> --
> 2.27.0
Qi Zhang Nov. 2, 2021, 12:08 a.m. UTC | #2 
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Tuesday, November 2, 2021 8:07 AM
> To: Yu, DapengX <dapengx.yu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>;
> stable@dpdk.org
> Subject: RE: [PATCH 1/2] net/ice: save meta on switch filter creation
> 
> 
> 
> > -----Original Message-----
> > From: Yu, DapengX <dapengx.yu@intel.com>
> > Sent: Monday, November 1, 2021 4:45 PM
> > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX
> > <dapengx.yu@intel.com>; stable@dpdk.org
> > Subject: [PATCH 1/2] net/ice: save meta on switch filter creation
> >
> > From: Dapeng Yu <dapengx.yu@intel.com>
> >
> > The meta is abandoned when switch filter is created in original
> > implementation.
> >
> > This patch saved the meta in RTE flow for future use.
> 
> Can be more specific, it is used for flow replay when handling exception at flow
> redirect.
> 
> Will update during merge.
> 
> >
> > Fixes: 47d460d63233 ("net/ice: rework switch filter")

This is not a fix, but can cc stable as the real fix patch depends on this. 
will remove fixline during merge.

> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
Ferruh Yigit Nov. 2, 2021, 4:11 p.m. UTC | #3 
On 11/1/2021 8:45 AM, dapengx.yu@intel.com wrote:
> From: Dapeng Yu<dapengx.yu@intel.com>
> 
> The meta is abandoned when switch filter is created in original
> implementation.
> 

What is 'meta', do you mean 'metadata'? Can you please clarify which
metadata you are referring to?

> This patch saved the meta in RTE flow for future use.
> 
> Fixes: 47d460d63233 ("net/ice: rework switch filter")
> Cc:stable@dpdk.org
> 
> Signed-off-by: Dapeng Yu<dapengx.yu@intel.com>
Yu, DapengX Nov. 3, 2021, 3:25 a.m. UTC | #4 
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Wednesday, November 3, 2021 12:11 AM
> To: Yu, DapengX <dapengx.yu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH 1/2] net/ice: save meta on switch filter
> creation
> 
> On 11/1/2021 8:45 AM, dapengx.yu@intel.com wrote:
> > From: Dapeng Yu<dapengx.yu@intel.com>
> >
> > The meta is abandoned when switch filter is created in original
> > implementation.
> >
> 
> What is 'meta', do you mean 'metadata'? Can you please clarify which
> metadata you are referring to?
> 
The commit log will be improved in the next version.

> > This patch saved the meta in RTE flow for future use.
> >
> > Fixes: 47d460d63233 ("net/ice: rework switch filter")
> > Cc:stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu<dapengx.yu@intel.com>
diff mbox series

Patch

diff --git a/drivers/net/ice/ice_switch_filter.c b/drivers/net/ice/ice_switch_filter.c
index 6b0c1bff1e..804cf9b812 100644
--- a/drivers/net/ice/ice_switch_filter.c
+++ b/drivers/net/ice/ice_switch_filter.c
@@ -180,6 +180,21 @@  struct sw_meta {
 	struct ice_adv_rule_info rule_info;
 };
 
+struct ice_switch_filter_conf {
+	bool added;
+
+	/* Only when added flag is true, the query data is valid */
+	struct ice_rule_query_data sw_query_data;
+
+	/*
+	 * The lookup elements and rule info are saved here when filter creation
+	 * succeeds.
+	 */
+	uint16_t lkups_num;
+	struct ice_adv_lkup_elem *lkups;
+	struct ice_adv_rule_info rule_info;
+};
+
 static struct ice_flow_parser ice_switch_dist_parser;
 static struct ice_flow_parser ice_switch_perm_parser;
 
@@ -359,7 +374,7 @@  ice_switch_create(struct ice_adapter *ad,
 	struct ice_pf *pf = &ad->pf;
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
 	struct ice_rule_query_data rule_added = {0};
-	struct ice_rule_query_data *filter_ptr;
+	struct ice_switch_filter_conf *filter_conf_ptr;
 	struct ice_adv_lkup_elem *list =
 		((struct sw_meta *)meta)->list;
 	uint16_t lkups_cnt =
@@ -381,18 +396,24 @@  ice_switch_create(struct ice_adapter *ad,
 	}
 	ret = ice_add_adv_rule(hw, list, lkups_cnt, rule_info, &rule_added);
 	if (!ret) {
-		filter_ptr = rte_zmalloc("ice_switch_filter",
-			sizeof(struct ice_rule_query_data), 0);
-		if (!filter_ptr) {
+		filter_conf_ptr = rte_zmalloc("ice_switch_filter",
+			sizeof(struct ice_switch_filter_conf), 0);
+		if (!filter_conf_ptr) {
 			rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 				   "No memory for ice_switch_filter");
 			goto error;
 		}
-		flow->rule = filter_ptr;
-		rte_memcpy(filter_ptr,
-			&rule_added,
-			sizeof(struct ice_rule_query_data));
+
+		filter_conf_ptr->sw_query_data = rule_added;
+
+		filter_conf_ptr->lkups = list;
+		filter_conf_ptr->lkups_num = lkups_cnt;
+		filter_conf_ptr->rule_info = *rule_info;
+
+		filter_conf_ptr->added = true;
+
+		flow->rule = filter_conf_ptr;
 	} else {
 		rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
@@ -400,7 +421,6 @@  ice_switch_create(struct ice_adapter *ad,
 		goto error;
 	}
 
-	rte_free(list);
 	rte_free(meta);
 	return 0;
 
@@ -411,6 +431,18 @@  ice_switch_create(struct ice_adapter *ad,
 	return -rte_errno;
 }
 
+static inline void
+ice_switch_filter_rule_free(struct rte_flow *flow)
+{
+	struct ice_switch_filter_conf *filter_conf_ptr =
+		(struct ice_switch_filter_conf *)flow->rule;
+
+	if (filter_conf_ptr)
+		rte_free(filter_conf_ptr->lkups);
+
+	rte_free(filter_conf_ptr);
+}
+
 static int
 ice_switch_destroy(struct ice_adapter *ad,
 		struct rte_flow *flow,
@@ -418,20 +450,23 @@  ice_switch_destroy(struct ice_adapter *ad,
 {
 	struct ice_hw *hw = &ad->hw;
 	int ret;
-	struct ice_rule_query_data *filter_ptr;
+	struct ice_switch_filter_conf *filter_conf_ptr;
 
-	filter_ptr = (struct ice_rule_query_data *)
+	filter_conf_ptr = (struct ice_switch_filter_conf *)
 		flow->rule;
 
-	if (!filter_ptr) {
+	if (!filter_conf_ptr || !filter_conf_ptr->added) {
 		rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 			"no such flow"
 			" create by switch filter");
+
+		ice_switch_filter_rule_free(flow);
+
 		return -rte_errno;
 	}
 
-	ret = ice_rem_adv_rule_by_id(hw, filter_ptr);
+	ret = ice_rem_adv_rule_by_id(hw, &filter_conf_ptr->sw_query_data);
 	if (ret) {
 		rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
@@ -439,16 +474,10 @@  ice_switch_destroy(struct ice_adapter *ad,
 		return -rte_errno;
 	}
 
-	rte_free(filter_ptr);
+	ice_switch_filter_rule_free(flow);
 	return ret;
 }
 
-static void
-ice_switch_filter_rule_free(struct rte_flow *flow)
-{
-	rte_free(flow->rule);
-}
-
 static bool
 ice_switch_parse_pattern(const struct rte_flow_item pattern[],
 		struct rte_flow_error *error,