On 12/07/2015 16:35, Ion Grigore wrote:
Hi Maxim,
I noticed you merged this patch and then you reverted it because the validation
test fails.
The problem is in the test not in the patch.
odp_checksum.c from line 192
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 != odp_be_to_cpu_16(0x7e5a))
status = -1;
Here is the packet used in the test :
0000 fe 0f 97 c9 e0 44 fe 0f 97 c9 e0 44 08 00 45 00
0010 00 34 00 01 00 00 00 11 39 65 c0 a8 00 01 c0 a8
0020 00 02 00 00 00 00 00 20 7e 5a 00 00 00 00 00 00
0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0040 00 00
The UDP checksum is 0x7e5a (in BE) and this is confirmed by Wireshark.
Thanks,
Grig
Yes, odp_be_to_cpu_16(0x7e5a) is logical, but change from 0xab2d to
0x7e5a means what we had really bad code before.
Grig, all patches should pass validation test. Can you please send
updated version with test case fixes included. You can also add
Bills review for it as he did it for main patch.
Thanks,
Maxim.
-----Original Message-----
From: lng-odp [mailto:[email protected]] On Behalf Of Maxim
Uvarov
Sent: Monday, December 07, 2015 1:09 PM
To: [email protected]
Subject: Re: [lng-odp] [PATCHv2] helper : Fix UDP checksum computation
Merged,
Maxim.
On 12/04/2015 18:20, Bill Fischofer wrote:
On Thu, Oct 22, 2015 at 7:02 AM, <[email protected]
<mailto:[email protected]>> wrote:
From: Grigore Ion <[email protected]
<mailto:[email protected]>>
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.
Signed-off-by: Grigore Ion <[email protected]
<mailto:[email protected]>>
Reviewed-by: Bill Fischofer <[email protected]
<mailto:[email protected]>>
---
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 | 32
+++++++++++++-------------------
1 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/helper/include/odp/helper/udp.h
b/helper/include/odp/helper/udp.h
index 06c439b..6fce3f2 100644
--- a/helper/include/odp/helper/udp.h
+++ b/helper/include/odp/helper/udp.h
@@ -44,20 +44,16 @@ 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;
+ uint32_t sum;
odph_udphdr_t *udph;
odph_ipv4hdr_t *iph;
- uint16_t udplen;
- uint8_t *buf;
+ uint16_t udplen, *buf;
- if (!odp_packet_l3_offset(pkt))
- return 0;
-
- 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);
@@ -67,23 +63,21 @@ static inline uint16_t
odph_ipv4_udp_chksum(odp_packet_t pkt)
/* 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 = (uint8_t *)udph; udplen > 1; udplen -= 2) {
- sum += ((*buf << 8) + *(buf + 1));
- buf += 2;
- }
+ odp_be_to_cpu_16(iph->proto) + udph->length;
+ for (buf = (uint16_t *)((void *)udph); udplen > 1; udplen
-= 2)
+ sum += *buf++;
+ if (udplen) /* If length is not a multiple of 2 bytes */
+ sum += odp_be_to_cpu_16(*((uint8_t *)buf) << 8);
/* Fold sum to 16 bits: add carrier to result */
- while (sum >> 16)
- sum = (sum & 0xFFFF) + (sum >> 16);
+ sum = (sum & 0xFFFF) + (sum >> 16);
+ sum += (sum >> 16);
/* 1's complement */
sum = ~sum;
- /* set computation result */
- sum = (sum == 0x0) ? 0xFFFF : sum;
-
- return sum;
+ /* set computation result in CPU endianness*/
+ return (sum == 0x0) ? 0xFFFF : odp_be_to_cpu_16(sum);
}
/** @internal Compile time assert */
--
1.7.3.4
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
