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