buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27771, Silly-window-avoidance decreases...

 
 

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

bug #27771: Silly-window-avoidance decreases window unnecessarily

Submitted by:  Simon Goldschmidt <goldsimon>
Submitted on:  Wed 21 Oct 2009 05:23:07 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: NoneStatus: Invalid
Privacy: PublicAssigned to: None
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.3.1

(Jump to the original submission Jump to the original submission)

Fri 23 Oct 2009 07:10:15 AM UTC, comment #8:

Closed as invalid after opening bug #27783: Silly window avoidance for small window sizes

Simon Goldschmidt <goldsimon>
Project Administrator
Thu 22 Oct 2009 03:29:44 PM UTC, comment #7:

No, the test in tcp_update_rcv_ann_wnd should look like this (second clause added):

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

i.e. if you change pcb->rcv_ann_wnd to TCP_WND you'd be right.

Kieran Mansley <kieranm>
Project Administrator
Thu 22 Oct 2009 03:20:10 PM UTC, comment #6:

> (iii) we can add a condition to the window update to also
> update if the increment is > WND/2 as suggested by David Empson.


Would that mean (in function tcp_update_rcv_ann_wnd()) to change the line

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((pcb->rcv_ann_wnd / 2), pcb->mss))) {

?

I did that change and (with MSS==WND==512) it resulted in:
- announced wnd=372
- received 40 bytes
- internal wnd=rcv_ann_wnd=332
- recved 40 bytes -> pcb->wnd = 512
-> window opens by more than (332/2)=166
- announced wnd=512

so even if I though differently, this seems to be OK.

Regarding your last comment, I guess you can argue that way or the other, but the old implementation (window always stays the same when directly calling tcp_recved()) seems indeed outdated and only applicable on local LANs...

I hope you agree with me that this is targeted to 1.3.2, too?

Simon Goldschmidt <goldsimon>
Project Administrator
Thu 22 Oct 2009 02:17:34 PM UTC, comment #5:

So it seems that both the summary of this bug as well as the bug itself is invalid? Let's wait to close this until we change the algorithm...

Simon Goldschmidt <goldsimon>
Project Administrator
Thu 22 Oct 2009 10:52:21 AM UTC, comment #4:

Imagine that the 10 bytes was just the first 10 bytes of a longer transfer, and that the sender has in fact got the other 502 possible bytes in flight. If we advertised the 10 bytes of new window after receiving it, the server would then get an ACK that allows it to send 10 more bytes. In reality it might want to send much more, but would be limited by the window, and so send just 10 bytes. This would result in a situation where the network descended to lots of small packets. If instead we batch up the window updates and give it a window updates of 1 MSS or more, the sender will always be able to send a full segment (when it is able to send at all) and so will result in a situation there the network sticks to MSS-sized frames: this is much more efficient.

The critical thing here is not the total amount of window we're advertising (as the receiver, we can't deduce the sender's state, and so can't know how much window is really outstanding) it's the size of the increments to the window that's important.

The downside is that it makes systems where WND = MSS behave a bit worse (the window will decrease to zero before getting an update of 1 MSS) but (i) these systems were never optimal; (ii) we can add a compile time warning; and (iii) we can add a condition to the window update to also update if the increment is > WND/2 as suggested by David Empson.

Kieran Mansley <kieranm>
Project Administrator
Thu 22 Oct 2009 10:38:19 AM UTC, comment #3:

What's happening in Jan's trace is:

- lwIP announces wnd of 512
- lwIP receives 10 bytes of data
- internal rcv_wnd is now 502
- calls recv callback
- recv callback calls tcp_recved
- internal rcv_wnd is now 512
- recv_ann_wnd is still 502
- lwIP sends ACK with an announced wnd of 502

This means the window is decreasing although it wouldn't have to: if the window size we have was OK to be announced before receiving, it should be OK to be announced after receiving, shouldn't it?

Simon Goldschmidt <goldsimon>
Project Administrator
Thu 22 Oct 2009 08:28:47 AM UTC, comment #2:

I think the current behaviour is right. I'd like to see a packet capture showing lwIP moving the right edge of the advertised window to give less to the sender. I think at the moment that it will keep this right edge constant instead of increasing it by small amounts, which is the correct behaviour. Once the small amounts add up to a sizeable increase (> 1 MSS) it will advertise that all in one go.

Kieran Mansley <kieranm>
Project Administrator
Wed 21 Oct 2009 08:44:04 PM UTC, comment #1:

The reason seems to be that tcp_update_rcv_ann_wnd() shrinks the window when called from tcp_receive(), but does not grow it when called from tcp_recved(). The reason is this line:

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

together with the line

if (wnd_inflation >= TCP_WND_UPDATE_THRESHOLD)

in tcp_recved(), this looks like the condition to not send updates is doubled: the first time we prevent growing the window at all, the second time we prevent sending a direct ACK. I wonder if we have to remove the ' + pcb->mss' in the first line or if we only have to call tcp_update_rcv_ann_wnd() only once per received packet instead of multiple times (i.e. call it after the recv callback is called)?

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 21 Oct 2009 05:23:07 PM UTC, original submission:

[This has been reported on lwip-users today (together with summaries "TCP payload is doubled" and "TCP problem" in the last days).]

The algorithm seems to shrink the window even if tcp_recved() is called inside the recv-callback and there hasn't been a shrinked window announced. When calling tcp_recved() for all data inside the recv-callback, no window change should occur at all.

Since this bug was introduced in 1.3.1, it would be a good idea to delay 1.3.2 until this is fixed, I guess.

Simon Goldschmidt <goldsimon>
Project Administrator

 

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 (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 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 23 Oct 2009 07:10:15 AM UTCgoldsimonStatusNone=>Invalid
      Open/ClosedOpen=>Closed
    Thu 22 Oct 2009 02:54:23 PM UTCgoldsimonSummarySilly-window-avoidance shrinks window unnecessarily=>Silly-window-avoidance decreases window unnecessarily

    Back to the top


    Powered by Savane 3.1-cleanup1