[v2,2/2] app/testpmd: fix tunnel offload private items location

Message ID 20210425155722.32477-2-getelson@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2,1/2] net/mlx5: fix tunnel offload private items location |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Gregory Etelson April 25, 2021, 3:57 p.m. UTC
  Tunnel offload API requires application to query PMD for specific flow
items and actions. Application uses these PMD specific elements to
build flow rules according to the tunnel offload model.
The model does not restrict private elements location in a flow rule,
but the current MLX5 PMD implementation expected that tunnel offload
rule will begin with PMD specific elements.
The patch places tunnel offload private PMD flow elements between
general RTE flow elements in a rule.

Cc: stable@dpdk.org
Fixes: 1b9f274623b8 ("app/testpmd: add commands for tunnel offload")

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 app/test-pmd/config.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
  

Comments

Ferruh Yigit April 30, 2021, 1:49 p.m. UTC | #1
On 4/25/2021 4:57 PM, Gregory Etelson wrote:
> Tunnel offload API requires application to query PMD for specific flow
> items and actions. Application uses these PMD specific elements to
> build flow rules according to the tunnel offload model.

Can you please give some samples what are "PMD specific elements" required to be
queried by application? To understand issue better.

> The model does not restrict private elements location in a flow rule,
> but the current MLX5 PMD implementation expected that tunnel offload
> rule will begin with PMD specific elements.

Why we need to refer the mlx5 pmd implementation in the testpmd patch? Is this
patch trying to align testpmd to the mlx5 implementation?

> The patch places tunnel offload private PMD flow elements between
> general RTE flow elements in a rule.
> 

Why?

Overall what was the problem, what was failing and what was its impact?

And how changing the private elements location in the flow rule solving the issue?

> Cc: stable@dpdk.org
> Fixes: 1b9f274623b8 ("app/testpmd: add commands for tunnel offload")
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
>  app/test-pmd/config.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 40b2b29725..1520b8193f 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1664,7 +1664,7 @@ port_flow_tunnel_offload_cmd_prep(portid_t port_id,
>  		     aptr->type != RTE_FLOW_ACTION_TYPE_END;
>  		     aptr++, num_actions++);
>  		pft->actions = malloc(
> -				(num_actions +  pft->num_pmd_actions) *
> +				(num_actions +  pft->num_pmd_actions + 1) *
>  				sizeof(actions[0]));
>  		if (!pft->actions) {
>  			rte_flow_tunnel_action_decap_release(
> @@ -1672,9 +1672,10 @@ port_flow_tunnel_offload_cmd_prep(portid_t port_id,
>  					pft->num_pmd_actions, &error);
>  			return NULL;
>  		}
> -		rte_memcpy(pft->actions, pft->pmd_actions,
> +		pft->actions[0].type = RTE_FLOW_ACTION_TYPE_VOID;
> +		rte_memcpy(pft->actions + 1, pft->pmd_actions,
>  			   pft->num_pmd_actions * sizeof(actions[0]));
> -		rte_memcpy(pft->actions + pft->num_pmd_actions, actions,
> +		rte_memcpy(pft->actions + pft->num_pmd_actions + 1, actions,
>  			   num_actions * sizeof(actions[0]));
>  	}
>  	if (tunnel_ops->items) {
> @@ -1692,7 +1693,7 @@ port_flow_tunnel_offload_cmd_prep(portid_t port_id,
>  		for (iptr = pattern, num_items = 1;
>  		     iptr->type != RTE_FLOW_ITEM_TYPE_END;
>  		     iptr++, num_items++);
> -		pft->items = malloc((num_items + pft->num_pmd_items) *
> +		pft->items = malloc((num_items + pft->num_pmd_items + 1) *
>  				    sizeof(pattern[0]));
>  		if (!pft->items) {
>  			rte_flow_tunnel_item_release(
> @@ -1700,9 +1701,10 @@ port_flow_tunnel_offload_cmd_prep(portid_t port_id,
>  					pft->num_pmd_items, &error);
>  			return NULL;
>  		}
> -		rte_memcpy(pft->items, pft->pmd_items,
> +		pft->items[0].type = RTE_FLOW_ITEM_TYPE_VOID;
> +		rte_memcpy(pft->items + 1, pft->pmd_items,
>  			   pft->num_pmd_items * sizeof(pattern[0]));
> -		rte_memcpy(pft->items + pft->num_pmd_items, pattern,
> +		rte_memcpy(pft->items + pft->num_pmd_items + 1, pattern,
>  			   num_items * sizeof(pattern[0]));
>  	}
>  
>
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 40b2b29725..1520b8193f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1664,7 +1664,7 @@  port_flow_tunnel_offload_cmd_prep(portid_t port_id,
 		     aptr->type != RTE_FLOW_ACTION_TYPE_END;
 		     aptr++, num_actions++);
 		pft->actions = malloc(
-				(num_actions +  pft->num_pmd_actions) *
+				(num_actions +  pft->num_pmd_actions + 1) *
 				sizeof(actions[0]));
 		if (!pft->actions) {
 			rte_flow_tunnel_action_decap_release(
@@ -1672,9 +1672,10 @@  port_flow_tunnel_offload_cmd_prep(portid_t port_id,
 					pft->num_pmd_actions, &error);
 			return NULL;
 		}
-		rte_memcpy(pft->actions, pft->pmd_actions,
+		pft->actions[0].type = RTE_FLOW_ACTION_TYPE_VOID;
+		rte_memcpy(pft->actions + 1, pft->pmd_actions,
 			   pft->num_pmd_actions * sizeof(actions[0]));
-		rte_memcpy(pft->actions + pft->num_pmd_actions, actions,
+		rte_memcpy(pft->actions + pft->num_pmd_actions + 1, actions,
 			   num_actions * sizeof(actions[0]));
 	}
 	if (tunnel_ops->items) {
@@ -1692,7 +1693,7 @@  port_flow_tunnel_offload_cmd_prep(portid_t port_id,
 		for (iptr = pattern, num_items = 1;
 		     iptr->type != RTE_FLOW_ITEM_TYPE_END;
 		     iptr++, num_items++);
-		pft->items = malloc((num_items + pft->num_pmd_items) *
+		pft->items = malloc((num_items + pft->num_pmd_items + 1) *
 				    sizeof(pattern[0]));
 		if (!pft->items) {
 			rte_flow_tunnel_item_release(
@@ -1700,9 +1701,10 @@  port_flow_tunnel_offload_cmd_prep(portid_t port_id,
 					pft->num_pmd_items, &error);
 			return NULL;
 		}
-		rte_memcpy(pft->items, pft->pmd_items,
+		pft->items[0].type = RTE_FLOW_ITEM_TYPE_VOID;
+		rte_memcpy(pft->items + 1, pft->pmd_items,
 			   pft->num_pmd_items * sizeof(pattern[0]));
-		rte_memcpy(pft->items + pft->num_pmd_items, pattern,
+		rte_memcpy(pft->items + pft->num_pmd_items + 1, pattern,
 			   num_items * sizeof(pattern[0]));
 	}