[1/1] helpers: fix udp checksum computation

Message ID 1430385858-29414-1-git-send-email-alexandru.badicioiu@linaro.org
State New
Headers show

Commit Message

Alexandru Badicioiu April 30, 2015, 9:24 a.m.
From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>

Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
---
 helper/include/odp/helper/udp.h |   35 +++++++++++------------------------
 1 files changed, 11 insertions(+), 24 deletions(-)

Comments

Maxim Uvarov May 13, 2015, 9:21 a.m. | #1
Bill, I think this patch is also ok to go in. Right?

Maxim.

On 04/30/2015 12:24, alexandru.badicioiu@linaro.org wrote:
> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>
> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> ---
>   helper/include/odp/helper/udp.h |   35 +++++++++++------------------------
>   1 files changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
> index 99a96f2..8565bab 100644
> --- a/helper/include/odp/helper/udp.h
> +++ b/helper/include/odp/helper/udp.h
> @@ -33,15 +33,6 @@ typedef struct ODP_PACKED {
>   	uint16be_t chksum;   /**< UDP header and data checksum (0 if not used)*/
>   } odph_udphdr_t;
>   
> -/** UDP pseudo header */
> -typedef struct ODPH_PACKET {
> -	uint32be_t src_addr; /**< Source addr */
> -	uint32be_t dst_addr; /**< Destination addr */
> -	uint8_t pad;	     /**< pad byte */
> -	uint8_t proto;	     /**< UDP protocol */
> -	uint16be_t length;   /**< data length */
> -} odph_udpphdr_t;
> -
>   /**
>    * UDP checksum
>    *
> @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET {
>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>   {
>   	uint32_t sum = 0;
> -	odph_udpphdr_t phdr;
>   	odph_udphdr_t *udph;
>   	odph_ipv4hdr_t *iph;
>   	uint16_t udplen;
> +	uint16_t *buf;
>   
>   	if (!odp_packet_l3_offset(pkt))
>   		return 0;
> @@ -68,24 +59,20 @@ static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>   	udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>   	udplen = odp_be_to_cpu_16(udph->length);
>   
> -	/* the source ip */
> -	phdr.src_addr = iph->src_addr;
> -	/* the dest ip */
> -	phdr.dst_addr = iph->dst_addr;
> -	/* proto */
> -	phdr.pad = 0;
> -	phdr.proto = ODPH_IPPROTO_UDP;
> -	/* the length */
> -	phdr.length = udph->length;
> -
> -	/* calc UDP pseudo header chksum */
> -	sum = (__odp_force uint32_t) odp_chksum(&phdr, sizeof(odph_udpphdr_t));
> -	/* calc udp header and data chksum */
> -	sum += (__odp_force uint32_t) odp_chksum(udph, udplen);
> +	/* 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 = (uint16_t *)udph; udplen > 1; udplen -= 2)
> +		sum += *buf++;
>   
>   	/* Fold sum to 16 bits: add carrier to result */
>   	while (sum >> 16)
>   		sum = (sum & 0xFFFF) + (sum >> 16);
> +
> +	/* 1's complement */
> +	sum = ~sum;
> +
>   	/* set computation result */
>   	sum = (sum == 0x0) ? 0xFFFF : sum;
>
Alexandru Badicioiu May 13, 2015, 10:23 a.m. | #2
Maxim,
this is an old one - there's a v1 submitted addressing a compile warning.

Alex

On 13 May 2015 at 12:21, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> Bill, I think this patch is also ok to go in. Right?
>
> Maxim.
>
>
> On 04/30/2015 12:24, alexandru.badicioiu@linaro.org wrote:
>
>> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>>
>> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>> ---
>>   helper/include/odp/helper/udp.h |   35
>> +++++++++++------------------------
>>   1 files changed, 11 insertions(+), 24 deletions(-)
>>
>> diff --git a/helper/include/odp/helper/udp.h
>> b/helper/include/odp/helper/udp.h
>> index 99a96f2..8565bab 100644
>> --- a/helper/include/odp/helper/udp.h
>> +++ b/helper/include/odp/helper/udp.h
>> @@ -33,15 +33,6 @@ typedef struct ODP_PACKED {
>>         uint16be_t chksum;   /**< UDP header and data checksum (0 if not
>> used)*/
>>   } odph_udphdr_t;
>>   -/** UDP pseudo header */
>> -typedef struct ODPH_PACKET {
>> -       uint32be_t src_addr; /**< Source addr */
>> -       uint32be_t dst_addr; /**< Destination addr */
>> -       uint8_t pad;         /**< pad byte */
>> -       uint8_t proto;       /**< UDP protocol */
>> -       uint16be_t length;   /**< data length */
>> -} odph_udpphdr_t;
>> -
>>   /**
>>    * UDP checksum
>>    *
>> @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET {
>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>   {
>>         uint32_t sum = 0;
>> -       odph_udpphdr_t phdr;
>>         odph_udphdr_t *udph;
>>         odph_ipv4hdr_t *iph;
>>         uint16_t udplen;
>> +       uint16_t *buf;
>>         if (!odp_packet_l3_offset(pkt))
>>                 return 0;
>> @@ -68,24 +59,20 @@ static inline uint16_t
>> odph_ipv4_udp_chksum(odp_packet_t pkt)
>>         udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>         udplen = odp_be_to_cpu_16(udph->length);
>>   -     /* the source ip */
>> -       phdr.src_addr = iph->src_addr;
>> -       /* the dest ip */
>> -       phdr.dst_addr = iph->dst_addr;
>> -       /* proto */
>> -       phdr.pad = 0;
>> -       phdr.proto = ODPH_IPPROTO_UDP;
>> -       /* the length */
>> -       phdr.length = udph->length;
>> -
>> -       /* calc UDP pseudo header chksum */
>> -       sum = (__odp_force uint32_t) odp_chksum(&phdr,
>> sizeof(odph_udpphdr_t));
>> -       /* calc udp header and data chksum */
>> -       sum += (__odp_force uint32_t) odp_chksum(udph, udplen);
>> +       /* 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 = (uint16_t *)udph; udplen > 1; udplen -= 2)
>> +               sum += *buf++;
>>         /* Fold sum to 16 bits: add carrier to result */
>>         while (sum >> 16)
>>                 sum = (sum & 0xFFFF) + (sum >> 16);
>> +
>> +       /* 1's complement */
>> +       sum = ~sum;
>> +
>>         /* set computation result */
>>         sum = (sum == 0x0) ? 0xFFFF : sum;
>>
>>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov May 13, 2015, 11:59 a.m. | #3
On 05/13/2015 13:23, Alexandru Badicioiu wrote:
> Maxim,
> this is an old one - there's a v1 submitted addressing a compile warning.
>
> Alex
>
Ah, ok, thanks.

Maxim.

> On 13 May 2015 at 12:21, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     Bill, I think this patch is also ok to go in. Right?
>
>     Maxim.
>
>
>     On 04/30/2015 12:24, alexandru.badicioiu@linaro.org
>     <mailto:alexandru.badicioiu@linaro.org> wrote:
>
>         From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org
>         <mailto:alexandru.badicioiu@linaro.org>>
>
>         Signed-off-by: Alexandru Badicioiu
>         <alexandru.badicioiu@linaro.org
>         <mailto:alexandru.badicioiu@linaro.org>>
>         ---
>           helper/include/odp/helper/udp.h |   35
>         +++++++++++------------------------
>           1 files changed, 11 insertions(+), 24 deletions(-)
>
>         diff --git a/helper/include/odp/helper/udp.h
>         b/helper/include/odp/helper/udp.h
>         index 99a96f2..8565bab 100644
>         --- a/helper/include/odp/helper/udp.h
>         +++ b/helper/include/odp/helper/udp.h
>         @@ -33,15 +33,6 @@ typedef struct ODP_PACKED {
>                 uint16be_t chksum;   /**< UDP header and data checksum
>         (0 if not used)*/
>           } odph_udphdr_t;
>           -/** UDP pseudo header */
>         -typedef struct ODPH_PACKET {
>         -       uint32be_t src_addr; /**< Source addr */
>         -       uint32be_t dst_addr; /**< Destination addr */
>         -       uint8_t pad;         /**< pad byte */
>         -       uint8_t proto;       /**< UDP protocol */
>         -       uint16be_t length;   /**< data length */
>         -} odph_udpphdr_t;
>         -
>           /**
>            * UDP checksum
>            *
>         @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET {
>           static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>           {
>                 uint32_t sum = 0;
>         -       odph_udpphdr_t phdr;
>                 odph_udphdr_t *udph;
>                 odph_ipv4hdr_t *iph;
>                 uint16_t udplen;
>         +       uint16_t *buf;
>                 if (!odp_packet_l3_offset(pkt))
>                         return 0;
>         @@ -68,24 +59,20 @@ static inline uint16_t
>         odph_ipv4_udp_chksum(odp_packet_t pkt)
>                 udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>                 udplen = odp_be_to_cpu_16(udph->length);
>           -     /* the source ip */
>         -       phdr.src_addr = iph->src_addr;
>         -       /* the dest ip */
>         -       phdr.dst_addr = iph->dst_addr;
>         -       /* proto */
>         -       phdr.pad = 0;
>         -       phdr.proto = ODPH_IPPROTO_UDP;
>         -       /* the length */
>         -       phdr.length = udph->length;
>         -
>         -       /* calc UDP pseudo header chksum */
>         -       sum = (__odp_force uint32_t) odp_chksum(&phdr,
>         sizeof(odph_udpphdr_t));
>         -       /* calc udp header and data chksum */
>         -       sum += (__odp_force uint32_t) odp_chksum(udph, udplen);
>         +       /* 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 = (uint16_t *)udph; udplen > 1; udplen -= 2)
>         +               sum += *buf++;
>                 /* Fold sum to 16 bits: add carrier to result */
>                 while (sum >> 16)
>                         sum = (sum & 0xFFFF) + (sum >> 16);
>         +
>         +       /* 1's complement */
>         +       sum = ~sum;
>         +
>                 /* set computation result */
>                 sum = (sum == 0x0) ? 0xFFFF : sum;
>
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>

Patch

diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
index 99a96f2..8565bab 100644
--- a/helper/include/odp/helper/udp.h
+++ b/helper/include/odp/helper/udp.h
@@ -33,15 +33,6 @@  typedef struct ODP_PACKED {
 	uint16be_t chksum;   /**< UDP header and data checksum (0 if not used)*/
 } odph_udphdr_t;
 
-/** UDP pseudo header */
-typedef struct ODPH_PACKET {
-	uint32be_t src_addr; /**< Source addr */
-	uint32be_t dst_addr; /**< Destination addr */
-	uint8_t pad;	     /**< pad byte */
-	uint8_t proto;	     /**< UDP protocol */
-	uint16be_t length;   /**< data length */
-} odph_udpphdr_t;
-
 /**
  * UDP checksum
  *
@@ -53,10 +44,10 @@  typedef struct ODPH_PACKET {
 static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
 {
 	uint32_t sum = 0;
-	odph_udpphdr_t phdr;
 	odph_udphdr_t *udph;
 	odph_ipv4hdr_t *iph;
 	uint16_t udplen;
+	uint16_t *buf;
 
 	if (!odp_packet_l3_offset(pkt))
 		return 0;
@@ -68,24 +59,20 @@  static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
 	udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
 	udplen = odp_be_to_cpu_16(udph->length);
 
-	/* the source ip */
-	phdr.src_addr = iph->src_addr;
-	/* the dest ip */
-	phdr.dst_addr = iph->dst_addr;
-	/* proto */
-	phdr.pad = 0;
-	phdr.proto = ODPH_IPPROTO_UDP;
-	/* the length */
-	phdr.length = udph->length;
-
-	/* calc UDP pseudo header chksum */
-	sum = (__odp_force uint32_t) odp_chksum(&phdr, sizeof(odph_udpphdr_t));
-	/* calc udp header and data chksum */
-	sum += (__odp_force uint32_t) odp_chksum(udph, udplen);
+	/* 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 = (uint16_t *)udph; udplen > 1; udplen -= 2)
+		sum += *buf++;
 
 	/* Fold sum to 16 bits: add carrier to result */
 	while (sum >> 16)
 		sum = (sum & 0xFFFF) + (sum >> 16);
+
+	/* 1's complement */
+	sum = ~sum;
+
 	/* set computation result */
 	sum = (sum == 0x0) ? 0xFFFF : sum;