diff mbox series

[v3,10/12] net/mlx5: support represented port flow action

Message ID 20211010143930.4985-11-ivan.malov@oktetlabs.ru (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: rework transfer flow API |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ivan Malov Oct. 10, 2021, 2:39 p.m. UTC 
  From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Semantics of the existing support for action PORT_ID suggests
that support for equal action REPRESENTED_PORT be implemented.

Helper functions keep port_id suffix since action
MLX5_FLOW_ACTION_PORT_ID is still used internally.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 doc/guides/nics/mlx5.rst        |  4 +--
 drivers/net/mlx5/mlx5_flow_dv.c | 62 ++++++++++++++++++++++++++-------
 2 files changed, 52 insertions(+), 14 deletions(-)

Comments

Slava Ovsiienko Oct. 12, 2021, 9:23 p.m. UTC | #1 
Hi, Ivan

> -----Original Message-----
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> Sent: Sunday, October 10, 2021 17:39
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ori Kam
> <orika@nvidia.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
> Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: [PATCH v3 10/12] net/mlx5: support represented port flow action
> 
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> Semantics of the existing support for action PORT_ID suggests that support
> for equal action REPRESENTED_PORT be implemented.
> 
> Helper functions keep port_id suffix since action
> MLX5_FLOW_ACTION_PORT_ID is still used internally.
> 
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>  doc/guides/nics/mlx5.rst        |  4 +--
>  drivers/net/mlx5/mlx5_flow_dv.c | 62 ++++++++++++++++++++++++++-------
>  2 files changed, 52 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index
> bae73f42d8..b76e979f47 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -431,8 +431,8 @@ Limitations
>       - yellow: NULL or END.
>       - RED: DROP / END.
>    - The only supported meter policy actions:
> -     - green: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG.
> -     - yellow: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG.
> +     - green: QUEUE, RSS, PORT_ID, REPRESENTED_PORT, JUMP, DROP, MARK
> and SET_TAG.
> +     - yellow: QUEUE, RSS, PORT_ID, REPRESENTED_PORT, JUMP, DROP,
> MARK and SET_TAG.
>       - RED: must be DROP.
>    - Policy actions of RSS for green and yellow should have the same
> configuration except queues.
>    - meter profile packet mode is supported.
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index c6370cd1d6..835cc5018c 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -5048,14 +5048,14 @@ flow_dv_validate_action_jump(struct
> rte_eth_dev *dev,  }
> 
>  /*
> - * Validate the port_id action.
> + * Validate action PORT_ID / REPRESENTED_PORT.
>   *
>   * @param[in] dev
>   *   Pointer to rte_eth_dev structure.
>   * @param[in] action_flags
>   *   Bit-fields that holds the actions detected until now.
>   * @param[in] action
> - *   Port_id RTE action structure.
> + *   PORT_ID / REPRESENTED_PORT action structure.
>   * @param[in] attr
>   *   Attributes of flow that includes this action.
>   * @param[out] error
> @@ -5072,6 +5072,7 @@ flow_dv_validate_action_port_id(struct
> rte_eth_dev *dev,
>  				struct rte_flow_error *error)
>  {
>  	const struct rte_flow_action_port_id *port_id;
> +	const struct rte_flow_action_ethdev *ethdev;
>  	struct mlx5_priv *act_priv;
>  	struct mlx5_priv *dev_priv;
>  	uint16_t port;
> @@ -5080,13 +5081,13 @@ flow_dv_validate_action_port_id(struct
> rte_eth_dev *dev,
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>  					  NULL,
> -					  "port id action is valid in transfer"
> +					  "port action is valid in transfer"
>  					  " mode only");
>  	if (!action || !action->conf)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ACTION_CONF,
>  					  NULL,
> -					  "port id action parameters must be"
> +					  "port action parameters must be"
>  					  " specified");
>  	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
>  			    MLX5_FLOW_FATE_ESWITCH_ACTIONS)) @@ -
> 5100,13 +5101,26 @@ flow_dv_validate_action_port_id(struct rte_eth_dev
> *dev,
> 
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>  					  NULL,
>  					  "failed to obtain E-Switch info");
> -	port_id = action->conf;
> -	port = port_id->original ? dev->data->port_id : port_id->id;
> +	switch (action->type) {
> +	case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +		port_id = action->conf;
> +		port = port_id->original ? dev->data->port_id : port_id->id;
> +		break;
> +	case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
> +		ethdev = action->conf;
> +		port = ethdev->port_id;
> +		break;
> +	default:

I would add MLX5_ASSERT(false) here.
It is not supposed to call flow_dv_validate_action_port_id() for other action types
but PORT_xxx only.

> +		return rte_flow_error_set
> +				(error, EINVAL,
> +				 RTE_FLOW_ERROR_TYPE_ACTION, action,
> +				 "unknown E-Switch action");
> +	}
>  	act_priv = mlx5_port_to_eswitch_info(port, false);
>  	if (!act_priv)
>  		return rte_flow_error_set
>  				(error, rte_errno,
> -				 RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> port_id,
> +				 RTE_FLOW_ERROR_TYPE_ACTION_CONF,
> action->conf,
>  				 "failed to obtain E-Switch port id for port");
>  	if (act_priv->domain_id != dev_priv->domain_id)
>  		return rte_flow_error_set
> @@ -5669,6 +5683,7 @@ flow_dv_validate_action_sample(uint64_t
> *action_flags,
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
>  			ret = flow_dv_validate_action_port_id(dev,
>  							      sub_action_flags,
>  							      act,
> @@ -7296,6 +7311,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
>  		case RTE_FLOW_ACTION_TYPE_VOID:
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
>  			ret = flow_dv_validate_action_port_id(dev,
>  							      action_flags,
>  							      actions,
> @@ -10770,12 +10786,12 @@ flow_dv_tag_release(struct rte_eth_dev *dev,
> }
> 
>  /**
> - * Translate port ID action to vport.
> + * Translate action PORT_ID / REPRESENTED_PORT to vport.
>   *
>   * @param[in] dev
>   *   Pointer to rte_eth_dev structure.
>   * @param[in] action
> - *   Pointer to the port ID action.
> + *   Pointer to action PORT_ID / REPRESENTED_PORT.
>   * @param[out] dst_port_id
>   *   The target port ID.
>   * @param[out] error
> @@ -10792,10 +10808,28 @@ flow_dv_translate_action_port_id(struct
> rte_eth_dev *dev,  {
>  	uint32_t port;
>  	struct mlx5_priv *priv;
> -	const struct rte_flow_action_port_id *conf =
> -			(const struct rte_flow_action_port_id *)action->conf;
> 
> -	port = conf->original ? dev->data->port_id : conf->id;
> +	switch (action->type) {
> +	case RTE_FLOW_ACTION_TYPE_PORT_ID: {
> +		const struct rte_flow_action_port_id *conf;
> +
> +		conf = (const struct rte_flow_action_port_id *)action->conf;
> +		port = conf->original ? dev->data->port_id : conf->id;
> +		break;
> +	}
> +	case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT: {
> +		const struct rte_flow_action_ethdev *ethdev;
> +
> +		ethdev = (const struct rte_flow_action_ethdev *)action->conf;
> +		port = ethdev->port_id;
> +		break;
> +	}
> +	default:
I would add MLX5_ASSERT(false) here as well.

With best regards,
Slava


> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> +					  "unknown E-Switch action");
> +	}
> +
>  	priv = mlx5_port_to_eswitch_info(port, false);
>  	if (!priv)
>  		return rte_flow_error_set(error, -rte_errno, @@ -11634,6
> +11668,7 @@ flow_dv_translate_action_sample(struct rte_eth_dev *dev,
>  			break;
>  		}
>  		case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
>  		{
>  			struct mlx5_flow_dv_port_id_action_resource
>  					port_id_resource;
> @@ -12714,6 +12749,7 @@ flow_dv_translate(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_VOID:
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
>  			if (flow_dv_translate_action_port_id(dev, action,
>  							     &port_id, error))
>  				return -rte_errno;
> @@ -15475,6 +15511,7 @@ __flow_dv_create_domain_policy_acts(struct
> rte_eth_dev *dev,
>  				break;
>  			}
>  			case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +			case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
>  			{
>  				struct mlx5_flow_dv_port_id_action_resource
>  					port_id_resource;
> @@ -17683,6 +17720,7 @@ flow_dv_validate_mtr_policy_acts(struct
> rte_eth_dev *dev,
>  					  NULL, "too many actions");
>  			switch (act->type) {
>  			case RTE_FLOW_ACTION_TYPE_PORT_ID:
> +			case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
>  				if (!priv->config.dv_esw_en)
>  					return -rte_mtr_error_set(error,
>  					ENOTSUP,
> --
> 2.20.1
diff mbox series

Patch

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index bae73f42d8..b76e979f47 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -431,8 +431,8 @@  Limitations
      - yellow: NULL or END.
      - RED: DROP / END.
   - The only supported meter policy actions:
-     - green: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG.
-     - yellow: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG.
+     - green: QUEUE, RSS, PORT_ID, REPRESENTED_PORT, JUMP, DROP, MARK and SET_TAG.
+     - yellow: QUEUE, RSS, PORT_ID, REPRESENTED_PORT, JUMP, DROP, MARK and SET_TAG.
      - RED: must be DROP.
   - Policy actions of RSS for green and yellow should have the same configuration except queues.
   - meter profile packet mode is supported.
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index c6370cd1d6..835cc5018c 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5048,14 +5048,14 @@  flow_dv_validate_action_jump(struct rte_eth_dev *dev,
 }
 
 /*
- * Validate the port_id action.
+ * Validate action PORT_ID / REPRESENTED_PORT.
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in] action_flags
  *   Bit-fields that holds the actions detected until now.
  * @param[in] action
- *   Port_id RTE action structure.
+ *   PORT_ID / REPRESENTED_PORT action structure.
  * @param[in] attr
  *   Attributes of flow that includes this action.
  * @param[out] error
@@ -5072,6 +5072,7 @@  flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
 				struct rte_flow_error *error)
 {
 	const struct rte_flow_action_port_id *port_id;
+	const struct rte_flow_action_ethdev *ethdev;
 	struct mlx5_priv *act_priv;
 	struct mlx5_priv *dev_priv;
 	uint16_t port;
@@ -5080,13 +5081,13 @@  flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL,
-					  "port id action is valid in transfer"
+					  "port action is valid in transfer"
 					  " mode only");
 	if (!action || !action->conf)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					  NULL,
-					  "port id action parameters must be"
+					  "port action parameters must be"
 					  " specified");
 	if (action_flags & (MLX5_FLOW_FATE_ACTIONS |
 			    MLX5_FLOW_FATE_ESWITCH_ACTIONS))
@@ -5100,13 +5101,26 @@  flow_dv_validate_action_port_id(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL,
 					  "failed to obtain E-Switch info");
-	port_id = action->conf;
-	port = port_id->original ? dev->data->port_id : port_id->id;
+	switch (action->type) {
+	case RTE_FLOW_ACTION_TYPE_PORT_ID:
+		port_id = action->conf;
+		port = port_id->original ? dev->data->port_id : port_id->id;
+		break;
+	case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
+		ethdev = action->conf;
+		port = ethdev->port_id;
+		break;
+	default:
+		return rte_flow_error_set
+				(error, EINVAL,
+				 RTE_FLOW_ERROR_TYPE_ACTION, action,
+				 "unknown E-Switch action");
+	}
 	act_priv = mlx5_port_to_eswitch_info(port, false);
 	if (!act_priv)
 		return rte_flow_error_set
 				(error, rte_errno,
-				 RTE_FLOW_ERROR_TYPE_ACTION_CONF, port_id,
+				 RTE_FLOW_ERROR_TYPE_ACTION_CONF, action->conf,
 				 "failed to obtain E-Switch port id for port");
 	if (act_priv->domain_id != dev_priv->domain_id)
 		return rte_flow_error_set
@@ -5669,6 +5683,7 @@  flow_dv_validate_action_sample(uint64_t *action_flags,
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_PORT_ID:
+		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 			ret = flow_dv_validate_action_port_id(dev,
 							      sub_action_flags,
 							      act,
@@ -7296,6 +7311,7 @@  flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
 		case RTE_FLOW_ACTION_TYPE_PORT_ID:
+		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 			ret = flow_dv_validate_action_port_id(dev,
 							      action_flags,
 							      actions,
@@ -10770,12 +10786,12 @@  flow_dv_tag_release(struct rte_eth_dev *dev,
 }
 
 /**
- * Translate port ID action to vport.
+ * Translate action PORT_ID / REPRESENTED_PORT to vport.
  *
  * @param[in] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in] action
- *   Pointer to the port ID action.
+ *   Pointer to action PORT_ID / REPRESENTED_PORT.
  * @param[out] dst_port_id
  *   The target port ID.
  * @param[out] error
@@ -10792,10 +10808,28 @@  flow_dv_translate_action_port_id(struct rte_eth_dev *dev,
 {
 	uint32_t port;
 	struct mlx5_priv *priv;
-	const struct rte_flow_action_port_id *conf =
-			(const struct rte_flow_action_port_id *)action->conf;
 
-	port = conf->original ? dev->data->port_id : conf->id;
+	switch (action->type) {
+	case RTE_FLOW_ACTION_TYPE_PORT_ID: {
+		const struct rte_flow_action_port_id *conf;
+
+		conf = (const struct rte_flow_action_port_id *)action->conf;
+		port = conf->original ? dev->data->port_id : conf->id;
+		break;
+	}
+	case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT: {
+		const struct rte_flow_action_ethdev *ethdev;
+
+		ethdev = (const struct rte_flow_action_ethdev *)action->conf;
+		port = ethdev->port_id;
+		break;
+	}
+	default:
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, action,
+					  "unknown E-Switch action");
+	}
+
 	priv = mlx5_port_to_eswitch_info(port, false);
 	if (!priv)
 		return rte_flow_error_set(error, -rte_errno,
@@ -11634,6 +11668,7 @@  flow_dv_translate_action_sample(struct rte_eth_dev *dev,
 			break;
 		}
 		case RTE_FLOW_ACTION_TYPE_PORT_ID:
+		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 		{
 			struct mlx5_flow_dv_port_id_action_resource
 					port_id_resource;
@@ -12714,6 +12749,7 @@  flow_dv_translate(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
 		case RTE_FLOW_ACTION_TYPE_PORT_ID:
+		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 			if (flow_dv_translate_action_port_id(dev, action,
 							     &port_id, error))
 				return -rte_errno;
@@ -15475,6 +15511,7 @@  __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev,
 				break;
 			}
 			case RTE_FLOW_ACTION_TYPE_PORT_ID:
+			case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 			{
 				struct mlx5_flow_dv_port_id_action_resource
 					port_id_resource;
@@ -17683,6 +17720,7 @@  flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev,
 					  NULL, "too many actions");
 			switch (act->type) {
 			case RTE_FLOW_ACTION_TYPE_PORT_ID:
+			case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 				if (!priv->config.dv_esw_en)
 					return -rte_mtr_error_set(error,
 					ENOTSUP,