Re: [dpdk-dev] [PATCH] net/ice: support flow redirect function

Re: [dpdk-dev] [PATCH] net/ice: support flow redirect function

[Yang, Qiming]

> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Tuesday, March 31, 2020 00:38
> To: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: [PATCH] net/ice: support flow redirect function
> 
> Add a new ops "redirect" to flow engine, it's used to implement the function
> that redirect a flow's destination. Currently only support VSI-Redirect which
> will be used by DCF for handling VSI mapping table change.
> A new API "ice_flow_redirect" is exposed, current usage is: it could be called
> when VSI mapping table change happens.

We can mention the case, like VF reset. The usage will be more clear.

> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  drivers/net/ice/ice_generic_flow.c | 22 ++++++++++++++++++++++
> drivers/net/ice/ice_generic_flow.h | 21 +++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/net/ice/ice_generic_flow.c
> b/drivers/net/ice/ice_generic_flow.c
> index a1648ee..823ff0e 100644
> --- a/drivers/net/ice/ice_generic_flow.c
> +++ b/drivers/net/ice/ice_generic_flow.c
> @@ -1961,3 +1961,25 @@ ice_flow_query(struct rte_eth_dev *dev,
>  	}
>  	return ret;
>  }
> +
> +int
> +ice_flow_redirect(struct ice_adapter *ad,
> +		  struct ice_flow_redirect *rd)
> +{
> +	struct ice_pf *pf = &ad->pf;
> +	struct rte_flow *p_flow;
> +	void *temp;
> +	int ret;
> +
> +	TAILQ_FOREACH_SAFE(p_flow, &pf->flow_list, node, temp) {
> +		if (!p_flow->engine->redirect)
> +			continue;
> +		ret = p_flow->engine->redirect(ad, p_flow, rd);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "Failed to redirect flows");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/net/ice/ice_generic_flow.h
> b/drivers/net/ice/ice_generic_flow.h
> index adc30ee..ede6ec8 100644
> --- a/drivers/net/ice/ice_generic_flow.h
> +++ b/drivers/net/ice/ice_generic_flow.h
> @@ -417,6 +417,20 @@ struct ice_pattern_match_item {
>  	void *meta;
>  };
> 
> +enum ice_flow_redirect_type {
> +	ICE_FLOW_REDIRECT_VSI,
> +};
> +
> +struct ice_flow_redirect {
> +	enum ice_flow_redirect_type type;
> +	union {
> +		struct {
> +			uint16_t vsi_handle;
> +			uint16_t new_vsi_num;
> +		};
> +	};
> +};
> +
>  typedef int (*engine_init_t)(struct ice_adapter *ad);  typedef void
> (*engine_uninit_t)(struct ice_adapter *ad);  typedef int
> (*engine_create_t)(struct ice_adapter *ad, @@ -430,6 +444,9 @@ typedef
> int (*engine_query_t)(struct ice_adapter *ad,
>  		struct rte_flow *flow,
>  		struct rte_flow_query_count *count,
>  		struct rte_flow_error *error);
> +typedef int(*engine_redirect_t)(struct ice_adapter *ad,
> +				struct rte_flow *flow,
> +				struct ice_flow_redirect *redirect);
>  typedef void (*engine_free_t) (struct rte_flow *flow);  typedef int
> (*parse_pattern_action_t)(struct ice_adapter *ad,
>  		struct ice_pattern_match_item *array, @@ -447,6 +464,7
> @@ struct ice_flow_engine {
>  	engine_create_t create;
>  	engine_destroy_t destroy;
>  	engine_query_t query_count;
> +	engine_redirect_t redirect;
>  	engine_free_t free;
>  	enum ice_flow_engine_type type;
>  };
> @@ -485,4 +503,7 @@ ice_search_pattern_match_item(const struct
> rte_flow_item pattern[],
>  		struct ice_pattern_match_item *array,
>  		uint32_t array_len,
>  		struct rte_flow_error *error);
> +int
> +ice_flow_redirect(struct ice_adapter *ad,
> +		  struct ice_flow_redirect *rd);
>  #endif
> --
> 2.7.4

Acked-by: Qiming Yang <qiming.yang@intel.com>

[dpdk-dev] [PATCH] net/ice: support flow redirect function

[Beilei Xing]

Add a new ops "redirect" to flow engine, it's used to implement the
function that redirect a flow's destination. Currently only support
VSI-Redirect which will be used by DCF for handling VSI mapping table
change.
A new API "ice_flow_redirect" is exposed, current usage is: it could
be called when VSI mapping table change happens.

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/ice/ice_generic_flow.c | 22 ++++++++++++++++++++++
 drivers/net/ice/ice_generic_flow.h | 21 +++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index a1648ee..823ff0e 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -1961,3 +1961,25 @@ ice_flow_query(struct rte_eth_dev *dev,
 	}
 	return ret;
 }
+
+int
+ice_flow_redirect(struct ice_adapter *ad,
+		  struct ice_flow_redirect *rd)
+{
+	struct ice_pf *pf = &ad->pf;
+	struct rte_flow *p_flow;
+	void *temp;
+	int ret;
+
+	TAILQ_FOREACH_SAFE(p_flow, &pf->flow_list, node, temp) {
+		if (!p_flow->engine->redirect)
+			continue;
+		ret = p_flow->engine->redirect(ad, p_flow, rd);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Failed to redirect flows");
+			return ret;
+		}
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ice/ice_generic_flow.h b/drivers/net/ice/ice_generic_flow.h
index adc30ee..ede6ec8 100644
--- a/drivers/net/ice/ice_generic_flow.h
+++ b/drivers/net/ice/ice_generic_flow.h
@@ -417,6 +417,20 @@ struct ice_pattern_match_item {
 	void *meta;
 };
 
+enum ice_flow_redirect_type {
+	ICE_FLOW_REDIRECT_VSI,
+};
+
+struct ice_flow_redirect {
+	enum ice_flow_redirect_type type;
+	union {
+		struct {
+			uint16_t vsi_handle;
+			uint16_t new_vsi_num;
+		};
+	};
+};
+
 typedef int (*engine_init_t)(struct ice_adapter *ad);
 typedef void (*engine_uninit_t)(struct ice_adapter *ad);
 typedef int (*engine_create_t)(struct ice_adapter *ad,
@@ -430,6 +444,9 @@ typedef int (*engine_query_t)(struct ice_adapter *ad,
 		struct rte_flow *flow,
 		struct rte_flow_query_count *count,
 		struct rte_flow_error *error);
+typedef int(*engine_redirect_t)(struct ice_adapter *ad,
+				struct rte_flow *flow,
+				struct ice_flow_redirect *redirect);
 typedef void (*engine_free_t) (struct rte_flow *flow);
 typedef int (*parse_pattern_action_t)(struct ice_adapter *ad,
 		struct ice_pattern_match_item *array,
@@ -447,6 +464,7 @@ struct ice_flow_engine {
 	engine_create_t create;
 	engine_destroy_t destroy;
 	engine_query_t query_count;
+	engine_redirect_t redirect;
 	engine_free_t free;
 	enum ice_flow_engine_type type;
 };
@@ -485,4 +503,7 @@ ice_search_pattern_match_item(const struct rte_flow_item pattern[],
 		struct ice_pattern_match_item *array,
 		uint32_t array_len,
 		struct rte_flow_error *error);
+int
+ice_flow_redirect(struct ice_adapter *ad,
+		  struct ice_flow_redirect *rd);
 #endif
-- 
2.7.4

[dpdk-dev] [PATCH v2 0/2] add flow redirect function

[Beilei Xing]

This patchset adds flow redirect function, also supports
flow ops thread safe

V2 changes:
 - Rework commit log.
 - Make a patchset.

Beilei Xing (2):
  net/ice: support flow redirect function
  net/ice: support flow ops thread safe

 drivers/net/ice/ice_ethdev.h       |  1 +
 drivers/net/ice/ice_generic_flow.c | 57 ++++++++++++++++++++++++++++++++------
 drivers/net/ice/ice_generic_flow.h | 21 ++++++++++++++
 3 files changed, 71 insertions(+), 8 deletions(-)

-- 
2.7.4

Acked-by: Qiming Yang <qiming.yang@intel.com>

[dpdk-dev] [PATCH v2 1/2] net/ice: support flow redirect function

[Beilei Xing]

Add a new ops "redirect" to flow engine, it's used to implement the
function that redirect a flow's destination. Currently only support
VSI-Redirect which will be used by DCF for handling VF-VSI mapping
table change.
A new API "ice_flow_redirect" is exposed, current usage is: it could
be called when there's VF-VSI mapping table change caused by VF reset.

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/ice/ice_generic_flow.c | 22 ++++++++++++++++++++++
 drivers/net/ice/ice_generic_flow.h | 21 +++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index a1648ee..823ff0e 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -1961,3 +1961,25 @@ ice_flow_query(struct rte_eth_dev *dev,
 	}
 	return ret;
 }
+
+int
+ice_flow_redirect(struct ice_adapter *ad,
+		  struct ice_flow_redirect *rd)
+{
+	struct ice_pf *pf = &ad->pf;
+	struct rte_flow *p_flow;
+	void *temp;
+	int ret;
+
+	TAILQ_FOREACH_SAFE(p_flow, &pf->flow_list, node, temp) {
+		if (!p_flow->engine->redirect)
+			continue;
+		ret = p_flow->engine->redirect(ad, p_flow, rd);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Failed to redirect flows");
+			return ret;
+		}
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ice/ice_generic_flow.h b/drivers/net/ice/ice_generic_flow.h
index adc30ee..ede6ec8 100644
--- a/drivers/net/ice/ice_generic_flow.h
+++ b/drivers/net/ice/ice_generic_flow.h
@@ -417,6 +417,20 @@ struct ice_pattern_match_item {
 	void *meta;
 };
 
+enum ice_flow_redirect_type {
+	ICE_FLOW_REDIRECT_VSI,
+};
+
+struct ice_flow_redirect {
+	enum ice_flow_redirect_type type;
+	union {
+		struct {
+			uint16_t vsi_handle;
+			uint16_t new_vsi_num;
+		};
+	};
+};
+
 typedef int (*engine_init_t)(struct ice_adapter *ad);
 typedef void (*engine_uninit_t)(struct ice_adapter *ad);
 typedef int (*engine_create_t)(struct ice_adapter *ad,
@@ -430,6 +444,9 @@ typedef int (*engine_query_t)(struct ice_adapter *ad,
 		struct rte_flow *flow,
 		struct rte_flow_query_count *count,
 		struct rte_flow_error *error);
+typedef int(*engine_redirect_t)(struct ice_adapter *ad,
+				struct rte_flow *flow,
+				struct ice_flow_redirect *redirect);
 typedef void (*engine_free_t) (struct rte_flow *flow);
 typedef int (*parse_pattern_action_t)(struct ice_adapter *ad,
 		struct ice_pattern_match_item *array,
@@ -447,6 +464,7 @@ struct ice_flow_engine {
 	engine_create_t create;
 	engine_destroy_t destroy;
 	engine_query_t query_count;
+	engine_redirect_t redirect;
 	engine_free_t free;
 	enum ice_flow_engine_type type;
 };
@@ -485,4 +503,7 @@ ice_search_pattern_match_item(const struct rte_flow_item pattern[],
 		struct ice_pattern_match_item *array,
 		uint32_t array_len,
 		struct rte_flow_error *error);
+int
+ice_flow_redirect(struct ice_adapter *ad,
+		  struct ice_flow_redirect *rd);
 #endif
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/2] net/ice: support flow ops thread safe

[Beilei Xing]

For DCF, flow ops may be executed in different threads,
so an thread safe option for generic flow APIs is needed.

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/ice/ice_ethdev.h       |  1 +
 drivers/net/ice/ice_generic_flow.c | 35 +++++++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 7b94a3c..f88f9dd 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -384,6 +384,7 @@ struct ice_pf {
 	bool offset_loaded;
 	bool adapter_stopped;
 	struct ice_flow_list flow_list;
+	rte_spinlock_t flow_ops_lock;
 	struct ice_parser_list rss_parser_list;
 	struct ice_parser_list perm_parser_list;
 	struct ice_parser_list dist_parser_list;
diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 823ff0e..c042079 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -1395,6 +1395,7 @@ ice_flow_init(struct ice_adapter *ad)
 	TAILQ_INIT(&pf->rss_parser_list);
 	TAILQ_INIT(&pf->perm_parser_list);
 	TAILQ_INIT(&pf->dist_parser_list);
+	rte_spinlock_init(&pf->flow_ops_lock);
 
 	TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
 		if (engine->init == NULL) {
@@ -1862,19 +1863,24 @@ ice_flow_create(struct rte_eth_dev *dev,
 		return flow;
 	}
 
+	rte_spinlock_lock(&pf->flow_ops_lock);
+
 	ret = ice_flow_process_filter(dev, flow, attr, pattern, actions,
 			&engine, ice_parse_engine_create, error);
-	if (ret < 0)
-		goto free_flow;
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "Failed to create flow");
+		rte_free(flow);
+		flow = NULL;
+		goto out;
+	}
+
 	flow->engine = engine;
 	TAILQ_INSERT_TAIL(&pf->flow_list, flow, node);
 	PMD_DRV_LOG(INFO, "Succeeded to create (%d) flow", engine->type);
-	return flow;
 
-free_flow:
-	PMD_DRV_LOG(ERR, "Failed to create flow");
-	rte_free(flow);
-	return NULL;
+out:
+	rte_spinlock_unlock(&pf->flow_ops_lock);
+	return flow;
 }
 
 static int
@@ -1894,8 +1900,9 @@ ice_flow_destroy(struct rte_eth_dev *dev,
 		return -rte_errno;
 	}
 
-	ret = flow->engine->destroy(ad, flow, error);
+	rte_spinlock_lock(&pf->flow_ops_lock);
 
+	ret = flow->engine->destroy(ad, flow, error);
 	if (!ret) {
 		TAILQ_REMOVE(&pf->flow_list, flow, node);
 		rte_free(flow);
@@ -1903,6 +1910,8 @@ ice_flow_destroy(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "Failed to destroy flow");
 	}
 
+	rte_spinlock_unlock(&pf->flow_ops_lock);
+
 	return ret;
 }
 
@@ -1937,6 +1946,7 @@ ice_flow_query(struct rte_eth_dev *dev,
 	struct ice_adapter *ad =
 		ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct rte_flow_query_count *count = data;
+	struct ice_pf *pf = &ad->pf;
 
 	if (!flow || !flow->engine || !flow->engine->query_count) {
 		rte_flow_error_set(error, EINVAL,
@@ -1945,6 +1955,8 @@ ice_flow_query(struct rte_eth_dev *dev,
 		return -rte_errno;
 	}
 
+	rte_spinlock_lock(&pf->flow_ops_lock);
+
 	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
 		switch (actions->type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
@@ -1959,6 +1971,9 @@ ice_flow_query(struct rte_eth_dev *dev,
 					"action not supported");
 		}
 	}
+
+	rte_spinlock_unlock(&pf->flow_ops_lock);
+
 	return ret;
 }
 
@@ -1971,6 +1986,8 @@ ice_flow_redirect(struct ice_adapter *ad,
 	void *temp;
 	int ret;
 
+	rte_spinlock_lock(&pf->flow_ops_lock);
+
 	TAILQ_FOREACH_SAFE(p_flow, &pf->flow_list, node, temp) {
 		if (!p_flow->engine->redirect)
 			continue;
@@ -1981,5 +1998,7 @@ ice_flow_redirect(struct ice_adapter *ad,
 		}
 	}
 
+	rte_spinlock_unlock(&pf->flow_ops_lock);
+
 	return 0;
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/2] net/ice: support flow redirect function

[Ye Xiaolong]

On 03/31, Beilei Xing wrote:
>Add a new ops "redirect" to flow engine, it's used to implement the
>function that redirect a flow's destination. Currently only support
>VSI-Redirect which will be used by DCF for handling VF-VSI mapping
>table change.
>A new API "ice_flow_redirect" is exposed, current usage is: it could
>be called when there's VF-VSI mapping table change caused by VF reset.
>
>Signed-off-by: Beilei Xing <beilei.xing@intel.com>
>Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>---
> drivers/net/ice/ice_generic_flow.c | 22 ++++++++++++++++++++++
> drivers/net/ice/ice_generic_flow.h | 21 +++++++++++++++++++++
> 2 files changed, 43 insertions(+)
>
>diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
>index a1648ee..823ff0e 100644
>--- a/drivers/net/ice/ice_generic_flow.c
>+++ b/drivers/net/ice/ice_generic_flow.c
>@@ -1961,3 +1961,25 @@ ice_flow_query(struct rte_eth_dev *dev,
> 	}
> 	return ret;
> }
>+
>+int
>+ice_flow_redirect(struct ice_adapter *ad,
>+		  struct ice_flow_redirect *rd)

No need to split into 2 lines.

>+{
>+	struct ice_pf *pf = &ad->pf;
>+	struct rte_flow *p_flow;
>+	void *temp;
>+	int ret;
>+
>+	TAILQ_FOREACH_SAFE(p_flow, &pf->flow_list, node, temp) {
>+		if (!p_flow->engine->redirect)
>+			continue;
>+		ret = p_flow->engine->redirect(ad, p_flow, rd);
>+		if (ret) {
>+			PMD_DRV_LOG(ERR, "Failed to redirect flows");
>+			return ret;
>+		}
>+	}
>+
>+	return 0;
>+}
>diff --git a/drivers/net/ice/ice_generic_flow.h b/drivers/net/ice/ice_generic_flow.h
>index adc30ee..ede6ec8 100644
>--- a/drivers/net/ice/ice_generic_flow.h
>+++ b/drivers/net/ice/ice_generic_flow.h
>@@ -417,6 +417,20 @@ struct ice_pattern_match_item {
> 	void *meta;
> };
> 
>+enum ice_flow_redirect_type {
>+	ICE_FLOW_REDIRECT_VSI,
>+};
>+
>+struct ice_flow_redirect {
>+	enum ice_flow_redirect_type type;
>+	union {
>+		struct {
>+			uint16_t vsi_handle;
>+			uint16_t new_vsi_num;
>+		};
>+	};
>+};
>+
> typedef int (*engine_init_t)(struct ice_adapter *ad);
> typedef void (*engine_uninit_t)(struct ice_adapter *ad);
> typedef int (*engine_create_t)(struct ice_adapter *ad,
>@@ -430,6 +444,9 @@ typedef int (*engine_query_t)(struct ice_adapter *ad,
> 		struct rte_flow *flow,
> 		struct rte_flow_query_count *count,
> 		struct rte_flow_error *error);
>+typedef int(*engine_redirect_t)(struct ice_adapter *ad,
>+				struct rte_flow *flow,
>+				struct ice_flow_redirect *redirect);
> typedef void (*engine_free_t) (struct rte_flow *flow);
> typedef int (*parse_pattern_action_t)(struct ice_adapter *ad,
> 		struct ice_pattern_match_item *array,
>@@ -447,6 +464,7 @@ struct ice_flow_engine {
> 	engine_create_t create;
> 	engine_destroy_t destroy;
> 	engine_query_t query_count;
>+	engine_redirect_t redirect;
> 	engine_free_t free;
> 	enum ice_flow_engine_type type;
> };
>@@ -485,4 +503,7 @@ ice_search_pattern_match_item(const struct rte_flow_item pattern[],
> 		struct ice_pattern_match_item *array,
> 		uint32_t array_len,
> 		struct rte_flow_error *error);
>+int
>+ice_flow_redirect(struct ice_adapter *ad,
>+		  struct ice_flow_redirect *rd);

Ditto

> #endif
>-- 
>2.7.4
>

Applied to next-net-intel with above fixes.

Re: [dpdk-dev] [PATCH v2 0/2] add flow redirect function

[Ye Xiaolong]

On 03/31, Beilei Xing wrote:
>This patchset adds flow redirect function, also supports
>flow ops thread safe
>
>V2 changes:
> - Rework commit log.
> - Make a patchset.
>
>Beilei Xing (2):
>  net/ice: support flow redirect function
>  net/ice: support flow ops thread safe
>
> drivers/net/ice/ice_ethdev.h       |  1 +
> drivers/net/ice/ice_generic_flow.c | 57 ++++++++++++++++++++++++++++++++------
> drivers/net/ice/ice_generic_flow.h | 21 ++++++++++++++
> 3 files changed, 71 insertions(+), 8 deletions(-)
>
>-- 
>2.7.4
>
>Acked-by: Qiming Yang <qiming.yang@intel.com>

Applied to dpdk-next-net-intel, Thanks.