Re: [lng-odp] [PATCHv4] helper : Fix UDP checksum computation
Resent with the fix inside.
Thanks,
Grig
-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Wednesday, December 09, 2015 8:02 AM
To: Grigore Ion-B17953 ; lng-odp@lists.linaro.org
Subject: Re: [PATCHv4] helper : Fix UDP checksum computation
It's a completely the same patch as v3.
Only commit message changed.
Best regards, Ilya Maximets.
On 08.12.2015 19:58, 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
> ---
> 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 | 55
> +
> helper/test/odp_chksum.c| 4 +--
> 2 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/helper/include/odp/helper/udp.h
> b/helper/include/odp/helper/udp.h index 06c439b..9e7256d 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
> * @{
> */
> @@ -44,46 +42,39 @@ 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;
> - 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) +
> + odp_be_to_cpu_16(iph->proto) + 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 += odp_be_to_cpu_16(*((uint8_t *)buf) << 8);
> + /* 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 : odp_be_to_cpu_16(sum);
> }
>
> /** @internal Compile time assert */
> diff --git a/helper/test/odp_chksum.c b/helper/test/odp_chksum.c index
> 1d417a8..2b32111 100644
> --- a/helper/test/odp_chksum.c
> +++ b/helper/test/odp_chksum.c
> @@ -189,14 +189,14 @@ int main(int argc TEST_UNUSED, char *argv[] TEST_UNUSED)
> udp->dst_port = 0;
> udp->length = odp_cpu_to_be_16(udat_size + ODPH_UDPHDR_LEN);
> udp->chksum = 0;
> - udp->chksum = odph_ipv4_udp_chksum(test_packet)
Re: [lng-odp] [PATCHv4] helper : Fix UDP checksum computation
It's a completely the same patch as v3.
Only commit message changed.
Best regards, Ilya Maximets.
On 08.12.2015 19:58, 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
> ---
> 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 | 55
> +
> helper/test/odp_chksum.c| 4 +--
> 2 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
> index 06c439b..9e7256d 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
> * @{
> */
> @@ -44,46 +42,39 @@ 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;
> - 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) +
> + odp_be_to_cpu_16(iph->proto) + 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 += odp_be_to_cpu_16(*((uint8_t *)buf) << 8);
> + /* 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 : odp_be_to_cpu_16(sum);
> }
>
> /** @internal Compile time assert */
> diff --git a/helper/test/odp_chksum.c b/helper/test/odp_chksum.c
> index 1d417a8..2b32111 100644
> --- a/helper/test/odp_chksum.c
> +++ b/helper/test/odp_chksum.c
> @@ -189,14 +189,14 @@ int main(int argc TEST_UNUSED, char *argv[] TEST_UNUSED)
> udp->dst_port = 0;
> udp->length = odp_cpu_to_be_16(udat_size + ODPH_UDPHDR_LEN);
> udp->chksum = 0;
> - udp->chksum = odph_ipv4_udp_chksum(test_packet);
> + 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 != 0xab2d)
> + if (udp->chksum != odp_be_to_cpu_16(0x7e5a))
>
[lng-odp] [PATCHv4] helper : Fix UDP checksum computation
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
---
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 | 55 +
helper/test/odp_chksum.c| 4 +--
2 files changed, 25 insertions(+), 34 deletions(-)
diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
index 06c439b..9e7256d 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
* @{
*/
@@ -44,46 +42,39 @@ 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;
- 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) +
+ odp_be_to_cpu_16(iph->proto) + 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 += odp_be_to_cpu_16(*((uint8_t *)buf) << 8);
+ /* 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 : odp_be_to_cpu_16(sum);
}
/** @internal Compile time assert */
diff --git a/helper/test/odp_chksum.c b/helper/test/odp_chksum.c
index 1d417a8..2b32111 100644
--- a/helper/test/odp_chksum.c
+++ b/helper/test/odp_chksum.c
@@ -189,14 +189,14 @@ int main(int argc TEST_UNUSED, char *argv[] TEST_UNUSED)
udp->dst_port = 0;
udp->length = odp_cpu_to_be_16(udat_size + ODPH_UDPHDR_LEN);
udp->chksum = 0;
- udp->chksum = odph_ipv4_udp_chksum(test_packet);
+ 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 != 0xab2d)
+ if (udp->chksum != odp_be_to_cpu_16(0x7e5a))
status = -1;
odp_packet_free(test_packet);
--
1.9.3
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
