DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix modify header action position
@ 2019-04-14 20:19 Ori Kam
  2019-04-14 20:19 ` Ori Kam
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Ori Kam @ 2019-04-14 20:19 UTC (permalink / raw)
  To: yskoh, shahafs; +Cc: dev, orika, dekelp, stable

According to RTE flow the action order should be the order that the
actions were given.
In the case of modify actions the position of the action was always
last.

This commit solves this issue by saving the position of the first modify
action, and then adds to this position the pointer to the modify action.

Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
Cc: dekelp@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3862b26..7b5eab7 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	struct mlx5_flow_dv_tag_resource tag_resource;
+	uint32_t modify_action_position = UINT32_MAX;
 
 	if (priority == MLX5_FLOW_PRIO_RSVD)
 		priority = priv->config.flow_prio - 1;
@@ -3419,6 +3420,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
 					MLX5_FLOW_ACTION_SET_MAC_SRC :
 					MLX5_FLOW_ACTION_SET_MAC_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
@@ -3429,6 +3432,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
 					MLX5_FLOW_ACTION_SET_IPV4_SRC :
 					MLX5_FLOW_ACTION_SET_IPV4_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
@@ -3439,6 +3444,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
 					MLX5_FLOW_ACTION_SET_IPV6_SRC :
 					MLX5_FLOW_ACTION_SET_IPV6_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
@@ -3450,6 +3457,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
 					MLX5_FLOW_ACTION_SET_TP_SRC :
 					MLX5_FLOW_ACTION_SET_TP_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl(&res, items,
@@ -3457,6 +3466,8 @@ struct field_modify_info modify_tcp[] = {
 								  error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl(&res, actions,
@@ -3464,6 +3475,8 @@ struct field_modify_info modify_tcp[] = {
 							     error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_END:
 			actions_end = true;
@@ -3474,7 +3487,7 @@ struct field_modify_info modify_tcp[] = {
 								 dev_flow,
 								 error))
 					return -rte_errno;
-				dev_flow->dv.actions[actions_n++] =
+				dev_flow->dv.actions[modify_action_position] =
 					dev_flow->dv.modify_hdr->verbs_action;
 			}
 			break;
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH] net/mlx5: fix modify header action position
  2019-04-14 20:19 [dpdk-dev] [PATCH] net/mlx5: fix modify header action position Ori Kam
@ 2019-04-14 20:19 ` Ori Kam
  2019-04-16 23:41 ` Yongseok Koh
  2019-04-17 20:01 ` [dpdk-dev] [PATCH v2] " Ori Kam
  2 siblings, 0 replies; 12+ messages in thread
From: Ori Kam @ 2019-04-14 20:19 UTC (permalink / raw)
  To: yskoh, shahafs; +Cc: dev, orika, dekelp, stable

According to RTE flow the action order should be the order that the
actions were given.
In the case of modify actions the position of the action was always
last.

This commit solves this issue by saving the position of the first modify
action, and then adds to this position the pointer to the modify action.

Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
Cc: dekelp@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3862b26..7b5eab7 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	struct mlx5_flow_dv_tag_resource tag_resource;
+	uint32_t modify_action_position = UINT32_MAX;
 
 	if (priority == MLX5_FLOW_PRIO_RSVD)
 		priority = priv->config.flow_prio - 1;
@@ -3419,6 +3420,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
 					MLX5_FLOW_ACTION_SET_MAC_SRC :
 					MLX5_FLOW_ACTION_SET_MAC_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
@@ -3429,6 +3432,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
 					MLX5_FLOW_ACTION_SET_IPV4_SRC :
 					MLX5_FLOW_ACTION_SET_IPV4_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
@@ -3439,6 +3444,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
 					MLX5_FLOW_ACTION_SET_IPV6_SRC :
 					MLX5_FLOW_ACTION_SET_IPV6_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
@@ -3450,6 +3457,8 @@ struct field_modify_info modify_tcp[] = {
 					RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
 					MLX5_FLOW_ACTION_SET_TP_SRC :
 					MLX5_FLOW_ACTION_SET_TP_DST;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl(&res, items,
@@ -3457,6 +3466,8 @@ struct field_modify_info modify_tcp[] = {
 								  error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl(&res, actions,
@@ -3464,6 +3475,8 @@ struct field_modify_info modify_tcp[] = {
 							     error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
+			if (modify_action_position == UINT32_MAX)
+				modify_action_position = actions_n++;
 			break;
 		case RTE_FLOW_ACTION_TYPE_END:
 			actions_end = true;
@@ -3474,7 +3487,7 @@ struct field_modify_info modify_tcp[] = {
 								 dev_flow,
 								 error))
 					return -rte_errno;
-				dev_flow->dv.actions[actions_n++] =
+				dev_flow->dv.actions[modify_action_position] =
 					dev_flow->dv.modify_hdr->verbs_action;
 			}
 			break;
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix modify header action position
  2019-04-14 20:19 [dpdk-dev] [PATCH] net/mlx5: fix modify header action position Ori Kam
  2019-04-14 20:19 ` Ori Kam
@ 2019-04-16 23:41 ` Yongseok Koh
  2019-04-16 23:41   ` Yongseok Koh
  2019-04-17  5:03   ` Ori Kam
  2019-04-17 20:01 ` [dpdk-dev] [PATCH v2] " Ori Kam
  2 siblings, 2 replies; 12+ messages in thread
From: Yongseok Koh @ 2019-04-16 23:41 UTC (permalink / raw)
  To: Ori Kam; +Cc: Shahaf Shuler, dev, Dekel Peled, stable

On Sun, Apr 14, 2019 at 08:19:57PM +0000, Ori Kam wrote:
> According to RTE flow the action order should be the order that the
> actions were given.
> In the case of modify actions the position of the action was always
> last.
> 
> This commit solves this issue by saving the position of the first modify
> action, and then adds to this position the pointer to the modify action.
> 
> Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
> Cc: dekelp@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 3862b26..7b5eab7 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
>  	};
>  	union flow_dv_attr flow_attr = { .attr = 0 };
>  	struct mlx5_flow_dv_tag_resource tag_resource;
> +	uint32_t modify_action_position = UINT32_MAX;

How about modify_action_idx and setting negative value as invalid? I don't force
it but it is just your choice.

	int modify_action_idx = -1;

>  	if (priority == MLX5_FLOW_PRIO_RSVD)
>  		priority = priv->config.flow_prio - 1;
> @@ -3419,6 +3420,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
>  					MLX5_FLOW_ACTION_SET_MAC_SRC :
>  					MLX5_FLOW_ACTION_SET_MAC_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;

Even though this does right thing, I hope it to be neater. Same lines are
repeating before 'break', so you can move it just out of this switch-case.
For example,

	for(..) {
		switch(..) {
		case ..
		default:
			break;
		}
		/* Check if a modify action is firstly seen. */
		if (modify_action_idx < 0 &&
		    (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
			modify_action_idx = actions_n++;
	}

thanks,
Yongseok

>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> @@ -3429,6 +3432,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
>  					MLX5_FLOW_ACTION_SET_IPV4_SRC :
>  					MLX5_FLOW_ACTION_SET_IPV4_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> @@ -3439,6 +3444,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
>  					MLX5_FLOW_ACTION_SET_IPV6_SRC :
>  					MLX5_FLOW_ACTION_SET_IPV6_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> @@ -3450,6 +3457,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
>  					MLX5_FLOW_ACTION_SET_TP_SRC :
>  					MLX5_FLOW_ACTION_SET_TP_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
>  			if (flow_dv_convert_action_modify_dec_ttl(&res, items,
> @@ -3457,6 +3466,8 @@ struct field_modify_info modify_tcp[] = {
>  								  error))
>  				return -rte_errno;
>  			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_TTL:
>  			if (flow_dv_convert_action_modify_ttl(&res, actions,
> @@ -3464,6 +3475,8 @@ struct field_modify_info modify_tcp[] = {
>  							     error))
>  				return -rte_errno;
>  			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_END:
>  			actions_end = true;
> @@ -3474,7 +3487,7 @@ struct field_modify_info modify_tcp[] = {
>  								 dev_flow,
>  								 error))
>  					return -rte_errno;
> -				dev_flow->dv.actions[actions_n++] =
> +				dev_flow->dv.actions[modify_action_position] =
>  					dev_flow->dv.modify_hdr->verbs_action;
>  			}
>  			break;
> -- 
> 1.8.3.1
> 

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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix modify header action position
  2019-04-16 23:41 ` Yongseok Koh
@ 2019-04-16 23:41   ` Yongseok Koh
  2019-04-17  5:03   ` Ori Kam
  1 sibling, 0 replies; 12+ messages in thread
From: Yongseok Koh @ 2019-04-16 23:41 UTC (permalink / raw)
  To: Ori Kam; +Cc: Shahaf Shuler, dev, Dekel Peled, stable

On Sun, Apr 14, 2019 at 08:19:57PM +0000, Ori Kam wrote:
> According to RTE flow the action order should be the order that the
> actions were given.
> In the case of modify actions the position of the action was always
> last.
> 
> This commit solves this issue by saving the position of the first modify
> action, and then adds to this position the pointer to the modify action.
> 
> Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
> Cc: dekelp@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 3862b26..7b5eab7 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
>  	};
>  	union flow_dv_attr flow_attr = { .attr = 0 };
>  	struct mlx5_flow_dv_tag_resource tag_resource;
> +	uint32_t modify_action_position = UINT32_MAX;

How about modify_action_idx and setting negative value as invalid? I don't force
it but it is just your choice.

	int modify_action_idx = -1;

>  	if (priority == MLX5_FLOW_PRIO_RSVD)
>  		priority = priv->config.flow_prio - 1;
> @@ -3419,6 +3420,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
>  					MLX5_FLOW_ACTION_SET_MAC_SRC :
>  					MLX5_FLOW_ACTION_SET_MAC_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;

Even though this does right thing, I hope it to be neater. Same lines are
repeating before 'break', so you can move it just out of this switch-case.
For example,

	for(..) {
		switch(..) {
		case ..
		default:
			break;
		}
		/* Check if a modify action is firstly seen. */
		if (modify_action_idx < 0 &&
		    (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
			modify_action_idx = actions_n++;
	}

thanks,
Yongseok

>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> @@ -3429,6 +3432,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
>  					MLX5_FLOW_ACTION_SET_IPV4_SRC :
>  					MLX5_FLOW_ACTION_SET_IPV4_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> @@ -3439,6 +3444,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
>  					MLX5_FLOW_ACTION_SET_IPV6_SRC :
>  					MLX5_FLOW_ACTION_SET_IPV6_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
>  		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> @@ -3450,6 +3457,8 @@ struct field_modify_info modify_tcp[] = {
>  					RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
>  					MLX5_FLOW_ACTION_SET_TP_SRC :
>  					MLX5_FLOW_ACTION_SET_TP_DST;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
>  			if (flow_dv_convert_action_modify_dec_ttl(&res, items,
> @@ -3457,6 +3466,8 @@ struct field_modify_info modify_tcp[] = {
>  								  error))
>  				return -rte_errno;
>  			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_SET_TTL:
>  			if (flow_dv_convert_action_modify_ttl(&res, actions,
> @@ -3464,6 +3475,8 @@ struct field_modify_info modify_tcp[] = {
>  							     error))
>  				return -rte_errno;
>  			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> +			if (modify_action_position == UINT32_MAX)
> +				modify_action_position = actions_n++;
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_END:
>  			actions_end = true;
> @@ -3474,7 +3487,7 @@ struct field_modify_info modify_tcp[] = {
>  								 dev_flow,
>  								 error))
>  					return -rte_errno;
> -				dev_flow->dv.actions[actions_n++] =
> +				dev_flow->dv.actions[modify_action_position] =
>  					dev_flow->dv.modify_hdr->verbs_action;
>  			}
>  			break;
> -- 
> 1.8.3.1
> 

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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix modify header action position
  2019-04-16 23:41 ` Yongseok Koh
  2019-04-16 23:41   ` Yongseok Koh
@ 2019-04-17  5:03   ` Ori Kam
  2019-04-17  5:03     ` Ori Kam
  1 sibling, 1 reply; 12+ messages in thread
From: Ori Kam @ 2019-04-17  5:03 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: Shahaf Shuler, dev, Dekel Peled, stable

PSB

> -----Original Message-----
> From: Yongseok Koh
> Sent: Wednesday, April 17, 2019 2:42 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org; Dekel Peled
> <dekelp@mellanox.com>; stable@dpdk.org
> Subject: Re: [PATCH] net/mlx5: fix modify header action position
> 
> On Sun, Apr 14, 2019 at 08:19:57PM +0000, Ori Kam wrote:
> > According to RTE flow the action order should be the order that the
> > actions were given.
> > In the case of modify actions the position of the action was always
> > last.
> >
> > This commit solves this issue by saving the position of the first modify
> > action, and then adds to this position the pointer to the modify action.
> >
> > Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
> > Cc: dekelp@mellanox.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow_dv.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> > index 3862b26..7b5eab7 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
> >  	};
> >  	union flow_dv_attr flow_attr = { .attr = 0 };
> >  	struct mlx5_flow_dv_tag_resource tag_resource;
> > +	uint32_t modify_action_position = UINT32_MAX;
> 
> How about modify_action_idx and setting negative value as invalid? I don't
> force
> it but it is just your choice.
> 

That was my first thought, but it gave warning when int is used as index to array.

> 	int modify_action_idx = -1;
> 
> >  	if (priority == MLX5_FLOW_PRIO_RSVD)
> >  		priority = priv->config.flow_prio - 1;
> > @@ -3419,6 +3420,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
> >  					MLX5_FLOW_ACTION_SET_MAC_SRC
> :
> >  					MLX5_FLOW_ACTION_SET_MAC_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> 
> Even though this does right thing, I hope it to be neater. Same lines are
> repeating before 'break', so you can move it just out of this switch-case.
> For example,
> 
> 	for(..) {
> 		switch(..) {
> 		case ..
> 		default:
> 			break;
> 		}
> 		/* Check if a modify action is firstly seen. */
> 		if (modify_action_idx < 0 &&
> 		    (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
> 			modify_action_idx = actions_n++;
> 	}
> 
> thanks,
> Yongseok
> 

I like your idea, will change.

> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> > @@ -3429,6 +3432,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
> >  					MLX5_FLOW_ACTION_SET_IPV4_SRC :
> >  					MLX5_FLOW_ACTION_SET_IPV4_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> > @@ -3439,6 +3444,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
> >  					MLX5_FLOW_ACTION_SET_IPV6_SRC :
> >  					MLX5_FLOW_ACTION_SET_IPV6_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
> >  		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> > @@ -3450,6 +3457,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
> >  					MLX5_FLOW_ACTION_SET_TP_SRC :
> >  					MLX5_FLOW_ACTION_SET_TP_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> >  			if (flow_dv_convert_action_modify_dec_ttl(&res,
> items,
> > @@ -3457,6 +3466,8 @@ struct field_modify_info modify_tcp[] = {
> >  								  error))
> >  				return -rte_errno;
> >  			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_TTL:
> >  			if (flow_dv_convert_action_modify_ttl(&res, actions,
> > @@ -3464,6 +3475,8 @@ struct field_modify_info modify_tcp[] = {
> >  							     error))
> >  				return -rte_errno;
> >  			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_END:
> >  			actions_end = true;
> > @@ -3474,7 +3487,7 @@ struct field_modify_info modify_tcp[] = {
> >  								 dev_flow,
> >  								 error))
> >  					return -rte_errno;
> > -				dev_flow->dv.actions[actions_n++] =
> > +				dev_flow->dv.actions[modify_action_position]
> =
> >  					dev_flow->dv.modify_hdr-
> >verbs_action;
> >  			}
> >  			break;
> > --
> > 1.8.3.1
> >

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

* Re: [dpdk-dev] [PATCH] net/mlx5: fix modify header action position
  2019-04-17  5:03   ` Ori Kam
@ 2019-04-17  5:03     ` Ori Kam
  0 siblings, 0 replies; 12+ messages in thread
From: Ori Kam @ 2019-04-17  5:03 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: Shahaf Shuler, dev, Dekel Peled, stable

PSB

> -----Original Message-----
> From: Yongseok Koh
> Sent: Wednesday, April 17, 2019 2:42 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org; Dekel Peled
> <dekelp@mellanox.com>; stable@dpdk.org
> Subject: Re: [PATCH] net/mlx5: fix modify header action position
> 
> On Sun, Apr 14, 2019 at 08:19:57PM +0000, Ori Kam wrote:
> > According to RTE flow the action order should be the order that the
> > actions were given.
> > In the case of modify actions the position of the action was always
> > last.
> >
> > This commit solves this issue by saving the position of the first modify
> > action, and then adds to this position the pointer to the modify action.
> >
> > Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
> > Cc: dekelp@mellanox.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow_dv.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> > index 3862b26..7b5eab7 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
> >  	};
> >  	union flow_dv_attr flow_attr = { .attr = 0 };
> >  	struct mlx5_flow_dv_tag_resource tag_resource;
> > +	uint32_t modify_action_position = UINT32_MAX;
> 
> How about modify_action_idx and setting negative value as invalid? I don't
> force
> it but it is just your choice.
> 

That was my first thought, but it gave warning when int is used as index to array.

> 	int modify_action_idx = -1;
> 
> >  	if (priority == MLX5_FLOW_PRIO_RSVD)
> >  		priority = priv->config.flow_prio - 1;
> > @@ -3419,6 +3420,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
> >  					MLX5_FLOW_ACTION_SET_MAC_SRC
> :
> >  					MLX5_FLOW_ACTION_SET_MAC_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> 
> Even though this does right thing, I hope it to be neater. Same lines are
> repeating before 'break', so you can move it just out of this switch-case.
> For example,
> 
> 	for(..) {
> 		switch(..) {
> 		case ..
> 		default:
> 			break;
> 		}
> 		/* Check if a modify action is firstly seen. */
> 		if (modify_action_idx < 0 &&
> 		    (action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
> 			modify_action_idx = actions_n++;
> 	}
> 
> thanks,
> Yongseok
> 

I like your idea, will change.

> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC:
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DST:
> > @@ -3429,6 +3432,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
> >  					MLX5_FLOW_ACTION_SET_IPV4_SRC :
> >  					MLX5_FLOW_ACTION_SET_IPV4_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC:
> >  		case RTE_FLOW_ACTION_TYPE_SET_IPV6_DST:
> > @@ -3439,6 +3444,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
> >  					MLX5_FLOW_ACTION_SET_IPV6_SRC :
> >  					MLX5_FLOW_ACTION_SET_IPV6_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_TP_SRC:
> >  		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> > @@ -3450,6 +3457,8 @@ struct field_modify_info modify_tcp[] = {
> >
> 	RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
> >  					MLX5_FLOW_ACTION_SET_TP_SRC :
> >  					MLX5_FLOW_ACTION_SET_TP_DST;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> >  			if (flow_dv_convert_action_modify_dec_ttl(&res,
> items,
> > @@ -3457,6 +3466,8 @@ struct field_modify_info modify_tcp[] = {
> >  								  error))
> >  				return -rte_errno;
> >  			action_flags |= MLX5_FLOW_ACTION_DEC_TTL;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_SET_TTL:
> >  			if (flow_dv_convert_action_modify_ttl(&res, actions,
> > @@ -3464,6 +3475,8 @@ struct field_modify_info modify_tcp[] = {
> >  							     error))
> >  				return -rte_errno;
> >  			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> > +			if (modify_action_position == UINT32_MAX)
> > +				modify_action_position = actions_n++;
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_END:
> >  			actions_end = true;
> > @@ -3474,7 +3487,7 @@ struct field_modify_info modify_tcp[] = {
> >  								 dev_flow,
> >  								 error))
> >  					return -rte_errno;
> > -				dev_flow->dv.actions[actions_n++] =
> > +				dev_flow->dv.actions[modify_action_position]
> =
> >  					dev_flow->dv.modify_hdr-
> >verbs_action;
> >  			}
> >  			break;
> > --
> > 1.8.3.1
> >

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

* [dpdk-dev] [PATCH v2] net/mlx5: fix modify header action position
  2019-04-14 20:19 [dpdk-dev] [PATCH] net/mlx5: fix modify header action position Ori Kam
  2019-04-14 20:19 ` Ori Kam
  2019-04-16 23:41 ` Yongseok Koh
@ 2019-04-17 20:01 ` Ori Kam
  2019-04-17 20:01   ` Ori Kam
  2019-04-18 13:25   ` Yongseok Koh
  2 siblings, 2 replies; 12+ messages in thread
From: Ori Kam @ 2019-04-17 20:01 UTC (permalink / raw)
  To: yskoh, shahafs; +Cc: dev, orika, dekelp, stable

According to RTE flow the action order should be the order that the
actions were given.
In the case of modify actions the position of the action was always
last.

This commit solves this issue by saving the position of the first modify
action, and then adds to this position the pointer to the modify action.

Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
Cc: dekelp@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Ori Kam <orika@mellanox.com>
---
v2:
* Move setting the index position outside the case.

---
 drivers/net/mlx5/mlx5_flow_dv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3862b26..c84779d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	struct mlx5_flow_dv_tag_resource tag_resource;
+	uint32_t modify_action_position = UINT32_MAX;
 
 	if (priority == MLX5_FLOW_PRIO_RSVD)
 		priority = priv->config.flow_prio - 1;
@@ -3474,13 +3475,16 @@ struct field_modify_info modify_tcp[] = {
 								 dev_flow,
 								 error))
 					return -rte_errno;
-				dev_flow->dv.actions[actions_n++] =
+				dev_flow->dv.actions[modify_action_position] =
 					dev_flow->dv.modify_hdr->verbs_action;
 			}
 			break;
 		default:
 			break;
 		}
+		if ((action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS) &&
+		    modify_action_position == UINT32_MAX)
+			modify_action_position = actions_n++;
 	}
 	dev_flow->dv.actions_n = actions_n;
 	flow->actions = action_flags;
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH v2] net/mlx5: fix modify header action position
  2019-04-17 20:01 ` [dpdk-dev] [PATCH v2] " Ori Kam
@ 2019-04-17 20:01   ` Ori Kam
  2019-04-18 13:25   ` Yongseok Koh
  1 sibling, 0 replies; 12+ messages in thread
From: Ori Kam @ 2019-04-17 20:01 UTC (permalink / raw)
  To: yskoh, shahafs; +Cc: dev, orika, dekelp, stable

According to RTE flow the action order should be the order that the
actions were given.
In the case of modify actions the position of the action was always
last.

This commit solves this issue by saving the position of the first modify
action, and then adds to this position the pointer to the modify action.

Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
Cc: dekelp@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Ori Kam <orika@mellanox.com>
---
v2:
* Move setting the index position outside the case.

---
 drivers/net/mlx5/mlx5_flow_dv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3862b26..c84779d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	struct mlx5_flow_dv_tag_resource tag_resource;
+	uint32_t modify_action_position = UINT32_MAX;
 
 	if (priority == MLX5_FLOW_PRIO_RSVD)
 		priority = priv->config.flow_prio - 1;
@@ -3474,13 +3475,16 @@ struct field_modify_info modify_tcp[] = {
 								 dev_flow,
 								 error))
 					return -rte_errno;
-				dev_flow->dv.actions[actions_n++] =
+				dev_flow->dv.actions[modify_action_position] =
 					dev_flow->dv.modify_hdr->verbs_action;
 			}
 			break;
 		default:
 			break;
 		}
+		if ((action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS) &&
+		    modify_action_position == UINT32_MAX)
+			modify_action_position = actions_n++;
 	}
 	dev_flow->dv.actions_n = actions_n;
 	flow->actions = action_flags;
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix modify header action position
  2019-04-17 20:01 ` [dpdk-dev] [PATCH v2] " Ori Kam
  2019-04-17 20:01   ` Ori Kam
@ 2019-04-18 13:25   ` Yongseok Koh
  2019-04-18 13:25     ` Yongseok Koh
  2019-04-18 18:55     ` Shahaf Shuler
  1 sibling, 2 replies; 12+ messages in thread
From: Yongseok Koh @ 2019-04-18 13:25 UTC (permalink / raw)
  To: Ori Kam; +Cc: Shahaf Shuler, dev, Dekel Peled, stable


> On Apr 17, 2019, at 1:01 PM, Ori Kam <orika@mellanox.com> wrote:
> 
> According to RTE flow the action order should be the order that the
> actions were given.
> In the case of modify actions the position of the action was always
> last.
> 
> This commit solves this issue by saving the position of the first modify
> action, and then adds to this position the pointer to the modify action.
> 
> Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
> Cc: dekelp@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>

> v2:
> * Move setting the index position outside the case.
> 
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 3862b26..c84779d 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
> 	};
> 	union flow_dv_attr flow_attr = { .attr = 0 };
> 	struct mlx5_flow_dv_tag_resource tag_resource;
> +	uint32_t modify_action_position = UINT32_MAX;
> 
> 	if (priority == MLX5_FLOW_PRIO_RSVD)
> 		priority = priv->config.flow_prio - 1;
> @@ -3474,13 +3475,16 @@ struct field_modify_info modify_tcp[] = {
> 								 dev_flow,
> 								 error))
> 					return -rte_errno;
> -				dev_flow->dv.actions[actions_n++] =
> +				dev_flow->dv.actions[modify_action_position] =
> 					dev_flow->dv.modify_hdr->verbs_action;
> 			}
> 			break;
> 		default:
> 			break;
> 		}
> +		if ((action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS) &&
> +		    modify_action_position == UINT32_MAX)
> +			modify_action_position = actions_n++;
> 	}
> 	dev_flow->dv.actions_n = actions_n;
> 	flow->actions = action_flags;
> -- 
> 1.8.3.1
> 

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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix modify header action position
  2019-04-18 13:25   ` Yongseok Koh
@ 2019-04-18 13:25     ` Yongseok Koh
  2019-04-18 18:55     ` Shahaf Shuler
  1 sibling, 0 replies; 12+ messages in thread
From: Yongseok Koh @ 2019-04-18 13:25 UTC (permalink / raw)
  To: Ori Kam; +Cc: Shahaf Shuler, dev, Dekel Peled, stable


> On Apr 17, 2019, at 1:01 PM, Ori Kam <orika@mellanox.com> wrote:
> 
> According to RTE flow the action order should be the order that the
> actions were given.
> In the case of modify actions the position of the action was always
> last.
> 
> This commit solves this issue by saving the position of the first modify
> action, and then adds to this position the pointer to the modify action.
> 
> Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct Verbs")
> Cc: dekelp@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>

> v2:
> * Move setting the index position outside the case.
> 
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 3862b26..c84779d 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3235,6 +3235,7 @@ struct field_modify_info modify_tcp[] = {
> 	};
> 	union flow_dv_attr flow_attr = { .attr = 0 };
> 	struct mlx5_flow_dv_tag_resource tag_resource;
> +	uint32_t modify_action_position = UINT32_MAX;
> 
> 	if (priority == MLX5_FLOW_PRIO_RSVD)
> 		priority = priv->config.flow_prio - 1;
> @@ -3474,13 +3475,16 @@ struct field_modify_info modify_tcp[] = {
> 								 dev_flow,
> 								 error))
> 					return -rte_errno;
> -				dev_flow->dv.actions[actions_n++] =
> +				dev_flow->dv.actions[modify_action_position] =
> 					dev_flow->dv.modify_hdr->verbs_action;
> 			}
> 			break;
> 		default:
> 			break;
> 		}
> +		if ((action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS) &&
> +		    modify_action_position == UINT32_MAX)
> +			modify_action_position = actions_n++;
> 	}
> 	dev_flow->dv.actions_n = actions_n;
> 	flow->actions = action_flags;
> -- 
> 1.8.3.1
> 


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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix modify header action position
  2019-04-18 13:25   ` Yongseok Koh
  2019-04-18 13:25     ` Yongseok Koh
@ 2019-04-18 18:55     ` Shahaf Shuler
  2019-04-18 18:55       ` Shahaf Shuler
  1 sibling, 1 reply; 12+ messages in thread
From: Shahaf Shuler @ 2019-04-18 18:55 UTC (permalink / raw)
  To: Yongseok Koh, Ori Kam; +Cc: dev, Dekel Peled, stable

Thursday, April 18, 2019 4:26 PM, Yongseok Koh:
> Subject: Re: [PATCH v2] net/mlx5: fix modify header action position
> 
> 
> > On Apr 17, 2019, at 1:01 PM, Ori Kam <orika@mellanox.com> wrote:
> >
> > According to RTE flow the action order should be the order that the
> > actions were given.
> > In the case of modify actions the position of the action was always
> > last.
> >
> > This commit solves this issue by saving the position of the first
> > modify action, and then adds to this position the pointer to the modify
> action.
> >
> > Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct
> > Verbs")
> > Cc: dekelp@mellanox.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> Acked-by: Yongseok Koh <yskoh@mellanox.com>

Applied to next-net-mlx, thanks. 

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

* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix modify header action position
  2019-04-18 18:55     ` Shahaf Shuler
@ 2019-04-18 18:55       ` Shahaf Shuler
  0 siblings, 0 replies; 12+ messages in thread
From: Shahaf Shuler @ 2019-04-18 18:55 UTC (permalink / raw)
  To: Yongseok Koh, Ori Kam; +Cc: dev, Dekel Peled, stable

Thursday, April 18, 2019 4:26 PM, Yongseok Koh:
> Subject: Re: [PATCH v2] net/mlx5: fix modify header action position
> 
> 
> > On Apr 17, 2019, at 1:01 PM, Ori Kam <orika@mellanox.com> wrote:
> >
> > According to RTE flow the action order should be the order that the
> > actions were given.
> > In the case of modify actions the position of the action was always
> > last.
> >
> > This commit solves this issue by saving the position of the first
> > modify action, and then adds to this position the pointer to the modify
> action.
> >
> > Fixes: 4bb14c83df95 ("net/mlx5: support modify header using Direct
> > Verbs")
> > Cc: dekelp@mellanox.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> Acked-by: Yongseok Koh <yskoh@mellanox.com>

Applied to next-net-mlx, thanks. 


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

end of thread, other threads:[~2019-04-18 18:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-14 20:19 [dpdk-dev] [PATCH] net/mlx5: fix modify header action position Ori Kam
2019-04-14 20:19 ` Ori Kam
2019-04-16 23:41 ` Yongseok Koh
2019-04-16 23:41   ` Yongseok Koh
2019-04-17  5:03   ` Ori Kam
2019-04-17  5:03     ` Ori Kam
2019-04-17 20:01 ` [dpdk-dev] [PATCH v2] " Ori Kam
2019-04-17 20:01   ` Ori Kam
2019-04-18 13:25   ` Yongseok Koh
2019-04-18 13:25     ` Yongseok Koh
2019-04-18 18:55     ` Shahaf Shuler
2019-04-18 18:55       ` Shahaf Shuler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).