From: Bin Huang <huangbin.mails@gmail.com>
To: Yongseok Koh <yskoh@mellanox.com>
Cc: "Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>,
dev@dpdk.org, "Adrien Mazarguil" <adrien.mazarguil@6wind.com>,
bin.huang@hxt-semitech.com, jie2.liu@hxt-semitech.com,
bing.zhao@hxt-semitech.com, zhiqiang.zhang@hxt-semitech.com
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: add ptype table idx for TCP ACK packets
Date: Fri, 30 Mar 2018 13:46:30 +0800 [thread overview]
Message-ID: <CANni5brr+xMGr=JmaVUgXTHN1srdMGQBaJYV0M2NLOMLj5SBXA@mail.gmail.com> (raw)
In-Reply-To: <20180329204624.GA23501@yongseok-MBP.local>
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
prev parent reply other threads:[~2018-03-30 5:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-29 11:25 Bin Huang
2018-03-29 20:46 ` Yongseok Koh
2018-03-30 5:46 ` Bin Huang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANni5brr+xMGr=JmaVUgXTHN1srdMGQBaJYV0M2NLOMLj5SBXA@mail.gmail.com' \
--to=huangbin.mails@gmail.com \
--cc=adrien.mazarguil@6wind.com \
--cc=bin.huang@hxt-semitech.com \
--cc=bing.zhao@hxt-semitech.com \
--cc=dev@dpdk.org \
--cc=jie2.liu@hxt-semitech.com \
--cc=nelio.laranjeiro@6wind.com \
--cc=yskoh@mellanox.com \
--cc=zhiqiang.zhang@hxt-semitech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).