[v2] net/mlx5: do not allow copy to mark via modify field

Message ID 20210716084305.646731-1-akozyrev@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] net/mlx5: do not allow copy to mark via modify field |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing fail Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Alexander Kozyrev July 16, 2021, 8:43 a.m. UTC
  The Mark action is a two-stage process in the Mellanox driver.
First, a hardware register is filled with the required value,
then this value is registered in the software resource table.

The MODIFY_FIELD action can instruct a Mellanox NIC to copy
some value from an arbitrary packet header field into the
hardware register, associated with the Mark item. But there
is no way NIC can modify the software resource table as well.

Due to these driver limitations the copying of arbitrary value
to the MARK can not be supported and should be rejected in the
MODIFY_FIELD action.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
v2: Rewrote the commit message to clarify the limitation better.
v1: https://patchwork.dpdk.org/project/dpdk/patch/20210616183444.2815030-1-akozyrev@nvidia.com/

 drivers/net/mlx5/mlx5_flow_dv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Slava Ovsiienko July 16, 2021, 10:47 a.m. UTC | #1
> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Friday, July 16, 2021 11:43
> To: dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Subject: [PATCH v2] net/mlx5: do not allow copy to mark via modify field
> 
> The Mark action is a two-stage process in the Mellanox driver.
> First, a hardware register is filled with the required value, then this value is
> registered in the software resource table.
> 
> The MODIFY_FIELD action can instruct a Mellanox NIC to copy some value
> from an arbitrary packet header field into the hardware register, associated
> with the Mark item. But there is no way NIC can modify the software
> resource table as well.
> 
> Due to these driver limitations the copying of arbitrary value to the MARK can
> not be supported and should be rejected in the MODIFY_FIELD action.
> 
Thank you, Alexander

> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
  
Thomas Monjalon July 22, 2021, 2:28 p.m. UTC | #2
16/07/2021 12:47, Slava Ovsiienko:
> > -----Original Message-----
> > From: Alexander Kozyrev <akozyrev@nvidia.com>
> > Sent: Friday, July 16, 2021 11:43
> > To: dev@dpdk.org
> > Cc: Raslan Darawsheh <rasland@nvidia.com>; Matan Azrad
> > <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> > Subject: [PATCH v2] net/mlx5: do not allow copy to mark via modify field
> > 
> > The Mark action is a two-stage process in the Mellanox driver.
> > First, a hardware register is filled with the required value, then this value is
> > registered in the software resource table.
> > 
> > The MODIFY_FIELD action can instruct a Mellanox NIC to copy some value
> > from an arbitrary packet header field into the hardware register, associated
> > with the Mark item. But there is no way NIC can modify the software
> > resource table as well.
> > 
> > Due to these driver limitations the copying of arbitrary value to the MARK can
> > not be supported and should be rejected in the MODIFY_FIELD action.
> > 
> Thank you, Alexander
> 
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Applied, thanks.
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d250486950..519c610e50 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4940,10 +4940,11 @@  flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 				"source and destination fields"
 				" cannot be the same");
 	if (action_modify_field->dst.field == RTE_FLOW_FIELD_VALUE ||
-	    action_modify_field->dst.field == RTE_FLOW_FIELD_POINTER)
+	    action_modify_field->dst.field == RTE_FLOW_FIELD_POINTER ||
+	    action_modify_field->dst.field == RTE_FLOW_FIELD_MARK)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
-				"immediate value or a pointer to it"
+				"mark, immediate value or a pointer to it"
 				" cannot be used as a destination");
 	if (action_modify_field->dst.field == RTE_FLOW_FIELD_START ||
 	    action_modify_field->src.field == RTE_FLOW_FIELD_START)