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

[Kieran Mansley]

On Thu, 25 Sep 2003, Paul Riley wrote:

> 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.

Interesting.  I've been looking at the checksum code today too, and
wondering about ways to make it faster.  I expect most of those would
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?

> Before I fix this has anyone else already done this?

Don't think so.  You can see the cutting edge source by checking out the
DEVEL tagged CVS tree - it's particularly useful if you do this before
making changes and submitting patches as otherwise the patches often fail
to apply.

> In addition I have already fixed a bug in the function
> update_arp_entry if ARPing for an address and hence a packet
> was queued for later transmission. On receipt of the reply
> the frame is sent, but without a correct source address. This
> is easily fixed with the change shown below
>
>           for (k = 0; k < netif->hwaddr_len; ++k) {
>             ethhdr->dest.addr[k] = ethaddr->addr[k];
> >>>>>            ethhdr->src.addr[k] = netif->hwaddr[k];
>           }

Thanks!  Will try to remember to add that when I get the chance.

> Lastly does anyone know how to get CVS write access. I would
> like to add in a new architecture port?

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.

> 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.

Thanks

Kieran