diff mbox

[dpdk-dev] net/mlx5: add ptype table idx for TCP ACK packets

Message ID 1522322725-6244-1-git-send-email-huangbin.mails@gmail.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Bin Huang March 29, 2018, 11:25 a.m. UTC 
  From: Bin Huang <bin.huang@hxt-semitech.com>

According to CQE format:
- l4_hdr_type:
     0 - None
     1 - TCP header was present in the packet
     2 - UDP header was present in the packet
     3 - TCP header was present in the packet with Empty
         TCP ACK indication. (TCP packet <ACK> flag is set,
         and packet carries no data)
     4 - TCP header was present in the packet with TCP ACK indication.
         (TCP packet <ACK> flag is set, and packet carries data).

A packet should be identified as TCP packet if l4_hdr_type is 1, 3 or 4.
Add corresponding idx of TCP ACK to ptype table.

previous discussion:
https://www.mail-archive.com/users@dpdk.org/msg02980.html

Signed-off-by: Bin Huang <bin.huang@hxt-semitech.com>
---
 drivers/net/mlx5/mlx5_rxtx.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

Comments

Yongseok Koh March 29, 2018, 8:46 p.m. UTC | #1 
On Thu, Mar 29, 2018 at 07:25:25PM +0800, Bin Huang wrote:
> From: Bin Huang <bin.huang@hxt-semitech.com>
> 
> According to CQE format:
> - l4_hdr_type:
>      0 - None
>      1 - TCP header was present in the packet
>      2 - UDP header was present in the packet
>      3 - TCP header was present in the packet with Empty
>          TCP ACK indication. (TCP packet <ACK> flag is set,
>          and packet carries no data)
>      4 - TCP header was present in the packet with TCP ACK indication.
>          (TCP packet <ACK> flag is set, and packet carries data).
> 
> A packet should be identified as TCP packet if l4_hdr_type is 1, 3 or 4.
> Add corresponding idx of TCP ACK to ptype table.
> 
> previous discussion:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fusers%40dpdk.org%2Fmsg02980.html&data=02%7C01%7Cyskoh%40mellanox.com%7C6e489616778d4440d68708d59567c921%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C636579195347091211&sdata=%2BPG5X1%2BOR0tnGMw4QKQyz1QrMNqOQ%2BQ5WeJRgjrY7j8%3D&reserved=0
> 
> Signed-off-by: Bin Huang <bin.huang@hxt-semitech.com>
> ---

Thank you for the patch and nice catch!
Only small comments.

>  drivers/net/mlx5/mlx5_rxtx.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index dc4ead9..e9c5a6f 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -81,11 +81,19 @@
>  		     RTE_PTYPE_L4_FRAG;
>  	(*p)[0x22] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>  		     RTE_PTYPE_L4_FRAG;
> -	/* TCP */
> +	/* TCP - Support TCP ACK indication */

This comment might not be needed. This can give wrong impression that the
following is to support TCP ACK. Let's leave it as is.

Or, in order to be informative, you can leave separate comments below for each
two. For example,

>  	(*p)[0x05] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>  		     RTE_PTYPE_L4_TCP;
>  	(*p)[0x06] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>  		     RTE_PTYPE_L4_TCP;
+	/* TCP ACK indication */
> +	(*p)[0x11] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_L4_TCP;
> +	(*p)[0x12] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_L4_TCP;
+	/* TCP ACK */
> +	(*p)[0x0d] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_L4_TCP;
> +	(*p)[0x0e] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_L4_TCP;

Please sort it. Better to move 0x0d and 0x0e earlier.
Can you also repeat it after 0x85 and 0x86?

>  	/* UDP */
>  	(*p)[0x09] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>  		     RTE_PTYPE_L4_UDP;
> @@ -141,12 +149,36 @@
>  	(*p)[0x46] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0x4d] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0x4e] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0x51] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0x52] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
>  	(*p)[0xc5] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L4_TCP;
>  	(*p)[0xc6] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0xcd] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0xce] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0xd1] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
> +	(*p)[0xd2] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
> +		     RTE_PTYPE_INNER_L4_TCP;
>  	/* Tunneled - UDP */
>  	(*p)[0x49] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>  		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |

Thanks,
Yongseok
Bin Huang March 30, 2018, 5:46 a.m. UTC | #2 
Hi Yongseok,

Thanks for the comments, I have submitted v2 patch following your comments.
https://dpdk.org/dev/patchwork/patch/36763/

On Fri, Mar 30, 2018 at 4:46 AM, Yongseok Koh <yskoh@mellanox.com> wrote:
> On Thu, Mar 29, 2018 at 07:25:25PM +0800, Bin Huang wrote:
>> From: Bin Huang <bin.huang@hxt-semitech.com>
>>
>> According to CQE format:
>> - l4_hdr_type:
>>      0 - None
>>      1 - TCP header was present in the packet
>>      2 - UDP header was present in the packet
>>      3 - TCP header was present in the packet with Empty
>>          TCP ACK indication. (TCP packet <ACK> flag is set,
>>          and packet carries no data)
>>      4 - TCP header was present in the packet with TCP ACK indication.
>>          (TCP packet <ACK> flag is set, and packet carries data).
>>
>> A packet should be identified as TCP packet if l4_hdr_type is 1, 3 or 4.
>> Add corresponding idx of TCP ACK to ptype table.
>>
>> previous discussion:
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fusers%40dpdk.org%2Fmsg02980.html&data=02%7C01%7Cyskoh%40mellanox.com%7C6e489616778d4440d68708d59567c921%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C636579195347091211&sdata=%2BPG5X1%2BOR0tnGMw4QKQyz1QrMNqOQ%2BQ5WeJRgjrY7j8%3D&reserved=0
>>
>> Signed-off-by: Bin Huang <bin.huang@hxt-semitech.com>
>> ---
>
> Thank you for the patch and nice catch!
> Only small comments.
>
>>  drivers/net/mlx5/mlx5_rxtx.c | 34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
>> index dc4ead9..e9c5a6f 100644
>> --- a/drivers/net/mlx5/mlx5_rxtx.c
>> +++ b/drivers/net/mlx5/mlx5_rxtx.c
>> @@ -81,11 +81,19 @@
>>                    RTE_PTYPE_L4_FRAG;
>>       (*p)[0x22] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>>                    RTE_PTYPE_L4_FRAG;
>> -     /* TCP */
>> +     /* TCP - Support TCP ACK indication */
>
> This comment might not be needed. This can give wrong impression that the
> following is to support TCP ACK. Let's leave it as is.
>
> Or, in order to be informative, you can leave separate comments below for each
> two. For example,
>
>>       (*p)[0x05] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>>                    RTE_PTYPE_L4_TCP;
>>       (*p)[0x06] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>>                    RTE_PTYPE_L4_TCP;
> +       /* TCP ACK indication */
>> +     (*p)[0x11] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_L4_TCP;
>> +     (*p)[0x12] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_L4_TCP;
> +       /* TCP ACK */
>> +     (*p)[0x0d] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_L4_TCP;
>> +     (*p)[0x0e] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_L4_TCP;
>
> Please sort it. Better to move 0x0d and 0x0e earlier.
> Can you also repeat it after 0x85 and 0x86?
>
>>       /* UDP */
>>       (*p)[0x09] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>>                    RTE_PTYPE_L4_UDP;
>> @@ -141,12 +149,36 @@
>>       (*p)[0x46] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0x4d] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0x4e] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0x51] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0x52] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>>       (*p)[0xc5] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L4_TCP;
>>       (*p)[0xc6] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0xcd] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0xce] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0xd1] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>> +     (*p)[0xd2] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
>> +                  RTE_PTYPE_INNER_L4_TCP;
>>       /* Tunneled - UDP */
>>       (*p)[0x49] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
>>                    RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
>
> Thanks,
> Yongseok

Thanks,
Bin
diff mbox

Patch

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index dc4ead9..e9c5a6f 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -81,11 +81,19 @@ 
 		     RTE_PTYPE_L4_FRAG;
 	(*p)[0x22] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
 		     RTE_PTYPE_L4_FRAG;
-	/* TCP */
+	/* TCP - Support TCP ACK indication */
 	(*p)[0x05] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
 		     RTE_PTYPE_L4_TCP;
 	(*p)[0x06] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
 		     RTE_PTYPE_L4_TCP;
+	(*p)[0x11] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_L4_TCP;
+	(*p)[0x12] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_L4_TCP;
+	(*p)[0x0d] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_L4_TCP;
+	(*p)[0x0e] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_L4_TCP;
 	/* UDP */
 	(*p)[0x09] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
 		     RTE_PTYPE_L4_UDP;
@@ -141,12 +149,36 @@ 
 	(*p)[0x46] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0x4d] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0x4e] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0x51] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0x52] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
 	(*p)[0xc5] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L4_TCP;
 	(*p)[0xc6] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0xcd] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0xce] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0xd1] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
+	(*p)[0xd2] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV6_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
+		     RTE_PTYPE_INNER_L4_TCP;
 	/* Tunneled - UDP */
 	(*p)[0x49] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
 		     RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |