[dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP checksum definition
Ferruh Yigit
ferruh.yigit at intel.com
Mon Oct 8 14:13:31 CEST 2018
On 10/8/2018 12:55 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Mon, 8 Oct 2018 11:53:01 +0100
>> From: Ferruh Yigit <ferruh.yigit at intel.com>
>> To: Jerin Jacob <jerin.jacob at caviumnetworks.com>, Thomas Monjalon
>> <thomas at monjalon.net>
>> CC: "Ananyev, Konstantin" <konstantin.ananyev at intel.com>, Andrew Rybchenko
>> <arybchenko at solarflare.com>, "Lu, Wenzhuo" <wenzhuo.lu at intel.com>, "Wu,
>> Jingjing" <jingjing.wu at intel.com>, "Iremonger, Bernard"
>> <bernard.iremonger at intel.com>, "Mcnamara, John" <john.mcnamara at intel.com>,
>> "Kovacevic, Marko" <marko.kovacevic at intel.com>, Olivier Matz
>> <olivier.matz at 6wind.com>, "dev at dpdk.org" <dev at dpdk.org>,
>> "shahafs at mellanox.com" <shahafs at mellanox.com>, "didier.pallard at 6wind.com"
>> <didier.pallard at 6wind.com>
>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP
>> checksum definition
>> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>> Thunderbird/52.9.1
>>
>> On 10/8/2018 10:37 AM, Jerin Jacob wrote:
>>> -----Original Message-----
>>>> Date: Mon, 08 Oct 2018 11:04:51 +0200
>>>> From: Thomas Monjalon <thomas at monjalon.net>
>>>> To: Jerin Jacob <jerin.jacob at caviumnetworks.com>, Ferruh Yigit
>>>> <ferruh.yigit at intel.com>, "Ananyev, Konstantin"
>>>> <konstantin.ananyev at intel.com>
>>>> Cc: Andrew Rybchenko <arybchenko at solarflare.com>, "Lu, Wenzhuo"
>>>> <wenzhuo.lu at intel.com>, "Wu, Jingjing" <jingjing.wu at intel.com>,
>>>> "Iremonger, Bernard" <bernard.iremonger at intel.com>, "Mcnamara, John"
>>>> <john.mcnamara at intel.com>, "Kovacevic, Marko" <marko.kovacevic at intel.com>,
>>>> Olivier Matz <olivier.matz at 6wind.com>, "dev at dpdk.org" <dev at dpdk.org>,
>>>> "shahafs at mellanox.com" <shahafs at mellanox.com>, "didier.pallard at 6wind.com"
>>>> <didier.pallard at 6wind.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP
>>>> checksum definition
>>>>
>>>> 08/10/2018 10:24, Jerin Jacob:
>>>>> From: Ferruh Yigit <ferruh.yigit at intel.com>
>>>>>> On 10/6/2018 1:18 PM, Ananyev, Konstantin wrote:
>>>>>>> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
>>>>>>>> From: Thomas Monjalon <thomas at monjalon.net>
>>>>>>>>> However, we should re-visit the flag PKT_RX_EIP_CKSUM_BAD.
>>>>>>>>
>>>>>>>> Do we need to block this patch due to the exiting PKT_RX_EIP_CKSUM_BAD
>>>>>>>> definition?
>>>>>>>>
>>>>>>>> I already added the author of the PKT_RX_EIP_CKSUM_BAD flag and ethdev and mbuf
>>>>>>>> maintainers in this list. So what else I need make forward progress
>>>>>>>> on this patch?
>>>>>>>>
>>>>>>>> I think, the definition of PKT_RX_EIP_CKSUM_BAD based on HW capability. It
>>>>>>>> is safe to assume that ALL HW can support CKSUM BAD if the feature is
>>>>>>>> available and hence it is more portable.
>>>>>>>
>>>>>>> Yes, as I remember PKT_RX_EIP_CKSUM_BAD is based on DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM.
>>>>>>
>>>>>> Switching to two bit won't reduce the portability, HW supports only reporting
>>>>>> CKSUM_BAD can set BAD || UNKNOWN.
>>>>>
>>>>> UNKNOWN is not a bit. It is represented as 0. It spec has 2 bit, then
>>>>> driver need to report GOOD as well.
>>>>>
>>>>> Same applies for PKT_RX_EL4_CKSUM as well.
>>>>>
>>>>>>
>>>>>> And I think patch is not blocked by PKT_RX_EIP_CKSUM_BAD, it can be changed
>>>>>> separately, for this patch question is can we represent PKT_RX_EL4_CKSUM_* with
>>>>>> two bits, to have BAD/GOOD/UNKNOWN?
>>>>
>>>> Yes, exact.
>>>>
>>>> PKT_RX_EIP_CKSUM_BAD must be left aside.
>>>> We should just avoid taking it as a reference.
>>>> And we can reconsider its definition later.
>>>
>>> OK.
>>>
>>> IMO, Using 2 bit scheme for tunneled checksum has following performance
>>> issue from driver side.
>>>
>>> Driver need to mark the packet as GOOD. All the HW can support
>>> detection of BAD. That not necessary mean GOOD in case of tunnel packet,
>>> so driver has to detect the packet is tunneled and packet is not BAD
>>> then mark GOOD.
>>
>> Yes UNKNOWN is not a bit, but a state, why don't use it? Why driver has to check
>> it is GOOD?
>
> The application is going to check is it GOOD or not. Not the driver,
> Right? My concern was, If application starts dropping the packet instead checking the BAD, if
> it checks == !GOOD.
Got it, but when 2 bits state introduced, app should check if check == BAD for
drop decision, because it is not GOOD || BAD anymore.
>
>>
>> 0x0 => UNKNOWN
>> 0x1 => BAD
>> 0x2 => GOOD
>> 0x3 => ? (invalid perhaps)
>>
>> HW that supports detecting good packets can set BAD || GOOD state, HW can detect
>> only BAD packet can set BAD || UNKNOWN state.
>>
>> If BAD is not set, there is an ambiguity of state, lets clarify it in lower
>> level, if it is UNKNOWN, let application know it is UNKNOWN.
>
> OK.
>
> How about the following then?
>
> /**
> * Mask of bits used to determine the status of outer RX L4 checksum.
> * - PKT_RX_EL4_CKSUM_UNKNOWN: no information about the outer RX L4 checksum
> * - PKT_RX_EL4_CKSUM_BAD: the outer L4 checksum in the packet is wrong
> * - PKT_RX_EL4_CKSUM_GOOD: the outer L4 checksum in the packet is valid
> * - PKT_RX_EL4_CKSUM_INVALID: invalid outer L4 checksum state.
> *
> * The detection of PKT_RX_EL4_CKSUM_GOOD shall be based on the given
> * HW capability, At minimum, the PMD should support
> * PKT_RX_EL4_CKSUM_UNKNOWN and PKT_RX_EL4_CKSUM_BAD states
> * if the offload is available.
> */
> #define PKT_RX_EL4_CKSUM_MASK ((1ULL << 21) | (1ULL << 22))
>
> #define PKT_RX_IP_CKSUM_UNKNOWN 0
> #define PKT_RX_IP_CKSUM_BAD (1ULL << 21)
> #define PKT_RX_IP_CKSUM_GOOD (1ULL << 22)
> #define PKT_RX_IP_CKSUM_INVALID ((1ULL << 21) | (1ULL << 22))
Looks good to me.
>
>
>
More information about the dev
mailing list