Thu 06 May 2010 02:55:42 PM UTC, comment #12:
I download the source code from cvs, and the patch work fine with avr32.
Thanks a lot
Juan
|
Wed 05 May 2010 12:42:51 PM UTC, comment #11:
If htons is a macro creating C code, the compiler will constant fold the redundant shift/ORs away. The advantage of #if/#else/#endif is there is nothing for the compiler to do. If htons generates inline assembly then there is waste because the compiler will be forced to generate the shift/OR and then the inline assembly which for little endian has to undo the shift/OR.
The following is legit too and a conforming compiler MUST not generate the test or code for the false half:
chk_sum = (BYTE_ORDER == LITTLE_ENDIAN) ? (proto << 8) | ttl : (ttl << 8) | proto;
|
Wed 05 May 2010 04:42:33 AM UTC, comment #10:
> I guess too it simply could be chk_sum = htons( (proto << 8) | ttl );
That would work, too, but for little endian, it would be shifting in the wrong direction and then shifting back again in htons(), wouldn't it?
|
Tue 04 May 2010 08:32:50 PM UTC, comment #9:
>That's what I meant with the "patch didn't apply to CVS HEAD" :-)
Yeah, I saw what that meant pretty quickly :-)
>Fixed by adding an endian-dependent macro to create an u16_t in def.h.
I guess too it simply could be chk_sum = htons( (proto << 8) | ttl );
|
Tue 04 May 2010 07:47:52 PM UTC, comment #8:
> When you do the patch, I download the source code from cvs to
> test it in the avr32.
When? It's already done in CVS.
|
Tue 04 May 2010 07:25:01 PM UTC, comment #7:
Hi Simon,
When you do the patch, I download the source code from cvs to test it in the avr32.
Regards,
Juan
|
Tue 04 May 2010 07:00:01 PM UTC, comment #6:
> The field _ttl_prot, don't exist in the struct ip_hdr, it has
> two 8 bits field: (proto, and ttl).
That's what I meant with the "patch didn't apply to CVS HEAD" :-) And, no, I was testing it with little endian (x86), and it worked.
Fixed by adding an endian-dependent macro to create an u16_t in def.h.
Thanks for reporting and testing!
|
Tue 04 May 2010 05:42:58 PM UTC, comment #5:
Ok, thanks Juan.
Simon, I think we need:
#if BYTE_ORDER == LITTLE_ENDIAN
chk_sum = (proto << 8) | ttl;
#else
chk_sum = (ttl<<8) | proto;
#endif
|
Tue 04 May 2010 04:48:48 PM UTC, comment #4:
Hi Bill,
The avr32 is big endian cpu.
The field _ttl_prot, don't exist in the struct ip_hdr, it has two 8 bits field: (proto, and ttl).
I change the order of [chk_sum = (proto << 8) | ttl;] for [chk_sum = (ttl<<8) | proto;], and work fine.
Thanks a lot.
|
Tue 04 May 2010 04:28:01 PM UTC, comment #3:
chk_sum = iphdr->_ttl_proto;
is correct for big and little endian - I tested both. What was checked in [chk_sum = (proto << 8) | ttl; ] apparently does not work for little endian? Is Avr32 little endian?
Simon, I assumed your changes where for #define names and format and other consistency things - I didn't check the actual C code.
Juan, can you plug in the line above [chk_sum = iphdr->_ttl_proto;] and take out your += 0x7F8 and see how it goes?
|
Tue 04 May 2010 03:58:32 PM UTC, comment #2:
That's because the original patch didn't apply to CVS HEAD and I seem to have forgotten about big-endian as it works for me??
|
Tue 04 May 2010 01:14:46 PM UTC, comment #1:
The code in CVS HEAD is not quite the same as the original patch. On line 600 (CVS HEAD version) it had:
chk_sum = iphdr->_ttl_proto;
It's checked in as:
chk_sum = (proto << 8) | ttl;
Your += 0x7F8 looks like it might be compensating for this difference.
|
Mon 03 May 2010 11:22:50 PM UTC, original submission:
Hi, I download the source code of lwip from the cvs, and port to the avr32 framework of Atmel, and found that checksum of IP header has wrong values when the CHECKSUM_GEN_IP_INLINE is defined to one. If I do iphdr->_chksum += 0x7F8; the checksum is good.
Thanks in advance,
Juan
|