Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-14 Thread Ion Grigore 
Hi Ilya,

I checked the changes you suggested. It's Ok.  I sent a new patch(v7).

Tkanks,
Grig

-Original Message-
From: Grigore Ion-B17953 
Sent: Friday, December 11, 2015 5:57 PM
To: 'Ilya Maximets' ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: RE: [PATCHv6] helper : Fix UDP checksum computation

Ok. I'll check your proposal on BE/LE.

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:54 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:47, Ion Grigore wrote:
> A more precise question. Did you check on a LE platform ?

Yes. On my LE i7.

> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0x) + (sum >> 16);
> -
> + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> + u8_pad_to_u16(iph->proto, 1) + udph->length;
 I meant something like:

 union {
uint8_t v8[2];
uint16_t v16;
 } val;

 val.v8[0] = 0;
 val.v8[1] = iph->proto;

>>>
>>> The val.val16 must be swapped on LE platforms. 
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
>>
>>>
>>>
 sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
val.v16 + udph->length;

> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
> + for ( ; udplen > 1; udplen -= 2)
> + sum += *buf++;
> + /* Length is not a multiple of 2 bytes */
> + if (udplen)
> + sum += u8_pad_to_u16(*buf, 0);
 val.v8[0] = *buf;
 val.v8[1] = 0;

 sum += val.v16;

 Isn't it more understandable, then artificial byte swapping?

>>>
>>> The val.val16 must be swapped on LE platforms.
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
> P.S. Just to be honest, I checked this a few minutes ago.
>  Helper test below works.
> 
> Yes on BE/LE platforms, but not with the change you proposed.


So why you talking that val.val16 must be swapped if you do not understand how 
it works and you have not tested it?
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets 
On 11.12.2015 12:19, Maxim Uvarov wrote:
> On 12/11/2015 10:39, Ilya Maximets wrote:
>> Comments inlined.
>>
>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>>> From: Grigore Ion 
>>>
>>> This patch fixes the following problems:
>>> - checksum computation for LE platforms
>>> - checksum is computed in the CPU endianness. The returned result
>>> must be converted to the BE ordering when it is used to update
>>> the UDP checksum in a packet.
>>> - checksum computation for packets having the UDP length not a
>>> multiple of 2
>>> - fixes the UDP checksum associated validation test
>>>
>>> Signed-off-by: Grigore Ion 
>>> ---
>>>   v6:
>>>   - Make code more understandable (Ilya Maximets)
>>>   v5:
>>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>>   v4:
>>>   - Verify checksum in CPU endianness in the associated test
>>>   (Ilya Maximets)
>>>   v3:
>>>   - fix the UDP checksum computation in the associated test
>>>   (Maxim Uvarov)
>>>   v2:
>>>   - patch updated to the last master (Maxim Uvarov)
>>>   v1:
>>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>>
>>>   helper/include/odp/helper/udp.h | 81 
>>> +
>>>   helper/test/odp_chksum.c|  4 +-
>>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/udp.h 
>>> b/helper/include/odp/helper/udp.h
>>> index 06c439b..d0599b1 100644
>>> --- a/helper/include/odp/helper/udp.h
>>> +++ b/helper/include/odp/helper/udp.h
>>> @@ -4,7 +4,6 @@
>>>* SPDX-License-Identifier: BSD-3-Clause
>>>*/
>>>   -
>>>   /**
>>>* @file
>>>*
>>> @@ -22,7 +21,6 @@ extern "C" {
>>>   #include 
>>>   #include 
>>>   -
>>>   /** @addtogroup odph_header ODPH HEADER
>>>*  @{
>>>*/
>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>>   } odph_udphdr_t;
>>> /**
>>> + * Perform byte swap required by UDP checksum computation algorithm
>>> + */
>>> +static inline uint16_t csum_bswap(uint16_t val)
>>> +{
>>> +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val);
>>> +#endif
>>> +return val;
>>> +}
>> Please, don't duplicate the code.
>>
>>> +
>>> +/**
>>> + * Pad a byte value to the left or to the right as required by UDP checksum
>>> + * computation algorithm and convert the result to CPU native uint16_t.
>>> + * Left padding is performed for the IP protocol field in the UDP
>>> + * pseudo-header (RFC 768). Right padding is performed in the case of the 
>>> odd
>>> + * byte in a UDP packet having the length not a 2 multiple.
>>> + */
>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left)
>>> +{
>>> +uint16_tret;
>>> +
>>> +ret = (left) ? val : val << 8;
>>> +return csum_bswap(ret);
>>> +}
>>> +
>>> +/**
>>>* UDP checksum
>>>*
>>>* This function uses odp packet to calc checksum
>>>*
>>>* @param pkt  calculate chksum for pkt
>>> - * @return  checksum value
>>> + * @return  checksum value in CPU endianness
>>>*/
>>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>>   {
>>> -uint32_t sum = 0;
>>> -odph_udphdr_t *udph;
>>> -odph_ipv4hdr_t *iph;
>>> -uint16_t udplen;
>>> -uint8_t *buf;
>>> -
>>> -if (!odp_packet_l3_offset(pkt))
>>> -return 0;
>>> +odph_ipv4hdr_t*iph;
>>> +odph_udphdr_t*udph;
>>> +uint32_tsum;
>>> +uint16_tudplen, *buf;
>>>   -if (!odp_packet_l4_offset(pkt))
>>> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>>   return 0;
>>> -
>>>   iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>>   udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>> -udplen = odp_be_to_cpu_16(udph->length);
>>> -
>>> -/* 32-bit sum of all 16-bit words covered by UDP chksum */
>>> +/* 32-bit sum of UDP pseudo-header */
>>>   sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>>> -  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>>> -  (uint16_t)iph->proto + udplen;
>>> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
>>> -sum += ((*buf << 8) + *(buf + 1));
>>> -buf += 2;
>>> -}
>>> -
>>> -/* Fold sum to 16 bits: add carrier to result */
>>> -while (sum >> 16)
>>> -sum = (sum & 0x) + (sum >> 16);
>>> -
>>> +(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>>> +u8_pad_to_u16(iph->proto, 1) + udph->length;
>> I meant something like:
>>
>> union {
>> uint8_t v8[2];
>> uint16_t v16;
>> } val;
>>
>> val.v8[0] = 0;
>> val.v8[1] = iph->proto;
>>
>> sum = (iph->src_addr &

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ola Liljedahl 
Something else but related to the reliable computation of checksums.

If you look how odp_chksum() is used e.g. in the packet generator example
(pack_icmp_pkt function), there is a lot of pointer casting and writes to
the resulting IPv4 and ICMP structures. Then we call odp_chksum() (which is
likely inlined) which casts the base pointer again and reads from it. Since
pointers to different types cannot alias in C (they cannot point to the
same memory), the compiler is free to rearrange memory accesses and indeed
read from memory (in odp_chksum) before the data has been written. I think
I saw this cause problems earlier. What saves us now is probably the call
to gettimeofday() before calling odp_chksum() because the compiler will not
reorder memory accesses over unknown function calls (the memcpy after might
get inlined so I don't see it as a barrier). If the call to gettimeofday()
would be moved after the computation (or removed), I believe we could have
problems.

The proper solution is to use a union of all relevant data structures and
perform all memory accesses through this union. From a compiler perspective
we are only using one pointer to one type so it will know that all memory
accesses are related and it can understand the dependencies. But we can't
trust the caller of odp_chksum() to do that. Instead odp_chksum() should
include a compiler memory barrier which ensures that the compiler will not
reorder memory accesses (in either direction) over this barrier. It is the
inlining of odp_chksum() that enables the problem, if odp_chksum() was not
inlined, the compiler would not be able to reorder memory accesses over the
function call.

static inline uint16sum_t odp_chksum(void *buffer, int len)

{

uint16_t *buf = buffer;

__asm __volatile("" ::: "memory");
/* Now 'buf' can be safely dereferenced */

An alternative could be to define a macro for these type-unsafe casts that
include the compiler barrier. I am not sure how such a macro would look
like.

On 11 December 2015 at 10:47, Ilya Maximets  wrote:

> On 11.12.2015 12:19, Maxim Uvarov wrote:
> > On 12/11/2015 10:39, Ilya Maximets wrote:
> >> Comments inlined.
> >>
> >> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
> >>> From: Grigore Ion 
> >>>
> >>> This patch fixes the following problems:
> >>> - checksum computation for LE platforms
> >>> - checksum is computed in the CPU endianness. The returned result
> >>> must be converted to the BE ordering when it is used to update
> >>> the UDP checksum in a packet.
> >>> - checksum computation for packets having the UDP length not a
> >>> multiple of 2
> >>> - fixes the UDP checksum associated validation test
> >>>
> >>> Signed-off-by: Grigore Ion 
> >>> ---
> >>>   v6:
> >>>   - Make code more understandable (Ilya Maximets)
> >>>   v5:
> >>>   - Checksum in CPU endianness fix added (Ilya Maximets)
> >>>   v4:
> >>>   - Verify checksum in CPU endianness in the associated test
> >>>   (Ilya Maximets)
> >>>   v3:
> >>>   - fix the UDP checksum computation in the associated test
> >>>   (Maxim Uvarov)
> >>>   v2:
> >>>   - patch updated to the last master (Maxim Uvarov)
> >>>   v1:
> >>>   - Move variables declaration on top of block. (Maxim Uvarov)
> >>>   - Check patch with checkpatch script.  (Maxim Uvarov)
> >>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
> >>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
> >>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
> >>>
> >>>   helper/include/odp/helper/udp.h | 81
> +
> >>>   helper/test/odp_chksum.c|  4 +-
> >>>   2 files changed, 51 insertions(+), 34 deletions(-)
> >>>
> >>> diff --git a/helper/include/odp/helper/udp.h
> b/helper/include/odp/helper/udp.h
> >>> index 06c439b..d0599b1 100644
> >>> --- a/helper/include/odp/helper/udp.h
> >>> +++ b/helper/include/odp/helper/udp.h
> >>> @@ -4,7 +4,6 @@
> >>>* SPDX-License-Identifier: BSD-3-Clause
> >>>*/
> >>>   -
> >>>   /**
> >>>* @file
> >>>*
> >>> @@ -22,7 +21,6 @@ extern "C" {
> >>>   #include 
> >>>   #include 
> >>>   -
> >>>   /** @addtogroup odph_header ODPH HEADER
> >>>*  @{
> >>>*/
> >>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
> >>>   } odph_udphdr_t;
> >>> /**
> >>> + * Perform byte swap required by UDP checksum computation algorithm
> >>> + */
> >>> +static inline uint16_t csum_bswap(uint16_t val)
> >>> +{
> >>> +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> >>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val);
> >>> +#endif
> >>> +return val;
> >>> +}
> >> Please, don't duplicate the code.
> >>
> >>> +
> >>> +/**
> >>> + * Pad a byte value to the left or to the right as required by UDP
> checksum
> >>> + * computation algorithm and convert the result to CPU native
> uint16_t.
> >>> + * Left padding is performed for the IP protocol

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore 
Comments inlined

-Original Message-
From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] 
Sent: Friday, December 11, 2015 11:20 AM
To: Ilya Maximets ; Grigore Ion-B17953 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 12/11/2015 10:39, Ilya Maximets wrote:
> Comments inlined.
>
> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>> From: Grigore Ion 
>>
>> This patch fixes the following problems:
>> - checksum computation for LE platforms
>> - checksum is computed in the CPU endianness. The returned result 
>> must be converted to the BE ordering when it is used to update the 
>> UDP checksum in a packet.
>> - checksum computation for packets having the UDP length not a 
>> multiple of 2
>> - fixes the UDP checksum associated validation test
>>
>> Signed-off-by: Grigore Ion 
>> ---
>>   v6:
>>   - Make code more understandable (Ilya Maximets)
>>   v5:
>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>   v4:
>>   - Verify checksum in CPU endianness in the associated test
>>   (Ilya Maximets)
>>   v3:
>>   - fix the UDP checksum computation in the associated test
>>   (Maxim Uvarov)
>>   v2:
>>   - patch updated to the last master (Maxim Uvarov)
>>   v1:
>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>
>>   helper/include/odp/helper/udp.h | 81 
>> +
>>   helper/test/odp_chksum.c|  4 +-
>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>
>> diff --git a/helper/include/odp/helper/udp.h 
>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
>> --- a/helper/include/odp/helper/udp.h
>> +++ b/helper/include/odp/helper/udp.h
>> @@ -4,7 +4,6 @@
>>* SPDX-License-Identifier: BSD-3-Clause
>>*/
>>   
>> -
>>   /**
>>* @file
>>*
>> @@ -22,7 +21,6 @@ extern "C" {
>>   #include 
>>   #include 
>>   
>> -
>>   /** @addtogroup odph_header ODPH HEADER
>>*  @{
>>*/
>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>   } odph_udphdr_t;
>>   
>>   /**
>> + * Perform byte swap required by UDP checksum computation algorithm  
>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
>> +return val;
>> +}
> Please, don't duplicate the code.
>
>> +
>> +/**
>> + * Pad a byte value to the left or to the right as required by UDP 
>> +checksum
>> + * computation algorithm and convert the result to CPU native uint16_t.
>> + * Left padding is performed for the IP protocol field in the UDP
>> + * pseudo-header (RFC 768). Right padding is performed in the case 
>> +of the odd
>> + * byte in a UDP packet having the length not a 2 multiple.
>> + */
>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
>> +uint16_tret;
>> +
>> +ret = (left) ? val : val << 8;
>> +return csum_bswap(ret);
>> +}
>> +
>> +/**
>>* UDP checksum
>>*
>>* This function uses odp packet to calc checksum
>>*
>>* @param pkt  calculate chksum for pkt
>> - * @return  checksum value
>> + * @return  checksum value in CPU endianness
>>*/
>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>   {
>> -uint32_t sum = 0;
>> -odph_udphdr_t *udph;
>> -odph_ipv4hdr_t *iph;
>> -uint16_t udplen;
>> -uint8_t *buf;
>> -
>> -if (!odp_packet_l3_offset(pkt))
>> -return 0;
>> +odph_ipv4hdr_t  *iph;
>> +odph_udphdr_t   *udph;
>> +uint32_tsum;
>> +uint16_tudplen, *buf;
>>   
>> -if (!odp_packet_l4_offset(pkt))
>> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>  return 0;
>> -
>>  iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>  udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>> -udplen = odp_be_to_cpu_16(udph->length);
>> -
>> -/* 32-bit sum of all 16-bit words covered by UDP chksum */
>> +/* 32-bit sum of UDP pseudo-header */
>>  sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>> -  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>> -  (uint16_t)iph->proto + udplen;
>> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
>> -sum += ((*buf << 8) + *(buf + 1));
>> -buf += 2;
>> -}
>> -
>> -/* Fold sum to 16 bits: add carrier to result */
>> -while (sum >> 16)
>> -sum = (sum & 0x) + (sum >> 16);
>> -
>> +(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>> +u8_pad_to_u16(iph->proto, 1) +

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ola Liljedahl 
On 11 December 2015 at 15:07, Ion Grigore  wrote:

> Comments inlined
>
> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
> Sent: Friday, December 11, 2015 11:20 AM
> To: Ilya Maximets ; Grigore Ion-B17953 <
> ion.grig...@freescale.com>; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>
> On 12/11/2015 10:39, Ilya Maximets wrote:
> > Comments inlined.
> >
> > On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
> >> From: Grigore Ion 
> >>
> >> This patch fixes the following problems:
> >> - checksum computation for LE platforms
> >> - checksum is computed in the CPU endianness. The returned result
> >> must be converted to the BE ordering when it is used to update the
> >> UDP checksum in a packet.
> >> - checksum computation for packets having the UDP length not a
> >> multiple of 2
> >> - fixes the UDP checksum associated validation test
> >>
> >> Signed-off-by: Grigore Ion 
> >> ---
> >>   v6:
> >>   - Make code more understandable (Ilya Maximets)
> >>   v5:
> >>   - Checksum in CPU endianness fix added (Ilya Maximets)
> >>   v4:
> >>   - Verify checksum in CPU endianness in the associated test
> >>   (Ilya Maximets)
> >>   v3:
> >>   - fix the UDP checksum computation in the associated test
> >>   (Maxim Uvarov)
> >>   v2:
> >>   - patch updated to the last master (Maxim Uvarov)
> >>   v1:
> >>   - Move variables declaration on top of block. (Maxim Uvarov)
> >>   - Check patch with checkpatch script.  (Maxim Uvarov)
> >>   - L3 header presence is tested twice. (Alexandru Badicioiu)
> >>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
> >>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
> >>
> >>   helper/include/odp/helper/udp.h | 81
> +
> >>   helper/test/odp_chksum.c|  4 +-
> >>   2 files changed, 51 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/helper/include/odp/helper/udp.h
> >> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
> >> --- a/helper/include/odp/helper/udp.h
> >> +++ b/helper/include/odp/helper/udp.h
> >> @@ -4,7 +4,6 @@
> >>* SPDX-License-Identifier: BSD-3-Clause
> >>*/
> >>
> >> -
> >>   /**
> >>* @file
> >>*
> >> @@ -22,7 +21,6 @@ extern "C" {
> >>   #include 
> >>   #include 
> >>
> >> -
> >>   /** @addtogroup odph_header ODPH HEADER
> >>*  @{
> >>*/
> >> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
> >>   } odph_udphdr_t;
> >>
> >>   /**
> >> + * Perform byte swap required by UDP checksum computation algorithm
> >> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if
> >> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> >> +val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
> >> +return val;
> >> +}
> > Please, don't duplicate the code.
> >
> >> +
> >> +/**
> >> + * Pad a byte value to the left or to the right as required by UDP
> >> +checksum
> >> + * computation algorithm and convert the result to CPU native uint16_t.
> >> + * Left padding is performed for the IP protocol field in the UDP
> >> + * pseudo-header (RFC 768). Right padding is performed in the case
> >> +of the odd
> >> + * byte in a UDP packet having the length not a 2 multiple.
> >> + */
> >> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
> >> +uint16_tret;
> >> +
> >> +ret = (left) ? val : val << 8;
> >> +return csum_bswap(ret);
> >> +}
> >> +
> >> +/**
> >>* UDP checksum
> >>*
> >>* This function uses odp packet to calc checksum
> >>*
> >>* @param pkt  calculate chksum for pkt
> >> - * @return  checksum value
> >> + * @return  checksum value in CPU endianness
> >>*/
> >>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
> >>   {
> >> -uint32_t sum = 0;
> >> -odph_udphdr_t *udph;
> >> -odph_ipv4hdr_t *iph;
> >> -uint16_t udplen;
> >> -uint8_t *buf;
> >> -
> >> -if (!odp_packet_l3_offset(pkt))
> >> -return 0;
> >> +odph_ipv4hdr_t  *iph;
> >> +odph_udphdr_t   *udph;
> >> +uint32_tsum;
> >> +uint16_tudplen, *buf;
> >>
> >> -if (!odp_packet_l4_offset(pkt))
> >> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
> >>  return 0;
> >> -
> >>  iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
> >>  udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
> >> -udplen = odp_be_to_cpu_16(udph->length);
> >> -
> >> -/* 32-bit sum of all 16-bit words covered by UDP chksum */
> >> +/* 32-bit sum of UDP pseudo-header */
> >>  sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
> >> -  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> >> -  (uint16_t)iph->proto + udplen;
> >> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
> >> -sum += ((*buf << 8)

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets 
On 11.12.2015 18:28, Ion Grigore wrote:
> 
> Comments inlined
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
> Sent: Friday, December 11, 2015 5:24 PM
> To: Grigore Ion-B17953 ; Maxim Uvarov 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 11.12.2015 17:07, Ion Grigore wrote:
>> Comments inlined
>>
>> -Original Message-
>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
>> Sent: Friday, December 11, 2015 11:20 AM
>> To: Ilya Maximets ; Grigore Ion-B17953 
>> ; lng-odp@lists.linaro.org
>> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>>
>> On 12/11/2015 10:39, Ilya Maximets wrote:
>>> Comments inlined.
>>>
>>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
 From: Grigore Ion 

 This patch fixes the following problems:
 - checksum computation for LE platforms
 - checksum is computed in the CPU endianness. The returned result 
 must be converted to the BE ordering when it is used to update the 
 UDP checksum in a packet.
 - checksum computation for packets having the UDP length not a 
 multiple of 2
 - fixes the UDP checksum associated validation test

 Signed-off-by: Grigore Ion 
 ---
   v6:
   - Make code more understandable (Ilya Maximets)
   v5:
   - Checksum in CPU endianness fix added (Ilya Maximets)
   v4:
   - Verify checksum in CPU endianness in the associated test
   (Ilya Maximets)
   v3:
   - fix the UDP checksum computation in the associated test
   (Maxim Uvarov)
   v2:
   - patch updated to the last master (Maxim Uvarov)
   v1:
   - Move variables declaration on top of block. (Maxim Uvarov)
   - Check patch with checkpatch script.  (Maxim Uvarov)
   - L3 header presence is tested twice. (Alexandru Badicioiu)
   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

   helper/include/odp/helper/udp.h | 81 
 +
   helper/test/odp_chksum.c|  4 +-
   2 files changed, 51 insertions(+), 34 deletions(-)

 diff --git a/helper/include/odp/helper/udp.h 
 b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
 --- a/helper/include/odp/helper/udp.h
 +++ b/helper/include/odp/helper/udp.h
 @@ -4,7 +4,6 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
   
 -
   /**
* @file
*
 @@ -22,7 +21,6 @@ extern "C" {
   #include 
   #include 
   
 -
   /** @addtogroup odph_header ODPH HEADER
*  @{
*/
 @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
   } odph_udphdr_t;
   
   /**
 + * Perform byte swap required by UDP checksum computation algorithm 
 +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
 +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
 +  val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
 +  return val;
 +}
>>> Please, don't duplicate the code.
>>>
 +
 +/**
 + * Pad a byte value to the left or to the right as required by UDP 
 +checksum
 + * computation algorithm and convert the result to CPU native uint16_t.
 + * Left padding is performed for the IP protocol field in the UDP
 + * pseudo-header (RFC 768). Right padding is performed in the case 
 +of the odd
 + * byte in a UDP packet having the length not a 2 multiple.
 + */
 +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
 +  uint16_tret;
 +
 +  ret = (left) ? val : val << 8;
 +  return csum_bswap(ret);
 +}
 +
 +/**
* UDP checksum
*
* This function uses odp packet to calc checksum
*
* @param pkt  calculate chksum for pkt
 - * @return  checksum value
 + * @return  checksum value in CPU endianness
*/
   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
   {
 -  uint32_t sum = 0;
 -  odph_udphdr_t *udph;
 -  odph_ipv4hdr_t *iph;
 -  uint16_t udplen;
 -  uint8_t *buf;
 -
 -  if (!odp_packet_l3_offset(pkt))
 -  return 0;
 +  odph_ipv4hdr_t  *iph;
 +  odph_udphdr_t   *udph;
 +  uint32_tsum;
 +  uint16_tudplen, *buf;
   
 -  if (!odp_packet_l4_offset(pkt))
 +  if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
return 0;
 -
iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
 -  udplen = odp_be_to_cpu_16(udph->length);
 -
 -  /* 32-bit sum of all 16-bit words covered by UDP chksum */

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets 
On 11.12.2015 18:47, Ion Grigore wrote:
> A more precise question. Did you check on a LE platform ?

Yes. On my LE i7.

> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0x) + (sum >> 16);
> -
> + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> + u8_pad_to_u16(iph->proto, 1) + udph->length;
 I meant something like:

 union {
uint8_t v8[2];
uint16_t v16;
 } val;

 val.v8[0] = 0;
 val.v8[1] = iph->proto;

>>>
>>> The val.val16 must be swapped on LE platforms. 
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
>>
>>>
>>>
 sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
val.v16 + udph->length;

> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
> + for ( ; udplen > 1; udplen -= 2)
> + sum += *buf++;
> + /* Length is not a multiple of 2 bytes */
> + if (udplen)
> + sum += u8_pad_to_u16(*buf, 0);
 val.v8[0] = *buf;
 val.v8[1] = 0;

 sum += val.v16;

 Isn't it more understandable, then artificial byte swapping?

>>>
>>> The val.val16 must be swapped on LE platforms.
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
> P.S. Just to be honest, I checked this a few minutes ago.
>  Helper test below works.
> 
> Yes on BE/LE platforms, but not with the change you proposed.


So why you talking that val.val16 must be swapped if you do not understand
how it works and you have not tested it?
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore 
Comments inlined

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:40 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:28, Ion Grigore wrote:
> 
> Comments inlined
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, December 11, 2015 5:24 PM
> To: Grigore Ion-B17953 ; Maxim Uvarov 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 11.12.2015 17:07, Ion Grigore wrote:
>> Comments inlined
>>
>> -Original Message-
>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
>> Sent: Friday, December 11, 2015 11:20 AM
>> To: Ilya Maximets ; Grigore Ion-B17953 
>> ; lng-odp@lists.linaro.org
>> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>>
>> On 12/11/2015 10:39, Ilya Maximets wrote:
>>> Comments inlined.
>>>
>>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
 From: Grigore Ion 

 This patch fixes the following problems:
 - checksum computation for LE platforms
 - checksum is computed in the CPU endianness. The returned result 
 must be converted to the BE ordering when it is used to update the 
 UDP checksum in a packet.
 - checksum computation for packets having the UDP length not a 
 multiple of 2
 - fixes the UDP checksum associated validation test

 Signed-off-by: Grigore Ion 
 ---
   v6:
   - Make code more understandable (Ilya Maximets)
   v5:
   - Checksum in CPU endianness fix added (Ilya Maximets)
   v4:
   - Verify checksum in CPU endianness in the associated test
   (Ilya Maximets)
   v3:
   - fix the UDP checksum computation in the associated test
   (Maxim Uvarov)
   v2:
   - patch updated to the last master (Maxim Uvarov)
   v1:
   - Move variables declaration on top of block. (Maxim Uvarov)
   - Check patch with checkpatch script.  (Maxim Uvarov)
   - L3 header presence is tested twice. (Alexandru Badicioiu)
   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

   helper/include/odp/helper/udp.h | 81 
 +
   helper/test/odp_chksum.c|  4 +-
   2 files changed, 51 insertions(+), 34 deletions(-)

 diff --git a/helper/include/odp/helper/udp.h 
 b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
 --- a/helper/include/odp/helper/udp.h
 +++ b/helper/include/odp/helper/udp.h
 @@ -4,7 +4,6 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
   
 -
   /**
* @file
*
 @@ -22,7 +21,6 @@ extern "C" {
   #include 
   #include 
   
 -
   /** @addtogroup odph_header ODPH HEADER
*  @{
*/
 @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
   } odph_udphdr_t;
   
   /**
 + * Perform byte swap required by UDP checksum computation 
 +algorithm */ static inline uint16_t csum_bswap(uint16_t val) { #if 
 +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
 +  val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
 +  return val;
 +}
>>> Please, don't duplicate the code.
>>>
 +
 +/**
 + * Pad a byte value to the left or to the right as required by UDP 
 +checksum
 + * computation algorithm and convert the result to CPU native uint16_t.
 + * Left padding is performed for the IP protocol field in the UDP
 + * pseudo-header (RFC 768). Right padding is performed in the case 
 +of the odd
 + * byte in a UDP packet having the length not a 2 multiple.
 + */
 +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
 +  uint16_tret;
 +
 +  ret = (left) ? val : val << 8;
 +  return csum_bswap(ret);
 +}
 +
 +/**
* UDP checksum
*
* This function uses odp packet to calc checksum
*
* @param pkt  calculate chksum for pkt
 - * @return  checksum value
 + * @return  checksum value in CPU endianness
*/
   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
   {
 -  uint32_t sum = 0;
 -  odph_udphdr_t *udph;
 -  odph_ipv4hdr_t *iph;
 -  uint16_t udplen;
 -  uint8_t *buf;
 -
 -  if (!odp_packet_l3_offset(pkt))
 -  return 0;
 +  odph_ipv4hdr_t  *iph;
 +  odph_udphdr_t   *udph;
 +  uint32_tsum;
 +  uint16_tudplen, *buf;
   
 -  if (!odp_packet_l4_offset(pkt))
 +  if

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore 
A more precise question. Did you check on a LE platform ?

-Original Message-
From: Grigore Ion-B17953 
Sent: Friday, December 11, 2015 5:44 PM
To: 'Ilya Maximets' ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: RE: [PATCHv6] helper : Fix UDP checksum computation

Comments inlined

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Friday, December 11, 2015 5:40 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:28, Ion Grigore wrote:
> 
> Comments inlined
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, December 11, 2015 5:24 PM
> To: Grigore Ion-B17953 ; Maxim Uvarov 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 11.12.2015 17:07, Ion Grigore wrote:
>> Comments inlined
>>
>> -Original Message-
>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
>> Sent: Friday, December 11, 2015 11:20 AM
>> To: Ilya Maximets ; Grigore Ion-B17953 
>> ; lng-odp@lists.linaro.org
>> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>>
>> On 12/11/2015 10:39, Ilya Maximets wrote:
>>> Comments inlined.
>>>
>>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
 From: Grigore Ion 

 This patch fixes the following problems:
 - checksum computation for LE platforms
 - checksum is computed in the CPU endianness. The returned result 
 must be converted to the BE ordering when it is used to update the 
 UDP checksum in a packet.
 - checksum computation for packets having the UDP length not a 
 multiple of 2
 - fixes the UDP checksum associated validation test

 Signed-off-by: Grigore Ion 
 ---
   v6:
   - Make code more understandable (Ilya Maximets)
   v5:
   - Checksum in CPU endianness fix added (Ilya Maximets)
   v4:
   - Verify checksum in CPU endianness in the associated test
   (Ilya Maximets)
   v3:
   - fix the UDP checksum computation in the associated test
   (Maxim Uvarov)
   v2:
   - patch updated to the last master (Maxim Uvarov)
   v1:
   - Move variables declaration on top of block. (Maxim Uvarov)
   - Check patch with checkpatch script.  (Maxim Uvarov)
   - L3 header presence is tested twice. (Alexandru Badicioiu)
   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

   helper/include/odp/helper/udp.h | 81 
 +
   helper/test/odp_chksum.c|  4 +-
   2 files changed, 51 insertions(+), 34 deletions(-)

 diff --git a/helper/include/odp/helper/udp.h 
 b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
 --- a/helper/include/odp/helper/udp.h
 +++ b/helper/include/odp/helper/udp.h
 @@ -4,7 +4,6 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
   
 -
   /**
* @file
*
 @@ -22,7 +21,6 @@ extern "C" {
   #include 
   #include 
   
 -
   /** @addtogroup odph_header ODPH HEADER
*  @{
*/
 @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
   } odph_udphdr_t;
   
   /**
 + * Perform byte swap required by UDP checksum computation 
 +algorithm */ static inline uint16_t csum_bswap(uint16_t val) { #if 
 +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
 +  val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
 +  return val;
 +}
>>> Please, don't duplicate the code.
>>>
 +
 +/**
 + * Pad a byte value to the left or to the right as required by UDP 
 +checksum
 + * computation algorithm and convert the result to CPU native uint16_t.
 + * Left padding is performed for the IP protocol field in the UDP
 + * pseudo-header (RFC 768). Right padding is performed in the case 
 +of the odd
 + * byte in a UDP packet having the length not a 2 multiple.
 + */
 +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
 +  uint16_tret;
 +
 +  ret = (left) ? val : val << 8;
 +  return csum_bswap(ret);
 +}
 +
 +/**
* UDP checksum
*
* This function uses odp packet to calc checksum
*
* @param pkt  calculate chksum for pkt
 - * @return  checksum value
 + * @return  checksum value in CPU endianness
*/
   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
   {
 -  uint32_t sum = 0;
 -  odph_udphdr_t *udph;
 -  odph_ipv4hdr_t *iph;

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets 
On 11.12.2015 17:07, Ion Grigore wrote:
> Comments inlined
> 
> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] 
> Sent: Friday, December 11, 2015 11:20 AM
> To: Ilya Maximets ; Grigore Ion-B17953 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 12/11/2015 10:39, Ilya Maximets wrote:
>> Comments inlined.
>>
>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>>> From: Grigore Ion 
>>>
>>> This patch fixes the following problems:
>>> - checksum computation for LE platforms
>>> - checksum is computed in the CPU endianness. The returned result 
>>> must be converted to the BE ordering when it is used to update the 
>>> UDP checksum in a packet.
>>> - checksum computation for packets having the UDP length not a 
>>> multiple of 2
>>> - fixes the UDP checksum associated validation test
>>>
>>> Signed-off-by: Grigore Ion 
>>> ---
>>>   v6:
>>>   - Make code more understandable (Ilya Maximets)
>>>   v5:
>>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>>   v4:
>>>   - Verify checksum in CPU endianness in the associated test
>>>   (Ilya Maximets)
>>>   v3:
>>>   - fix the UDP checksum computation in the associated test
>>>   (Maxim Uvarov)
>>>   v2:
>>>   - patch updated to the last master (Maxim Uvarov)
>>>   v1:
>>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>>
>>>   helper/include/odp/helper/udp.h | 81 
>>> +
>>>   helper/test/odp_chksum.c|  4 +-
>>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/udp.h 
>>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
>>> --- a/helper/include/odp/helper/udp.h
>>> +++ b/helper/include/odp/helper/udp.h
>>> @@ -4,7 +4,6 @@
>>>* SPDX-License-Identifier: BSD-3-Clause
>>>*/
>>>   
>>> -
>>>   /**
>>>* @file
>>>*
>>> @@ -22,7 +21,6 @@ extern "C" {
>>>   #include 
>>>   #include 
>>>   
>>> -
>>>   /** @addtogroup odph_header ODPH HEADER
>>>*  @{
>>>*/
>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>>   } odph_udphdr_t;
>>>   
>>>   /**
>>> + * Perform byte swap required by UDP checksum computation algorithm  
>>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
>>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +   val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
>>> +   return val;
>>> +}
>> Please, don't duplicate the code.
>>
>>> +
>>> +/**
>>> + * Pad a byte value to the left or to the right as required by UDP 
>>> +checksum
>>> + * computation algorithm and convert the result to CPU native uint16_t.
>>> + * Left padding is performed for the IP protocol field in the UDP
>>> + * pseudo-header (RFC 768). Right padding is performed in the case 
>>> +of the odd
>>> + * byte in a UDP packet having the length not a 2 multiple.
>>> + */
>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
>>> +   uint16_tret;
>>> +
>>> +   ret = (left) ? val : val << 8;
>>> +   return csum_bswap(ret);
>>> +}
>>> +
>>> +/**
>>>* UDP checksum
>>>*
>>>* This function uses odp packet to calc checksum
>>>*
>>>* @param pkt  calculate chksum for pkt
>>> - * @return  checksum value
>>> + * @return  checksum value in CPU endianness
>>>*/
>>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>>   {
>>> -   uint32_t sum = 0;
>>> -   odph_udphdr_t *udph;
>>> -   odph_ipv4hdr_t *iph;
>>> -   uint16_t udplen;
>>> -   uint8_t *buf;
>>> -
>>> -   if (!odp_packet_l3_offset(pkt))
>>> -   return 0;
>>> +   odph_ipv4hdr_t  *iph;
>>> +   odph_udphdr_t   *udph;
>>> +   uint32_tsum;
>>> +   uint16_tudplen, *buf;
>>>   
>>> -   if (!odp_packet_l4_offset(pkt))
>>> +   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>> return 0;
>>> -
>>> iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>> -   udplen = odp_be_to_cpu_16(udph->length);
>>> -
>>> -   /* 32-bit sum of all 16-bit words covered by UDP chksum */
>>> +   /* 32-bit sum of UDP pseudo-header */
>>> sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>>> - (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>>> - (uint16_t)iph->proto + udplen;
>>> -   for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
>>> -   sum += ((*buf << 8) + *(buf + 1));
>>> -   buf += 2;
>>> -   }
>>> -
>>> -   /* Fold sum to 16 bits: add carrier to result */
>>> -   while (sum >> 16)
>>> -   sum = (sum

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore 

Comments inlined

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:24 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 17:07, Ion Grigore wrote:
> Comments inlined
> 
> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
> Sent: Friday, December 11, 2015 11:20 AM
> To: Ilya Maximets ; Grigore Ion-B17953 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 12/11/2015 10:39, Ilya Maximets wrote:
>> Comments inlined.
>>
>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>>> From: Grigore Ion 
>>>
>>> This patch fixes the following problems:
>>> - checksum computation for LE platforms
>>> - checksum is computed in the CPU endianness. The returned result 
>>> must be converted to the BE ordering when it is used to update the 
>>> UDP checksum in a packet.
>>> - checksum computation for packets having the UDP length not a 
>>> multiple of 2
>>> - fixes the UDP checksum associated validation test
>>>
>>> Signed-off-by: Grigore Ion 
>>> ---
>>>   v6:
>>>   - Make code more understandable (Ilya Maximets)
>>>   v5:
>>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>>   v4:
>>>   - Verify checksum in CPU endianness in the associated test
>>>   (Ilya Maximets)
>>>   v3:
>>>   - fix the UDP checksum computation in the associated test
>>>   (Maxim Uvarov)
>>>   v2:
>>>   - patch updated to the last master (Maxim Uvarov)
>>>   v1:
>>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>>
>>>   helper/include/odp/helper/udp.h | 81 
>>> +
>>>   helper/test/odp_chksum.c|  4 +-
>>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/udp.h 
>>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
>>> --- a/helper/include/odp/helper/udp.h
>>> +++ b/helper/include/odp/helper/udp.h
>>> @@ -4,7 +4,6 @@
>>>* SPDX-License-Identifier: BSD-3-Clause
>>>*/
>>>   
>>> -
>>>   /**
>>>* @file
>>>*
>>> @@ -22,7 +21,6 @@ extern "C" {
>>>   #include 
>>>   #include 
>>>   
>>> -
>>>   /** @addtogroup odph_header ODPH HEADER
>>>*  @{
>>>*/
>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>>   } odph_udphdr_t;
>>>   
>>>   /**
>>> + * Perform byte swap required by UDP checksum computation algorithm 
>>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
>>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +   val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
>>> +   return val;
>>> +}
>> Please, don't duplicate the code.
>>
>>> +
>>> +/**
>>> + * Pad a byte value to the left or to the right as required by UDP 
>>> +checksum
>>> + * computation algorithm and convert the result to CPU native uint16_t.
>>> + * Left padding is performed for the IP protocol field in the UDP
>>> + * pseudo-header (RFC 768). Right padding is performed in the case 
>>> +of the odd
>>> + * byte in a UDP packet having the length not a 2 multiple.
>>> + */
>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
>>> +   uint16_tret;
>>> +
>>> +   ret = (left) ? val : val << 8;
>>> +   return csum_bswap(ret);
>>> +}
>>> +
>>> +/**
>>>* UDP checksum
>>>*
>>>* This function uses odp packet to calc checksum
>>>*
>>>* @param pkt  calculate chksum for pkt
>>> - * @return  checksum value
>>> + * @return  checksum value in CPU endianness
>>>*/
>>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>>   {
>>> -   uint32_t sum = 0;
>>> -   odph_udphdr_t *udph;
>>> -   odph_ipv4hdr_t *iph;
>>> -   uint16_t udplen;
>>> -   uint8_t *buf;
>>> -
>>> -   if (!odp_packet_l3_offset(pkt))
>>> -   return 0;
>>> +   odph_ipv4hdr_t  *iph;
>>> +   odph_udphdr_t   *udph;
>>> +   uint32_tsum;
>>> +   uint16_tudplen, *buf;
>>>   
>>> -   if (!odp_packet_l4_offset(pkt))
>>> +   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>> return 0;
>>> -
>>> iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>> -   udplen = odp_be_to_cpu_16(udph->length);
>>> -
>>> -   /* 32-bit sum of all 16-bit words covered by UDP chksum */
>>> +   /* 32-bit sum of UDP pseudo-header */
>>> sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>>> - (iph->dst_addr & 0x) + (iph->dst_addr >>

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore 
Ok. I'll check your proposal on BE/LE.

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:54 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:47, Ion Grigore wrote:
> A more precise question. Did you check on a LE platform ?

Yes. On my LE i7.

> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0x) + (sum >> 16);
> -
> + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> + u8_pad_to_u16(iph->proto, 1) + udph->length;
 I meant something like:

 union {
uint8_t v8[2];
uint16_t v16;
 } val;

 val.v8[0] = 0;
 val.v8[1] = iph->proto;

>>>
>>> The val.val16 must be swapped on LE platforms. 
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
>>
>>>
>>>
 sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
val.v16 + udph->length;

> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
> + for ( ; udplen > 1; udplen -= 2)
> + sum += *buf++;
> + /* Length is not a multiple of 2 bytes */
> + if (udplen)
> + sum += u8_pad_to_u16(*buf, 0);
 val.v8[0] = *buf;
 val.v8[1] = 0;

 sum += val.v16;

 Isn't it more understandable, then artificial byte swapping?

>>>
>>> The val.val16 must be swapped on LE platforms.
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
> P.S. Just to be honest, I checked this a few minutes ago.
>  Helper test below works.
> 
> Yes on BE/LE platforms, but not with the change you proposed.


So why you talking that val.val16 must be swapped if you do not understand how 
it works and you have not tested it?
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Maxim Uvarov 

On 12/11/2015 10:39, Ilya Maximets wrote:

Comments inlined.

On 10.12.2015 12:59, ion.grig...@freescale.com wrote:

From: Grigore Ion 

This patch fixes the following problems:
- checksum computation for LE platforms
- checksum is computed in the CPU endianness. The returned result
must be converted to the BE ordering when it is used to update
the UDP checksum in a packet.
- checksum computation for packets having the UDP length not a
multiple of 2
- fixes the UDP checksum associated validation test

Signed-off-by: Grigore Ion 
---
  v6:
  - Make code more understandable (Ilya Maximets)
  v5:
  - Checksum in CPU endianness fix added (Ilya Maximets)
  v4:
  - Verify checksum in CPU endianness in the associated test
  (Ilya Maximets)
  v3:
  - fix the UDP checksum computation in the associated test
  (Maxim Uvarov)
  v2:
  - patch updated to the last master (Maxim Uvarov)
  v1:
  - Move variables declaration on top of block. (Maxim Uvarov)
  - Check patch with checkpatch script.  (Maxim Uvarov)
  - L3 header presence is tested twice. (Alexandru Badicioiu)
  - Remove unnecessary check for L3 header presence. (Bill Fischofer)
  - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

  helper/include/odp/helper/udp.h | 81 +
  helper/test/odp_chksum.c|  4 +-
  2 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
index 06c439b..d0599b1 100644
--- a/helper/include/odp/helper/udp.h
+++ b/helper/include/odp/helper/udp.h
@@ -4,7 +4,6 @@
   * SPDX-License-Identifier: BSD-3-Clause
   */
  
-

  /**
   * @file
   *
@@ -22,7 +21,6 @@ extern "C" {
  #include 
  #include 
  
-

  /** @addtogroup odph_header ODPH HEADER
   *  @{
   */
@@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
  } odph_udphdr_t;
  
  /**

+ * Perform byte swap required by UDP checksum computation algorithm
+ */
+static inline uint16_t csum_bswap(uint16_t val)
+{
+#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+   val = __odp_builtin_bswap16((__odp_force uint16_t)val);
+#endif
+   return val;
+}

Please, don't duplicate the code.


+
+/**
+ * Pad a byte value to the left or to the right as required by UDP checksum
+ * computation algorithm and convert the result to CPU native uint16_t.
+ * Left padding is performed for the IP protocol field in the UDP
+ * pseudo-header (RFC 768). Right padding is performed in the case of the odd
+ * byte in a UDP packet having the length not a 2 multiple.
+ */
+static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left)
+{
+   uint16_tret;
+
+   ret = (left) ? val : val << 8;
+   return csum_bswap(ret);
+}
+
+/**
   * UDP checksum
   *
   * This function uses odp packet to calc checksum
   *
   * @param pkt  calculate chksum for pkt
- * @return  checksum value
+ * @return  checksum value in CPU endianness
   */
  static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
  {
-   uint32_t sum = 0;
-   odph_udphdr_t *udph;
-   odph_ipv4hdr_t *iph;
-   uint16_t udplen;
-   uint8_t *buf;
-
-   if (!odp_packet_l3_offset(pkt))
-   return 0;
+   odph_ipv4hdr_t  *iph;
+   odph_udphdr_t   *udph;
+   uint32_tsum;
+   uint16_tudplen, *buf;
  
-	if (!odp_packet_l4_offset(pkt))

+   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
return 0;
-
iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
-   udplen = odp_be_to_cpu_16(udph->length);
-
-   /* 32-bit sum of all 16-bit words covered by UDP chksum */
+   /* 32-bit sum of UDP pseudo-header */
sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
- (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
- (uint16_t)iph->proto + udplen;
-   for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
-   sum += ((*buf << 8) + *(buf + 1));
-   buf += 2;
-   }
-
-   /* Fold sum to 16 bits: add carrier to result */
-   while (sum >> 16)
-   sum = (sum & 0x) + (sum >> 16);
-
+   (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
+   u8_pad_to_u16(iph->proto, 1) + udph->length;

I meant something like:

union {
uint8_t v8[2];
uint16_t v16;
} val;

val.v8[0] = 0;
val.v8[1] = iph->proto;

sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
   (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
   val.v16 + udph->length;


+   udplen = odp_be_to_cpu_16(udph->length);
+   buf = (uint16_t *)((void *)udph);
+   /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
+   for ( ; udplen > 1; udplen -= 2)
+   sum += *buf++;
+   /* Length is not a multiple of 2 bytes */
+   if (udplen)
+

[lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-10 Thread ion.grigore 
From: Grigore Ion 

This patch fixes the following problems:
- checksum computation for LE platforms
- checksum is computed in the CPU endianness. The returned result
must be converted to the BE ordering when it is used to update
the UDP checksum in a packet.
- checksum computation for packets having the UDP length not a
multiple of 2
- fixes the UDP checksum associated validation test

Signed-off-by: Grigore Ion 
---
 v6:
 - Make code more understandable (Ilya Maximets)
 v5:
 - Checksum in CPU endianness fix added (Ilya Maximets)
 v4:
 - Verify checksum in CPU endianness in the associated test
 (Ilya Maximets)
 v3:
 - fix the UDP checksum computation in the associated test
 (Maxim Uvarov)
 v2:
 - patch updated to the last master (Maxim Uvarov)
 v1:
 - Move variables declaration on top of block. (Maxim Uvarov)
 - Check patch with checkpatch script.  (Maxim Uvarov)
 - L3 header presence is tested twice. (Alexandru Badicioiu)
 - Remove unnecessary check for L3 header presence. (Bill Fischofer)
 - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

 helper/include/odp/helper/udp.h | 81 +
 helper/test/odp_chksum.c|  4 +-
 2 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
index 06c439b..d0599b1 100644
--- a/helper/include/odp/helper/udp.h
+++ b/helper/include/odp/helper/udp.h
@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-
 /**
  * @file
  *
@@ -22,7 +21,6 @@ extern "C" {
 #include 
 #include 
 
-
 /** @addtogroup odph_header ODPH HEADER
  *  @{
  */
@@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
 } odph_udphdr_t;
 
 /**
+ * Perform byte swap required by UDP checksum computation algorithm
+ */
+static inline uint16_t csum_bswap(uint16_t val)
+{
+#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+   val = __odp_builtin_bswap16((__odp_force uint16_t)val);
+#endif
+   return val;
+}
+
+/**
+ * Pad a byte value to the left or to the right as required by UDP checksum
+ * computation algorithm and convert the result to CPU native uint16_t.
+ * Left padding is performed for the IP protocol field in the UDP
+ * pseudo-header (RFC 768). Right padding is performed in the case of the odd
+ * byte in a UDP packet having the length not a 2 multiple.
+ */
+static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left)
+{
+   uint16_tret;
+
+   ret = (left) ? val : val << 8;
+   return csum_bswap(ret);
+}
+
+/**
  * UDP checksum
  *
  * This function uses odp packet to calc checksum
  *
  * @param pkt  calculate chksum for pkt
- * @return  checksum value
+ * @return  checksum value in CPU endianness
  */
 static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
 {
-   uint32_t sum = 0;
-   odph_udphdr_t *udph;
-   odph_ipv4hdr_t *iph;
-   uint16_t udplen;
-   uint8_t *buf;
-
-   if (!odp_packet_l3_offset(pkt))
-   return 0;
+   odph_ipv4hdr_t  *iph;
+   odph_udphdr_t   *udph;
+   uint32_tsum;
+   uint16_tudplen, *buf;
 
-   if (!odp_packet_l4_offset(pkt))
+   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
return 0;
-
iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
-   udplen = odp_be_to_cpu_16(udph->length);
-
-   /* 32-bit sum of all 16-bit words covered by UDP chksum */
+   /* 32-bit sum of UDP pseudo-header */
sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
- (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
- (uint16_t)iph->proto + udplen;
-   for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
-   sum += ((*buf << 8) + *(buf + 1));
-   buf += 2;
-   }
-
-   /* Fold sum to 16 bits: add carrier to result */
-   while (sum >> 16)
-   sum = (sum & 0x) + (sum >> 16);
-
+   (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
+   u8_pad_to_u16(iph->proto, 1) + udph->length;
+   udplen = odp_be_to_cpu_16(udph->length);
+   buf = (uint16_t *)((void *)udph);
+   /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
+   for ( ; udplen > 1; udplen -= 2)
+   sum += *buf++;
+   /* Length is not a multiple of 2 bytes */
+   if (udplen)
+   sum += u8_pad_to_u16(*buf, 0);
+   /* Fold sum to 16 bits */
+   sum = (sum & 0x) + (sum >> 16);
+   /* Add carrier (0/1) to result */
+   sum += (sum >> 16);
/* 1's complement */
sum = ~sum;
-
-   /* set computation result */
-   sum = (sum == 0x0) ? 0x : sum;
-
-   return sum;
+   /* Set computation result in CPU endianness */
+   return (sum == 0x0) ? 0x : csum_bswap(sum);
 }
 
 /** @internal Compile

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-10 Thread Ilya Maximets 
Comments inlined.

On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
> From: Grigore Ion 
> 
> This patch fixes the following problems:
> - checksum computation for LE platforms
> - checksum is computed in the CPU endianness. The returned result
> must be converted to the BE ordering when it is used to update
> the UDP checksum in a packet.
> - checksum computation for packets having the UDP length not a
> multiple of 2
> - fixes the UDP checksum associated validation test
> 
> Signed-off-by: Grigore Ion 
> ---
>  v6:
>  - Make code more understandable (Ilya Maximets)
>  v5:
>  - Checksum in CPU endianness fix added (Ilya Maximets)
>  v4:
>  - Verify checksum in CPU endianness in the associated test
>  (Ilya Maximets)
>  v3:
>  - fix the UDP checksum computation in the associated test
>  (Maxim Uvarov)
>  v2:
>  - patch updated to the last master (Maxim Uvarov)
>  v1:
>  - Move variables declaration on top of block. (Maxim Uvarov)
>  - Check patch with checkpatch script.  (Maxim Uvarov)
>  - L3 header presence is tested twice. (Alexandru Badicioiu)
>  - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>  - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
> 
>  helper/include/odp/helper/udp.h | 81 
> +
>  helper/test/odp_chksum.c|  4 +-
>  2 files changed, 51 insertions(+), 34 deletions(-)
> 
> diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
> index 06c439b..d0599b1 100644
> --- a/helper/include/odp/helper/udp.h
> +++ b/helper/include/odp/helper/udp.h
> @@ -4,7 +4,6 @@
>   * SPDX-License-Identifier: BSD-3-Clause
>   */
>  
> -
>  /**
>   * @file
>   *
> @@ -22,7 +21,6 @@ extern "C" {
>  #include 
>  #include 
>  
> -
>  /** @addtogroup odph_header ODPH HEADER
>   *  @{
>   */
> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>  } odph_udphdr_t;
>  
>  /**
> + * Perform byte swap required by UDP checksum computation algorithm
> + */
> +static inline uint16_t csum_bswap(uint16_t val)
> +{
> +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> + val = __odp_builtin_bswap16((__odp_force uint16_t)val);
> +#endif
> + return val;
> +}

Please, don't duplicate the code.

> +
> +/**
> + * Pad a byte value to the left or to the right as required by UDP checksum
> + * computation algorithm and convert the result to CPU native uint16_t.
> + * Left padding is performed for the IP protocol field in the UDP
> + * pseudo-header (RFC 768). Right padding is performed in the case of the odd
> + * byte in a UDP packet having the length not a 2 multiple.
> + */
> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left)
> +{
> + uint16_tret;
> +
> + ret = (left) ? val : val << 8;
> + return csum_bswap(ret);
> +}
> +
> +/**
>   * UDP checksum
>   *
>   * This function uses odp packet to calc checksum
>   *
>   * @param pkt  calculate chksum for pkt
> - * @return  checksum value
> + * @return  checksum value in CPU endianness
>   */
>  static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>  {
> - uint32_t sum = 0;
> - odph_udphdr_t *udph;
> - odph_ipv4hdr_t *iph;
> - uint16_t udplen;
> - uint8_t *buf;
> -
> - if (!odp_packet_l3_offset(pkt))
> - return 0;
> + odph_ipv4hdr_t  *iph;
> + odph_udphdr_t   *udph;
> + uint32_tsum;
> + uint16_tudplen, *buf;
>  
> - if (!odp_packet_l4_offset(pkt))
> + if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>   return 0;
> -
>   iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>   udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
> - udplen = odp_be_to_cpu_16(udph->length);
> -
> - /* 32-bit sum of all 16-bit words covered by UDP chksum */
> + /* 32-bit sum of UDP pseudo-header */
>   sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
> -   (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> -   (uint16_t)iph->proto + udplen;
> - for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
> - sum += ((*buf << 8) + *(buf + 1));
> - buf += 2;
> - }
> -
> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0x) + (sum >> 16);
> -
> + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> + u8_pad_to_u16(iph->proto, 1) + udph->length;

I meant something like:

union {
uint8_t v8[2];
uint16_t v16;
} val;

val.v8[0] = 0;
val.v8[1] = iph->proto;

sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
  val.v16 + udph->length;

> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
> + for ( ; udplen > 1; udplen -= 2)
> +