Re: [lwip-devel] Bug in Checksum implementation and ARP fix

[Paul Riley]

Leon,

Two points first and foremost you stated.

"I assumed our compiler was the problem,
but the cast to a u16_t makes dataptr required to be on a 16-bit boundary,
right?"

I'm afraid this is wrong, what it tells the compiler is access this memory 
location as a 16 bit value. The memory location/address is determined at 
run-time and the compiler has no control over this. This will work on some 
processor architectures such as x86 which allow 16 bit accesses on unaligned 
addresses, on other architectures eg. ARM you will get an exception, and on 
other architectures you just get the wrong value.

Secondly you asked for my Savannah ID to get CVS access I'm showing my 
ignorance here, but what's one of those and where do I get one from?

Thanks very much,

Paul


Date: Thu, 25 Sep 2003 23:07:30 +0200
From: "Leon Woestenberg" <address@hidden>
Subject: Re: [lwip-devel] Bug in  Checksum implementation and ARP fix
To: "lwip-devel" <address@hidden>
Message-ID: <address@hidden>
Content-Type: text/plain;       charset="iso-8859-1"

Hello Paul, Kieran,

> > There is a bug in the implementation of lwip_chksum.
> > Basically it assumes that you can perform 16 bit access on
> > non 16 bit aligned address
> >
> > The offending line is
> >
> >     acc += *(u16_t *)dataptr;
> >
> > This is not valid on all processor architectures.
>
Funny. We had a local patch on this. I assumed our compiler was the problem,
but the cast to a u16_t makes dataptr required to be on a 16-bit boundary,
right?

> break some architectures too.  Maybe we should move the checksum code into
> an architecture specific place in the source tree, and keep a
> generic-should-work-on-most implementation (ie. the one there is
> already) as the default.  Anyone got opinions on this?
>
Yup. Could we do it the same way as we handle htonl() etc? I think
we used macro's there.

> >           for (k = 0; k < netif->hwaddr_len; ++k) {
> >             ethhdr->dest.addr[k] = ethaddr->addr[k];
> > >>>>>            ethhdr->src.addr[k] = netif->hwaddr[k];
> >           }
>
Thanks, good bug hunting!

> > Lastly does anyone know how to get CVS write access. I would
> > like to add in a new architecture port?
>
No problem, finding two bugs qualifies you for CVS access :-)

BTW, I am very interested to hear which architecture you have ported
to. Wouldn't that be Altera's NIOS?

Please email your Savannah ID and I can add you to the cvs committers.

Please make sure to make changes solely in the DEVEL branch!

> I'll let Leon answer this one.  I'll be interested to see what the
> architecture is - it's good to hear that a company like Altera is using
> lwip.
>
We already had Xilinx on the list, so a bit of competition is never bad :-)

> > P.S. I can send patches in diff -u format if required
>
> In general, that's useful, but for fixes as small as this probably
> overkill.  Easiest thing to do is send mail (as you have done) and submit
> a bug report on savannah to ensure it doesn't get forgotten about.
>
Fully agreed.

Regards,

Leon Woestenberg.