Re: [dpdk-dev] [PATCH v2 03/20] i40e: call i40e_txd_enable_checksum only for offloaded packets

[Olivier MATZ <olivier.matz@6wind.com>]

Hi,

On 02/13/2015 03:25 AM, Zhang, Helin wrote:
>> On 02/11/2015 06:32 AM, Zhang, Helin wrote:
>>>> On 02/10/2015 07:03 AM, Zhang, Helin wrote:
>>>>>>    		/* Enable checksum offloading */
>>>>>>    		cd_tunneling_params = 0;
>>>>>> -		i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
>>>>>> -						l2_len, l3_len, outer_l2_len,
>>>>>> -						outer_l3_len,
>>>>>> -						&cd_tunneling_params);
>>>>>> +		if (ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK) {
>>>>> likely should be added.
>>>>
>>>> I would say unlikely() instead. I think the non-offload case should
>>>> be the default one. What do you think?
>>
>> Maybe you missed this comment. Any thoughts?
> Ohh, sorry for the missing!
> I'd prefer to have likely, as hardware offload is preferred if there is. If you
> don't think so, how about to keep nothing (no likely/unlikely) as it is.

OK, I'll use likely() as you initially suggested.

>>> As 40G is quite sensitive on cpu cycles, we'd better to avoid any
>>> performance drop during our modifying the code for fast path.
>>> Performance is what we care about too much. Based on my experiences,
>>> even minor code changes may result in big performance impact.
>>> It seems that we may need to help you on performance measurement.
>>
>> Thanks, indeed it's helpful if you can check performance non-regression.
> I have asked our validation guys here to help you on that, but might not in
> high priority. In addition, we all will take vocation for the coming Chinese new year.

OK, it's noted


Thanks,
Olivier