DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <arybchenko@solarflare.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Bernard Iremonger <bernard.iremonger@intel.com>,
	John McNamara <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	 Olivier Matz <olivier.matz@6wind.com>, <dev@dpdk.org>,
	<shahafs@mellanox.com>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP checksum definition
Date: Wed, 3 Oct 2018 22:47:15 +0300	[thread overview]
Message-ID: <8195dbcb-4b05-b47b-ebdf-2a8c36fa2974@solarflare.com> (raw)
In-Reply-To: <20181003181412.GA8662@jerin>

Hi Jerin,

On 03.10.2018 21:14, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Wed, 3 Oct 2018 21:00:37 +0300
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
>>   Bernard Iremonger <bernard.iremonger@intel.com>, John McNamara
>>   <john.mcnamara@intel.com>, Marko Kovacevic <marko.kovacevic@intel.com>,
>>   Thomas Monjalon <thomas@monjalon.net>, Ferruh Yigit
>>   <ferruh.yigit@intel.com>, Olivier Matz <olivier.matz@6wind.com>,
>>   dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin"
>>   <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP
>>   checksum definition
>> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
>>   Thunderbird/52.9.1
>>
>> On 03.10.2018 20:12, Jerin Jacob wrote:
>>> -----Original Message-----
>>>> Date: Wed, 3 Oct 2018 13:27:13 +0530
>>>> From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>>> To: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> CC: Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
>>>>    Bernard Iremonger <bernard.iremonger@intel.com>, John McNamara
>>>>    <john.mcnamara@intel.com>, Marko Kovacevic <marko.kovacevic@intel.com>,
>>>>    Thomas Monjalon <thomas@monjalon.net>, Ferruh Yigit
>>>>    <ferruh.yigit@intel.com>, Olivier Matz <olivier.matz@6wind.com>,
>>>>    dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin"
>>>>    <konstantin.ananyev@intel.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP
>>>>    checksum definition
>>>> User-Agent: Mutt/1.10.1 (2018-07-13)
>>>>
>>>> External Email
>>>>
>>>> -----Original Message-----
>>>>> Date: Wed, 3 Oct 2018 10:34:52 +0300
>>>>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Wenzhuo Lu
>>>>>    <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>, Bernard
>>>>>    Iremonger <bernard.iremonger@intel.com>, John McNamara
>>>>>    <john.mcnamara@intel.com>, Marko Kovacevic <marko.kovacevic@intel.com>,
>>>>>    Thomas Monjalon <thomas@monjalon.net>, Ferruh Yigit
>>>>>    <ferruh.yigit@intel.com>, Olivier Matz <olivier.matz@6wind.com>
>>>>> CC: dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin"
>>>>>    <konstantin.ananyev@intel.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP
>>>>>    checksum definition
>>>>> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
>>>>>    Thunderbird/60.0
>>>>>
>>>>>
>>>>> On 10/2/18 10:24 PM, Jerin Jacob wrote:
>>>>>
>>>>> Introduced DEV_RX_OFFLOAD_OUTER_UDP_CKSUM Rx offload flag and
>>>>> PKT_RX_EL4_CKSUM_BAD mbuf ol_flags to detect outer UDP checksum
>>>>> failure.
>>>>>
>>>>> - To use hardware Rx outer UDP checksum offload, the user needs to
>>>>> configure DEV_RX_OFFLOAD_OUTER_UDP_CKSUM offload flags in slowpath.
>>>>>
>>>>> - Driver updates the PKT_RX_EL4_CKSUM_BAD mbuf ol_flag on checksum failure
>>>>> similar to the outer L3 PKT_RX_EIP_CKSUM_BAD flag.
>>>>>
>>>>> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com><mailto:jerin.jacob@caviumnetworks.com>
>>>>>
>>>>> 1. I'm not sure that it is OK that mbuf and ethdev changes go in one patch.
>>>>>      It seems typically mbuf changes go separately and mbuf changes should
>>>>>      be applied to main dpdk repo.
>>>> I don't have strong opinion on this. If there are no other objection, I
>>>> will split the patch further as mbuf and ethdev as you pointed out.
>>>>
>>>>> 2. I'd like to see thought why single bit is used for outer L2 checksum when
>>>>>      2 bits (UNKNOWN, BAD, GOOD, NONE) are used for PKT_RX_L4_CKSUM.
>>>>>      May be it is OK, but it would be useful to state explicitly why it is decided
>>>>>      to go this way.
>>>> I am following the scheme similar to OUTER IP checksum where we have only
>>>> one bit filed(PKT_RX_EIP_CKSUM_BAD). I will mention in the git commit.
>>>>
>>>>
>>>>> 3. PKT_RX_L4_CKSUM_MASK description says nothing if it is inner or outer.
>>>>>      May be it is not directly related to changeset, but I think it would be really
>>>>>      useful to clarify it.
>>>> I will update the comment.
>>> Hi Andrew,
>>>
>>> I looked at the other definitions in mbuf.h, according the documentation,
>>> If nothing is mentioned it is treated as inner if the packet is
>>> tunneled else it is outer most. So I would like avoid confusion by
>>> adding "inner" in the exiting PKT_RX_L4_CKSUM_MASK comment.
>>> Technically it is not correct to say "inner" if the packet is not
>>> tunneled. So I am untouching the exiting comment.
>>>
>> Yes, it is incorrect to say that it is inner. How does application find
>> how to treat PKT_RX_L4_CKSUM (inner or outer)?
>> Should it rely on packet type provided in mbuf?
> AFAIK, Finding is it a tunneled packet or not is through ptype or SW has
> to parse the packet. For example, testpmd chooses later method using
> "csum parse-tunnel on <port>" to detect the presence of the tunnel.

SW parsing of the packet cannot help, since app should be sure
that HW has classified the packet as tunneled and provided information
about inner and outer checksum checks.

>> Is it specified/mentioned somewhere?
> I don't know. It it not directly related to this change set, Olivier may know
> additional details.

I disagree. You're adding one more offload flag. Yes, it simply follows
existing RX_EIP_CKSUM_BAD pattern. But, IMHO, RX_EIP_CKSUM_BAD
has many open questions. Why should these open questions be preserved
here? It is similar to the code with a bug which is cloned once again with
the bug :)

If everyone else is fine with the description of Rx checksum offloads and
it is only me who is unhappy with it - no problem.

Thanks for your patience and I'm sorry that I'm really boring with it.
My goal is just to make it clear and as the result have less bugs in
networking PMDs and applications.

Andrew.

  reply	other threads:[~2018-10-03 19:47 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-13 13:47 [dpdk-dev] [PATCH 1/4] ethdev: add SCTP Rx checksum offload support Jerin Jacob
2018-09-13 13:47 ` [dpdk-dev] [PATCH 2/4] mbuf: fix Tx offload mask Jerin Jacob
2018-10-01 13:45   ` Ferruh Yigit
2018-10-01 15:53     ` Jerin Jacob
2018-10-01 16:13       ` Ferruh Yigit
2018-09-13 13:47 ` [dpdk-dev] [PATCH 3/4] ethdev: add Rx offload outer L4 checksum definitions Jerin Jacob
2018-09-13 17:24   ` Shahaf Shuler
2018-09-14  3:05     ` Jerin Jacob
2018-09-16  5:53       ` Shahaf Shuler
2018-09-16  9:32         ` Jerin Jacob
2018-09-13 13:47 ` [dpdk-dev] [PATCH 4/4] ethdev: add Tx " Jerin Jacob
2018-10-01 13:45   ` Ferruh Yigit
2018-10-02  9:52     ` Jerin Jacob
2018-10-01 13:45 ` [dpdk-dev] [PATCH 1/4] ethdev: add SCTP Rx checksum offload support Ferruh Yigit
2018-10-01 13:46 ` Ferruh Yigit
2018-10-01 15:59   ` Jerin Jacob
2018-10-01 16:11     ` Ferruh Yigit
2018-10-02  8:53       ` Jerin Jacob
2018-10-02  9:13     ` Ferruh Yigit
2018-10-02 10:51 ` [dpdk-dev] [PATCH v2 1/2] " Jerin Jacob
2018-10-02 10:51   ` [dpdk-dev] [PATCH v2 2/2] mbuf: fix Tx offload mask Jerin Jacob
2018-10-04  2:31     ` Hu, Jiayu
2018-10-04 16:05       ` Ferruh Yigit
2018-10-03 18:52   ` [dpdk-dev] [PATCH v2 1/2] ethdev: add SCTP Rx checksum offload support Ferruh Yigit
2018-10-02 19:24 ` [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP checksum definition Jerin Jacob
2018-10-02 19:24   ` [dpdk-dev] [PATCH v2 2/4] ethdev: add Tx " Jerin Jacob
2018-10-03  7:41     ` Andrew Rybchenko
2018-10-03  7:58       ` Jerin Jacob
2018-10-03  8:02         ` Ferruh Yigit
2018-10-03  8:36           ` Thomas Monjalon
2018-10-03 10:52     ` Iremonger, Bernard
2018-10-02 19:24   ` [dpdk-dev] [PATCH v2 3/4] app/testpmd: add outer UDP HW checksum support Jerin Jacob
2018-10-03 13:23     ` Iremonger, Bernard
2018-10-02 19:24   ` [dpdk-dev] [PATCH v2 4/4] app/testpmd: collect bad outer L4 checksum for csum engine Jerin Jacob
2018-10-03  8:29     ` Andrew Rybchenko
2018-10-03  7:34   ` [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP checksum definition Andrew Rybchenko
2018-10-03  7:57     ` Jerin Jacob
2018-10-03  8:35       ` Thomas Monjalon
2018-10-03  8:36         ` Andrew Rybchenko
2018-10-03 17:12       ` Jerin Jacob
2018-10-03 18:00         ` Andrew Rybchenko
2018-10-03 18:14           ` Jerin Jacob
2018-10-03 19:47             ` Andrew Rybchenko [this message]
2018-10-03 20:08               ` Thomas Monjalon
2018-10-04  5:59               ` Jerin Jacob
2018-10-05 19:48                 ` Ferruh Yigit
2018-10-05 20:04                 ` Ferruh Yigit
2018-10-05 22:44                   ` Thomas Monjalon
2018-10-06  8:15                     ` Jerin Jacob
2018-10-06 12:18                       ` Ananyev, Konstantin
2018-10-08  8:12                         ` Ferruh Yigit
2018-10-08  8:24                           ` Jerin Jacob
2018-10-08  9:04                             ` Thomas Monjalon
2018-10-08  9:37                               ` Jerin Jacob
2018-10-08 10:53                                 ` Ferruh Yigit
2018-10-08 11:55                                   ` Jerin Jacob
2018-10-08 12:13                                     ` Ferruh Yigit
2018-10-08 12:25                                       ` Jerin Jacob
2018-10-08 13:03                                         ` Thomas Monjalon
2018-10-08 13:08                                           ` Jerin Jacob
2018-10-03  8:53   ` Ananyev, Konstantin
2018-10-03  8:59     ` Jerin Jacob
2018-10-03  9:17       ` Ananyev, Konstantin
2018-10-03  9:22         ` Jerin Jacob
2018-10-03 10:16           ` Ananyev, Konstantin
2018-10-03 11:15             ` Jerin Jacob
2018-10-03 10:51   ` Iremonger, Bernard
2018-10-03 11:19     ` Jerin Jacob
2018-10-03 13:00       ` Iremonger, Bernard
2018-10-03 18:16 ` [dpdk-dev] [PATCH v3 " Jerin Jacob
2018-10-03 18:16   ` [dpdk-dev] [PATCH v3 2/4] ethdev: add Tx " Jerin Jacob
2018-10-03 18:16   ` [dpdk-dev] [PATCH v3 3/4] app/testpmd: add outer UDP HW checksum support Jerin Jacob
2018-10-03 18:16   ` [dpdk-dev] [PATCH v3 4/4] app/testpmd: collect bad outer L4 checksum for csum engine Jerin Jacob
2018-10-04 13:45     ` Iremonger, Bernard
2018-10-04 14:16       ` Jerin Jacob
2018-10-04 15:06         ` Iremonger, Bernard
2018-10-08 16:09   ` [dpdk-dev] [PATCH v4 1/4] ethdev: add Rx offload outer UDP checksum definition Jerin Jacob
2018-10-08 16:09     ` [dpdk-dev] [PATCH v4 2/4] ethdev: add Tx " Jerin Jacob
2018-10-09 10:06       ` Andrew Rybchenko
2018-10-08 16:09     ` [dpdk-dev] [PATCH v4 3/4] app/testpmd: add outer UDP HW checksum support Jerin Jacob
2018-10-08 16:09     ` [dpdk-dev] [PATCH v4 4/4] app/testpmd: collect bad outer L4 checksum for csum engine Jerin Jacob
2018-10-09 10:06     ` [dpdk-dev] [PATCH v4 1/4] ethdev: add Rx offload outer UDP checksum definition Andrew Rybchenko
2018-10-09 14:18     ` [dpdk-dev] [PATCH v5 " Jerin Jacob
2018-10-09 14:18       ` [dpdk-dev] [PATCH v5 2/4] ethdev: add Tx " Jerin Jacob
2018-10-09 14:18       ` [dpdk-dev] [PATCH v5 3/4] app/testpmd: add outer UDP HW checksum support Jerin Jacob
2018-10-09 14:18       ` [dpdk-dev] [PATCH v5 4/4] app/testpmd: collect bad outer L4 checksum for csum engine Jerin Jacob
2018-10-09 16:46       ` [dpdk-dev] [PATCH v5 1/4] ethdev: add Rx offload outer UDP checksum definition Ferruh Yigit

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=8195dbcb-4b05-b47b-ebdf-2a8c36fa2974@solarflare.com \
    --to=arybchenko@solarflare.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=jingjing.wu@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.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).