buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50914, TCP_CHECKSUM_ON_COPY when adding...

 
 

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

bug #50914: TCP_CHECKSUM_ON_COPY when adding data to retransmission

Submitter:  shilei yu <kyliuxing>
Submitted:  Tue 02 May 2017 11:58:33 AM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  2.1.0
lwIP version:  2.0.0

Jump to the original submission

Mon 12 Feb 2018 11:39:01 AM UTC, comment #17: 

To fix this, tcp_output_segment has to restore seg->chksum_swapped
before returning.

Fixed, thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 11 May 2017 07:31:11 PM UTC, comment #16: 

In any case, this seems like yet another bug in calculating the a header checksum (at least #43840 fised in db4844 on 2014-12-17 was also in this region somewhere)

-> updated summary to reflect this

Simon Goldschmidt <goldsimon>
Group administrator
Thu 11 May 2017 07:03:56 PM UTC, comment #15: 


> I believe checksum-on-copy is off by default


Oh, right. That's bad.

Well, it's probably good that way if we still have a bug there, but it's bad since it should be much faster - at least for the case where we copy data using memcpy and have a slow access to it when loading it into the CPU again for calculating the checksum.

For checksum-offload setups, the speed change should not be that important.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 11 May 2017 01:45:47 PM UTC, comment #14: 


> Clearly, checksum-on-copy should be faster, which is why it is the default. However, your log output indicates there's a problem with this and the generated checksum is off by one.


I believe checksum-on-copy is off by default.  From opt.h:

/**
 * LWIP_CHECKSUM_ON_COPY==1: Calculate checksum when copying data from
 * application buffers to pbufs.
 */
#if !defined LWIP_CHECKSUM_ON_COPY || defined __DOXYGEN__
#define LWIP_CHECKSUM_ON_COPY           0
#endif


Joel Cunningham <jcunningham>
Group Member
Thu 11 May 2017 09:06:10 AM UTC, comment #13: 

I added the log as attachment, it happened twice but i didn't get wireshark capture.i feel if the PC's ack between the same segment's seq and seq+len, problem will be reproduced.
I simplified the debug info, so if you need more, i will do more test.
Thanks for your help and patience.

shilei yu <kyliuxing>
Thu 11 May 2017 07:25:55 AM UTC, comment #12: 

LWIP_CHECKSUM_ON_COPY
== 1: for headers, checksum is calculated byte by byte when the header is filled; for data, checksum is created during memcpy when the data is already in the processor's registers
== 0: first headers and data are put in place and just before transmitting, checksum is created by fetching all bytes into the processor's registers again and summing them up.

Clearly, checksum-on-copy should be faster, which is why it is the default. However, your log output indicates there's a problem with this and the generated checksum is off by one.

Could you post such a log? Hopefully, I can see which segments sizes where sent.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 11 May 2017 06:10:58 AM UTC, comment #11: 

@goldsimon
It works well by setting TCP_CHECKSUM_ON_COPY_SANITY_CHECK == 1.
but i also found "tcp_output_segment: calculated checksum is ffff instead of 0" in my log, it seems has some error but fix by the sanity check.
i'm not clear about the modification, could you kindly make an explanation?
thanks!

shilei yu <kyliuxing>
Mon 08 May 2017 07:15:44 PM UTC, comment #10: 

Right. (Were you waiting for that?)

I'd need the output of the SANITY_CHECK_FAIL() to fix this (if there's really still a bug in this feature).

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 May 2017 09:49:50 AM UTC, comment #9: 

TCP_CHECKSUM_ON_COPY_SANITY_CHECK seems not work if i set LWIP_CHECKSUM_ON_COPY==0(TCP_CHECKSUM_ON_COPY will be 0 too),
So i should set LWIP_CHECKSUM_ON_COPY==0 or LWIP_CHECKSUM_ON_COPY==1 and TCP_CHECKSUM_ON_COPY_SANITY_CHECK == 1 to check whether the checksum is error, right?

Thanks for your response! it is very helpful.

shilei yu <kyliuxing>
Fri 05 May 2017 07:37:17 AM UTC, comment #8: 

Oh, and you might also want to try:


#define TCP_CHECKSUM_ON_COPY_SANITY_CHECK==1


and optionally to fail on such an error:


#define TCP_CHECKSUM_ON_COPY_SANITY_CHECK_FAIL(msg) LWIP_ASSERT(msg, 0)


Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 May 2017 07:33:19 AM UTC, comment #7: 


> 'm not clear about LWIP_CHECKSUM_ON_COPY, it seems only related UDP packet


It's not. Just try LWIP_CHECKSUM_ON_COPY==0 as I suggested.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 May 2017 07:20:05 AM UTC, comment #6: 

@goldsimon
LWIP_CHECKSUM_ON_COPY = 1 and CHECKSUM_GEN_TCP = 1 too.
CHECKSUM_GEN_TCP should be 1 because we can't calculate checksum by hardware.is it right?
I'm not clear about LWIP_CHECKSUM_ON_COPY, it seems only related UDP packet,how it can effect on TCP checksum?
thanks!

shilei yu <kyliuxing>
Thu 04 May 2017 11:02:50 AM UTC, comment #5: 

Yes. There are many "[TCP CHECKSUM INCORRECT]" packets from the PC as checksum offloading is done after wireshark captures the local packets.

However, the retransmitted packet from the device (e.g. no. 4155) has an invalid checksum, too. That shouldn't happen. There's something wrong in calculating the checksum of that packet.

What's your setting for LWIP_CHECKSUM_ON_COPY and CHECKSUM_GEN_TCP? Could you try again with LWIP_CHECKSUM_ON_COPY ==0?

Simon Goldschmidt <goldsimon>
Group administrator
Wed 03 May 2017 02:09:59 AM UTC, comment #4: 

@goldsimon
Thanks for your response.
My PC(TCP server) is windows7 and device(TCP client) runs mbed OS which use lwip as TCP/IP stack.
I have confirmed lwip version is 2.0.0 in init.h.
I Use TCPUDPDbg as TCP server on PC, but i don't think it was related the issue, it is just application software, the ack num depends on windows TCP/IP stack.
i don't understand about the checksum, do you mean the PC receive the packet but the checksum is wrong, so drop it?

shilei yu <kyliuxing>
Tue 02 May 2017 07:23:43 PM UTC, comment #3: 

And after another look, did you notice both sides produce invalid checksums? Normally, only the PC running wireshark should have invalid checksums.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 02 May 2017 07:11:46 PM UTC, comment #2: 

Reading the pcap, I can only assume you have a lot of lost packets, possibly due to recvmbox limitations (or other limitations, as you don't say what lwIP API you are using).

As to the PC, how could I possibly say anything without knowing what software it runs?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 02 May 2017 07:02:52 PM UTC, comment #1: 

I assume the PC is running lwIP? ;-)

If you want to find out the lwIP version, check init.h, LWIP_VERSION or LWIP_VERSION_STRING.

If it's not a release, I strongly suggest upgrading!

Simon Goldschmidt <goldsimon>
Group administrator
Tue 02 May 2017 11:58:33 AM UTC, original submission:  

I met a issue when made TCP test. I built a TCP server on PC and make my device to connect it, they sent data to each other.
From the capture of PC(TCP server), firstly, device send seq 6825983(len 1447 ,line 4139), PC ack 6827430, but device may be not receive the ack, so device retransmit seq 6825983(len 1457), the len is larger. then the PC still ack 6827430 and the lwip stack retransmit 6825983(len 1457 ,line 4155), until reached max retry times.
Why the PC won't ack the added 10 bytes?
Once the PC's ack num is between seq and seq+len, lwip stack will retransmit the whole segment, but PC won't update the ack num.
The attachment is uart_log of device(client) and wireshark cpature of PC(server), you can refer the line 4139 and line 4155 in Wireshark_server.pcap.
by the way, i'm not sure of my lwip version, but the file CHANGELOG include  "2016-07-27: Simon Goldschmidt * opt.h, timeouts.h/.c: added LWIP_TIMERS_CUSTOM to override the default implementation of timeouts"
Thank you!

shilei yu <kyliuxing>

 

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

Attached Files
file #40663:  SANITY_CHECK_log.7z added by kyliuxing (184KiB - application/octet-stream - uart log with TCP_CHECKSUM_ON_COPY_SANITY_CHECK == 1)
file #40555:  UART_client.log added by kyliuxing (2MiB - text/plain - uart_log of device(client) and wireshark cpature of PC(server))
file #40556:  Wireshark_server.pcap added by kyliuxing (2MiB - application/octet-stream - uart_log of device(client) and wireshark cpature of PC(server))

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jcunningham (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kyliuxing (Submitted the item)
  •  

    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
    2018-02-12 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2018-01-02 goldsimon Planned ReleaseNone 2.1.0
    2017-05-11 goldsimon SummaryTCP server don't update the ack num TCP_CHECKSUM_ON_COPY when adding data to retransmission
    2017-05-11 kyliuxing Attached File- Added SANITY_CHECK_log.7z, #40663
    2017-05-02 kyliuxing Attached File- Added UART_client.log, #40555
        Attached File- Added Wireshark_server.pcap, #40556

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code