DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix actions validation on root table
@ 2020-04-29 12:52 Bing Zhao
  0 siblings, 0 replies; 3+ messages in thread
From: Bing Zhao @ 2020-04-29 12:52 UTC (permalink / raw)
  To: viacheslavo, rasland; +Cc: orika, matan, dev, stable

The maximal supported header modifications number of a single modify
context on the root table cannot be queried from firmware directly.
It is a fixed value of 16 in the latest releases. In the validation
stage, PMD driver should ensure that no more than 16 header modify
actions exist in a single context.
In some old firmware releases, the supported value is 8. PMD driver
should try its best to create the flow. Firmware will return error
and refuse to create the flow if the actions number exceeds the
maximal value.

Fixes: 72a944dba163 ("net/mlx5: fix header modify action validation")
Cc: stable@dpdk.org

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>>
---
 drivers/net/mlx5/mlx5_flow.h    | 12 +++++++-----
 drivers/net/mlx5/mlx5_flow_dv.c | 17 +++++++----------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 2a1f596..75b8288 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -415,14 +415,16 @@ struct mlx5_flow_dv_tag_resource {
 
 /*
  * Number of modification commands.
- * If extensive metadata registers are supported, the maximal actions amount is
- * 16 and 8 otherwise on root table. The validation could also be done in the
- * lower driver layer.
- * On non-root table, there is no limitation, but 32 is enough right now.
+ * The maximal actions amount in FW is some constant, and it is 16 in the
+ * latest releases. In some old releases, it will be limited to 8.
+ * Since there is no interface to query the capacity, the maximal value should
+ * be used to allow PMD to create the flow. The validation will be done in the
+ * lower driver layer or FW. A failure will be returned if exceeds the maximal
+ * supported actions number on the root table.
+ * On non-root tables, there is no limitation, but 32 is enough right now.
  */
 #define MLX5_MAX_MODIFY_NUM			32
 #define MLX5_ROOT_TBL_MODIFY_NUM		16
-#define MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG	8
 
 /* Modify resource structure */
 struct mlx5_flow_dv_modify_hdr_resource {
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6263ecc..ba2febf 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3640,21 +3640,18 @@ struct field_modify_info modify_tcp[] = {
  * @return
  *   Max number of modify header actions device can support.
  */
-static unsigned int
-flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev, uint64_t flags)
+static inline unsigned int
+flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev __rte_unused,
+			      uint64_t flags)
 {
 	/*
-	 * There's no way to directly query the max cap. Although it has to be
-	 * acquried by iterative trial, it is a safe assumption that more
-	 * actions are supported by FW if extensive metadata register is
-	 * supported. (Only in the root table)
+	 * There's no way to directly query the max capacity from FW.
+	 * The maximal value on root table should be assumed to be supported.
 	 */
 	if (!(flags & MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL))
 		return MLX5_MAX_MODIFY_NUM;
 	else
-		return mlx5_flow_ext_mreg_supported(dev) ?
-					MLX5_ROOT_TBL_MODIFY_NUM :
-					MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG;
+		return MLX5_ROOT_TBL_MODIFY_NUM;
 }
 
 /**
@@ -5347,7 +5344,7 @@ struct field_modify_info modify_tcp[] = {
 	    dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    mlx5_flow_ext_mreg_supported(dev))
 		rw_act_num += MLX5_ACT_NUM_SET_TAG;
-	if ((uint32_t)rw_act_num >=
+	if ((uint32_t)rw_act_num >
 			flow_dv_modify_hdr_action_max(dev, is_root)) {
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] net/mlx5: fix actions validation on root table
  2020-04-29 12:54 Bing Zhao
@ 2020-04-30 12:26 ` Raslan Darawsheh
  0 siblings, 0 replies; 3+ messages in thread
From: Raslan Darawsheh @ 2020-04-30 12:26 UTC (permalink / raw)
  To: Bing Zhao, Slava Ovsiienko; +Cc: Ori Kam, Matan Azrad, dev, stable

Hi,

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Wednesday, April 29, 2020 3:54 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix actions validation on root table
> 
> The maximal supported header modifications number of a single modify
> context on the root table cannot be queried from firmware directly.
> It is a fixed value of 16 in the latest releases. In the validation
> stage, PMD driver should ensure that no more than 16 header modify
> actions exist in a single context.
> In some old firmware releases, the supported value is 8. PMD driver
> should try its best to create the flow. Firmware will return error
> and refuse to create the flow if the actions number exceeds the
> maximal value.
> 
> Fixes: 72a944dba163 ("net/mlx5: fix header modify action validation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.h    | 12 +++++++-----
>  drivers/net/mlx5/mlx5_flow_dv.c | 17 +++++++----------
>  2 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 2a1f596..75b8288 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -415,14 +415,16 @@ struct mlx5_flow_dv_tag_resource {
> 
>  /*
>   * Number of modification commands.
> - * If extensive metadata registers are supported, the maximal actions
> amount is
> - * 16 and 8 otherwise on root table. The validation could also be done in the
> - * lower driver layer.
> - * On non-root table, there is no limitation, but 32 is enough right now.
> + * The maximal actions amount in FW is some constant, and it is 16 in the
> + * latest releases. In some old releases, it will be limited to 8.
> + * Since there is no interface to query the capacity, the maximal value
> should
> + * be used to allow PMD to create the flow. The validation will be done in
> the
> + * lower driver layer or FW. A failure will be returned if exceeds the maximal
> + * supported actions number on the root table.
> + * On non-root tables, there is no limitation, but 32 is enough right now.
>   */
>  #define MLX5_MAX_MODIFY_NUM			32
>  #define MLX5_ROOT_TBL_MODIFY_NUM		16
> -#define MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG	8
> 
>  /* Modify resource structure */
>  struct mlx5_flow_dv_modify_hdr_resource {
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 6263ecc..ba2febf 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3640,21 +3640,18 @@ struct field_modify_info modify_tcp[] = {
>   * @return
>   *   Max number of modify header actions device can support.
>   */
> -static unsigned int
> -flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev, uint64_t flags)
> +static inline unsigned int
> +flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev __rte_unused,
> +			      uint64_t flags)
>  {
>  	/*
> -	 * There's no way to directly query the max cap. Although it has to be
> -	 * acquried by iterative trial, it is a safe assumption that more
> -	 * actions are supported by FW if extensive metadata register is
> -	 * supported. (Only in the root table)
> +	 * There's no way to directly query the max capacity from FW.
> +	 * The maximal value on root table should be assumed to be
> supported.
>  	 */
>  	if (!(flags & MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL))
>  		return MLX5_MAX_MODIFY_NUM;
>  	else
> -		return mlx5_flow_ext_mreg_supported(dev) ?
> -					MLX5_ROOT_TBL_MODIFY_NUM :
> -
> 	MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG;
> +		return MLX5_ROOT_TBL_MODIFY_NUM;
>  }
> 
>  /**
> @@ -5347,7 +5344,7 @@ struct field_modify_info modify_tcp[] = {
>  	    dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
>  	    mlx5_flow_ext_mreg_supported(dev))
>  		rw_act_num += MLX5_ACT_NUM_SET_TAG;
> -	if ((uint32_t)rw_act_num >=
> +	if ((uint32_t)rw_act_num >
>  			flow_dv_modify_hdr_action_max(dev, is_root)) {
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> --
> 1.8.3.1

Patch applied on next-net-mlx,

Kindest regards,
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-dev] [PATCH] net/mlx5: fix actions validation on root table
@ 2020-04-29 12:54 Bing Zhao
  2020-04-30 12:26 ` Raslan Darawsheh
  0 siblings, 1 reply; 3+ messages in thread
From: Bing Zhao @ 2020-04-29 12:54 UTC (permalink / raw)
  To: viacheslavo, rasland; +Cc: orika, matan, dev, stable

The maximal supported header modifications number of a single modify
context on the root table cannot be queried from firmware directly.
It is a fixed value of 16 in the latest releases. In the validation
stage, PMD driver should ensure that no more than 16 header modify
actions exist in a single context.
In some old firmware releases, the supported value is 8. PMD driver
should try its best to create the flow. Firmware will return error
and refuse to create the flow if the actions number exceeds the
maximal value.

Fixes: 72a944dba163 ("net/mlx5: fix header modify action validation")
Cc: stable@dpdk.org

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.h    | 12 +++++++-----
 drivers/net/mlx5/mlx5_flow_dv.c | 17 +++++++----------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 2a1f596..75b8288 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -415,14 +415,16 @@ struct mlx5_flow_dv_tag_resource {
 
 /*
  * Number of modification commands.
- * If extensive metadata registers are supported, the maximal actions amount is
- * 16 and 8 otherwise on root table. The validation could also be done in the
- * lower driver layer.
- * On non-root table, there is no limitation, but 32 is enough right now.
+ * The maximal actions amount in FW is some constant, and it is 16 in the
+ * latest releases. In some old releases, it will be limited to 8.
+ * Since there is no interface to query the capacity, the maximal value should
+ * be used to allow PMD to create the flow. The validation will be done in the
+ * lower driver layer or FW. A failure will be returned if exceeds the maximal
+ * supported actions number on the root table.
+ * On non-root tables, there is no limitation, but 32 is enough right now.
  */
 #define MLX5_MAX_MODIFY_NUM			32
 #define MLX5_ROOT_TBL_MODIFY_NUM		16
-#define MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG	8
 
 /* Modify resource structure */
 struct mlx5_flow_dv_modify_hdr_resource {
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6263ecc..ba2febf 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3640,21 +3640,18 @@ struct field_modify_info modify_tcp[] = {
  * @return
  *   Max number of modify header actions device can support.
  */
-static unsigned int
-flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev, uint64_t flags)
+static inline unsigned int
+flow_dv_modify_hdr_action_max(struct rte_eth_dev *dev __rte_unused,
+			      uint64_t flags)
 {
 	/*
-	 * There's no way to directly query the max cap. Although it has to be
-	 * acquried by iterative trial, it is a safe assumption that more
-	 * actions are supported by FW if extensive metadata register is
-	 * supported. (Only in the root table)
+	 * There's no way to directly query the max capacity from FW.
+	 * The maximal value on root table should be assumed to be supported.
 	 */
 	if (!(flags & MLX5DV_DR_ACTION_FLAGS_ROOT_LEVEL))
 		return MLX5_MAX_MODIFY_NUM;
 	else
-		return mlx5_flow_ext_mreg_supported(dev) ?
-					MLX5_ROOT_TBL_MODIFY_NUM :
-					MLX5_ROOT_TBL_MODIFY_NUM_NO_MREG;
+		return MLX5_ROOT_TBL_MODIFY_NUM;
 }
 
 /**
@@ -5347,7 +5344,7 @@ struct field_modify_info modify_tcp[] = {
 	    dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 	    mlx5_flow_ext_mreg_supported(dev))
 		rw_act_num += MLX5_ACT_NUM_SET_TAG;
-	if ((uint32_t)rw_act_num >=
+	if ((uint32_t)rw_act_num >
 			flow_dv_modify_hdr_action_max(dev, is_root)) {
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-30 12:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 12:52 [dpdk-dev] [PATCH] net/mlx5: fix actions validation on root table Bing Zhao
2020-04-29 12:54 Bing Zhao
2020-04-30 12:26 ` Raslan Darawsheh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).