All of lore.kernel.org
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: do not allow copy to mark via modify field
@ 2021-06-16 18:34 Alexander Kozyrev
  2021-07-14  6:30 ` Slava Ovsiienko
  2021-07-16  8:43 ` [dpdk-dev] [PATCH v2] " Alexander Kozyrev
  0 siblings, 2 replies; 5+ messages in thread
From: Alexander Kozyrev @ 2021-06-16 18:34 UTC (permalink / raw)
  To: dev; +Cc: rasland, viacheslavo, matan

Mark requires a tag resource to be registered as part of
the value assigning. It is not possible during a copy
operation from a packet field. Forbid this in MODIFY_FIELD.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index dafd37ab93..26b901e32e 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4797,10 +4797,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)
-- 
2.18.2


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

* Re: [dpdk-dev] [PATCH] net/mlx5: do not allow copy to mark via modify field
  2021-06-16 18:34 [dpdk-dev] [PATCH] net/mlx5: do not allow copy to mark via modify field Alexander Kozyrev
@ 2021-07-14  6:30 ` Slava Ovsiienko
  2021-07-16  8:43 ` [dpdk-dev] [PATCH v2] " Alexander Kozyrev
  1 sibling, 0 replies; 5+ messages in thread
From: Slava Ovsiienko @ 2021-07-14  6:30 UTC (permalink / raw)
  To: Alexander Kozyrev, dev; +Cc: Raslan Darawsheh, Matan Azrad

Hi, Alexander

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, June 16, 2021 21:35
> To: dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>
> Subject: [PATCH] net/mlx5: do not allow copy to mark via modify field
> 
> Mark requires a tag resource to be registered as part of the value assigning. It
> is not possible during a copy operation from a packet field. Forbid this in
> MODIFY_FIELD.

Sorry, commit message seems not to be clear even for one who is in context.
"tag resource" - is this hardware resource?
"not possible" - due to hardware limitation?

And I would add few words about the final table "match-set flow tag resource" -
not too many details, just explain - if we copy packet field to MARK substitution register -
we can't handle actual value copying in the "match-set" table.

With best regards,
Slava

> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index dafd37ab93..26b901e32e 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -4797,10 +4797,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)
> --
> 2.18.2


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

* [dpdk-dev] [PATCH v2] net/mlx5: do not allow copy to mark via modify field
  2021-06-16 18:34 [dpdk-dev] [PATCH] net/mlx5: do not allow copy to mark via modify field Alexander Kozyrev
  2021-07-14  6:30 ` Slava Ovsiienko
@ 2021-07-16  8:43 ` Alexander Kozyrev
  2021-07-16 10:47   ` Slava Ovsiienko
  1 sibling, 1 reply; 5+ messages in thread
From: Alexander Kozyrev @ 2021-07-16  8:43 UTC (permalink / raw)
  To: dev; +Cc: rasland, matan, viacheslavo

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(-)

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)
-- 
2.18.2


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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: do not allow copy to mark via modify field
  2021-07-16  8:43 ` [dpdk-dev] [PATCH v2] " Alexander Kozyrev
@ 2021-07-16 10:47   ` Slava Ovsiienko
  2021-07-22 14:28     ` Thomas Monjalon
  0 siblings, 1 reply; 5+ messages in thread
From: Slava Ovsiienko @ 2021-07-16 10:47 UTC (permalink / raw)
  To: Alexander Kozyrev, dev; +Cc: Raslan Darawsheh, Matan Azrad

> -----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>


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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: do not allow copy to mark via modify field
  2021-07-16 10:47   ` Slava Ovsiienko
@ 2021-07-22 14:28     ` Thomas Monjalon
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2021-07-22 14:28 UTC (permalink / raw)
  To: Alexander Kozyrev; +Cc: dev, Raslan Darawsheh, Matan Azrad, Slava Ovsiienko

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.




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

end of thread, other threads:[~2021-07-22 14:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-16 18:34 [dpdk-dev] [PATCH] net/mlx5: do not allow copy to mark via modify field Alexander Kozyrev
2021-07-14  6:30 ` Slava Ovsiienko
2021-07-16  8:43 ` [dpdk-dev] [PATCH v2] " Alexander Kozyrev
2021-07-16 10:47   ` Slava Ovsiienko
2021-07-22 14:28     ` Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.