[bug #27783] Silly window avoidance for small window sizes

[Sylvain Beucler-4]

URL:
  <http://savannah.nongnu.org/bugs/?27783>

                 Summary: Silly window avoidance for small window sizes
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: dempson
            Submitted on: Thu 22 Oct 2009 21:19:50 GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release:
            lwIP version: 1.3.1

    _______________________________________________________

Details:

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.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #1, bug #27783 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Update of bug #27783 (project lwip):

         Planned Release:                         => 1.3.2                  

    _______________________________________________________

Follow-up Comment #2:

Any objections against checking in?

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #3, bug #27783 (project lwip):

Looks good, thanks for following this through.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Update of bug #27783 (project lwip):

                  Status:                    None => Fixed                  
             Assigned to:                    None => goldsimon              
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #4:

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #5, bug #27783 (project lwip):

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?

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #6, bug #27783 (project lwip):

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)

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #7, bug #27783 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #8, bug #27783 (project lwip):

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".  

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Sylvain Beucler-4]

Follow-up Comment #9, bug #27783 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27783>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel