buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27783, Silly window avoidance for small...

 
 

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

bug #27783: Silly window avoidance for small window sizes

Submitter:  David Empson <dempson>
Submitted:  Thu 22 Oct 2009 09:19:50 PM 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

Jump to the original submission

Fri 30 Oct 2009 10:03:10 AM UTC, comment #9: 

Changed TCP_WND to 4*TCP_MSS and TCP_MSS to 536 in opt.h.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 30 Oct 2009 09:51:32 AM UTC, comment #8: 

Yes, please change the default TCP_WND to 4MSS.  We might want to change the default MSS as well as it is currently very small.  536 bytes is the normal default value for MSS.

I think things should still work well with TCP_WND = 2MSS.  They'll work better with bigger windows of course, but even if delayed ACKs aren't being used, I'd still class that as "working well".

Kieran Mansley <kieranm>
Group Member
Fri 30 Oct 2009 09:39:23 AM UTC, comment #7: 

Hm, but that would make it worse for WND==MSS again. Maybe you're right and sending that many ACKs is OK if the window is that small (since it helps minimize latencies) - which means leave it as it is.

However, we should document this somewhere and make sure the default definitions in opt.h are OK:

- opt.h says that TCP_WND must be at least 2*MSS "for things to work well". I wonder if that's still the case since we changed the ACK mechanism (WND == MSS works quite well for me). I think we should change that to "This should be at least 4*MSS for things to work well (otherwise, delayed ACK mechanism won't work)".

- I think we should change the default definition of TCP_WND from 2048 to (4*TCP_MSS). That way, the window is automatically adjusted if someone only defines TCP_MSS (but not TCP_WND) in lwipopts.h.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 30 Oct 2009 09:04:55 AM UTC, comment #6: 

Sounds reasonable.

Only downside is it makes it a little harder to reason about when people report problems.  I guess it might also be arguable that with a small window having lots of ACKs is a good idea.

I think on balance your suggestion is a good idea.  I just find it odd that we need a special case for this small range of window sizes.

One other thing:
for TCP_WND = 4MSS we get a threshold of MSS.
for TCP_WND = 4MSS - 1 we get a threshold of 2MSS.
That big step seems wrong: why not set it to MSS for WND < 4MSS?
That would make it better at TCP_WND = 2MSS too: you'd have a threshold of MSS (i.e. less than the TCP_WND) instead of 2MSS (same as the TCP_WND)

Kieran Mansley <kieranm>
Group Member
Thu 29 Oct 2009 05:22:44 PM UTC, comment #5: 

I just discovered another problem with our silliy-window-avoidance algorithm: with TCP_WND <= ((TCP_MSS*4)-4) (i.e. window smaller than 4 segments), the algorithm effectively disables the delayed ACK mechanism, i.e. an ACK is sent for every (full-MSS-sized) segment received.

As a fix, I guess the definition of TCP_WND_UPDATE_THRESHOLD in opt.h must be changed:

#ifndef TCP_WND_UPDATE_THRESHOLD
#if (TCP_WND < (TCP_MSS 4)) && (TCP_WND >= (TCP_MSS 2))
#define TCP_WND_UPDATE_THRESHOLD        (TCP_MSS * 2)
#else
#define TCP_WND_UPDATE_THRESHOLD        (TCP_WND / 4)
#endif
#endif

This both fixes the delayed-ACK mechanism for windows between 2*MSS and 4*MSS as well as effectively disables that mechanism for windows < 2*MSS.

Any thoughts? Kieran?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 23 Oct 2009 01:18:11 PM UTC, comment #4: 

Checked in. Thanks for tracking this down with us, David.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 23 Oct 2009 08:10:17 AM UTC, comment #3: 

Looks good, thanks for following this through.

Kieran Mansley <kieranm>
Group Member
Fri 23 Oct 2009 07:12:40 AM UTC, comment #2: 

Any objections against checking in?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 23 Oct 2009 07:12:01 AM UTC, comment #1: 

As was written in bug #27771, to achieve this, the first if-statement in tcp_update_rcv_ann_wnd() should be changed from

if (TCP_SEQ_GEQ(new_right_edge, pcb->rcv_ann_right_edge + pcb->mss))

to

if (TCP_SEQ_GEQ(new_right_edge, pcb->rcv_ann_right_edge + LWIP_MIN((TCP_WND / 2), pcb->mss)))

I've just tested it and it is working fine.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 22 Oct 2009 09:19:50 PM UTC, original submission:  

Kieran Mansley wrote:

> There were two changes in 1.3.1 [in the area of receive window management]:
> - only send an explicit window update if the change is greater than TCP_WND_UPDATE_THRESHOLD.
> - only change the window advertised (including in normal ACKs) if the change is greater than 1 MSS.
> This is silly window avoidance, and designed to prevent the sender getting small amounts of window
> and then sending small packets.


The description of silly window avoidance in RFC1122 (section 4.2.3.3) says that there is supposed to be a second test.

The threshold to advertise an increase in window size should be the lesser of MSS and WND/2 (assuming the recommended fraction of 1/2). This would allow the use of window sizes smaller than MSS*2. The absolute minimum (but horribly inefficient) value of WND would be 2.

Assuming WND is set equal to MSS:

1. If the application is reading data slowly a few bytes at a time with a sender able to supply data continuously, then the typical pattern should be to get an initial transmit burst of the full window size, followed by half-full segements (WND/2 = MSS/2) once the receiver has processed half the initial data.

2. If the sender is delivering small packets at a moderate pace and the receiver is able to keep up, the expected pattern should be to see the window size decreasing in each ACK until just after half the window is gone, then it should advertise an almost full window again.

It seems reasonable for LWIP to recommend that users set WND to at least MSS*2, but unless LWIP enforces this minimum, the silly window avoidance code should allow for small windows by checking WND/2 as well as MSS.

As it stands, the code has a degenerate case when WND = MSS, which in (2) results in the window closing all the way to zero before opening up to full again. It isn't quite as bad for MSS < WND < MSS*2, but still results in the window getting smaller than TCP says it should.

David Empson <dempson>

 

(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 kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by dempson (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-23 goldsimon Assigned toNone goldsimon
        Open/ClosedOpen Closed
        StatusNone Fixed
    2009-10-23 goldsimon Planned Release 1.3.2

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code