On 04.01.2016 15:48, ion.grig...@freescale.com wrote:
> From: Grigore Ion <ion.grig...@freescale.com>
>
> This patch fixes the following problems:
> - checksum computation for LE platforms
> - checksum computation for packets having the UDP length not a
> multiple of 2
> - checksum computation in the test and the example applications
>
> Signed-off-by: Grigore Ion <ion.grig...@freescale.com>
> ---
> v8:
> - Checksum in BE endianness (Ilya Maximets)
> v7:
> - Make code simpler and more understandable as it was
> sugessted by Ilya Maximets
> v6:
> - Make code more understandable. Not done as it was
> sugessted by 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)
>
> example/generator/odp_generator.c | 2 +-
> helper/include/odp/helper/udp.h | 65
> ++++++++++++++++++++-------------------
> helper/test/odp_chksum.c | 4 +--
> test/validation/pktio/pktio.c | 2 +-
> 4 files changed, 37 insertions(+), 36 deletions(-)
>
> diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> index cdbc761..757dc54 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -242,7 +242,7 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
> udp->dst_port = 0;
> udp->length = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN);
> udp->chksum = 0;
> - udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(pkt));
> + udp->chksum = odph_ipv4_udp_chksum(pkt);
>
> return pkt;
> }
> diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
> index 06c439b..afdb4fc 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 <odp/debug.h>
> #include <odp/byteorder.h>
>
> -
> /** @addtogroup odph_header ODPH HEADER
> * @{
> */
> @@ -44,46 +42,49 @@ 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 BE 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))
> + odph_ipv4hdr_t *iph;
> + odph_udphdr_t *udph;
> + uint32_t sum;
> + uint16_t udplen, *buf;
> + union {
> + uint8_t v8[2];
> + uint16_t v16;
> + } val;
> +
> + if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
> return 0;
> -
> - if (!odp_packet_l4_offset(pkt))
> - 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 */
I think, it should be specified, that it's a sum of 16-bit words.
> 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;
> + (iph->dst_addr & 0xFFFF) + (iph->dst_addr >> 16) +
> + udph->length;
> + val.v8[0] = 0;
> + val.v8[1] = iph->proto;
> + sum += val.v16;
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
Same here.
> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + for ( ; udplen > 1; udplen -= 2)
> + sum += *buf++;
> + /* Length is not a multiple of 2 bytes */
> + if (udplen) {
> + val.v8[0] = *buf;
> + val.v8[1] = 0;
> + sum += val.v16;
> }
> -
> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0xFFFF) + (sum >> 16);
> -
> + /* Fold sum to 16 bits */
> + sum = (sum & 0xFFFF) + (sum >> 16);
> + /* Add carrier (0/1) to result */
> + sum += (sum >> 16);
> /* 1's complement */
> sum = ~sum;
> -
> - /* set computation result */
> - sum = (sum == 0x0) ? 0xFFFF : sum;
> -
> - return sum;
> + /* Return checksum in BE endianness */
> + return (sum == 0x0) ? 0xFFFF : sum;
> }
>
> /** @internal Compile time assert */
> diff --git a/helper/test/odp_chksum.c b/helper/test/odp_chksum.c
> index 1d417a8..b71fabc 100644
> --- a/helper/test/odp_chksum.c
> +++ b/helper/test/odp_chksum.c
> @@ -194,9 +194,9 @@ int main(int argc TEST_UNUSED, char *argv[] TEST_UNUSED)
> if (udp->chksum == 0)
> return -1;
>
> - printf("chksum = 0x%x\n", udp->chksum);
> + printf("chksum = 0x%x\n", odp_be_to_cpu_16(udp->chksum));
>
> - if (udp->chksum != 0xab2d)
> + if (odp_be_to_cpu_16(udp->chksum) != 0x7e5a)
> status = -1;
>
> odp_packet_free(test_packet);
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index e473a1d..8121f1e 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -232,7 +232,7 @@ static int pktio_fixup_checksums(odp_packet_t pkt)
> ip->chksum = 0;
> odph_ipv4_csum_update(pkt);
> udp->chksum = 0;
> - udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(pkt));
> + udp->chksum = odph_ipv4_udp_chksum(pkt);
>
> return 0;
> }
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
