net/mlx5: update VLAN and encap actions validation
diff mbox series

Message ID e65aba6e5a07e35628c9fff6f454a49a6d8dd7aa.1585054493.git.dekelp@mellanox.com
State Accepted
Delegated to: Raslan Darawsheh
Headers show
Series
  • net/mlx5: update VLAN and encap actions validation
Related show

Checks

Context Check Description
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance fail Performance Testing issues
ci/checkpatch success coding style OK

Commit Message

Dekel Peled March 24, 2020, 12:58 p.m. UTC
Flow rule in NIC table on VF representor should not contain VLAN pop
or push actions, and encap or decap actions. Using these actions in
NIC table on VF representor is not a valid use case.
This patch updates the various validation functions to reject such
rules.

Cc: stable@dpdk.org

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
Acked-by: Jack Min <jackmin@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 82 +++++++++++++++++++++++++++++++++--------
 1 file changed, 67 insertions(+), 15 deletions(-)

Comments

Raslan Darawsheh March 29, 2020, 10:53 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Dekel Peled <dekelp@mellanox.com>
> Sent: Tuesday, March 24, 2020 2:58 PM
> To: Matan Azrad <matan@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] net/mlx5: update VLAN and encap actions validation
> 
> Flow rule in NIC table on VF representor should not contain VLAN pop
> or push actions, and encap or decap actions. Using these actions in
> NIC table on VF representor is not a valid use case.
> This patch updates the various validation functions to reject such
> rules.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> Acked-by: Jack Min <jackmin@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 82
> +++++++++++++++++++++++++++++++++--------
>  1 file changed, 67 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 2090631..7fe61c2 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1698,7 +1698,7 @@ struct field_modify_info modify_tcp[] = {
>  				 const struct rte_flow_attr *attr,
>  				 struct rte_flow_error *error)
>  {
> -	struct mlx5_priv *priv = dev->data->dev_private;
> +	const struct mlx5_priv *priv = dev->data->dev_private;
> 
>  	(void)action;
>  	(void)attr;
> @@ -1729,6 +1729,11 @@ struct field_modify_info modify_tcp[] = {
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
>  					  "wrong action order, port_id should
> "
>  					  "be after pop VLAN action");
> +	if (!attr->transfer && priv->representor)
> +		return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "pop vlan action for VF representor "
> +					  "not supported on NIC table");
>  	return 0;
>  }
> 
> @@ -1792,6 +1797,8 @@ struct field_modify_info modify_tcp[] = {
>  /**
>   * Validate the push VLAN action.
>   *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
>   * @param[in] action_flags
>   *   Holds the actions detected until now.
>   * @param[in] item_flags
> @@ -1807,13 +1814,15 @@ struct field_modify_info modify_tcp[] = {
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
>  static int
> -flow_dv_validate_action_push_vlan(uint64_t action_flags,
> +flow_dv_validate_action_push_vlan(struct rte_eth_dev *dev,
> +				  uint64_t action_flags,
>  				  uint64_t item_flags __rte_unused,
>  				  const struct rte_flow_action *action,
>  				  const struct rte_flow_attr *attr,
>  				  struct rte_flow_error *error)
>  {
>  	const struct rte_flow_action_of_push_vlan *push_vlan = action-
> >conf;
> +	const struct mlx5_priv *priv = dev->data->dev_private;
> 
>  	if (!attr->transfer && attr->ingress)
>  		return rte_flow_error_set(error, ENOTSUP,
> @@ -1836,6 +1845,11 @@ struct field_modify_info modify_tcp[] = {
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
>  					  "wrong action order, port_id should
> "
>  					  "be after push VLAN");
> +	if (!attr->transfer && priv->representor)
> +		return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "push vlan action for VF representor
> "
> +					  "not supported on NIC table");
>  	(void)attr;
>  	return 0;
>  }
> @@ -2201,10 +2215,14 @@ struct field_modify_info modify_tcp[] = {
>  /**
>   * Validate the L2 encap action.
>   *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
>   * @param[in] action_flags
>   *   Holds the actions detected until now.
>   * @param[in] action
>   *   Pointer to the action structure.
> + * @param[in] attr
> + *   Pointer to flow attributes.
>   * @param[out] error
>   *   Pointer to error structure.
>   *
> @@ -2212,10 +2230,14 @@ struct field_modify_info modify_tcp[] = {
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
>  static int
> -flow_dv_validate_action_l2_encap(uint64_t action_flags,
> +flow_dv_validate_action_l2_encap(struct rte_eth_dev *dev,
> +				 uint64_t action_flags,
>  				 const struct rte_flow_action *action,
> +				 const struct rte_flow_attr *attr,
>  				 struct rte_flow_error *error)
>  {
> +	const struct mlx5_priv *priv = dev->data->dev_private;
> +
>  	if (!(action->conf))
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> @@ -2225,12 +2247,19 @@ struct field_modify_info modify_tcp[] = {
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
>  					  "can only have a single encap action
> "
>  					  "in a flow");
> +	if (!attr->transfer && priv->representor)
> +		return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "encap action for VF representor "
> +					  "not supported on NIC table");
>  	return 0;
>  }
> 
>  /**
>   * Validate a decap action.
>   *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
>   * @param[in] action_flags
>   *   Holds the actions detected until now.
>   * @param[in] attr
> @@ -2242,10 +2271,13 @@ struct field_modify_info modify_tcp[] = {
>   *   0 on success, a negative errno value otherwise and rte_errno is set.
>   */
>  static int
> -flow_dv_validate_action_decap(uint64_t action_flags,
> -				 const struct rte_flow_attr *attr,
> -				 struct rte_flow_error *error)
> +flow_dv_validate_action_decap(struct rte_eth_dev *dev,
> +			      uint64_t action_flags,
> +			      const struct rte_flow_attr *attr,
> +			      struct rte_flow_error *error)
>  {
> +	const struct mlx5_priv *priv = dev->data->dev_private;
> +
>  	if (action_flags & MLX5_FLOW_XCAP_ACTIONS)
>  		return rte_flow_error_set(error, ENOTSUP,
>  					  RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> @@ -2264,6 +2296,11 @@ struct field_modify_info modify_tcp[] = {
>  					  NULL,
>  					  "decap action not supported for "
>  					  "egress");
> +	if (!attr->transfer && priv->representor)
> +		return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "decap action for VF representor "
> +					  "not supported on NIC table");
>  	return 0;
>  }
> 
> @@ -2272,6 +2309,8 @@ struct field_modify_info modify_tcp[] = {
>  /**
>   * Validate the raw encap and decap actions.
>   *
> + * @param[in] dev
> + *   Pointer to the rte_eth_dev structure.
>   * @param[in] decap
>   *   Pointer to the decap action.
>   * @param[in] encap
> @@ -2290,11 +2329,13 @@ struct field_modify_info modify_tcp[] = {
>   */
>  static int
>  flow_dv_validate_action_raw_encap_decap
> -	(const struct rte_flow_action_raw_decap *decap,
> +	(struct rte_eth_dev *dev,
> +	 const struct rte_flow_action_raw_decap *decap,
>  	 const struct rte_flow_action_raw_encap *encap,
>  	 const struct rte_flow_attr *attr, uint64_t *action_flags,
>  	 int *actions_n, struct rte_flow_error *error)
>  {
> +	const struct mlx5_priv *priv = dev->data->dev_private;
>  	int ret;
> 
>  	if (encap && (!encap->size || !encap->data))
> @@ -2327,7 +2368,8 @@ struct field_modify_info modify_tcp[] = {
>  				"encap combination");
>  	}
>  	if (decap) {
> -		ret = flow_dv_validate_action_decap(*action_flags, attr,
> error);
> +		ret = flow_dv_validate_action_decap(dev, *action_flags,
> attr,
> +						    error);
>  		if (ret < 0)
>  			return ret;
>  		*action_flags |= MLX5_FLOW_ACTION_DECAP;
> @@ -2344,6 +2386,12 @@ struct field_modify_info modify_tcp[] = {
> 
> RTE_FLOW_ERROR_TYPE_ACTION,
>  						  NULL,
>  						  "more than one encap
> action");
> +		if (!attr->transfer && priv->representor)
> +			return rte_flow_error_set
> +					(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					 "encap action for VF representor "
> +					 "not supported on NIC table");
>  		*action_flags |= MLX5_FLOW_ACTION_ENCAP;
>  		++(*actions_n);
>  	}
> @@ -4888,7 +4936,8 @@ struct field_modify_info modify_tcp[] = {
>  			++actions_n;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> -			ret =
> flow_dv_validate_action_push_vlan(action_flags,
> +			ret = flow_dv_validate_action_push_vlan(dev,
> +								action_flags,
>  								item_flags,
>  								actions, attr,
>  								error);
> @@ -4916,8 +4965,10 @@ struct field_modify_info modify_tcp[] = {
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
>  		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
> -			ret =
> flow_dv_validate_action_l2_encap(action_flags,
> -							       actions, error);
> +			ret = flow_dv_validate_action_l2_encap(dev,
> +							       action_flags,
> +							       actions, attr,
> +							       error);
>  			if (ret < 0)
>  				return ret;
>  			action_flags |= MLX5_FLOW_ACTION_ENCAP;
> @@ -4925,8 +4976,8 @@ struct field_modify_info modify_tcp[] = {
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
>  		case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
> -			ret = flow_dv_validate_action_decap(action_flags,
> attr,
> -							    error);
> +			ret = flow_dv_validate_action_decap(dev,
> action_flags,
> +							    attr, error);
>  			if (ret < 0)
>  				return ret;
>  			action_flags |= MLX5_FLOW_ACTION_DECAP;
> @@ -4934,7 +4985,7 @@ struct field_modify_info modify_tcp[] = {
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
>  			ret = flow_dv_validate_action_raw_encap_decap
> -				(NULL, actions->conf, attr, &action_flags,
> +				(dev, NULL, actions->conf, attr,
> &action_flags,
>  				 &actions_n, error);
>  			if (ret < 0)
>  				return ret;
> @@ -4950,7 +5001,8 @@ struct field_modify_info modify_tcp[] = {
>  				encap = actions->conf;
>  			}
>  			ret = flow_dv_validate_action_raw_encap_decap
> -					   (decap ? decap : &empty_decap,
> encap,
> +					   (dev,
> +					    decap ? decap : &empty_decap,
> encap,
>  					    attr, &action_flags, &actions_n,
>  					    error);
>  			if (ret < 0)
> --
> 1.8.3.1


Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Patch
diff mbox series

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2090631..7fe61c2 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1698,7 +1698,7 @@  struct field_modify_info modify_tcp[] = {
 				 const struct rte_flow_attr *attr,
 				 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
+	const struct mlx5_priv *priv = dev->data->dev_private;
 
 	(void)action;
 	(void)attr;
@@ -1729,6 +1729,11 @@  struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "wrong action order, port_id should "
 					  "be after pop VLAN action");
+	if (!attr->transfer && priv->representor)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "pop vlan action for VF representor "
+					  "not supported on NIC table");
 	return 0;
 }
 
@@ -1792,6 +1797,8 @@  struct field_modify_info modify_tcp[] = {
 /**
  * Validate the push VLAN action.
  *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[in] item_flags
@@ -1807,13 +1814,15 @@  struct field_modify_info modify_tcp[] = {
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_push_vlan(uint64_t action_flags,
+flow_dv_validate_action_push_vlan(struct rte_eth_dev *dev,
+				  uint64_t action_flags,
 				  uint64_t item_flags __rte_unused,
 				  const struct rte_flow_action *action,
 				  const struct rte_flow_attr *attr,
 				  struct rte_flow_error *error)
 {
 	const struct rte_flow_action_of_push_vlan *push_vlan = action->conf;
+	const struct mlx5_priv *priv = dev->data->dev_private;
 
 	if (!attr->transfer && attr->ingress)
 		return rte_flow_error_set(error, ENOTSUP,
@@ -1836,6 +1845,11 @@  struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
 					  "wrong action order, port_id should "
 					  "be after push VLAN");
+	if (!attr->transfer && priv->representor)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "push vlan action for VF representor "
+					  "not supported on NIC table");
 	(void)attr;
 	return 0;
 }
@@ -2201,10 +2215,14 @@  struct field_modify_info modify_tcp[] = {
 /**
  * Validate the L2 encap action.
  *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[in] action
  *   Pointer to the action structure.
+ * @param[in] attr
+ *   Pointer to flow attributes.
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -2212,10 +2230,14 @@  struct field_modify_info modify_tcp[] = {
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_l2_encap(uint64_t action_flags,
+flow_dv_validate_action_l2_encap(struct rte_eth_dev *dev,
+				 uint64_t action_flags,
 				 const struct rte_flow_action *action,
+				 const struct rte_flow_attr *attr,
 				 struct rte_flow_error *error)
 {
+	const struct mlx5_priv *priv = dev->data->dev_private;
+
 	if (!(action->conf))
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, action,
@@ -2225,12 +2247,19 @@  struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
 					  "can only have a single encap action "
 					  "in a flow");
+	if (!attr->transfer && priv->representor)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "encap action for VF representor "
+					  "not supported on NIC table");
 	return 0;
 }
 
 /**
  * Validate a decap action.
  *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
  * @param[in] action_flags
  *   Holds the actions detected until now.
  * @param[in] attr
@@ -2242,10 +2271,13 @@  struct field_modify_info modify_tcp[] = {
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-flow_dv_validate_action_decap(uint64_t action_flags,
-				 const struct rte_flow_attr *attr,
-				 struct rte_flow_error *error)
+flow_dv_validate_action_decap(struct rte_eth_dev *dev,
+			      uint64_t action_flags,
+			      const struct rte_flow_attr *attr,
+			      struct rte_flow_error *error)
 {
+	const struct mlx5_priv *priv = dev->data->dev_private;
+
 	if (action_flags & MLX5_FLOW_XCAP_ACTIONS)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -2264,6 +2296,11 @@  struct field_modify_info modify_tcp[] = {
 					  NULL,
 					  "decap action not supported for "
 					  "egress");
+	if (!attr->transfer && priv->representor)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "decap action for VF representor "
+					  "not supported on NIC table");
 	return 0;
 }
 
@@ -2272,6 +2309,8 @@  struct field_modify_info modify_tcp[] = {
 /**
  * Validate the raw encap and decap actions.
  *
+ * @param[in] dev
+ *   Pointer to the rte_eth_dev structure.
  * @param[in] decap
  *   Pointer to the decap action.
  * @param[in] encap
@@ -2290,11 +2329,13 @@  struct field_modify_info modify_tcp[] = {
  */
 static int
 flow_dv_validate_action_raw_encap_decap
-	(const struct rte_flow_action_raw_decap *decap,
+	(struct rte_eth_dev *dev,
+	 const struct rte_flow_action_raw_decap *decap,
 	 const struct rte_flow_action_raw_encap *encap,
 	 const struct rte_flow_attr *attr, uint64_t *action_flags,
 	 int *actions_n, struct rte_flow_error *error)
 {
+	const struct mlx5_priv *priv = dev->data->dev_private;
 	int ret;
 
 	if (encap && (!encap->size || !encap->data))
@@ -2327,7 +2368,8 @@  struct field_modify_info modify_tcp[] = {
 				"encap combination");
 	}
 	if (decap) {
-		ret = flow_dv_validate_action_decap(*action_flags, attr, error);
+		ret = flow_dv_validate_action_decap(dev, *action_flags, attr,
+						    error);
 		if (ret < 0)
 			return ret;
 		*action_flags |= MLX5_FLOW_ACTION_DECAP;
@@ -2344,6 +2386,12 @@  struct field_modify_info modify_tcp[] = {
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  NULL,
 						  "more than one encap action");
+		if (!attr->transfer && priv->representor)
+			return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					 "encap action for VF representor "
+					 "not supported on NIC table");
 		*action_flags |= MLX5_FLOW_ACTION_ENCAP;
 		++(*actions_n);
 	}
@@ -4888,7 +4936,8 @@  struct field_modify_info modify_tcp[] = {
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
-			ret = flow_dv_validate_action_push_vlan(action_flags,
+			ret = flow_dv_validate_action_push_vlan(dev,
+								action_flags,
 								item_flags,
 								actions, attr,
 								error);
@@ -4916,8 +4965,10 @@  struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
 		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
-			ret = flow_dv_validate_action_l2_encap(action_flags,
-							       actions, error);
+			ret = flow_dv_validate_action_l2_encap(dev,
+							       action_flags,
+							       actions, attr,
+							       error);
 			if (ret < 0)
 				return ret;
 			action_flags |= MLX5_FLOW_ACTION_ENCAP;
@@ -4925,8 +4976,8 @@  struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
 		case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
-			ret = flow_dv_validate_action_decap(action_flags, attr,
-							    error);
+			ret = flow_dv_validate_action_decap(dev, action_flags,
+							    attr, error);
 			if (ret < 0)
 				return ret;
 			action_flags |= MLX5_FLOW_ACTION_DECAP;
@@ -4934,7 +4985,7 @@  struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			ret = flow_dv_validate_action_raw_encap_decap
-				(NULL, actions->conf, attr, &action_flags,
+				(dev, NULL, actions->conf, attr, &action_flags,
 				 &actions_n, error);
 			if (ret < 0)
 				return ret;
@@ -4950,7 +5001,8 @@  struct field_modify_info modify_tcp[] = {
 				encap = actions->conf;
 			}
 			ret = flow_dv_validate_action_raw_encap_decap
-					   (decap ? decap : &empty_decap, encap,
+					   (dev,
+					    decap ? decap : &empty_decap, encap,
 					    attr, &action_flags, &actions_n,
 					    error);
 			if (ret < 0)