buglwIP - A Lightweight TCP/IP stack - Bugs: bug #32417, TCP_OVERSIZE seems to have...

 
 

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

bug #32417: TCP_OVERSIZE seems to have problems with (fast-)retransmission

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Thu 10 Feb 2011 08:15:45 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Mon 24 Apr 2017 06:51:11 PM UTC, comment #8: 

It's been a while since the last comments here, I've come back here via bug #50694 (TX exist more pbufs after enable LWIP_NETIF_TX_SINGLE_PBUF).

I just wanted to make clear Joe is wrong: segments get removed only if an ACK acknowledges all bytes in that segment. Even if only one byte is unacked in a segment, that segment will be completely retransmitted until all bytes are ACKed. Bein like that, adding data to an already transmitted segment should do no harm.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 07 Dec 2011 07:02:43 PM UTC, comment #7: 


>> 2.
>
> Is that so? If so, that wouldn't be a bug of TCP_OVERSIZE but a general
> problem with retransmission. I.e. segments should be resent as long as they
> contain unacked data. The remote host is free to send ACKs to any amount of
> data, so that could always hit us in the middle of a segment. By adding your
> change (TF_SEG_SENT flag), I fear we would only hide a more general problem.


If you want to fix it differently, then that should be filed as a separate bug.

I think it's pretty unlikely for a host to acknowledge just part of a packet
it received (i.e., it actually received a frame containing sequence 100 - 200
but decides to ack only to 150), but it certainly would be allowed.  I suppose
it could happen if it only acks once the upper layers read the stream, and the
application reads in smaller chunks and stops without closing.
If you modified a host to do that, and LwIP then appended past sequence 200 to
the same pbuf, I think LwIP would never retransmit it.

It's been awhile so I don't remember all of the analysis.  I did write the
description at the time, though.  If a segment is allowed
to grow after it's sent once, it'll never get resent.  I think it's
a clean enough fix to just not append to a segment once it's been sent.

Otherwise you could do a fix to trim pbufs once they get acked.  However,
the pbuf may already be in use by the driver (for a re-send) so modifying
it once it is sent may cause problems.  I think once a pbuf is given to
a driver, it shouldn't be changed, even just to increase the length.

I agree with you on 1 and 3.

    Regards,
    Joe

Joe Eykholt <jre1>
Tue 06 Dec 2011 08:18:36 PM UTC, comment #6: 


> 1. pcb->last_oversize would not match last_seg->oversize_left


Agreed: we need to reset oversize_left when sending a segment.

> 2.


Is that so? If so, that wouldn't be a bug of TCP_OVERSIZE but a general problem with retransmission. I.e. segments should be resent as long as they contain unacked data. The remote host is free to send ACKs to any amount of data, so that could always hit us in the middle of a segment. By adding your change (TF_SEG_SENT flag), I fear we would only hide a more general problem.

> 3.


Agreed, but that's already fixed differently than in your patch (I changed tcp_pbuf_prealloc() to use pcb->mss for allocation instead of TCP_MSS, so there's no need for #ifdef in tcp_write.c:428).


Summed up, what's left is a debug-assert that triggers, but in release mode, no problems should appear.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 06 Dec 2011 05:18:24 PM UTC, comment #5: 

On September 4, 2011, I posted this on the developer list:

I think I've come up with fixes for this and a related problem.

    1. pcb->last_oversize would not match last_seg->oversize_left
    causing an assert failure.  This occurs after retransmission when
    the retransmitted frame becomes the only frame on the unsent list.
    Fix by clearing seg->oversize_left when sending the frame for the
    first time.
   
    2.  When a segment is requeued on the unsent list for retransmission,
    occasionally tcp_write() would tack on more data either in the oversize,
    or as additional pbufs.  If the window size is sufficiently small,
    and the original transmission was eventually acked, the frame would
    not be re-sent, causing the (one-way) connection to hang.
   
    3.  The fix to bug 34019 may also help, so I included that in my patch.
    Note that the single-PBUF #define should keep tcp_write() from
    appending additional PBUFs, but doesn't.  I haven't fixed that yet.

The patch I used is attached.  This is for 1.4.0-rc1, but should apply to
later versions as well.  I'll verify later and may be back with an update.
http://old.nabble.com/file/p32397830/lwip-32417.patch lwip-32417.patch

I think this explains the root cause (at least in my setup) for this problem,
and I've been running this fix for three months.  Could these changes be
checked in as well?

Joe Eykholt <jre1>
Thu 29 Sep 2011 06:58:05 PM UTC, comment #4: 

I'll close this as fixed: there were 2 places where the last segment in pcb->unsent could be changed where pcb->unsent_oversize wasn't reset, so this might well fix the bug without the pcb->unsent bug being fixed.

To cleanly separate this, I've opened bug #34435 ("fast retransmits can result in unordered unsent list") for the other issue (assigned to Kieran, this one's for me :)

Simon Goldschmidt <goldsimon>
Group administrator
Thu 01 Sep 2011 07:35:18 PM UTC, comment #3: 

Thank you for the detailed analysis.  This is a serious bug, and I don't think the solution will be trivial.  I'm assigning to me to try and come up with something that will allow us to sort the list without corrupting the order.

Kieran Mansley <kieranm>
Group Member
Wed 31 Aug 2011 02:54:25 PM UTC, comment #2: 

When seqno is cycled from 2^32 to 0, it's low possible that the unack_list will become an unordered list.

Supposed that:
1: unack_list is NULL;
2: cwnd >= 8 && MSS == 1024
3: unsent_list is:
packet1: seqno = 0xFFFFFF00;
packet2: seqno = 0x00000300;
packet3: seqno = 0x00000700;
...;
packet8: seqno = 0x00001B00;
4: after tcp_output called, Fast-retransmission occurred(happens on a spurious event) , retransmitting the first segment of unack_list and now the unsent_list is(unordered unack_list):
packet2: seqno = 0x00000300;
packet3: seqno = 0x00000700;
...;
packet8: seqno = 0x00001B00;
packet1: seqno = 0xFFFFFF00;
5: unfortunately, timeout-retransmission occurred, the cwnd is reset to 1.
6: after total TCP_MAXRTX times retransmit packet2, the pcb will be purged.

tao tang <opensms4>
Tue 12 Apr 2011 08:03:16 PM UTC, comment #1: 

I'm having a hard time reproducing this in unit tests and the code seems correct to me: pcb->unsent_oversize always represents the oversize bytes in the last unsent segment. If segments in the unsent queue are ordered by segno, the 'last_unsent' segment does not even get changed by the fast-retsansmission code: (fast-)retransmitted segments get sorted in to the before 'last_unsent' and thus pcb->oversize_left should not get changed in this scenario.

Reading the code doesn't get me anywhere, too. MK said he (or she?) was using 1.4.0 RC2 and line 437 fits to CVS HEAD's line '#if TCP_OVERSIZE', too...

Simon Goldschmidt <goldsimon>
Group administrator
Thu 10 Feb 2011 08:15:45 PM UTC, original submission:  

observed by MK on lwip-devel (09.02.2011):

I am triggering the following assertion from tcp_out.c : 437

#if TCP_OVERSIZE
#if TCP_OVERSIZE_DBGCHECK
   /* check that pcb->unsent_oversize matches last_unsent->unsent_oversize */
   LWIP_ASSERT("unsent_oversize mismatch (pcb vs. last_unsent)",
               pcb->unsent_oversize == last_unsent->oversize_left);
#endif /* TCP_OVERSIZE_DBGCHECK */


As far as I can tell, here is what happens before the above gets triggered
- I have one directional flow from sender to receiver
- sender tcp has sent a lot of data (happens on a spurious event)
causing the receiving tcp ' s window to be at full / near full
- there is a network loss. sender tcp goes into fast recovery
- it stays in fast recovery state for a while (keeps getting dupacks)
- finally it gets the next ack and exits fast recovery. All this
happens before tcp timer has a time to fire
- Then a subsequent attempt (not sure if it is the very next attempt
or a few attempts down the line) by application to send more data
triggers the above.

>From what I see, the fast retransmit will do some segment reordering

and put the unacked segment into the unsent queue. While it is doing
this, it is not changing pcb->unsent_oversize. This somehow seems
related to the assertion. But I do not really get that code, it may
not be the problem at all.

Simon Goldschmidt <goldsimon>
Group administrator

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jre1 (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 goldsimon (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-29 goldsimon StatusNone Fixed
        Assigned tokieranm goldsimon
        Open/ClosedOpen Closed
        Planned Release 1.4.1
    2011-09-01 kieranm StatusWorks For Me None
        Assigned toNone kieranm
    2011-04-13 goldsimon StatusNone Works For Me

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code