Hi Ilya, I checked the changes you suggested. It's Ok. I sent a new patch(v7).
Tkanks, Grig -----Original Message----- From: Grigore Ion-B17953 Sent: Friday, December 11, 2015 5:57 PM To: 'Ilya Maximets' <i.maxim...@samsung.com>; Maxim Uvarov <maxim.uva...@linaro.org>; lng-odp@lists.linaro.org Subject: RE: [PATCHv6] helper : Fix UDP checksum computation Ok. I'll check your proposal on BE/LE. -----Original Message----- From: Ilya Maximets [mailto:i.maxim...@samsung.com] Sent: Friday, December 11, 2015 5:54 PM To: Grigore Ion-B17953 <ion.grig...@freescale.com>; Maxim Uvarov <maxim.uva...@linaro.org>; lng-odp@lists.linaro.org Subject: Re: [PATCHv6] helper : Fix UDP checksum computation On 11.12.2015 18:47, Ion Grigore wrote: > A more precise question. Did you check on a LE platform ? Yes. On my LE i7. >>>>> - /* Fold sum to 16 bits: add carrier to result */ >>>>> - while (sum >> 16) >>>>> - sum = (sum & 0xFFFF) + (sum >> 16); >>>>> - >>>>> + (iph->dst_addr & 0xFFFF) + (iph->dst_addr >> 16) + >>>>> + u8_pad_to_u16(iph->proto, 1) + udph->length; >>>> I meant something like: >>>> >>>> union { >>>> uint8_t v8[2]; >>>> uint16_t v16; >>>> } val; >>>> >>>> val.v8[0] = 0; >>>> val.v8[1] = iph->proto; >>>> >>> >>> The val.val16 must be swapped on LE platforms. >> >> NO. >> >> Did you checked this ? > > And you? > >> >>> >>> >>>> sum = (iph->src_addr & 0xFFFF) + (iph->src_addr >> 16) + >>>> (iph->dst_addr & 0xFFFF) + (iph->dst_addr >> 16) + >>>> val.v16 + udph->length; >>>> >>>>> + udplen = odp_be_to_cpu_16(udph->length); >>>>> + buf = (uint16_t *)((void *)udph); >>>>> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */ >>>>> + for ( ; udplen > 1; udplen -= 2) >>>>> + sum += *buf++; >>>>> + /* Length is not a multiple of 2 bytes */ >>>>> + if (udplen) >>>>> + sum += u8_pad_to_u16(*buf, 0); >>>> val.v8[0] = *buf; >>>> val.v8[1] = 0; >>>> >>>> sum += val.v16; >>>> >>>> Isn't it more understandable, then artificial byte swapping? >>>> >>> >>> The val.val16 must be swapped on LE platforms. >> >> NO. >> >> Did you checked this ? > > And you? > > P.S. Just to be honest, I checked this a few minutes ago. > Helper test below works. > > Yes on BE/LE platforms, but not with the change you proposed. So why you talking that val.val16 must be swapped if you do not understand how it works and you have not tested it? _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp