Fri 01 May 2009 12:27:41 PM UTC, comment #21:
Closing as fixed (again) due to no reports to the contrary.
|
Thu 26 Mar 2009 02:43:09 PM UTC, comment #20:
I've checked in a change to set the ACK flag on those probes. Can anyone confirm whether there is a still an outstanding issue with this bug?
|
Thu 12 Feb 2009 01:39:13 PM UTC, comment #19:
Status is: "not enough time at the moment"
I've bumped it up to 1.3.1 - should have done that when reopening the bug
|
Wed 11 Feb 2009 07:38:14 PM UTC, comment #18:
Kieran, what's the status on this? It's still marked as planned for 1.3.0 ...
|
Thu 18 Dec 2008 02:57:33 PM UTC, comment #17:
It is definitely lack of ACK flag in zero window probe.
Without it windows host for example doesn't reply to this probe.
I think it should be special tested case for last packet with FIN flag (no data, only FIN flag).
In this case we should allocate not
pbuf_alloc(PBUF_IP, TCP_HLEN + 1, PBUF_RAM)
but only
pbuf_alloc(PBUF_IP, TCP_HLEN, PBUF_RAM)
+ FIN flag
|
Fri 28 Nov 2008 10:23:02 AM UTC, comment #16:
OK, I'll have to give that some thought. I think it would need some integration with the other parts of the tcp retransmission algorithm as it could result in a large number of packets being retransmitted, and so we should be updating some of the state. It has the potential to be harmful in that it could put packets onto the wire quite aggresively at a time of loss (i.e. when you should be backing off).
I suspect the root of the problem is failing to recover properly when we do get the window-opening ACK, rather than the probe that we send.
As to the condition in tcp_receive: no, it's deliberately like that to cope with cases where packets have been reordered. In words, it's approximating:
if segment contains new data
or segment acks new data
or segment increases the window
then...
|
Fri 28 Nov 2008 10:06:19 AM UTC, comment #15:
(A) This modification below resends the unacked segments till the send window allows.
This is the only change I made in tcp_out.cpp.
I can confirm using Wireshark that 9-10 segments are retransmitted when segment lost.
I tested in my test environment and it was running well over the night (~16hrs) without breaking the socket.
Therefore I decided to use it in my code - if you accept it, please commit also into lwip CVS repository.
(B) Please note that the above is a solution for small sender window only, and not for zero receiver window. I cannot test the latter because it doesn't happen in my system. However I tried to set TCP_ACK flag of the 1-byte probe and then I got reply from the receiver. But despite of this, lwip sent out the 1-byte probe again, therefore I suspect that some further checks needed, so would be essential to test that part in
real situation.
WRT tcp_receive, I don't really know the meaning of that condition. It's just an
intuitive fix because it looks like the following pattern:
if ((a1 < a2) ||
(a1 == a2 && b1 < b2) ||
(a1 == a2 && b1 == b2 && c1 < c2))
But if "a1 == a2" is missing from the third line, the expression can be true in case of
a1 > a2 which is probably unwanted.
|
Thu 27 Nov 2008 02:08:09 PM UTC, comment #14:
OK, reopening. I'll take another look when I get a chance, but not sure when that will be.
If the previous version works for you, you should use that for the time being, but it does have the problem that it won't probe for more window when there is only a small amount available, and lwIP won't send in that case as we refuse to segment packets to fill a small window.
I haven't had a look at the trace you provide in detail yet, but it will be very useful when working on this, thanks. The other end should ACK these probes as they are effectively retransmissions. It might not ACK the first one, but the second will be a duplicate and so should always be ACKed.
I can't remember why I didn't set the ACK flag, probably just didn't want to work out what to put in the ACK field. PSH isn't appropriate here.
Regarding the condition in tcp_receive, I think the condition is wrong as it stands, but I'd prefer to change it to use the right_wnd_edge rather than the absolute value of the window so that we only take notice of window changes that increase the window. By the time we get to that test we've established that the segment contains no new data and acknowledges no new data, so the only reason it would be interesting is if it had a larger window. This is probably a separate bug - could you open one and describe the problem you're seeing with that condition?
|
Thu 27 Nov 2008 01:23:03 PM UTC, comment #13:
Kieran, the problem I described in bug #24212 caused by your last modification in tcp_out.c: Rev 1.71 works well, but I get the error using 1.72. The difference is that Zero Window Probe is sent not when snd_wnd is zero, but also when the window is not enough to send the data.
RFC793 "Managing the Window" says:
"The sending TCP must be prepared to accept from the user and send at least one octet of new data even if the send window is zero." and
"When the receiving TCP has a zero window and a segment arrives it must still send an acknowledgment showing its next expected sequence number and current window (zero)."
In my understanding it means that the receiver is not liable for sending ACK to this probe, if its window is nonzero.
This happens in my attached scenario:
https://savannah.nongnu.org/bugs/download.php?file_id=16890
Note that receiver window is 1280 always (and thus non-zero).
Frame#21: Sender sends a segment as usual.
#21-22: segment#1847 lost
#22-26: sender sends 5*142=710 bytes. Including the lost segment, the sent data is 710+142=852 bytes.
#27: receiver ACKs frame#21 and expects the lost segment#1989 and advertises receiver window 1280.
#28: sender sends 426 (=3*142) bytes, so totally 852+426=1278 bytes. Close, right? With the next 142 bytes, 1278+142 > 1280, therefore lwip starts the persist timer in a next timeslice.
#29-34: dark and weird things happen: sender (lwip) and receiver tries to get back to normal way, finally lwip resends the lost segment
#35: receiver ACKs seg#1989
#36: are you still with me? Finally, persist timer event occurs, and lwip sends 1-byte Zero Window Probe.
#37: there's almost 3.4s gap between #36 and 37, which is very big comparing to the normal 0.2s cycle time. It means that receiver doesn't ACK the 1-byte message, perhaps because its window is non-zero.
#38...: lwip tries the Zero Window Probe again, but no success, finally the receiver gets bored and closes the socket.
Remarks:
A) A possible solution could be that instead of sending 1-byte in tcp_zero_window_probe, check if snd_wnd is greater or equal than the segment (seg->tcphdr->seqno) size, then (re-)send the whole segment.
B) Wireshark identifies Zero Window Probe segments as "Broken TCP" - maybe it is wrong, but wouldn't be better to set ACK (and PSH) flag?
So I suggest to re-open this bug. It takes some time to me to investigate in A) and B) as I have to carefully test the solution and this problem occurs randomly (5min...3hours). Welcome for any comment.
- Tamas
PS: In tcp_receive, the condition (pcb->snd_wl2 == ackno && tcphdr->wnd > pcb->snd_wnd) shouldn't be (pcb->snd_wl1 == seqno && pcb->snd_wl2 == ackno && tcphdr->wnd > pcb->snd_wnd)?
|
Tue 04 Mar 2008 01:12:39 PM UTC, comment #12:
Closing this as fixed due to lack of reports to the contrary.
|
Tue 15 Jan 2008 01:02:38 PM UTC, comment #11:
I've modified it to activate the persist timer when we're prevented from sending by a small send window (not just a zero send window).
Any more comments?
|
Fri 04 Jan 2008 05:23:52 PM UTC, comment #10:
If we receive whole sized packet, it will be trimmed till window size.
If client application do not receive data,
with one byte probe we receive window update. (Window now one byte less).
And this one byte will be added to the receive queue.
In worst case we can have 100 packets...
>> Persist timer is always bounded between 5 and 60 seconds
>Do you have a reference for that? RFC793 or 1122 don't mention this bound as far as I could see.
At least Stevens say about that.
http://www.uic.rsu.ru/doc/inet/tcp_stevens/tcp_pers.htm
|
Fri 04 Jan 2008 01:13:17 PM UTC, comment #9:
> I don't think that send 1 byte probe very good for our
> implementation.
> Imagine 100 TCP segments in receive queue each one byte long.
> May be better to send whole segment without split?
I wouldn't actually split the packet, just send one byte from it, and leave it unsplit on the unsent queue. The one byte probe should hopefully result in an ACK and a window update from the other side, which would then allow us to send the whole packet. So there should only ever be one 1-byte-probe in the receive queue, rather than 100. The reason I like this method is that it requires very little extra code. Sending the whole packet would be good in that sense too, but I was a bit uneasy about relying on the receiver to deal with the oversize packet. If the one byte probe turns out to be causing problems, it should be easy to change in the future to send a whole packet, but I'd rather try one byte probes first.
> Persist timer is always bounded between 5 and 60 seconds
Do you have a reference for that? RFC793 or 1122 don't mention this bound as far as I could see.
|
Fri 04 Jan 2008 12:53:21 PM UTC, comment #8:
That is right.
The missed case - the window is not zero, but too small to send whole packet.
I don't think that send 1 byte probe very good for our implementation.
Imagine 100 TCP segments in receive queue each one byte long.
May be better to send whole segment without split?
And one more issue.
In suggested patch initial timeout for persistent timer is 1.5 seconds.
But the Persist timer is always bounded between 5 and 60 seconds.
So instead
const u8_t tcp_persist_backoff[7] = { 3, 6, 12, 24, 48, 96, 120 };
should be something like this
const u8_t tcp_persist_backoff[7] = { 10, 10, 12, 24, 48, 96, 120 };
|
Thu 03 Jan 2008 08:54:00 AM UTC, comment #7:
Thanks for taking a look.
For the avoidance of confusion, I think you're describing the case where the window is not zero, but we can't send because the head of the send queue is bigger than the available window.
This is something I'd overlooked, so I think you're right. Probably the easiest solution would be to just start the persist timer in that case, and send the one-byte probes just as we would in the zero-window case. I'll look into this later.
|
Wed 02 Jan 2008 05:31:38 PM UTC, comment #6:
Hi Kieran,
in original patch if send window is not 0
and we will send segment that greater then send window,
this segment will be splitted, and only first part sent.
In your implementation if send window is not 0
persistent timer is not started and segment can not be
moved from unsent queue to unacked queue.
It seems to me that this case is not covered with this patch.
May be instead of splitted segmented is possible to send whole segment, and other site should trim this segment if it does not have enough place.
|
Mon 31 Dec 2007 01:36:01 PM UTC, comment #5:
I've checked this in, but again would like to see it more widely tested before we close this bug.
|
Thu 20 Dec 2007 03:49:07 PM UTC, comment #4:
I've merged and simplified the persist timer portion of the patch but I think this at least needs testing to make sure it doesn't break anything else before it can be committed. Any volunteers? I'm not in a position to test it myself at the moment and this is the last issue currently gating 1.3.0
(file #14676)
|
Tue 18 Dec 2007 01:50:39 PM UTC, comment #3:
(from lwip-users, by Per-Henrik Lundblom)
Hi,
I guess it is about time to commit the mods and fixes we have done on the lwIP stack. I have included and attached one large patch (I know it's bad with one patch to fix several problems but take it or leave it). The patch was made against a CVS copy from 2007-09-20.
The patch should fix the following things:
- Silly window problem (bug #20199)
- Implementation of the TCP persist timer which lwIP curretly lacks. The
persist timer is MANDATORY for any TCP implementation. This
implementation can be slimmed down. This implementation doesn't share
any code with the retransmit timer even though it should be possible.
Also, the tcp_split_unsent_seq() function has code copied from other
tcp_*() functions. I guess you could leave out the split part of the
persist timer if you want to reduce code size but then you don't fully
implement the functionality for the persist timer.
Note that this code was 100% written during work time at connectBlue AB
(www.connectblue.se) but is released under the same license as the rest of lwIP. We really hope it will find its way into the the trunk and that others will find it useful.
Merry Christmas to all of you!
/PH
(file #14652)
|
Tue 06 Nov 2007 01:03:47 PM UTC, comment #2:
(from bug #21522, by Oleg Tyshev)
This document has good description of different approaches to zero window and keep alive implementations. http://www.usenix.org/publications/library/proceedings/bos94/full_papers/lin.a
|
Thu 19 Jul 2007 01:06:16 AM UTC, comment #1:
This problem can show up under the following circumstances:
If lwIP has send enough data that the "remote" has filled its received window and has returned a zero (or smaller than the segment lwIP wants to send) window, lwIP stops. When the window opens, the remote sends an ACK with a window update. If this packet gets lost (this is a common occurrence), then lwIP won't send data until the other end sends another window update. It will only do this if it has some data to send back. So in a unidirectional data transfer setup, one missed window update/ACK packet results in lwIP locking up.
The reference for this is in the section "Managing the Window" on
page 42 of RFC793:
ftp://ftp.rfc-editor.org/in-notes/rfc793.txt
And in section 4.2.2.17 of:
ftp://ftp.rfc-editor.org/in-notes/rfc1122.txt
For previous discussion on this issue, and why it might be a bit tricky, please refer to the following posts in the lwip-users list:
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00033.html
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00034.html
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00037.html
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00042.html
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00043.html
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00032.html
http://lists.nongnu.org/archive/html/lwip-users/2007-06/msg00077.html
|
Wed 18 Jul 2007 09:31:50 AM UTC, original submission:
TCP implementations must have the persist timer to send zero-window probes allowing it to cope with a lost window update. We don't currently have one. Should be straightforward enough as rather similar to the keepalive probes and timer.
|