buglwIP - A Lightweight TCP/IP stack - Bugs: bug #36153, TCP Cheksum error if ...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #36153: TCP Cheksum error if LWIP_CHECKSUM_ON_COPY=1

Submitted by:  Alexander <pronkin>
Submitted on:  Thu 12 Apr 2012 07:58:16 AM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.4.0

(Jump to the original submission Jump to the original submission)

Fri 14 Mar 2014 06:48:07 PM UTC, comment #9:

Finally fixed, thanks for reporting.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 16 Apr 2012 01:10:55 PM UTC, comment #8:

I finally realized what going on.
It's lwip bug(. checksum of delayed tcp_seg not correctly summed with new sended data. Here is example with my comments:

//my test values for reproduce
#define TCP_SND_BUF (24)
#define TCP_MSS (10) //yes, I know that TCP_MSS must be more then 536, but it's for easily debug only
#define LWIP_CHECKSUM_ON_COPY 1
#define TCP_CHECKSUM_ON_COPY_SANITY_CHECK 1

with this options sending 27 bytes of any data will fail

log:
TCP connection request 4163 -> 80. //new client connected
tcp_enqueue_flags: queueing 6510:6511 (0x12)
tcp_output_segment: 6510:6510
TCP connection established 4163 -> 80.
tcp_output: nothing to send (0x0) //here we send 24 bytes of data
tcp_write(pcb=0x200019a8, data=0x80273a0, len=24, apiflags=2) //24 writes because of TCP_SND_BUF limit
tcp_write: queueing 6511:6521 //full segment1 len = 10
tcp_write: queueing 6521:6531 //full segment2 len = 10
tcp_write: queueing 6531:6535 //NOT full segment3 len = 4 (will wait for TCP_MORE_DATA)
tcp_output_segment: 6511:6521 //segment1 out
tcp_output_segment: 6521:6531 //segment2 out
tcp_write(pcb=0x200019a8, data=0x80273b8, len=3, apiflags=0) //after ACK received we write remaining data (3 bytes)
tcp_output_segment: 6531:6538 //it's segment with old 4 bytes + new 3 bytes
tcp_output_segment: calculated checksum is a64a instead of ae6 //checksum of this segment incorrect!!

now deep into the code:
tcp_write function declare next variables if LWIP_CHECKSUM_ON_COPY=1

u16_t concat_chksum = 0; //checksum of new added data
u8_t concat_chksum_swapped = 0; //is checksum swapped or not
u16_t concat_chksummed = 0; //len of new data witch added to delayed segment

//in 2 phase we set this variables to right values
after all we concat_p to last_unsent->p and sum checksums

#if TCP_CHECKSUM_ON_COPY
if (concat_chksummed) {

/ERROR if concat_chksum swapped!! we must swap it back /

tcp_seg_add_chksum(concat_chksum, concat_chksummed, &last_unsent->chksum,
&last_unsent->chksum_swapped);
last_unsent->flags |= TF_SEG_DATA_CHECKSUMMED;
}
#endif /* TCP_CHECKSUM_ON_COPY */

I add this before call tcp_seg_add_chksum (also patch in attach)
/if concat checksumm swapped - swap it back /
if (concat_chksum_swapped){
concat_chksum = SWAP_BYTES_IN_WORD(concat_chksum);
}

This solve the problem.

After viewing the code, I think, that concat_chksum_swapped not necessary,
because we don't accumulate checksum in concat_chksum. (why tcp_seg_add_chksum called for compute new data checksum in phase 2??)
I need someone to fix the code properly, I'm afraid to break the code.

(file #25657)

Alexander <pronkin>
Fri 13 Apr 2012 09:03:02 AM UTC, comment #7:

OK. through the debug I found that bug reproducing
depending on TCP_SND_BUF option.
total len of data is 4337 but my TCP_SND_BUF is 4096
if I set TCP_SND_BUF to 5120 (slightly more than 4337) error does not occur (what is your TCP_SND_BUF opt on win32?).

//all my TCP options
#define LWIP_TCP 1
#define TCP_SND_BUF (1024*4)
#define MEMP_NUM_TCP_SEG 16
#define TCP_SND_QUEUELEN 16
#define TCP_MSS 1460

Alexander <pronkin>
Thu 12 Apr 2012 08:40:45 PM UTC, comment #6:

About Endianness. STM32F103 Reference manual says:

"The bytes are coded in memory in Little Endian format. The lowest numbered byte in a word
is considered the word’s least significant byte and the highest numbered byte the most
significant."

I even checked it out with debugger.
I think yes - my STM32 supporting only little-endian.

About Hardware. My CPU does not have any MAC module.
I use ENC28J60 SPI Stand-Alone Ethernet Controller, all checksum calculations are made by the software.

About LWIP_CHKSUM_ALGORITHM. I test with 1 and 2 and 3, bug reproduced with 100%. now it set by default (2).

About LWIP_CHKSUM, LWIP_CHKSUM_COPY, LWIP_CHKSUM_COPY_ALGORITHM
I have not defined any user versions of this.

About bug reproducing.
Are you sure that you set TCP_MSS set to 1460?
some info about my reproducing:
100% probability,
if send one packet of 1417 bytes checksum is good
if send two packets 1460 + 1417 checksum is good too.
if last packet has even length (for e.g 1418) checksum is good too.
no matter how many full 1460 packets were sent before this 3 packets. checksum is incorrect

I will try reproduce bug with a small MSS. In this case it will be more easily to debug.

and some more info:
I use tcpip_thread (FreeRTOS port from FreeRTOS team). Everything work perfect
(if LWIP_CHECKSUM_ON_COPY=0) - netif_api, ICMP, DHCP, TCP(test web server) .
I do not think that the problem in porting.

Alexander <pronkin>
Thu 12 Apr 2012 06:21:48 PM UTC, comment #5:

> Is this perhaps an issue involving the STM32 hardware TCP checksum?


Hmm, I think in that case, TCP_CHECKSUM_ON_COPY_SANITY_CHECK wouldn't have yielded a warning...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 12 Apr 2012 06:18:37 PM UTC, comment #4:

> only little-endian supported


An STM32 supporting only little-endian? I thought those only work with big-endian...

Anyway, I checked on win32 (which is little-endian since running on an intel), and I cannot reproduce this.

Which LWIP_CHKSUM_ALGORITHM setting are you using? Have you provided a user-defined version of LWIP_CHKSUM(), LWIP_CHKSUM_COPY() or LWIP_CHKSUM_COPY_ALGORITHM()?

If this is realy a bgu in lwIP, I'd love to fix it for 1.4.1 (depending on what changes are needed).

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 12 Apr 2012 06:12:29 PM UTC, comment #3:

Is this perhaps an issue involving the STM32 hardware TCP checksum? You have to turn off one of the lwip software checksum or the STM32 hardware checksum.

Jeff Barlow <jcbarlow>
Thu 12 Apr 2012 09:46:39 AM UTC, comment #2:

only little-endian supported

in cc.h also
#define BYTE_ORDER LITTLE_ENDIAN

I can make some tests in my configuration if needed, or write more information

Alexander <pronkin>
Thu 12 Apr 2012 08:21:24 AM UTC, comment #1:

A question which might be relevant: Is your CPU configured for little-endian or big-endian memory accesses?

Mason <mason>
Thu 12 Apr 2012 07:58:16 AM UTC, original submission:

I found that if LWIP_CHECKSUM_ON_COPY=1 tcp checksum errors may occur.

It happened for this situation:

options used:

#define TCP_MSS 1460
#define LWIP_CHECKSUM_ON_COPY 1

const char badPaket[] = { ..full data in attach file (size = 4337) .. };
netconn_write(newconn, badPaket, sizeof(badPaket), NETCONN_NOCOPY);

//this sends 3 packets (1460 + 1460 + 1417)
//in last packet checksum incorrect (if last packet has even length all OK)

if I add sanity check option
#define TCP_CHECKSUM_ON_COPY_SANITY_CHECK 1

now everything works fine but error message printed
tcp_output_segment: calculated checksum is 65c1 instead of 1017

I attached pcap dump and demo code to reproduce the problem
about may configuration:
32bit ARM Cortex-M3 (STM32)
gcc 4.6.0
netconn API used

Alexander <pronkin>

 

Attached Files
file #25657:  tcp_out.c.patch added by pronkin (683B - application/octet-stream)
file #25632:  bad_checksum.pcap added by pronkin (10KiB - application/octet-stream)
file #25633:  bad_checksum.c added by pronkin (26KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by jcbarlow (Posted a comment)
  • -unavailable- added by mason (Posted a comment)
  • -unavailable- added by pronkin (Submitted the item)
  • -unavailable- added by pronkin
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 14 Mar 2014 06:48:07 PM UTCgoldsimonStatusNeed Info=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Mon 16 Apr 2012 01:10:55 PM UTCpronkinAttached File-=>Added tcp_out.c.patch, #25657
    Thu 12 Apr 2012 06:18:37 PM UTCgoldsimonStatusNone=>Need Info
    Thu 12 Apr 2012 07:58:16 AM UTCpronkinAttached File-=>Added bad_checksum.pcap, #25632
      Attached File-=>Added bad_checksum.c, #25633
      Carbon-Copy-=>Added pronkin

    Back to the top


    Powered by Savane 3.1-cleanup1