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

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

[Greg Ren]

Hi all:

I am new to u-boot and got assignment to debug some networking issue. I
traced the checksum failure and was able to fix it with the patch below.

This change fixes:
1. The checksum generation failure for odd-length packet.
2. Do not ignore the LSB of checksum value - (Again, I am new to this.
Maybe someone can educate me about the idea of ignoring one bit of
checksum)

The patch is a git commit log from my local git reposite.

Thanks for your time and advice.

% git show cffd5fb03e0c3f116cce9f3ff825c5445a1eca3f
commit cffd5fb03e0c3f116cce9f3ff825c5445a1eca3f
Author: gren <gren@ubicom.com>
Date:   Wed Dec 2 13:07:53 2009 -0800

    Fix issue with checksum failure of odd-length packets

diff --git a/net/net.c b/net/net.c
old mode 100644
new mode 100755
index 5637cf5..5bbee04
--- a/net/net.c
+++ b/net/net.c
@@ -703,14 +703,14 @@ int PingSend(void)
        ip->ip_sum   = 0;
        NetCopyIP((void*)&ip->ip_src, &NetOurIP); /* already in network
byte order */
        NetCopyIP((void*)&ip->ip_dst, &NetPingIP);         /* - "" - */
-       ip->ip_sum   = ~NetCksum((uchar *)ip, IP_HDR_SIZE_NO_UDP / 2);
+       ip->ip_sum   = ~NetCksum((uchar *)ip, IP_HDR_SIZE_NO_UDP);

        s = &ip->udp_src;               /* XXX ICMP starts here */
        s[0] = htons(0x0800);           /* echo-request, code */
        s[1] = 0;                       /* checksum */
        s[2] = 0;                       /* identifier */
        s[3] = htons(PingSeqNo++);      /* sequence number */
-       s[1] = ~NetCksum((uchar *)s, 8/2);
+       s[1] = ~NetCksum((uchar *)s, 8);

        /* size of the waiting packet */
        NetArpWaitTxPacketSize = (pkt - NetArpWaitTxPacket) +
IP_HDR_SIZE_NO_UDP + 8;
@@ -1363,7 +1363,7 @@ NetReceive(volatile uchar * inpkt, int len)
                if ((ip->ip_hl_v & 0x0f) > 0x05) {
                        return;
                }
-               if (!NetCksumOk((uchar *)ip, IP_HDR_SIZE_NO_UDP / 2)) {
+               if (!NetCksumOk((uchar *)ip, IP_HDR_SIZE_NO_UDP)) {
                        puts ("checksum bad\n");
                        return;
                }
@@ -1420,12 +1420,12 @@ NetReceive(volatile uchar * inpkt, int len)
                                ip->ip_off = 0;
                                NetCopyIP((void*)&ip->ip_dst,
&ip->ip_src);
                                NetCopyIP((void*)&ip->ip_src,
&NetOurIP);
-                               ip->ip_sum = ~NetCksum((uchar *)ip,
IP_HDR_SIZE_NO_UDP >> 1);
+                               ip->ip_sum = ~NetCksum((uchar *)ip,
IP_HDR_SIZE_NO_UDP);

                                icmph->type = ICMP_ECHO_REPLY;
                                icmph->checksum = 0;
                                icmph->checksum = ~NetCksum((uchar
*)icmph,
-                                               (len -
IP_HDR_SIZE_NO_UDP) >> 1);
+                                               (len -
IP_HDR_SIZE_NO_UDP));
                                (void) eth_send((uchar *)et,
ETHER_HDR_SIZE + len);
                                return;
 #endif
@@ -1577,7 +1577,7 @@ static int net_check_prereq (proto_t protocol)
 int
 NetCksumOk(uchar * ptr, int len)
 {
-       return !((NetCksum(ptr, len) + 1) & 0xfffe);
+       return (NetCksum(ptr, len) == 0xffff);
 }


@@ -1588,8 +1588,13 @@ NetCksum(uchar * ptr, int len)
        ushort *p = (ushort *)ptr;

        xsum = 0;
-       while (len-- > 0)
+       while (len > 1) {
                xsum += *p++;
+               len -= 2;
+       }
+       if (len == 1) {
+               xsum += (*p & 0xff00);
+       }
        xsum = (xsum & 0xffff) + (xsum >> 16);
        xsum = (xsum & 0xffff) + (xsum >> 16);
        return (xsum & 0xffff);
@@ -1663,7 +1668,7 @@ NetSetIP(volatile uchar * xip, IPaddr_t dest, int
dport, int sport, int len)
        ip->udp_dst  = htons(dport);
        ip->udp_len  = htons(8 + len);
        ip->udp_xsum = 0;
-       ip->ip_sum   = ~NetCksum((uchar *)ip, IP_HDR_SIZE_NO_UDP / 2);
+       ip->ip_sum   = ~NetCksum((uchar *)ip, IP_HDR_SIZE_NO_UDP);
 }

 void copy_filename (char *dst, char *src, int size)

Regards

Greg Ren
SW Engineer
Phone: (408)649-2703
195 Baypointe Pkwy. San Jose CA 95134

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

[Wolfgang Denk]

Dear "Greg Ren",

In message <CB2DD11991B27C4F99935E6229450D3204E5CB91@STORK.scenix.com> you wrote:
> 
> I am new to u-boot and got assignment to debug some networking issue. I
> traced the checksum failure and was able to fix it with the patch below.

It would be important to know on which system(s) you have actually
tested your patch - and on which you experienced any issues in the
first place. Please mention CPU, board, and network driver used.

> The patch is a git commit log from my local git reposite.
> 
> Thanks for your time and advice.
> 
> % git show cffd5fb03e0c3f116cce9f3ff825c5445a1eca3f

Please use git-format-patch / git-send-email to submit patches, see
http://www.denx.de/wiki/U-Boot/Patches for details.


> @@ -1420,12 +1420,12 @@ NetReceive(volatile uchar * inpkt, int len)
>                                 ip->ip_off = 0;
>                                 NetCopyIP((void*)&ip->ip_dst,
> &ip->ip_src);
>                                 NetCopyIP((void*)&ip->ip_src,
> &NetOurIP);
> -                               ip->ip_sum = ~NetCksum((uchar *)ip,
> IP_HDR_SIZE_NO_UDP >> 1);
> +                               ip->ip_sum = ~NetCksum((uchar *)ip,
> IP_HDR_SIZE_NO_UDP);

Your mailer has line-wrapped the patch which makes it useless.

> +       if (len == 1) {
> +               xsum += (*p & 0xff00);

I doubt that this code is endianess-clean.

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 at denx.de
This restaurant was advertising breakfast  any  time.  So  I  ordered
french toast in the renaissance.            - Steven Wright, comedian

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

[Joakim Tjernlund]

> Dear "Greg Ren",
>
> In message <CB2DD11991B27C4F99935E6229450D3204E5CB91@STORK.scenix.com> you wrote:
> >
> > I am new to u-boot and got assignment to debug some networking issue. I
> > traced the checksum failure and was able to fix it with the patch below.
>
> It would be important to know on which system(s) you have actually
> tested your patch - and on which you experienced any issues in the
> first place. Please mention CPU, board, and network driver used.
>
> > The patch is a git commit log from my local git reposite.
> >
> > Thanks for your time and advice.
> >
> > % git show cffd5fb03e0c3f116cce9f3ff825c5445a1eca3f
>
> Please use git-format-patch / git-send-email to submit patches, see
> http://www.denx.de/wiki/U-Boot/Patches for details.
>
>
> > @@ -1420,12 +1420,12 @@ NetReceive(volatile uchar * inpkt, int len)
> >                                 ip->ip_off = 0;
> >                                 NetCopyIP((void*)&ip->ip_dst,
> > &ip->ip_src);
> >                                 NetCopyIP((void*)&ip->ip_src,
> > &NetOurIP);
> > -                               ip->ip_sum = ~NetCksum((uchar *)ip,
> > IP_HDR_SIZE_NO_UDP >> 1);
> > +                               ip->ip_sum = ~NetCksum((uchar *)ip,
> > IP_HDR_SIZE_NO_UDP);
>
> Your mailer has line-wrapped the patch which makes it useless.
>
> > +       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 */

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

[Wolfgang Denk]

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;
	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;
	}

	xsum1 = xsum + (*p & 0xff00);

	xsum2 = xsum + *(unsigned char *)(p);

	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 at denx.de
       There is enough for the need of everyone in this world,
       but not for the greed of everyone.     - Mahatma Gandhi

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

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 03/12/2009 15:08:24:

> Dear Joakim Tjernlund,
>
> In message <OFDFF8A95F.374EB267-ONC1257681.0042613C-C1257681.
> 004485DD at 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 at denx.de
>        There is enough for the need of everyone in this world,
>        but not for the greed of everyone.     - Mahatma Gandhi
>

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

[Greg Ren]

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 at transmode.se] 
Sent: Thursday, December 03, 2009 6:41 AM
To: Wolfgang Denk
Cc: Greg Ren; u-boot at 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 at 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 at denx.de
>        There is enough for the need of everyone in this world,
>        but not for the greed of everyone.     - Mahatma Gandhi
>

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

[Greg Ren]

Ah. I just realize that the endianess can be supported as simple as:

In stead of this:
               xsum += (*p & 0xff00);
Use this:
               xsum += (*p & ntohs(0xff00));

Greg Ren

-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de] 
Sent: Thursday, December 03, 2009 3:31 AM
To: Greg Ren
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH]Fix checksum to handle odd-length packet

Dear "Greg Ren",

In message <CB2DD11991B27C4F99935E6229450D3204E5CB91@STORK.scenix.com>
you wrote:
> 
> I am new to u-boot and got assignment to debug some networking issue.
I
> traced the checksum failure and was able to fix it with the patch
below.

It would be important to know on which system(s) you have actually
tested your patch - and on which you experienced any issues in the
first place. Please mention CPU, board, and network driver used.

> The patch is a git commit log from my local git reposite.
> 
> Thanks for your time and advice.
> 
> % git show cffd5fb03e0c3f116cce9f3ff825c5445a1eca3f

Please use git-format-patch / git-send-email to submit patches, see
http://www.denx.de/wiki/U-Boot/Patches for details.


> @@ -1420,12 +1420,12 @@ NetReceive(volatile uchar * inpkt, int len)
>                                 ip->ip_off = 0;
>                                 NetCopyIP((void*)&ip->ip_dst,
> &ip->ip_src);
>                                 NetCopyIP((void*)&ip->ip_src,
> &NetOurIP);
> -                               ip->ip_sum = ~NetCksum((uchar *)ip,
> IP_HDR_SIZE_NO_UDP >> 1);
> +                               ip->ip_sum = ~NetCksum((uchar *)ip,
> IP_HDR_SIZE_NO_UDP);

Your mailer has line-wrapped the patch which makes it useless.

> +       if (len == 1) {
> +               xsum += (*p & 0xff00);

I doubt that this code is endianess-clean.

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 at denx.de
This restaurant was advertising breakfast  any  time.  So  I  ordered
french toast in the renaissance.            - Steven Wright, comedian

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

[Joakim Tjernlund]

>
> Ah. I just realize that the endianess can be supported as simple as:
>
> In stead of this:
>                xsum += (*p & 0xff00);
> Use this:
>                xsum += (*p & ntohs(0xff00));

Did you look at the suggestion I sent you?
I know it works because I use in ospf.

 Jocke

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

[Greg Ren]

I must have missed it. The only suggestion that I remember was referring
to how to generate patch using git email. As for technical discussion,
it was ended as "not endianess clean".

Greg Ren


-----Original Message-----
From: Joakim Tjernlund [mailto:joakim.tjernlund at transmode.se] 
Sent: Tuesday, December 08, 2009 3:43 PM
To: Greg Ren
Cc: u-boot at lists.denx.de; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH]Fix checksum to handle odd-length packet

>
> Ah. I just realize that the endianess can be supported as simple as:
>
> In stead of this:
>                xsum += (*p & 0xff00);
> Use this:
>                xsum += (*p & ntohs(0xff00));

Did you look at the suggestion I sent you?
I know it works because I use in ospf.

 Jocke

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

[Ben Warren]

Hi Greg,

Greg Ren wrote:
> I must have missed it. The only suggestion that I remember was referring
> to how to generate patch using git email. As for technical discussion,
> it was ended as "not endianess clean".
>
>   
Please don't top post.
While I agree that the network code should handle this properly, how do 
you end up with odd-length packets?  U-boot's been around for years and 
nobody else has had this problem...

regards,
Ben

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

[Greg Ren]

Hi Ben:

Ben Warren wrote:
> Please don't top post.
> While I agree that the network code should handle this properly, how
do 
> you end up with odd-length packets?  U-boot's been around for years
and 
> nobody else has had this problem...

I simply ping it with odd length. I also checked the latest u-boot
online and the issue has not been fixed. It could be that most usage had
involved only with even length packet, and no one had seen this issue.
It is a boot loader and may not get used as often as some popular
applications.

Regards

Greg

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

[Jerry Van Baren]

Hi Greg,

Greg Ren wrote:
> Hi Ben:
> 
> Ben Warren wrote:
>> Please don't top post.
>> While I agree that the network code should handle this properly, how
> do 
>> you end up with odd-length packets?  U-boot's been around for years
> and 
>> nobody else has had this problem...
> 
> I simply ping it with odd length. I also checked the latest u-boot
> online and the issue has not been fixed. It could be that most usage had
> involved only with even length packet, and no one had seen this issue.
> It is a boot loader and may not get used as often as some popular
> applications.
> 
> Regards
> 
> Greg

When you respin your patch, please put the "ping with an odd length 
causes an incorrect checksum bug" in the commit message for future 
reference.

Thanks,
gvb

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

[Greg Ren]

Jerry Van Baren wrote:

> When you respin your patch, please put the "ping with an odd length 
> causes an incorrect checksum bug" in the commit message for future 
> reference.

> Thanks,
> gvb

The change was to the general checksum calculation. So there is enough
reason to believe that any odd-length packet may suffer the same fate.
Ping is just an easier way to test and verify the fix.

regards
Greg Ren

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

[Jerry Van Baren]

Greg Ren wrote:
> Jerry Van Baren wrote:
> 
>> When you respin your patch, please put the "ping with an odd length 
>> causes an incorrect checksum bug" in the commit message for future 
>> reference.
> 
>> Thanks,
>> gvb
> 
> The change was to the general checksum calculation. So there is enough
> reason to believe that any odd-length packet may suffer the same fate.
> Ping is just an easier way to test and verify the fix.
> 
> regards
> Greg Ren

Understood.  My point was your original commit message did not identify 
*how* to exercise the bug (i.e. ping with an odd length packet).  That 
is very valuable information because it helps us remember what was 
broken and how to test both the brokenness and the fix.

Thanks,
gvb