| Message ID | 1405414460-13833-1-git-send-email-weilong.chen@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 07/15/2014 11:54 AM, Weilong Chen wrote: > This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. > According to https://www.ietf.org/rfc/rfc1071.txt. > The checksum algorithm is "Byte Order Independence". > > Signed-off-by: Weilong Chen <weilong.chen@linaro.org> > --- > include/helper/odp_chksum.h | 5 +---- > include/helper/odp_ip.h | 11 ++++------- > include/helper/odp_udp.h | 2 +- > test/generator/odp_generator.c | 6 +++--- > 4 files changed, 9 insertions(+), 15 deletions(-) > > diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h > index 37078b6..12ef61f 100644 > --- a/include/helper/odp_chksum.h > +++ b/include/helper/odp_chksum.h > @@ -22,13 +22,10 @@ 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 > + * @return checksum value > */ > static inline uint16_t odp_chksum(void *buffer, int len) > { > diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h > index d8366bb..a364049 100644 > --- a/include/helper/odp_ip.h > +++ b/include/helper/odp_ip.h > @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t pkt) > /** > * Calculate and fill in IPv4 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 pkt ODP packet > * > - * @return IPv4 checksum in host cpu order, or 0 on failure > + * @return IPv4 checksum, or 0 on failure > */ > -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) > +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) > { > - uint16_t res = 0; > + uint16be_t res = 0; If checksum calculation algorithm is byteorder independent, than no need to operate with BE types. Just change .chksum field to uint16_t and operate in native endianness.
See http://locklessinc.com/articles/tcp_checksum/ for a good article on this subject and the hazards of optimizations. Since these checksums are universally implemented in HW anyway for all performance paths, ODP's software implementations should simply follow the RFCs without embellishment. Bill On Tue, Jul 15, 2014 at 4:30 AM, Taras Kondratiuk < taras.kondratiuk@linaro.org> wrote: > On 07/15/2014 11:54 AM, Weilong Chen wrote: > >> This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. >> According to https://www.ietf.org/rfc/rfc1071.txt. >> The checksum algorithm is "Byte Order Independence". >> >> Signed-off-by: Weilong Chen <weilong.chen@linaro.org> >> --- >> include/helper/odp_chksum.h | 5 +---- >> include/helper/odp_ip.h | 11 ++++------- >> include/helper/odp_udp.h | 2 +- >> test/generator/odp_generator.c | 6 +++--- >> 4 files changed, 9 insertions(+), 15 deletions(-) >> >> diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h >> index 37078b6..12ef61f 100644 >> --- a/include/helper/odp_chksum.h >> +++ b/include/helper/odp_chksum.h >> @@ -22,13 +22,10 @@ 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 >> + * @return checksum value >> */ >> static inline uint16_t odp_chksum(void *buffer, int len) >> { >> diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h >> index d8366bb..a364049 100644 >> --- a/include/helper/odp_ip.h >> +++ b/include/helper/odp_ip.h >> @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t >> pkt) >> /** >> * Calculate and fill in IPv4 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 pkt ODP packet >> * >> - * @return IPv4 checksum in host cpu order, or 0 on failure >> + * @return IPv4 checksum, or 0 on failure >> */ >> -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) >> +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) >> { >> - uint16_t res = 0; >> + uint16be_t res = 0; >> > > If checksum calculation algorithm is byteorder independent, than > no need to operate with BE types. Just change .chksum field to > uint16_t and operate in native endianness. > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
I don't think this patch is the correct solution, it re introduces the sparse warnings, which are there to indicate we have messed something up. udp->chksum is defined as BE uint16be_t chksum; Possibly that field is incorrect in our structure and should not be defined as BE, but reverting this patch mixes endian assignments again and that cannot be correct. On 15 July 2014 05:30, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 07/15/2014 11:54 AM, Weilong Chen wrote: > >> This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. >> According to https://www.ietf.org/rfc/rfc1071.txt. >> The checksum algorithm is "Byte Order Independence". >> >> Signed-off-by: Weilong Chen <weilong.chen@linaro.org> >> --- >> include/helper/odp_chksum.h | 5 +---- >> include/helper/odp_ip.h | 11 ++++------- >> include/helper/odp_udp.h | 2 +- >> test/generator/odp_generator.c | 6 +++--- >> 4 files changed, 9 insertions(+), 15 deletions(-) >> >> diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h >> index 37078b6..12ef61f 100644 >> --- a/include/helper/odp_chksum.h >> +++ b/include/helper/odp_chksum.h >> @@ -22,13 +22,10 @@ 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 >> + * @return checksum value >> */ >> static inline uint16_t odp_chksum(void *buffer, int len) >> { >> diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h >> index d8366bb..a364049 100644 >> --- a/include/helper/odp_ip.h >> +++ b/include/helper/odp_ip.h >> @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t >> pkt) >> /** >> * Calculate and fill in IPv4 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 pkt ODP packet >> * >> - * @return IPv4 checksum in host cpu order, or 0 on failure >> + * @return IPv4 checksum, or 0 on failure >> */ >> -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) >> +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) >> { >> - uint16_t res = 0; >> + uint16be_t res = 0; >> > > If checksum calculation algorithm is byteorder independent, than > no need to operate with BE types. Just change .chksum field to > uint16_t and operate in native endianness. > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
Does this issue need some attention from a wider audience ? There appears to be an incompatibility shown by the generator code between the ODP defined structures expectation of BE data, and the ODP checksum utilities that work in the native ENDIAN format. I thought that it was clearly decided that the checksum functions should be native endian, sparse appears to agree, I also take that from your comment above Taras. Weilong, I don't think I know if this is the real answer, but changing the ODP code so that icmp->chksum in the generator code is not a BE type should make everything work without reverting the patch. This still leaves us with something "twisted" - network byte order experts - any comments ? I think we need two things. 1. A hangout to review the code using the generator as the use case 2. Generate some proper unit tests for the checksum code I will schedule a HO to follow on from this thread because it is now a blocker the the l2fwd application for the demo in September, and the l2fwd with isolation work. Mike On 15 July 2014 08:13, Mike Holmes <mike.holmes@linaro.org> wrote: > I don't think this patch is the correct solution, it re introduces the > sparse warnings, which are there to indicate we have messed something up. > > udp->chksum is defined as BE uint16be_t chksum; > > Possibly that field is incorrect in our structure and should not be > defined as BE, but reverting this patch mixes endian assignments again and > that cannot be correct. > > > > > On 15 July 2014 05:30, Taras Kondratiuk <taras.kondratiuk@linaro.org> > wrote: > >> On 07/15/2014 11:54 AM, Weilong Chen wrote: >> >>> This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. >>> According to https://www.ietf.org/rfc/rfc1071.txt. >>> The checksum algorithm is "Byte Order Independence". >>> >>> Signed-off-by: Weilong Chen <weilong.chen@linaro.org> >>> --- >>> include/helper/odp_chksum.h | 5 +---- >>> include/helper/odp_ip.h | 11 ++++------- >>> include/helper/odp_udp.h | 2 +- >>> test/generator/odp_generator.c | 6 +++--- >>> 4 files changed, 9 insertions(+), 15 deletions(-) >>> >>> diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h >>> index 37078b6..12ef61f 100644 >>> --- a/include/helper/odp_chksum.h >>> +++ b/include/helper/odp_chksum.h >>> @@ -22,13 +22,10 @@ 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 >>> + * @return checksum value >>> */ >>> static inline uint16_t odp_chksum(void *buffer, int len) >>> { >>> diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h >>> index d8366bb..a364049 100644 >>> --- a/include/helper/odp_ip.h >>> +++ b/include/helper/odp_ip.h >>> @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t >>> pkt) >>> /** >>> * Calculate and fill in IPv4 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 pkt ODP packet >>> * >>> - * @return IPv4 checksum in host cpu order, or 0 on failure >>> + * @return IPv4 checksum, or 0 on failure >>> */ >>> -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) >>> +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) >>> { >>> - uint16_t res = 0; >>> + uint16be_t res = 0; >>> >> >> If checksum calculation algorithm is byteorder independent, than >> no need to operate with BE types. Just change .chksum field to >> uint16_t and operate in native endianness. >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
The checksum algorithms are endian-neutral however the various pseudo-headers that need to be prepended to the checksums are where htons() are used. See http://www.winpcap.org/pipermail/winpcap-users/2007-July/001984.html for a reasonably succinct code example. There are lots of these available and we should just pick one rather than trying to invent our own. The normative reference for this is RFC 1071 <http://tools.ietf.org/html/rfc1071>. The follow-ons (RFC 1141 <http://tools.ietf.org/html/rfc1141> and RFC 1624 <http://tools.ietf.org/html/rfc1624>) and discuss incremental updates of the checksum which is used by routers when they do things like just decrement the TTL field of the IP header since this avoids having to recalculate the entire checksum. On Tue, Jul 15, 2014 at 4:22 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > Does this issue need some attention from a wider audience ? > > There appears to be an incompatibility shown by the generator code between > the ODP defined structures expectation of BE data, and the ODP checksum > utilities that work in the native ENDIAN format. > > I thought that it was clearly decided that the checksum functions should > be native endian, sparse appears to agree, I also take that from your > comment above Taras. > > Weilong, I don't think I know if this is the real answer, but changing the > ODP code so that icmp->chksum in the generator code is not a BE type > should make everything work without reverting the patch. This still > leaves us with something "twisted" - network byte order experts - any > comments ? > > I think we need two things. > > 1. A hangout to review the code using the generator as the use case > 2. Generate some proper unit tests for the checksum code > > I will schedule a HO to follow on from this thread because it is now a > blocker the the l2fwd application for the demo in September, and the l2fwd > with isolation work. > > Mike > > > > On 15 July 2014 08:13, Mike Holmes <mike.holmes@linaro.org> wrote: > >> I don't think this patch is the correct solution, it re introduces the >> sparse warnings, which are there to indicate we have messed something up. >> >> udp->chksum is defined as BE uint16be_t chksum; >> >> Possibly that field is incorrect in our structure and should not be >> defined as BE, but reverting this patch mixes endian assignments again and >> that cannot be correct. >> >> >> >> >> On 15 July 2014 05:30, Taras Kondratiuk <taras.kondratiuk@linaro.org> >> wrote: >> >>> On 07/15/2014 11:54 AM, Weilong Chen wrote: >>> >>>> This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. >>>> According to https://www.ietf.org/rfc/rfc1071.txt. >>>> The checksum algorithm is "Byte Order Independence". >>>> >>>> Signed-off-by: Weilong Chen <weilong.chen@linaro.org> >>>> --- >>>> include/helper/odp_chksum.h | 5 +---- >>>> include/helper/odp_ip.h | 11 ++++------- >>>> include/helper/odp_udp.h | 2 +- >>>> test/generator/odp_generator.c | 6 +++--- >>>> 4 files changed, 9 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h >>>> index 37078b6..12ef61f 100644 >>>> --- a/include/helper/odp_chksum.h >>>> +++ b/include/helper/odp_chksum.h >>>> @@ -22,13 +22,10 @@ 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 >>>> + * @return checksum value >>>> */ >>>> static inline uint16_t odp_chksum(void *buffer, int len) >>>> { >>>> diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h >>>> index d8366bb..a364049 100644 >>>> --- a/include/helper/odp_ip.h >>>> +++ b/include/helper/odp_ip.h >>>> @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t >>>> pkt) >>>> /** >>>> * Calculate and fill in IPv4 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 pkt ODP packet >>>> * >>>> - * @return IPv4 checksum in host cpu order, or 0 on failure >>>> + * @return IPv4 checksum, or 0 on failure >>>> */ >>>> -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) >>>> +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) >>>> { >>>> - uint16_t res = 0; >>>> + uint16be_t res = 0; >>>> >>> >>> If checksum calculation algorithm is byteorder independent, than >>> no need to operate with BE types. Just change .chksum field to >>> uint16_t and operate in native endianness. >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> > > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
Thanks Bill, we have working checksum code as far as I can tell, did you get to compare it to the RFC ? My assumption is based on the fact that it has been working fine until we fixed the mixing of BE and LE types when assigning its result into our structures, where BE is specified. I think we need a code review to find why we apparently incorrectly specify BE at some point after the checksum is calculated. or why the "to_be" functions are not correct on some platforms. On 15 July 2014 18:52, Bill Fischofer <bill.fischofer@linaro.org> wrote: > The checksum algorithms are endian-neutral however the various > pseudo-headers that need to be prepended to the checksums are where htons() > are used. See > http://www.winpcap.org/pipermail/winpcap-users/2007-July/001984.html for > a reasonably succinct code example. There are lots of these available and > we should just pick one rather than trying to invent our own. > > The normative reference for this is RFC 1071 > <http://tools.ietf.org/html/rfc1071>. The follow-ons (RFC 1141 > <http://tools.ietf.org/html/rfc1141> and RFC 1624 > <http://tools.ietf.org/html/rfc1624>) and discuss incremental updates of > the checksum which is used by routers when they do things like just > decrement the TTL field of the IP header since this avoids having to > recalculate the entire checksum. > > > On Tue, Jul 15, 2014 at 4:22 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> Does this issue need some attention from a wider audience ? >> >> There appears to be an incompatibility shown by the generator code >> between the ODP defined structures expectation of BE data, and the ODP >> checksum utilities that work in the native ENDIAN format. >> >> I thought that it was clearly decided that the checksum functions should >> be native endian, sparse appears to agree, I also take that from your >> comment above Taras. >> >> Weilong, I don't think I know if this is the real answer, but changing >> the ODP code so that icmp->chksum in the generator code is not a BE type >> should make everything work without reverting the patch. This still >> leaves us with something "twisted" - network byte order experts - any >> comments ? >> >> I think we need two things. >> >> 1. A hangout to review the code using the generator as the use case >> 2. Generate some proper unit tests for the checksum code >> >> I will schedule a HO to follow on from this thread because it is now a >> blocker the the l2fwd application for the demo in September, and the l2fwd >> with isolation work. >> >> Mike >> >> >> >> On 15 July 2014 08:13, Mike Holmes <mike.holmes@linaro.org> wrote: >> >>> I don't think this patch is the correct solution, it re introduces the >>> sparse warnings, which are there to indicate we have messed something up. >>> >>> udp->chksum is defined as BE uint16be_t chksum; >>> >>> Possibly that field is incorrect in our structure and should not be >>> defined as BE, but reverting this patch mixes endian assignments again and >>> that cannot be correct. >>> >>> >>> >>> >>> On 15 July 2014 05:30, Taras Kondratiuk <taras.kondratiuk@linaro.org> >>> wrote: >>> >>>> On 07/15/2014 11:54 AM, Weilong Chen wrote: >>>> >>>>> This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. >>>>> According to https://www.ietf.org/rfc/rfc1071.txt. >>>>> The checksum algorithm is "Byte Order Independence". >>>>> >>>>> Signed-off-by: Weilong Chen <weilong.chen@linaro.org> >>>>> --- >>>>> include/helper/odp_chksum.h | 5 +---- >>>>> include/helper/odp_ip.h | 11 ++++------- >>>>> include/helper/odp_udp.h | 2 +- >>>>> test/generator/odp_generator.c | 6 +++--- >>>>> 4 files changed, 9 insertions(+), 15 deletions(-) >>>>> >>>>> diff --git a/include/helper/odp_chksum.h b/include/helper/odp_chksum.h >>>>> index 37078b6..12ef61f 100644 >>>>> --- a/include/helper/odp_chksum.h >>>>> +++ b/include/helper/odp_chksum.h >>>>> @@ -22,13 +22,10 @@ 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 >>>>> + * @return checksum value >>>>> */ >>>>> static inline uint16_t odp_chksum(void *buffer, int len) >>>>> { >>>>> diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h >>>>> index d8366bb..a364049 100644 >>>>> --- a/include/helper/odp_ip.h >>>>> +++ b/include/helper/odp_ip.h >>>>> @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t >>>>> pkt) >>>>> /** >>>>> * Calculate and fill in IPv4 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 pkt ODP packet >>>>> * >>>>> - * @return IPv4 checksum in host cpu order, or 0 on failure >>>>> + * @return IPv4 checksum, or 0 on failure >>>>> */ >>>>> -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) >>>>> +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) >>>>> { >>>>> - uint16_t res = 0; >>>>> + uint16be_t res = 0; >>>>> >>>> >>>> If checksum calculation algorithm is byteorder independent, than >>>> no need to operate with BE types. Just change .chksum field to >>>> uint16_t and operate in native endianness. >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> >>> >>> -- >>> *Mike Holmes* >>> Linaro Technical Manager / Lead >>> LNG - ODP >>> >> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> 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..12ef61f 100644 --- a/include/helper/odp_chksum.h +++ b/include/helper/odp_chksum.h @@ -22,13 +22,10 @@ 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 + * @return checksum value */ static inline uint16_t odp_chksum(void *buffer, int len) { diff --git a/include/helper/odp_ip.h b/include/helper/odp_ip.h index d8366bb..a364049 100644 --- a/include/helper/odp_ip.h +++ b/include/helper/odp_ip.h @@ -94,16 +94,13 @@ static inline int odp_ipv4_csum_valid(odp_packet_t pkt) /** * Calculate and fill in IPv4 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 pkt ODP packet * - * @return IPv4 checksum in host cpu order, or 0 on failure + * @return IPv4 checksum, or 0 on failure */ -static inline uint16_t odp_ipv4_csum_update(odp_packet_t pkt) +static inline uint16be_t odp_ipv4_csum_update(odp_packet_t pkt) { - uint16_t res = 0; + uint16be_t res = 0; uint16_t *w; odp_ipv4hdr_t *ip; int nleft = sizeof(odp_ipv4hdr_t); @@ -114,7 +111,7 @@ 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); + ip->chksum = res; return res; } diff --git a/include/helper/odp_udp.h b/include/helper/odp_udp.h index ae31f6b..5aedb36 100644 --- a/include/helper/odp_udp.h +++ b/include/helper/odp_udp.h @@ -89,7 +89,7 @@ static inline uint16_t odp_ipv4_udp_chksum(odp_packet_t pkt) /* set computation result */ sum = (sum == 0x0) ? 0xFFFF : sum; - return sum; + return (uint16_t)sum; } /** @internal Compile time assert */ diff --git a/test/generator/odp_generator.c b/test/generator/odp_generator.c index 8f6ad49..fd85455 100644 --- a/test/generator/odp_generator.c +++ b/test/generator/odp_generator.c @@ -212,7 +212,7 @@ static void pack_udp_pkt(odp_buffer_t obuf) udp->dst_port = 0; udp->length = odp_cpu_to_be_16(args->appl.payload + ODP_UDPHDR_LEN); udp->chksum = 0; - udp->chksum = odp_cpu_to_be_16(odp_ipv4_udp_chksum(pkt)); + udp->chksum = odp_ipv4_udp_chksum(pkt); odp_packet_set_len(pkt, args->appl.payload + ODP_UDPHDR_LEN + ODP_IPV4HDR_LEN + ODP_ETHHDR_LEN); } @@ -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);
This reverts commit acdbb05449d8d6f80caf39d035e6a6c7a5e2dabb. According to https://www.ietf.org/rfc/rfc1071.txt. The checksum algorithm is "Byte Order Independence". Signed-off-by: Weilong Chen <weilong.chen@linaro.org> --- include/helper/odp_chksum.h | 5 +---- include/helper/odp_ip.h | 11 ++++------- include/helper/odp_udp.h | 2 +- test/generator/odp_generator.c | 6 +++--- 4 files changed, 9 insertions(+), 15 deletions(-)