Re: [dpdk-dev] [PATCH] app/testpmd: fix IP checksum calculation

[George Prekas <prekageo@amazon.com>]

On 12/4/2020 2:59 AM, Ferruh Yigit wrote:
> CAUTION: This email originated from outside of the organization. Do 
> not click links or open attachments unless you can confirm the sender 
> and know the content is safe.
>
>
>
> On 12/3/2020 1:59 PM, George Prekas wrote:
>> Insert a compiler barrier to make sure that the IP checksum calculation
>> happens after setting all the fields of the IP header.
>>
>
> Can you please provide the compiler details, and if there is any specific
> instruction on how to reproduce this failure?

This happens with GCC 9 and GCC 10. It works fine on GCC 8.

Stephen was right that a compiler barrier here is not the right 
solution. After spending some time on it, I realized that it is an 
aliasing problem when casting the IP header to uint16_t*. As far as I 
understand, this is not allowed by the C standard. As far as I know, 
there are 3 ways to fix this problem: Use a union, use memcpy, or set 
the compiler flag -fno-strict-aliasing. I assume that the last option is 
the least intrusive. I've submitted a second version of the patch with it.

Let me know of your opinion.

>
>> Signed-off-by: George Prekas <prekageo@amazon.com>
>> ---
>>   app/test-pmd/flowgen.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
>> index acf3e2460..893b4b0b8 100644
>> --- a/app/test-pmd/flowgen.c
>> +++ b/app/test-pmd/flowgen.c
>> @@ -150,6 +150,7 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
>> next_flow);
>>               ip_hdr->total_length    = RTE_CPU_TO_BE_16(pkt_size -
>> sizeof(*eth_hdr));
>> +             rte_compiler_barrier();
>>               ip_hdr->hdr_checksum    = ip_sum((unaligned_uint16_t 
>> *)ip_hdr,
>>                                                sizeof(*ip_hdr));
>>
>>
>