diff mbox

[PATCHv2] Introduce sum type to solve sparse warnings

Message ID 1405633613-48330-1-git-send-email-mike.holmes@linaro.org
State Accepted
Commit 8f7fd2f16bed95341951856a8ebe77671c3391e5
Headers show

Commit Message

Mike Holmes July 17, 2014, 9:46 p.m. UTC
ODP follows the linux kernel with the use of sparse and BE data type
checking.  This patch introduces the sum type which is used by Linux
but was not part of ODP see:
http://lxr.free-electrons.com/source/include/uapi/linux/icmp.h#L71
This patch allows SPARSE to pass cleanly and the odp_generator test
case also passes.

Signed-off-by: Mike Holmes <mike.holmes@linaro.org>

---
Note 1 The bitwise attribute forbids arithmetic so the checksum
code uses regular types and forces the result in the return from
the function.
Note 2 The timer ping code is an application that uses a different
structure definition that does not have the bitwise attribute and so
this application is left issuing a sparse warning and is a separate
issue.
---
 include/helper/odp_chksum.h    |  9 +++------
 include/helper/odp_icmp.h      |  2 +-
 include/helper/odp_ip.h        | 10 ++++------
 include/helper/odp_udp.h       |  4 ++--
 include/odp_byteorder.h        |  3 ++-
 test/generator/odp_generator.c |  4 ++--
 6 files changed, 14 insertions(+), 18 deletions(-)

Comments

Anders Roxell July 18, 2014, 4:05 a.m. UTC | #1
On 2014-07-17 17:46, Mike Holmes wrote:
> ODP follows the linux kernel with the use of sparse and BE data type
> checking.  This patch introduces the sum type which is used by Linux
> but was not part of ODP see:
> http://lxr.free-electrons.com/source/include/uapi/linux/icmp.h#L71
> This patch allows SPARSE to pass cleanly and the odp_generator test
> case also passes.
> 
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>

Reviewed-and-Tested-by: Anders Roxell <anders.roxell@linaro.org>

> 
> ---
> Note 1 The bitwise attribute forbids arithmetic so the checksum
> code uses regular types and forces the result in the return from
> the function.
> Note 2 The timer ping code is an application that uses a different
> structure definition that does not have the bitwise attribute and so
> this application is left issuing a sparse warning and is a separate
> issue.
> ---
>  include/helper/odp_chksum.h    |  9 +++------
>  include/helper/odp_icmp.h      |  2 +-
>  include/helper/odp_ip.h        | 10 ++++------
>  include/helper/odp_udp.h       |  4 ++--
>  include/odp_byteorder.h        |  3 ++-
>  test/generator/odp_generator.c |  4 ++--
>  6 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h
> index 37078b6..710711a 100644
> --- a/include/helper/odp_chksum.h
> +++ b/include/helper/odp_chksum.h
> @@ -22,18 +22,15 @@ extern "C" {
>  /**
>   * Checksum
>   *
> - * @note when using this api to populate data destined for the wire
> - * odp_cpu_to_be_16() can be used to remove sparse warnings
> - *
>   * @param buffer calculate chksum for buffer
>   * @param len    buffer length
>   *
>   * @return checksum value in host cpu order
>   */
> -static inline uint16_t odp_chksum(void *buffer, int len)
> +static inline uint16sum_t odp_chksum(void *buffer, int len)
>  {
>  	uint16_t *buf = buffer;
> -	unsigned int sum = 0;
> +	uint32_t sum = 0;
>  	uint16_t result;
>  
>  	for (sum = 0; len > 1; len -= 2)
> @@ -46,7 +43,7 @@ static inline uint16_t odp_chksum(void *buffer, int len)
>  	sum += (sum >> 16);
>  	result = ~sum;
>  
> -	return result;
> +	return  (__odp_force uint16sum_t) result;
>  }
>  
>  #ifdef __cplusplus
> diff --git a/include/helper/odp_icmp.h b/include/helper/odp_icmp.h
> index f34b68e..ab8c409 100644
> --- a/include/helper/odp_icmp.h
> +++ b/include/helper/odp_icmp.h
> @@ -29,7 +29,7 @@ extern "C" {
>  typedef struct ODP_PACKED {
>  	uint8_t type;		/**< message type */
>  	uint8_t code;		/**< type sub-code */
> -	uint16be_t chksum;	/**< checksum of icmp header */
> +	uint16sum_t chksum;	/**< checksum of icmp header */
>  	union {
>  		struct {
>  			uint16be_t id;
> diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h
> index d8366bb..6f4e028 100644
> --- a/include/helper/odp_ip.h
> +++ b/include/helper/odp_ip.h
> @@ -56,7 +56,7 @@ typedef struct ODP_PACKED {
>  	uint16be_t frag_offset; /**< Fragmentation offset */
>  	uint8_t    ttl;         /**< Time to live */
>  	uint8_t    proto;       /**< Protocol */
> -	uint16be_t chksum;      /**< Checksum */
> +	uint16sum_t chksum;      /**< Checksum */
>  	uint32be_t src_addr;    /**< Source address */
>  	uint32be_t dst_addr;    /**< Destination address */
>  } odp_ipv4hdr_t;
> @@ -101,9 +101,8 @@ static inline int odp_ipv4_csum_valid(odp_packet_t pkt)
>   *
>   * @return IPv4 checksum in host cpu order, or 0 on failure
>   */
> -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt)
> +static inline uint16sum_t odp_ipv4_csum_update(odp_packet_t pkt)
>  {
> -	uint16_t res = 0;
>  	uint16_t *w;
>  	odp_ipv4hdr_t *ip;
>  	int nleft = sizeof(odp_ipv4hdr_t);
> @@ -113,9 +112,8 @@ static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt)
>  
>  	ip = (odp_ipv4hdr_t *)odp_packet_l3(pkt);
>  	w = (uint16_t *)(void *)ip;
> -	res = odp_chksum(w, nleft);
> -	ip->chksum = odp_cpu_to_be_16(res);
> -	return res;
> +	ip->chksum = odp_chksum(w, nleft);
> +	return ip->chksum;
>  }
>  
>  /** IPv6 version */
> diff --git a/include/helper/odp_udp.h b/include/helper/odp_udp.h
> index ae31f6b..0ad1541 100644
> --- a/include/helper/odp_udp.h
> +++ b/include/helper/odp_udp.h
> @@ -79,9 +79,9 @@ static inline uint16_t odp_ipv4_udp_chksum(odp_packet_t pkt)
>  	phdr.length = udph->length;
>  
>  	/* calc UDP pseudo header chksum */
> -	sum = odp_chksum(&phdr, sizeof(odp_udpphdr_t));
> +	sum =   (__odp_force uint32_t) odp_chksum(&phdr, sizeof(odp_udpphdr_t));
>  	/* calc udp header and data chksum */
> -	sum += odp_chksum(udph, udplen);
> +	sum += (__odp_force uint32_t) odp_chksum(udph, udplen);
>  
>  	/* Fold sum to 16 bits: add carrier to result */
>  	while (sum >> 16)
> diff --git a/include/odp_byteorder.h b/include/odp_byteorder.h
> index 80f6081..e0f7a17 100644
> --- a/include/odp_byteorder.h
> +++ b/include/odp_byteorder.h
> @@ -71,7 +71,8 @@ typedef uint32_t __odp_bitwise	uint32be_t; /**< unsigned 32bit big endian */
>  typedef uint64_t __odp_bitwise	uint64le_t; /**< unsigned 64bit little endian */
>  typedef uint64_t __odp_bitwise	uint64be_t; /**< unsigned 64bit big endian */
>  
> -
> +typedef uint16_t __odp_bitwise  uint16sum_t; /**< unsigned 16bit bitwise */
> +typedef uint32_t __odp_bitwise  uint32sum_t; /**< unsigned 32bit bitwise */
>  /*
>   * Big Endian -> CPU byte order:
>   */
> diff --git a/test/generator/odp_generator.c b/test/generator/odp_generator.c
> index 8f6ad49..e4a72fa 100644
> --- a/test/generator/odp_generator.c
> +++ b/test/generator/odp_generator.c
> @@ -275,8 +275,8 @@ static void pack_icmp_pkt(odp_buffer_t obuf)
>  	gettimeofday(&tval, NULL);
>  	memcpy(tval_d, &tval, sizeof(struct timeval));
>  	icmp->chksum = 0;
> -	icmp->chksum = odp_cpu_to_be_16(odp_chksum(icmp, args->appl.payload +
> -				  ODP_ICMPHDR_LEN));
> +	icmp->chksum = odp_chksum(icmp, args->appl.payload +
> +				  ODP_ICMPHDR_LEN);
>  
>  	odp_packet_set_len(pkt, args->appl.payload + ODP_ICMPHDR_LEN +
>  			   ODP_IPV4HDR_LEN + ODP_ETHHDR_LEN);
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h
index 37078b6..710711a 100644
--- a/include/helper/odp_chksum.h
+++ b/include/helper/odp_chksum.h
@@ -22,18 +22,15 @@  extern "C" {
 /**
  * Checksum
  *
- * @note when using this api to populate data destined for the wire
- * odp_cpu_to_be_16() can be used to remove sparse warnings
- *
  * @param buffer calculate chksum for buffer
  * @param len    buffer length
  *
  * @return checksum value in host cpu order
  */
-static inline uint16_t odp_chksum(void *buffer, int len)
+static inline uint16sum_t odp_chksum(void *buffer, int len)
 {
 	uint16_t *buf = buffer;
-	unsigned int sum = 0;
+	uint32_t sum = 0;
 	uint16_t result;
 
 	for (sum = 0; len > 1; len -= 2)
@@ -46,7 +43,7 @@  static inline uint16_t odp_chksum(void *buffer, int len)
 	sum += (sum >> 16);
 	result = ~sum;
 
-	return result;
+	return  (__odp_force uint16sum_t) result;
 }
 
 #ifdef __cplusplus
diff --git a/include/helper/odp_icmp.h b/include/helper/odp_icmp.h
index f34b68e..ab8c409 100644
--- a/include/helper/odp_icmp.h
+++ b/include/helper/odp_icmp.h
@@ -29,7 +29,7 @@  extern "C" {
 typedef struct ODP_PACKED {
 	uint8_t type;		/**< message type */
 	uint8_t code;		/**< type sub-code */
-	uint16be_t chksum;	/**< checksum of icmp header */
+	uint16sum_t chksum;	/**< checksum of icmp header */
 	union {
 		struct {
 			uint16be_t id;
diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h
index d8366bb..6f4e028 100644
--- a/include/helper/odp_ip.h
+++ b/include/helper/odp_ip.h
@@ -56,7 +56,7 @@  typedef struct ODP_PACKED {
 	uint16be_t frag_offset; /**< Fragmentation offset */
 	uint8_t    ttl;         /**< Time to live */
 	uint8_t    proto;       /**< Protocol */
-	uint16be_t chksum;      /**< Checksum */
+	uint16sum_t chksum;      /**< Checksum */
 	uint32be_t src_addr;    /**< Source address */
 	uint32be_t dst_addr;    /**< Destination address */
 } odp_ipv4hdr_t;
@@ -101,9 +101,8 @@  static inline int odp_ipv4_csum_valid(odp_packet_t pkt)
  *
  * @return IPv4 checksum in host cpu order, or 0 on failure
  */
-static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt)
+static inline uint16sum_t odp_ipv4_csum_update(odp_packet_t pkt)
 {
-	uint16_t res = 0;
 	uint16_t *w;
 	odp_ipv4hdr_t *ip;
 	int nleft = sizeof(odp_ipv4hdr_t);
@@ -113,9 +112,8 @@  static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt)
 
 	ip = (odp_ipv4hdr_t *)odp_packet_l3(pkt);
 	w = (uint16_t *)(void *)ip;
-	res = odp_chksum(w, nleft);
-	ip->chksum = odp_cpu_to_be_16(res);
-	return res;
+	ip->chksum = odp_chksum(w, nleft);
+	return ip->chksum;
 }
 
 /** IPv6 version */
diff --git a/include/helper/odp_udp.h b/include/helper/odp_udp.h
index ae31f6b..0ad1541 100644
--- a/include/helper/odp_udp.h
+++ b/include/helper/odp_udp.h
@@ -79,9 +79,9 @@  static inline uint16_t odp_ipv4_udp_chksum(odp_packet_t pkt)
 	phdr.length = udph->length;
 
 	/* calc UDP pseudo header chksum */
-	sum = odp_chksum(&phdr, sizeof(odp_udpphdr_t));
+	sum =   (__odp_force uint32_t) odp_chksum(&phdr, sizeof(odp_udpphdr_t));
 	/* calc udp header and data chksum */
-	sum += odp_chksum(udph, udplen);
+	sum += (__odp_force uint32_t) odp_chksum(udph, udplen);
 
 	/* Fold sum to 16 bits: add carrier to result */
 	while (sum >> 16)
diff --git a/include/odp_byteorder.h b/include/odp_byteorder.h
index 80f6081..e0f7a17 100644
--- a/include/odp_byteorder.h
+++ b/include/odp_byteorder.h
@@ -71,7 +71,8 @@  typedef uint32_t __odp_bitwise	uint32be_t; /**< unsigned 32bit big endian */
 typedef uint64_t __odp_bitwise	uint64le_t; /**< unsigned 64bit little endian */
 typedef uint64_t __odp_bitwise	uint64be_t; /**< unsigned 64bit big endian */
 
-
+typedef uint16_t __odp_bitwise  uint16sum_t; /**< unsigned 16bit bitwise */
+typedef uint32_t __odp_bitwise  uint32sum_t; /**< unsigned 32bit bitwise */
 /*
  * Big Endian -> CPU byte order:
  */
diff --git a/test/generator/odp_generator.c b/test/generator/odp_generator.c
index 8f6ad49..e4a72fa 100644
--- a/test/generator/odp_generator.c
+++ b/test/generator/odp_generator.c
@@ -275,8 +275,8 @@  static void pack_icmp_pkt(odp_buffer_t obuf)
 	gettimeofday(&tval, NULL);
 	memcpy(tval_d, &tval, sizeof(struct timeval));
 	icmp->chksum = 0;
-	icmp->chksum = odp_cpu_to_be_16(odp_chksum(icmp, args->appl.payload +
-				  ODP_ICMPHDR_LEN));
+	icmp->chksum = odp_chksum(icmp, args->appl.payload +
+				  ODP_ICMPHDR_LEN);
 
 	odp_packet_set_len(pkt, args->appl.payload + ODP_ICMPHDR_LEN +
 			   ODP_IPV4HDR_LEN + ODP_ETHHDR_LEN);