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

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

(Jump to the original submission 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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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 2MSS "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 4MSS 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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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 2MSS and 4MSS as well as effectively disables that mechanism for windows < 2*MSS.

Any thoughts? Kieran?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 23 Oct 2009 01:18:11 PM UTC, comment #4:

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

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 23 Oct 2009 08:10:17 AM UTC, comment #3:

Looks good, thanks for following this through.

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

Any objections against checking in?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by dempson (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
    Fri 23 Oct 2009 01:18:11 PM UTCgoldsimonAssigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
      StatusNone=>Fixed
    Fri 23 Oct 2009 07:12:40 AM UTCgoldsimonPlanned Release=>1.3.2

    Back to the top


    Powered by Savane 3.1-cleanup1