[PATCH] app/testpmd: add size parameter to raw_encap action

[PATCH] app/testpmd: add size parameter to raw_encap action

[Gregory Etelson]

Testpmd always provides RAW_ENCAP flow action configuration with
encap buffer and the buffer size.
That implementation does not allow to create non-masked raw_encap
action in the template API actions template.

The patch adds the `size` parameter to testpmd `raw_encap` action
configuration.
Testpmd can create non-masked raw-encap action template and specify
encap buffer during flow creation.

Example:

# total data size is 50
testpmd> set raw_encap 0 \
         eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
         ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
         vxlan vni is 1 / end_set

testpmd> flow actions_template 0 create ingress \
         actions_template_id 50 \
         template raw_encap size 50 / jump / end \
         mask raw_encap size 50 / jump / end \

tstpmd> flow queue 0 create 0 template_table 0 \
        pattern_template 0 actions_template 0 postpone no \
        pattern ... end \
        actions raw_encap index 0 / jump group 1 / end

The new `size` parameter is mutually exclusive with the existing
`index` parameter.

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 6c8571154e..b231c3fd03 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -647,6 +647,7 @@ enum index {
 	ACTION_DEC_TCP_ACK_VALUE,
 	ACTION_RAW_ENCAP,
 	ACTION_RAW_DECAP,
+	ACTION_RAW_ENCAP_SIZE,
 	ACTION_RAW_ENCAP_INDEX,
 	ACTION_RAW_ENCAP_INDEX_VALUE,
 	ACTION_RAW_DECAP_INDEX,
@@ -2389,6 +2390,7 @@ static const enum index action_dec_tcp_ack[] = {
 };
 
 static const enum index action_raw_encap[] = {
+	ACTION_RAW_ENCAP_SIZE,
 	ACTION_RAW_ENCAP_INDEX,
 	ACTION_NEXT,
 	ZERO,
@@ -6761,6 +6763,14 @@ static const struct token token_list[] = {
 		.next = NEXT(action_raw_encap),
 		.call = parse_vc_action_raw_encap,
 	},
+	[ACTION_RAW_ENCAP_SIZE] = {
+		.name = "size",
+		.help = "raw encap size",
+		.next = NEXT(NEXT_ENTRY(ACTION_NEXT),
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_raw_encap, size)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_RAW_ENCAP_INDEX] = {
 		.name = "index",
 		.help = "the index of raw_encap_confs",
@@ -9456,8 +9466,6 @@ parse_vc_action_raw_encap(struct context *ctx, const struct token *token,
 			  unsigned int size)
 {
 	struct buffer *out = buf;
-	struct rte_flow_action *action;
-	struct action_raw_encap_data *action_raw_encap_data = NULL;
 	int ret;
 
 	ret = parse_vc(ctx, token, str, len, buf, size);
@@ -9468,16 +9476,9 @@ parse_vc_action_raw_encap(struct context *ctx, const struct token *token,
 		return ret;
 	if (!out->args.vc.actions_n)
 		return -1;
-	action = &out->args.vc.actions[out->args.vc.actions_n - 1];
 	/* Point to selected object. */
 	ctx->object = out->args.vc.data;
 	ctx->objmask = NULL;
-	/* Copy the headers to the buffer. */
-	action_raw_encap_data = ctx->object;
-	action_raw_encap_data->conf.data = raw_encap_confs[0].data;
-	action_raw_encap_data->conf.preserve = NULL;
-	action_raw_encap_data->conf.size = raw_encap_confs[0].size;
-	action->conf = &action_raw_encap_data->conf;
 	return ret;
 }
 
-- 
2.39.2

Re: [PATCH] app/testpmd: add size parameter to raw_encap action

[Ferruh Yigit]

On 10/26/2023 8:30 AM, Gregory Etelson wrote:
> Testpmd always provides RAW_ENCAP flow action configuration with
> encap buffer and the buffer size.
> That implementation does not allow to create non-masked raw_encap
> action in the template API actions template.
> 
> The patch adds the `size` parameter to testpmd `raw_encap` action
> configuration.
> Testpmd can create non-masked raw-encap action template and specify
> encap buffer during flow creation.
> 
> Example:
> 
> # total data size is 50
> testpmd> set raw_encap 0 \
>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>          vxlan vni is 1 / end_set
> 
> testpmd> flow actions_template 0 create ingress \
>          actions_template_id 50 \
>          template raw_encap size 50 / jump / end \
>          mask raw_encap size 50 / jump / end \
> 
> tstpmd> flow queue 0 create 0 template_table 0 \
>         pattern_template 0 actions_template 0 postpone no \
>         pattern ... end \
>         actions raw_encap index 0 / jump group 1 / end
> 
> The new `size` parameter is mutually exclusive with the existing
> `index` parameter.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> 

Hi Ori, Can you please support reviewing this patch?

RE: [PATCH] app/testpmd: add size parameter to raw_encap action

[Dariusz Sosnowski]

Hi Gregory,

> -----Original Message-----
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Thursday, October 26, 2023 09:31
> To: dev@dpdk.org
> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
>
> Testpmd always provides RAW_ENCAP flow action configuration with encap
> buffer and the buffer size.
> That implementation does not allow to create non-masked raw_encap action
> in the template API actions template.
> 
> The patch adds the `size` parameter to testpmd `raw_encap` action
> configuration.
> Testpmd can create non-masked raw-encap action template and specify encap
> buffer during flow creation.
> 
> Example:
> 
> # total data size is 50
> testpmd> set raw_encap 0 \
>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>          vxlan vni is 1 / end_set
> 
> testpmd> flow actions_template 0 create ingress \
>          actions_template_id 50 \
>          template raw_encap size 50 / jump / end \
>          mask raw_encap size 50 / jump / end \
> 
> tstpmd> flow queue 0 create 0 template_table 0 \
>         pattern_template 0 actions_template 0 postpone no \
>         pattern ... end \
>         actions raw_encap index 0 / jump group 1 / end
> 
> The new `size` parameter is mutually exclusive with the existing `index`
> parameter.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>

The following sequence of commands results in "Bad arguments" error, but I think it should be accepted. 

testpmd> port stop all
Stopping ports...
Checking link statuses...
Done
testpmd> flow configure 0 queues_number 4 queues_size 64
Configure flows on port 0: number of queues 4 with 64 elements
testpmd> port start all
Port 0: B8:CE:F6:7B:D8:E0
Checking link statuses...
Done
testpmd> set raw_encap 0 eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / vxlan vni is 1 / end_set
testpmd> flow actions_template 0 create ingress actions_template_id 100 template raw_encap index 0 / jump / end mask raw_encap index 0 / jump / end
Bad arguments

Could you please take a look at that?

Best regards,
Dariusz Sosnowski

RE: [PATCH] app/testpmd: add size parameter to raw_encap action

[Dariusz Sosnowski]

> -----Original Message-----
> From: Dariusz Sosnowski
> Sent: Friday, February 9, 2024 12:04
> To: Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org
> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> Subject: RE: [PATCH] app/testpmd: add size parameter to raw_encap action
> 
> Hi Gregory,
> 
> > -----Original Message-----
> > From: Gregory Etelson <getelson@nvidia.com>
> > Sent: Thursday, October 26, 2023 09:31
> > To: dev@dpdk.org
> > Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
> > <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
> > <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> > Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
> >
> > Testpmd always provides RAW_ENCAP flow action configuration with encap
> > buffer and the buffer size.
> > That implementation does not allow to create non-masked raw_encap
> > action in the template API actions template.
> >
> > The patch adds the `size` parameter to testpmd `raw_encap` action
> > configuration.
> > Testpmd can create non-masked raw-encap action template and specify
> > encap buffer during flow creation.
> >
> > Example:
> >
> > # total data size is 50
> > testpmd> set raw_encap 0 \
> >          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
> >          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
> >          vxlan vni is 1 / end_set
> >
> > testpmd> flow actions_template 0 create ingress \
> >          actions_template_id 50 \
> >          template raw_encap size 50 / jump / end \
> >          mask raw_encap size 50 / jump / end \
> >
> > tstpmd> flow queue 0 create 0 template_table 0 \
> >         pattern_template 0 actions_template 0 postpone no \
> >         pattern ... end \
> >         actions raw_encap index 0 / jump group 1 / end
> >
> > The new `size` parameter is mutually exclusive with the existing
> > `index` parameter.
> >
> > Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> 
> The following sequence of commands results in "Bad arguments" error, but I
> think it should be accepted.
> 
> testpmd> port stop all
> Stopping ports...
> Checking link statuses...
> Done
> testpmd> flow configure 0 queues_number 4 queues_size 64
> Configure flows on port 0: number of queues 4 with 64 elements
> testpmd> port start all
> Port 0: B8:CE:F6:7B:D8:E0
> Checking link statuses...
> Done
> testpmd> set raw_encap 0 eth src is 11:22:33:44:55:66 dst is
> testpmd> aa:bb:cc:dd:01:aa / ipv4 src is 31.31.31.31 dst is 63.63.63.1 /
> testpmd> udp src is 1 / vxlan vni is 1 / end_set flow actions_template 0
> testpmd> create ingress actions_template_id 100 template raw_encap index
> testpmd> 0 / jump / end mask raw_encap index 0 / jump / end
> Bad arguments

I bisected the tree, and it appears that this issue is not caused by this commit, but it's an existing problem in testpmd.
The root cause of "Bad arguments" is the fact that when parsing function for raw_encap index is called,
parse_int() is called with size == 0, but raw_encap index size is sizeof(size_t).
This causes a failure in validation introduced in commit 913b919906da ("app/testpmd: add size validation to token parsers").

The same issue appears for other cases where parse_int() is called with size == 0 e.g., parsing RSS queues in RSS flow action.
I'll provide a patch for testpmd which addresses that.

Best regards,
Dariusz Sosnowski

Re: [PATCH] app/testpmd: add size parameter to raw_encap action

[Ferruh Yigit]

On 2/9/2024 1:43 PM, Dariusz Sosnowski wrote:
>> -----Original Message-----
>> From: Dariusz Sosnowski
>> Sent: Friday, February 9, 2024 12:04
>> To: Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org
>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>> Subject: RE: [PATCH] app/testpmd: add size parameter to raw_encap action
>>
>> Hi Gregory,
>>
>>> -----Original Message-----
>>> From: Gregory Etelson <getelson@nvidia.com>
>>> Sent: Thursday, October 26, 2023 09:31
>>> To: dev@dpdk.org
>>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>>> Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
>>>
>>> Testpmd always provides RAW_ENCAP flow action configuration with encap
>>> buffer and the buffer size.
>>> That implementation does not allow to create non-masked raw_encap
>>> action in the template API actions template.
>>>
>>> The patch adds the `size` parameter to testpmd `raw_encap` action
>>> configuration.
>>> Testpmd can create non-masked raw-encap action template and specify
>>> encap buffer during flow creation.
>>>
>>> Example:
>>>
>>> # total data size is 50
>>> testpmd> set raw_encap 0 \
>>>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>>>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>>>          vxlan vni is 1 / end_set
>>>
>>> testpmd> flow actions_template 0 create ingress \
>>>          actions_template_id 50 \
>>>          template raw_encap size 50 / jump / end \
>>>          mask raw_encap size 50 / jump / end \
>>>
>>> tstpmd> flow queue 0 create 0 template_table 0 \
>>>         pattern_template 0 actions_template 0 postpone no \
>>>         pattern ... end \
>>>         actions raw_encap index 0 / jump group 1 / end
>>>
>>> The new `size` parameter is mutually exclusive with the existing
>>> `index` parameter.
>>>
>>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>>
>> The following sequence of commands results in "Bad arguments" error, but I
>> think it should be accepted.
>>
>> testpmd> port stop all
>> Stopping ports...
>> Checking link statuses...
>> Done
>> testpmd> flow configure 0 queues_number 4 queues_size 64
>> Configure flows on port 0: number of queues 4 with 64 elements
>> testpmd> port start all
>> Port 0: B8:CE:F6:7B:D8:E0
>> Checking link statuses...
>> Done
>> testpmd> set raw_encap 0 eth src is 11:22:33:44:55:66 dst is
>> testpmd> aa:bb:cc:dd:01:aa / ipv4 src is 31.31.31.31 dst is 63.63.63.1 /
>> testpmd> udp src is 1 / vxlan vni is 1 / end_set flow actions_template 0
>> testpmd> create ingress actions_template_id 100 template raw_encap index
>> testpmd> 0 / jump / end mask raw_encap index 0 / jump / end
>> Bad arguments
> 
> I bisected the tree, and it appears that this issue is not caused by this commit, but it's an existing problem in testpmd.
> The root cause of "Bad arguments" is the fact that when parsing function for raw_encap index is called,
> parse_int() is called with size == 0, but raw_encap index size is sizeof(size_t).
> This causes a failure in validation introduced in commit 913b919906da ("app/testpmd: add size validation to token parsers").
> 
> The same issue appears for other cases where parse_int() is called with size == 0 e.g., parsing RSS queues in RSS flow action.
> I'll provide a patch for testpmd which addresses that.
> 

Hi Dariusz,

I merged that patch recently, and it is still not merged to main repo,
so we can drop it from next-net if required, instead of having a fix on
top of it.

That patch is because of a defect that overwrites some memory, and side
impact you mentioned was not expected.
Can you please sync with Gregory about it first?

Re: [PATCH] app/testpmd: add size parameter to raw_encap action

[Gregory Etelson]

[-- Attachment #1: Type: text/plain, Size: 4737 bytes --]

Hello Ferruh,

The patch did not fix an existing issue.
It was proposed the code improvement.
Let's drop it for now.
I'll post a general solution in the next iteration.

Regards,
Gregory
________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Friday, February 9, 2024 15:55
To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org <dev@dpdk.org>
Cc: Maayan Kashani <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
Subject: Re: [PATCH] app/testpmd: add size parameter to raw_encap action

External email: Use caution opening links or attachments


On 2/9/2024 1:43 PM, Dariusz Sosnowski wrote:
>> -----Original Message-----
>> From: Dariusz Sosnowski
>> Sent: Friday, February 9, 2024 12:04
>> To: Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org
>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>> Subject: RE: [PATCH] app/testpmd: add size parameter to raw_encap action
>>
>> Hi Gregory,
>>
>>> -----Original Message-----
>>> From: Gregory Etelson <getelson@nvidia.com>
>>> Sent: Thursday, October 26, 2023 09:31
>>> To: dev@dpdk.org
>>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>>> Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
>>>
>>> Testpmd always provides RAW_ENCAP flow action configuration with encap
>>> buffer and the buffer size.
>>> That implementation does not allow to create non-masked raw_encap
>>> action in the template API actions template.
>>>
>>> The patch adds the `size` parameter to testpmd `raw_encap` action
>>> configuration.
>>> Testpmd can create non-masked raw-encap action template and specify
>>> encap buffer during flow creation.
>>>
>>> Example:
>>>
>>> # total data size is 50
>>> testpmd> set raw_encap 0 \
>>>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>>>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>>>          vxlan vni is 1 / end_set
>>>
>>> testpmd> flow actions_template 0 create ingress \
>>>          actions_template_id 50 \
>>>          template raw_encap size 50 / jump / end \
>>>          mask raw_encap size 50 / jump / end \
>>>
>>> tstpmd> flow queue 0 create 0 template_table 0 \
>>>         pattern_template 0 actions_template 0 postpone no \
>>>         pattern ... end \
>>>         actions raw_encap index 0 / jump group 1 / end
>>>
>>> The new `size` parameter is mutually exclusive with the existing
>>> `index` parameter.
>>>
>>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>>
>> The following sequence of commands results in "Bad arguments" error, but I
>> think it should be accepted.
>>
>> testpmd> port stop all
>> Stopping ports...
>> Checking link statuses...
>> Done
>> testpmd> flow configure 0 queues_number 4 queues_size 64
>> Configure flows on port 0: number of queues 4 with 64 elements
>> testpmd> port start all
>> Port 0: B8:CE:F6:7B:D8:E0
>> Checking link statuses...
>> Done
>> testpmd> set raw_encap 0 eth src is 11:22:33:44:55:66 dst is
>> testpmd> aa:bb:cc:dd:01:aa / ipv4 src is 31.31.31.31 dst is 63.63.63.1 /
>> testpmd> udp src is 1 / vxlan vni is 1 / end_set flow actions_template 0
>> testpmd> create ingress actions_template_id 100 template raw_encap index
>> testpmd> 0 / jump / end mask raw_encap index 0 / jump / end
>> Bad arguments
>
> I bisected the tree, and it appears that this issue is not caused by this commit, but it's an existing problem in testpmd.
> The root cause of "Bad arguments" is the fact that when parsing function for raw_encap index is called,
> parse_int() is called with size == 0, but raw_encap index size is sizeof(size_t).
> This causes a failure in validation introduced in commit 913b919906da ("app/testpmd: add size validation to token parsers").
>
> The same issue appears for other cases where parse_int() is called with size == 0 e.g., parsing RSS queues in RSS flow action.
> I'll provide a patch for testpmd which addresses that.
>

Hi Dariusz,

I merged that patch recently, and it is still not merged to main repo,
so we can drop it from next-net if required, instead of having a fix on
top of it.

That patch is because of a defect that overwrites some memory, and side
impact you mentioned was not expected.
Can you please sync with Gregory about it first?


[-- Attachment #2: Type: text/html, Size: 7959 bytes --]

Re: [PATCH] app/testpmd: add size parameter to raw_encap action

[Ferruh Yigit]

On 2/9/2024 2:01 PM, Gregory Etelson wrote:
> Hello Ferruh,
> 
> The patch did not fix an existing issue.
>

I remember it is for protection against similar buffer overwrite issue,
not exactly for that specific issue.


> It was proposed the code improvement.
> Let's drop it for now.
>

Agree, I will drop.

> I'll post a general solution in the next iteration.
> 

Ack, thanks.

> Regards,
> Gregory
> ------------------------------------------------------------------------
> *From:* Ferruh Yigit <ferruh.yigit@amd.com>
> *Sent:* Friday, February 9, 2024 15:55
> *To:* Dariusz Sosnowski <dsosnowski@nvidia.com>; Gregory Etelson
> <getelson@nvidia.com>; dev@dpdk.org <dev@dpdk.org>
> *Cc:* Maayan Kashani <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>;
> Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>
> *Subject:* Re: [PATCH] app/testpmd: add size parameter to raw_encap action
>  
> External email: Use caution opening links or attachments
> 
> 
> On 2/9/2024 1:43 PM, Dariusz Sosnowski wrote:
>>> -----Original Message-----
>>> From: Dariusz Sosnowski
>>> Sent: Friday, February 9, 2024 12:04
>>> To: Gregory Etelson <getelson@nvidia.com>; dev@dpdk.org
>>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>>> Subject: RE: [PATCH] app/testpmd: add size parameter to raw_encap action
>>>
>>> Hi Gregory,
>>>
>>>> -----Original Message-----
>>>> From: Gregory Etelson <getelson@nvidia.com>
>>>> Sent: Thursday, October 26, 2023 09:31
>>>> To: dev@dpdk.org
>>>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>>>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>>>> Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
>>>>
>>>> Testpmd always provides RAW_ENCAP flow action configuration with encap
>>>> buffer and the buffer size.
>>>> That implementation does not allow to create non-masked raw_encap
>>>> action in the template API actions template.
>>>>
>>>> The patch adds the `size` parameter to testpmd `raw_encap` action
>>>> configuration.
>>>> Testpmd can create non-masked raw-encap action template and specify
>>>> encap buffer during flow creation.
>>>>
>>>> Example:
>>>>
>>>> # total data size is 50
>>>> testpmd> set raw_encap 0 \
>>>>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>>>>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>>>>          vxlan vni is 1 / end_set
>>>>
>>>> testpmd> flow actions_template 0 create ingress \
>>>>          actions_template_id 50 \
>>>>          template raw_encap size 50 / jump / end \
>>>>          mask raw_encap size 50 / jump / end \
>>>>
>>>> tstpmd> flow queue 0 create 0 template_table 0 \
>>>>         pattern_template 0 actions_template 0 postpone no \
>>>>         pattern ... end \
>>>>         actions raw_encap index 0 / jump group 1 / end
>>>>
>>>> The new `size` parameter is mutually exclusive with the existing
>>>> `index` parameter.
>>>>
>>>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>>>
>>> The following sequence of commands results in "Bad arguments" error, but I
>>> think it should be accepted.
>>>
>>> testpmd> port stop all
>>> Stopping ports...
>>> Checking link statuses...
>>> Done
>>> testpmd> flow configure 0 queues_number 4 queues_size 64
>>> Configure flows on port 0: number of queues 4 with 64 elements
>>> testpmd> port start all
>>> Port 0: B8:CE:F6:7B:D8:E0
>>> Checking link statuses...
>>> Done
>>> testpmd> set raw_encap 0 eth src is 11:22:33:44:55:66 dst is
>>> testpmd> aa:bb:cc:dd:01:aa / ipv4 src is 31.31.31.31 dst is 63.63.63.1 /
>>> testpmd> udp src is 1 / vxlan vni is 1 / end_set flow actions_template 0
>>> testpmd> create ingress actions_template_id 100 template raw_encap index
>>> testpmd> 0 / jump / end mask raw_encap index 0 / jump / end
>>> Bad arguments
>>
>> I bisected the tree, and it appears that this issue is not caused by this commit, but it's an existing problem in testpmd.
>> The root cause of "Bad arguments" is the fact that when parsing function for raw_encap index is called,
>> parse_int() is called with size == 0, but raw_encap index size is sizeof(size_t).
>> This causes a failure in validation introduced in commit 913b919906da ("app/testpmd: add size validation to token parsers").
>>
>> The same issue appears for other cases where parse_int() is called with size == 0 e.g., parsing RSS queues in RSS flow action.
>> I'll provide a patch for testpmd which addresses that.
>>
> 
> Hi Dariusz,
> 
> I merged that patch recently, and it is still not merged to main repo,
> so we can drop it from next-net if required, instead of having a fix on
> top of it.
> 
> That patch is because of a defect that overwrites some memory, and side
> impact you mentioned was not expected.
> Can you please sync with Gregory about it first?
> 

RE: [PATCH] app/testpmd: add size parameter to raw_encap action

[Dariusz Sosnowski]

> -----Original Message-----
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Thursday, October 26, 2023 09:31
> To: dev@dpdk.org
> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
> 
> Testpmd always provides RAW_ENCAP flow action configuration with encap
> buffer and the buffer size.
> That implementation does not allow to create non-masked raw_encap action
> in the template API actions template.
> 
> The patch adds the `size` parameter to testpmd `raw_encap` action
> configuration.
> Testpmd can create non-masked raw-encap action template and specify encap
> buffer during flow creation.
> 
> Example:
> 
> # total data size is 50
> testpmd> set raw_encap 0 \
>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>          vxlan vni is 1 / end_set
> 
> testpmd> flow actions_template 0 create ingress \
>          actions_template_id 50 \
>          template raw_encap size 50 / jump / end \
>          mask raw_encap size 50 / jump / end \
> 
> tstpmd> flow queue 0 create 0 template_table 0 \
>         pattern_template 0 actions_template 0 postpone no \
>         pattern ... end \
>         actions raw_encap index 0 / jump group 1 / end
> 
> The new `size` parameter is mutually exclusive with the existing `index`
> parameter.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

Best regards,
Dariusz Sosnowski

Re: [PATCH] app/testpmd: add size parameter to raw_encap action

[Ferruh Yigit]

On 2/9/2024 4:00 PM, Dariusz Sosnowski wrote:
>> -----Original Message-----
>> From: Gregory Etelson <getelson@nvidia.com>
>> Sent: Thursday, October 26, 2023 09:31
>> To: dev@dpdk.org
>> Cc: Gregory Etelson <getelson@nvidia.com>; Maayan Kashani
>> <mkashani@nvidia.com>; Ori Kam <orika@nvidia.com>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>> Subject: [PATCH] app/testpmd: add size parameter to raw_encap action
>>
>> Testpmd always provides RAW_ENCAP flow action configuration with encap
>> buffer and the buffer size.
>> That implementation does not allow to create non-masked raw_encap action
>> in the template API actions template.
>>
>> The patch adds the `size` parameter to testpmd `raw_encap` action
>> configuration.
>> Testpmd can create non-masked raw-encap action template and specify encap
>> buffer during flow creation.
>>
>> Example:
>>
>> # total data size is 50
>> testpmd> set raw_encap 0 \
>>          eth src is 11:22:33:44:55:66 dst is aa:bb:cc:dd:01:aa / \
>>          ipv4 src is 31.31.31.31 dst is 63.63.63.1 / udp src is 1 / \
>>          vxlan vni is 1 / end_set
>>
>> testpmd> flow actions_template 0 create ingress \
>>          actions_template_id 50 \
>>          template raw_encap size 50 / jump / end \
>>          mask raw_encap size 50 / jump / end \
>>
>> tstpmd> flow queue 0 create 0 template_table 0 \
>>         pattern_template 0 actions_template 0 postpone no \
>>         pattern ... end \
>>         actions raw_encap index 0 / jump group 1 / end
>>
>> The new `size` parameter is mutually exclusive with the existing `index`
>> parameter.
>>
>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 

Applied to dpdk-next-net/main, thanks.