| Message ID | 1405613316-14656-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 2014-07-17 12:08, Mike Holmes wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > 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. I think you should move this above the s-o-b so it is part of the commit log. Cheers, Anders > 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 --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);
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- 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. 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(-)