Hi Helin,
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?
>> + i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
>> + l2_len, l3_len, outer_l2_len,
>> + outer_l3_len,
>> + &cd_tunneling_params);
>> + }
> As this code changes are in fast path, performance regression test is needed.
> I would
> like to see the performance difference with or without this patch set.
> Hopefully nothing
> different. If you need any helps, just let me know.
I'm sorry, I won't have the needed resources to bench this as I
would have to setup a performance platform with i40e devices.
But I'm pretty sure that the code in non-offload case would be faster
with this patch as it will avoid many operations in
i40e_txd_enable_checksum().
For the offload case, as we also removed the if (l2_len == 0)
and if (l3_len == 0), I think there are also less tests than before
my patch series.
So in my opinion, adding this test does not really justify to check the
performance.
Regards,
Olivier
