buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27504, tcp_enqueue wrongly concatenates...

 
 

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

bug #27504: tcp_enqueue wrongly concatenates segments which are not consecutive when retransmitting unacked segments

Submitter:  Jörg Kesten <jkesten>
Submitted:  Wed 23 Sep 2009 09:02:49 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.1

Sun 11 Oct 2009 01:07:21 PM UTC, comment #2: 


> Could tcp_rexmit perhaps call tcp_output_segment directly rather
> than mucking about with the queues?


That would also be faster than adding the segment to the unsent list, but if direct sending fails (e.g. due to ERR_MEM), the segment won't be sent again until the next call to the timer.

I prefer the patch as a solution as it does not change the behaviour much.

Checked in, thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 23 Sep 2009 10:32:52 AM UTC, comment #1: 

The patch looks good to me.

I am a little worried about the tcp_rexmit behaviour. I thought that unacked + unsent were supposed to form a continuous, monotonic sequence of segments? Apparently, so did tcp_enqueue().

Note that tcp_rexmit_rto preserves this invariant as well.

Could tcp_rexmit perhaps call tcp_output_segment directly rather than mucking about with the queues?

Jakob Stoklund Olesen <stoklund>
Group Member
Wed 23 Sep 2009 09:02:49 AM UTC, original submission:  

tcp_enqueue tries to append a new segment to the last one in the unsent list if the max len is not reached yet.
It is not checked if the new segment is really the next one after the last unsent segment, leading to the following faulty behavior if unacked data has to be resend:

  • some segments are send and added to the unacked queue (say n to n+5)
  • segment "n" gets lost
  • after some time tcp_rexmit moves segment n from the unacked back to the unsent queue.
  • The current state is

   - one segment (number n) is in the unsent queue
   - some further segments are still in the unacked queue (n+1 .. n+5) (the actual count is not important here)

  • Now tcp_enqueue is called, segments n+6 and n+7 get enqueued


!!* tcp_enqueue detects one non-full segment in the unsent queue (n) and concatenates/chains the first new segment (n+6) to it
!!* so what happens is that the data from segment n+6 gets appended to segment n, and n+7 gets added to the unsent-queue.

  • tcp_output (re)sends segment n (piggybacking the data from n+6 which by this ends up in the nirvana)
  • segment n is received correctly by the peer and all unacked segments are acked at once (up to n+5)
  • next segment send is n+7


Attached is a simple fix which just verifies that the seqno of the new segment is equal to the seqno of the old segment + len. This seems to be sufficient, at least for the above described error case.

Jörg Kesten <jkesten>

 

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

Attached Files
file #18759:  tcp_enqueue.patch added by jkesten (820B - application/octet-stream)

 

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 stoklund (Posted a comment)
  • -email is unavailable- added by jkesten (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-10-11 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2009-09-23 jkesten Attached File- Added tcp_enqueue.patch, #18759

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code