[PATCH] net/mlx5: fix modify field operation validation

[PATCH] net/mlx5: fix modify field operation validation

[Dariusz Sosnowski]

This patch removes the following checks from validation
of modify field action:

- rejection of ADD operation,
- offsets should be aligned to 4 bytes.

These limitations were removed in
commit 0f4aa72b99da ("net/mlx5: support flow modify field with HWS"),
but non-HWS validation was not updated.

Notes about these limitations are removed from mlx5 PMD docs.
On top of that, the current offsetting behavior in modify field action
is clarified in the mlx5 docs.

Fixes: 0f4aa72b99da ("net/mlx5: support flow modify field with HWS")
Cc: suanmingm@nvidia.com

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 doc/guides/nics/mlx5.rst        | 10 ++++++++--
 drivers/net/mlx5/mlx5_flow_dv.c | 21 ++++++++-------------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 4f0db21dde..203bbd9d27 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -447,17 +447,23 @@ Limitations
 
 - Modify Field flow:
 
-  - Supports the 'set' operation only for ``RTE_FLOW_ACTION_TYPE_MODIFY_FIELD`` action.
+  - Supports the 'set' and 'add' operations for ``RTE_FLOW_ACTION_TYPE_MODIFY_FIELD`` action.
   - Modification of an arbitrary place in a packet via the special ``RTE_FLOW_FIELD_START`` Field ID is not supported.
   - Modification of the 802.1Q Tag, VXLAN Network or GENEVE Network ID's is not supported.
   - Encapsulation levels are not supported, can modify outermost header fields only.
-  - Offsets must be 32-bits aligned, cannot skip past the boundary of a field.
+  - Offsets cannot skip past the boundary of a field.
   - If the field type is ``RTE_FLOW_FIELD_MAC_TYPE``
     and packet contains one or more VLAN headers,
     the meaningful type field following the last VLAN header
     is used as modify field operation argument.
     The modify field action is not intended to modify VLAN headers type field,
     dedicated VLAN push and pop actions should be used instead.
+  - For packet fields (e.g. MAC addresses, IPv4 addresses or L4 ports)
+    offset specifies the number of bits to skip from field's start,
+    starting from MSB in the first byte, in the network order.
+  - For flow metadata fields (e.g. META or TAG)
+    offset specifies the number of bits to skip from field's start,
+    starting from LSB in the least significant byte, in the host order.
 
 - Age action:
 
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index bc9a75f225..f1a3868e48 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5035,13 +5035,11 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 				" the width of a field");
 	if (action_modify_field->dst.field != RTE_FLOW_FIELD_VALUE &&
 	    action_modify_field->dst.field != RTE_FLOW_FIELD_POINTER) {
-		if ((action_modify_field->dst.offset +
-		     action_modify_field->width > dst_width) ||
-		    (action_modify_field->dst.offset % 32))
+		if (action_modify_field->dst.offset +
+		    action_modify_field->width > dst_width)
 			return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"destination offset is too big"
-					" or not aligned to 4 bytes");
+					"destination offset is too big");
 		if (action_modify_field->dst.level &&
 		    action_modify_field->dst.field != RTE_FLOW_FIELD_TAG)
 			return rte_flow_error_set(error, ENOTSUP,
@@ -5056,13 +5054,11 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"modify field action is not"
 					" supported for group 0");
-		if ((action_modify_field->src.offset +
-		     action_modify_field->width > src_width) ||
-		    (action_modify_field->src.offset % 32))
+		if (action_modify_field->src.offset +
+		    action_modify_field->width > src_width)
 			return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"source offset is too big"
-					" or not aligned to 4 bytes");
+					"source offset is too big");
 		if (action_modify_field->src.level &&
 		    action_modify_field->src.field != RTE_FLOW_FIELD_TAG)
 			return rte_flow_error_set(error, ENOTSUP,
@@ -5132,11 +5128,10 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 					"cannot modify meta without"
 					" extensive registers available");
 	}
-	if (action_modify_field->operation != RTE_FLOW_MODIFY_SET)
+	if (action_modify_field->operation == RTE_FLOW_MODIFY_SUB)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
-				"add and sub operations"
-				" are not supported");
+				"sub operations are not supported");
 	if (action_modify_field->dst.field == RTE_FLOW_FIELD_IPV4_ECN ||
 	    action_modify_field->src.field == RTE_FLOW_FIELD_IPV4_ECN ||
 	    action_modify_field->dst.field == RTE_FLOW_FIELD_IPV6_ECN ||
-- 
2.25.1

RE: [PATCH] net/mlx5: fix modify field operation validation

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Thursday, November 17, 2022 4:42 PM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> Subject: [PATCH] net/mlx5: fix modify field operation validation
> 
> This patch removes the following checks from validation
> of modify field action:
> 
> - rejection of ADD operation,
> - offsets should be aligned to 4 bytes.
> 
> These limitations were removed in
> commit 0f4aa72b99da ("net/mlx5: support flow modify field with HWS"),
> but non-HWS validation was not updated.
> 
> Notes about these limitations are removed from mlx5 PMD docs.
> On top of that, the current offsetting behavior in modify field action
> is clarified in the mlx5 docs.
> 
> Fixes: 0f4aa72b99da ("net/mlx5: support flow modify field with HWS")
> Cc: suanmingm@nvidia.com
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh