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

Submitter:  Alexander <pronkin>
Submitted:  Thu 12 Apr 2012 07:58:16 AM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.4.0

Jump to the original submission

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

Finally fixed, thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
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>
Group administrator
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>
Group administrator
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>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

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
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jcbarlow (Posted a comment)
  • -email is unavailable- added by mason (Posted a comment)
  • -email is unavailable- added by pronkin (Submitted the item)
  • -email is unavailable- added by pronkin
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-03-14 goldsimon StatusNeed Info Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2012-04-16 pronkin Attached File- Added tcp_out.c.patch, #25657
    2012-04-12 goldsimon StatusNone Need Info
    2012-04-12 pronkin Attached 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.13-4448.
    Corresponding source code