buglwIP - A Lightweight TCP/IP stack - Bugs: bug #34517, persist timer is started although...

 
 

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

bug #34517: persist timer is started although no zero window is received

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Mon 10 Oct 2011 07:50:11 AM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Fri 04 Nov 2016 06:42:38 PM UTC, comment #9: 

Opened bug #49533 to move the discussion of the identified bug, also provide a patch

Joel Cunningham <jcunningham>
Group Member
Tue 01 Nov 2016 08:39:05 PM UTC, comment #8: 

Simon,

What is the bug you reference which handles the case where LwIP treats a non-zero window less than 1 MSS as a zero window?

I'm running into this issue currently with the git master branch. 

What I'm seeing is where the receiver's window is less than 1 MSS but the next packet on the unsent queue is 1 MSS, so LwIP stops sending.  Then the window update from the receiver is dropped (verified update is sent in wireshark, but tcphdr in tcp_in.c only shows the previous ACK with < 1 MSS window).

Since the zero window timer isn't running and there are no segments on the unacked queue, LwIP doesn't send anything and the connection deadlocks

Joel Cunningham <jcunningham>
Group Member
Fri 14 Oct 2011 05:24:01 PM UTC, comment #7: 

Yeah, I also thought about that. However, we have another bug for that, which I regard mandatory for 1.4.1 - although I do not have a plan how to implement it.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 14 Oct 2011 03:27:47 PM UTC, comment #6: 

Given that (if I remember correctly) we treat a window that is smaller than an MSS as a zero window so we don't have to split packets, is there a risk here that we could be prevented from sending due to the window being small (effectively a zero window) but we won't have started the persist timer as the window is not actually zero?

Kieran Mansley <kieranm>
Group Member
Thu 13 Oct 2011 07:26:47 PM UTC, comment #5: 

Fixed by starting the persist timer when a zero window is received, not when we have more data queued for sending than fits into the window.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 13 Oct 2011 07:23:20 PM UTC, comment #4: 

Updated summary: the actual bug is that the persist timer is started when we think the window is zero, not when we actually received a zero window.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 11 Oct 2011 06:03:08 AM UTC, comment #3: 

Having had another look at this, I think that our condition when to start the persist timer is somewhat insecure. TCP/IP Illustrated (and other sources) say the persist timer should be started when we receive a zero-window announcement. However, we start it when we have more data to send than the window allows.

This can lead to starting the persist timer when we have much data to send and the remote host is simply disconnected. I think this is insecure since the persist timer prevents connection timeout and we should only use standard retransmission in the case of having too much data to send without receiving a zero window.

This would mean starting the persist timer would be done in tcp_receive when processing an ACK / window update (same place where it is stopped when the window grows > 0) instead of starting it in tcp_output when we have more data to send than the window allows.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 10 Oct 2011 09:04:35 AM UTC, comment #2: 

We have implemented both: if tehre is already-sent (unacked) data, we send that. If not, we send 1 out-of-window byte.

However, aside from the bug I just fixed (wrong byte was sent), I think that if we have unacked data, we should send that whole segment instead of creating a new segment and sending only one byte from it. From reading about TCP implementations, it seems to be a common thing to combine RTO with persist timer (which is roughly what my change would be - although still using 2 counter variables and the persist timer would still fire before the RTO timer does).

Simon Goldschmidt <goldsimon>
Group administrator
Mon 10 Oct 2011 08:21:00 AM UTC, comment #1: 

I think you are correct that the RTO mechanism will discover lost acks when there is unacked data, and the persist (or zero-window) timer is there to discover lost acks when there is no unacked data.

Having the persist timer running when this is unacked data shouldn't cause any harm though, and it may be easier to start it that way.

The mechanism is I think fine.  All it needs to do is guarantee that the receiver will send an ACK when it gets it.  This means either (i) sending a byte we've already sent; or (ii) sending a byte that is out of window.  I prefer (i) which is I think what we have now.

Kieran Mansley <kieranm>
Group Member
Mon 10 Oct 2011 07:50:11 AM UTC, original submission:  

Is it correct that the persist timer is started although there is unacked data? I thought that this timer is responsible to prevent losing ACKs that open the window and I would have assumed that in the case where the window is not 0, the normal RTO-timeout retransmission would take place.

Like it is now (e.g. when the complete window is unacked), the persist timer retransmits 1 byte of the oldest unacked segment instead of transmitting 1 byte out-of-window or retransmitting the complete oldest unacked segment.

Simon Goldschmidt <goldsimon>
Group administrator

 

(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 jcunningham (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-10-13 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2011-10-13 goldsimon Summarypersist timer is started although there is unacked data persist timer is started although no zero window is received
    2011-10-11 goldsimon StatusNone In Progress
        Assigned toNone goldsimon
        Planned Release 1.4.1

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code