Hi Maxim,

I noticed you merged this patch and then you reverted it because the validation 
test fails.
The problem is in the test not in the patch.

odp_checksum.c  from line 192 
        udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(test_packet));

        if (udp->chksum == 0)
                return -1;

        printf("chksum = 0x%x\n", udp->chksum);

        if (udp->chksum != odp_be_to_cpu_16(0x7e5a))
                status = -1;

Here is the packet used in the test :

0000 fe 0f 97 c9 e0 44 fe 0f 97 c9 e0 44 08 00 45 00
0010 00 34 00 01 00 00 00 11 39 65 c0 a8 00 01 c0 a8
0020 00 02 00 00 00 00 00 20 7e 5a 00 00 00 00 00 00
0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0040 00 00

The UDP checksum is 0x7e5a (in BE) and this is confirmed by Wireshark.

Thanks,
Grig

-----Original Message-----
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Maxim 
Uvarov
Sent: Monday, December 07, 2015 1:09 PM
To: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCHv2] helper : Fix UDP checksum computation

Merged,
Maxim.

On 12/04/2015 18:20, Bill Fischofer wrote:
>
>
> On Thu, Oct 22, 2015 at 7:02 AM, <ion.grig...@freescale.com 
> <mailto:ion.grig...@freescale.com>> wrote:
>
>     From: Grigore Ion <ion.grig...@freescale.com
>     <mailto:ion.grig...@freescale.com>>
>
>     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.
>
>     Signed-off-by: Grigore Ion <ion.grig...@freescale.com
>     <mailto:ion.grig...@freescale.com>>
>
>
> Reviewed-by: Bill Fischofer <bill.fischo...@linaro.org 
> <mailto:bill.fischo...@linaro.org>>
>
>     ---
>      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 |   32
>     +++++++++++++-------------------
>      1 files changed, 13 insertions(+), 19 deletions(-)
>
>     diff --git a/helper/include/odp/helper/udp.h
>     b/helper/include/odp/helper/udp.h
>     index 06c439b..6fce3f2 100644
>     --- a/helper/include/odp/helper/udp.h
>     +++ b/helper/include/odp/helper/udp.h
>     @@ -44,20 +44,16 @@ typedef struct ODP_PACKED {
>       * 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;
>     +       uint32_t sum;
>             odph_udphdr_t *udph;
>             odph_ipv4hdr_t *iph;
>     -       uint16_t udplen;
>     -       uint8_t *buf;
>     +       uint16_t udplen, *buf;
>
>     -       if (!odp_packet_l3_offset(pkt))
>     -               return 0;
>     -
>     -       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);
>     @@ -67,23 +63,21 @@ static inline uint16_t
>     odph_ipv4_udp_chksum(odp_packet_t pkt)
>             /* 32-bit sum of all 16-bit words covered by UDP chksum */
>             sum = (iph->src_addr & 0xFFFF) + (iph->src_addr >> 16) +
>                   (iph->dst_addr & 0xFFFF) + (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;
>     -       }
>     +             odp_be_to_cpu_16(iph->proto) + udph->length;
>     +       for (buf = (uint16_t *)((void *)udph); udplen > 1; udplen
>     -= 2)
>     +               sum += *buf++;
>     +       if (udplen) /* If length is not a multiple of 2 bytes */
>     +               sum += odp_be_to_cpu_16(*((uint8_t *)buf) << 8);
>
>             /* Fold sum to 16 bits: add carrier to result */
>     -       while (sum >> 16)
>     -               sum = (sum & 0xFFFF) + (sum >> 16);
>     +       sum = (sum & 0xFFFF) + (sum >> 16);
>     +       sum += (sum >> 16);
>
>             /* 1's complement */
>             sum = ~sum;
>
>     -       /* set computation result */
>     -       sum = (sum == 0x0) ? 0xFFFF : sum;
>     -
>     -       return sum;
>     +       /* set computation result in CPU endianness*/
>     +       return (sum == 0x0) ? 0xFFFF : odp_be_to_cpu_16(sum);
>      }
>
>      /** @internal Compile time assert */
>     --
>     1.7.3.4
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to