Re: [U-Boot] [PATCH]Fix checksum to handle odd-length packet

3 Dec 2009


      Thanks.
It was not a clean solution as I only experimented on our processor
which is big-endian.
The fact is that the original code is not endianess proof. It was coded
for big-endian processors.
Greg Ren
-----Original Message-----
From: Joakim Tjernlund [mailto:joakim.tjernlund@transmode.se] 
Sent: Thursday, December 03, 2009 6:41 AM
To: Wolfgang Denk
Cc: Greg Ren; u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH]Fix checksum to handle odd-length packet
Wolfgang Denk wd@denx.de wrote on 03/12/2009 15:08:24:
...
Dear Joakim Tjernlund,
In message <OFDFF8A95F.374EB267-ONC1257681.0042613C-C1257681.
004485DD@transmode.se> you wrote:
...
...
...

  if (len == 1) {


          xsum += (*p & 0xff00);


I doubt that this code is endianess-clean.
Nope, I would think some thing like this would work better:
      count = len >> 1; /* div by 2 */
   for(p--; count; --count)
     xsum += *++p;
if (len & 1) /* Odd */
     xsum += *(u_char *)(++p);   /* one byte only */
It probably depends on the definition of "better" ;-)
Running this test code:

#include <stdio.h>
#include <string.h>
#define LENGTH   6
int main (void)
{
   char string[LENGTH] = { 1, 2, 3, 4, 5, 0, };
   unsigned short array[LENGTH/2];
   unsigned short *p;
   unsigned short xsum, xsum1, xsum2;
ulong, not short :)
...
int i, count;
memcpy (array, string, LENGTH);
count = LENGTH / 2;
xsum = 0;
   p = array;
while (count > 1) {
      printf ("Adding 0x%04x\n", *p);
      xsum += *p++;
      --count;
   }
for(p--; count; --count)
      xsum += *++p;
if (LENGTH & 1) /* Odd */
      xsum += *(unsigned char *)(++p);   /* one byte only */
...
xsum1 = xsum + (*p & 0xff00);
ehh, this has to go.
...
xsum2 = xsum + *(unsigned char *)(p);
You are not folding the sum, unsure if that has any significans
...
printf ("*p = 0x%04x\n", *p);
   printf ("xsum = %04x,  xsum1 = %04x,  xsum2 = %04x\n",
      xsum, xsum1, xsum2);
return (0);
}

on a little endian system gives:
-> ./f-le
   Adding 0x0201
   Adding 0x0403
   *p = 0x0005
   xsum = 0604,  xsum1 = 0604,  xsum2 = 0609
while running it on a big endian system gives
-> ./f-be
   Adding 0x0102
   Adding 0x0304
   *p = 0x0500
   xsum = 0406,  xsum1 = 0906,  xsum2 = 040b
I don't know what you consider to be the exact result,  but  fact  is
that  even if we ignore byte swapping, none of the implementations is
endianess clean.
Of course, there is a chance that I mis-implemented your suggestion.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
       There is enough for the need of everyone in this world,
       but not for the greed of everyone.     - Mahatma Gandhi