buglwIP - A Lightweight TCP/IP stack - Bugs: bug #33952, PUSH flag in incoming packet is...

 
 

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

bug #33952: PUSH flag in incoming packet is lost when packet is aggregated and sent to application

Submitter:  Klaus Hansen <foss_eswp>
Submitted:  Fri 05 Aug 2011 10:37:32 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.3.0

Sat 03 Sep 2011 08:01:30 PM UTC, comment #3: 

This is my fix: just set PBUF_FLAG_PSH on every pbuf passed to tcp_input() (if PSH or FIN flag is given) instead of setting it just before passing it to the application.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 01 Sep 2011 07:36:17 PM UTC, comment #2: 

Makes sense to me.  There won't be any data after the FIN, so the FIN is an implicit PSH

Kieran Mansley <kieranm>
Group Member
Wed 31 Aug 2011 03:11:46 PM UTC, comment #1: 

I'v downloaded the newest code and check this issue, I found that PBUF_FLAG_PUSH has been added once a packet with PUSH flag.
--I suggest adding PBUF_FLAG_PUSH when lwIP received a packet with FIN flag.

tao tang <opensms4>
Fri 05 Aug 2011 10:37:32 AM UTC, original submission:  

I noticed this faulty behaviour when I was trying to "put" a file from a Windows FTP client to a FTP server using version 1.3.0 of the lwIP stack. Sometimes the last bytes in the file were missing.

The FTP server would call lwip_recvfrom which again would call netconn_recv.
In this case the file to be transferred was 52228 bytes long and the tcp packets eache contained 536 bytes with the last one containing only 236 bytes (97 * 536 + 236).
By instrumenting the lwip_recvfrom code I could see that most of the calls to netconn_recv returned just 536 bytes from one tcp packet. And when this one tcp packet contained a PUSH flag (I had WireShark running), the packet returned from the netconn_recv call also had the PUSH flag set.
But some of the calls to netconn_recv returned an aggregate of several tcp packets - I suspect because the receiving application had been busy doing other things. And if one of the tcp packets had the PUSH flag set, the flag was not set in the aggregate packet.
In this situation, the last 7 packets were aggregated (6 * 536 + 236). The last 256 byte packet had the PUSH flag set, but the aggregate packet returned from the netconn_recv call had no PUSH flag. So the lwip_recvfrom function would loop and call netconn_recv again, this time finding out that the connection had been closed, and returning 0 bytes.

I found out that the aggregate packet was a result of the last 7 packets being taken out of the out-of-sequence queue in the tcp_receive function. When a packet is inserted into the out-of-sequence queue, the tcp flags are remembered. But when the packets are taken out of the queue, the PUSH flag is forgotten.

I fixed the bug by adding a TF_GOT_PSH flag (tcp.h) which the tcp_receive function would set in the global recv_flags, if one of the packets in the out-of-sequence queue has the PUSH flag set.

In the tcp_input function, where the received packets (simple or aggregate) are returned from the tcp_process call, I added a check for the TF_GOT_PSH flag in the global recv_flags, and set the PUSH flag in the recv_data->flags accordingly before returning recv_data to the application.

I checked version 1.4.0 to see if this bug had already been fixed, but it didn't seem so.

I have attached sockets.c, tcp_in.c and tcp.h with my debug code and bug fix. The bug is named TD#10887.


Klaus Hansen <foss_eswp>

 

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

Attached Files
file #23744:  sockets.c added by foss_eswp (58KiB - text/plain)
file #23745:  tcp.h added by foss_eswp (25KiB - text/plain)
file #23746:  tcp_in.c added by foss_eswp (55KiB - 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 kieranm (Posted a comment)
  • -email is unavailable- added by opensms4 (Posted a comment)
  • -email is unavailable- added by foss_eswp (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-09-03 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
        Planned Release 1.4.1
    2011-08-05 foss_eswp Attached File- Added sockets.c, #23744
        Attached File- Added tcp.h, #23745
        Attached File- Added tcp_in.c, #23746

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code