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

Submitted by:  Jörg Kesten <jkesten>
Submitted on:  Wed 23 Sep 2009 09:02:49 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.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>
Project AdministratorIn charge of this item.
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>
Project 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>

 

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
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by stoklund (Posted a comment)
  • -unavailable- added by jkesten (Submitted the item)
  •  

    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 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 11 Oct 2009 01:07:21 PM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Wed 23 Sep 2009 09:02:49 AM UTCjkestenAttached File-=>Added tcp_enqueue.patch, #18759

    Back to the top


    Powered by Savane 3.1-cleanup1