[quagga-dev 8816] Re: [PATCH 1/2] lib: Optimize Fletcher and Internet checksums

Joakim Tjernlund joakim.tjernlund at transmode.se
Thu Sep 8 12:40:22 BST 2011


>
> David Lamparter <equinox at diac24.net> wrote on 2011/08/31 17:29:02:
> >
> > From: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> >
> > Did some simple optimizations to reduce the number
> > of instructions in the hot path for both fletcher_checksum()
> > and in_cksum().
> > ---
> >  lib/checksum.c |   53 +++++++++++++++++++----------------------------------
> >  1 files changed, 19 insertions(+), 34 deletions(-)
> ...
>
> > -            /* mop up an odd byte, if necessary */
> > -   if (nbytes == 1) {
> > -      oddbyte = 0;      /* make sure top half is zero */
> > -      *((u_char *) &oddbyte) = *(u_char *)ptr;   /* one byte only */
> > -      sum += oddbyte;
> > -   }
> > +   count = nbytes >> 1; /* div by 2 */
> > +   for(ptr--; count; --count)
> > +     sum += *++ptr;
> > +
> > +   if (nbytes & 1) /* Odd */
> > +     sum += *(u_char *)(++ptr);   /* one byte only */
> >
>
> Just found out that the odd byte handling in my patch is wrong for
> BE archs(even the RFC got this wrong apparently) See
>  http://lists.busybox.net/pipermail/busybox/2011-September/076598.html
> for some clues. I haven't looked deeper into this though.
>
> It should be as before:
>    if (nbytes & 1) {
>       oddbyte = 0;      /* make sure top half is zero */
>       *((u_char *) &oddbyte) = *(u_char *)ptr;   /* one byte only */
>       sum += oddbyte;
>    }

Actually, the odd type casting confuses gcc and generates bad code. Using
an union instead is better:

   if (nbytes & 1) {
       union uu {
	    u_short val;
	    u_char b[2];
	  } oddbyte = {0};
        /* Make sure that the left-over byte is added correctly both
         * with little and big endian hosts */
      oddbyte.b[0] = *(u_char *)ptr;   /* one byte only */
      sum += oddbyte.val;
   }





More information about the Quagga-dev mailing list