buglwIP - A Lightweight TCP/IP stack - Bugs: bug #20126, Zero ssthresh bug

 
 

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

bug #20126: Zero ssthresh bug

Submitter:  Per-Henrik Lundblom <vajper>
Submitted:  Fri 08 Jun 2007 10:35:17 AM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  fbernon
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Mon 18 Jun 2007 02:06:35 PM UTC, comment #12: 

Ok, it's closed. Thank you to report that...

Frédéric Bernon <fbernon>
Group Member
Mon 18 Jun 2007 09:49:38 AM UTC, comment #11: 

Confirmed!

Per-Henrik Lundblom <vajper>
Sun 17 Jun 2007 05:08:08 PM UTC, comment #10: 

Ok, it's check in. Per-Henrik, Kieran, can you confirm we can close it?

Frédéric Bernon <fbernon>
Group Member
Fri 15 Jun 2007 09:13:59 AM UTC, comment #9: 

Yes please, sorry for the delay in replying.

Kieran Mansley <kieranm>
Group Member
Tue 12 Jun 2007 04:43:25 PM UTC, comment #8: 

Kieran, do you want I check in that (if you always got problems with CVS)?

Frédéric Bernon <fbernon>
Group Member
Tue 12 Jun 2007 04:42:01 PM UTC, comment #7: 

I agree...

Per-Henrik Lundblom <vajper>
Tue 12 Jun 2007 03:42:25 PM UTC, comment #6: 

Yes.

Kieran Mansley <kieranm>
Group Member
Tue 12 Jun 2007 03:34:16 PM UTC, comment #5: 

So, you propose to add this to fix this problem?

if (pcb->sstresh < 2*pcb->mss)
  pcb->sstresh = 2*pcb->mss;

Frédéric Bernon <fbernon>
Group Member
Tue 12 Jun 2007 03:26:43 PM UTC, comment #4: 

I'd be happier with the 2*MSS rather than MSS.  When you first reported the problem I thought 2*MSS was right, but didn't like to question you and didn't have time to look it up.  It's probably something to do with interaction with the delayed ACK mechanism, or Nagle, or something like that.

Anyway, adding a 2*MSS lower limit to ssthresh is fine by me.

Kieran Mansley <kieranm>
Group Member
Mon 11 Jun 2007 03:53:38 PM UTC, comment #3: 

Is it this book release ?

http://angrypacket.com/~enz00/txts/tcpip.pdf

(Note, I don't know if this link is very legal...)

Frédéric Bernon <fbernon>
Group Member
Mon 11 Jun 2007 03:39:40 PM UTC, comment #2: 

Oh, as always when you go public with something it appears you are wrong =/

Actually, my interpretion was a bit wrong. Now when I read the text I would say ssthresh never can be less than 2*MSS. This haven't been tested with lwIP because I don't have the test setup at hand today.

Anyway, I get the minimum value of 2*MSS from TCP/IP Illustrated chapter 21.6 from the part describing the congestion avoidance algoritm. Point 3 reads:

"When congestion occurs (indicated by a timeout or the reception of duplicate ACKs), one-half of the current window size (the minimum of cwnd and the receiver's advertised window, but at least two segments) is saved in ssthresh. Additionally, if the congestion is indicated by a timeout, cwnd is set to one segment (i.e., slow start)."

It clearly says that at least two segments are saved in ssthresh.

/PH

Per-Henrik Lundblom <vajper>
Mon 11 Jun 2007 02:53:48 PM UTC, comment #1: 

Is there any TCP expert to confirm that...

if (pcb->sstresh < pcb->mss)
  pcb->sstresh = pcb->mss;

...is a valid fix?

I suppose that lwIP's "sstresh" is the "Slow-start threshold (ssthresh)" described in  http://www.faqs.org/rfcs/rfc2581.html

Per-Henrik, can you tell me which part of the RFC tell that "sstresh never should be assigned a value less than one MSS"?

Frédéric Bernon <fbernon>
Group Member
Fri 08 Jun 2007 10:35:17 AM UTC, original submission:  

When heavily opening and closing the tcp receive window on one side of a TCP connection, I ran into the problem with the TCP sstresh value being set to 0 resulting in a stop of transmission. My interpretion of the TCP standards (carefully studying of TCP/IP Illustrated) is that sstresh never should be assigned a value less than one MSS. Adding zero value check to the code solved all the transmissions stops.

My proposed patch against 1.2.0 is:

--- lwip-1.2.0/src/core/tcp_in.c        2006-08-18 12:18:42.000000000
+0200
+++ lwip-1.2.0_patched/src/core/tcp_in.c        2007-06-08
10:24:42.451875000 +0200
@@ -678,6 +678,9 @@
               pcb->ssthresh = pcb->snd_wnd / 2;
             else
               pcb->ssthresh = pcb->cwnd / 2;
+
+            if (pcb->sstresh == 0)
+              pcb->sstresh = pcb->mss;

             pcb->cwnd = pcb->ssthresh + 3 * pcb->mss;
             pcb->flags |= TF_INFR;


I have only seen the sstresh being set to 0, but probably it is better to do:

            if (pcb->sstresh < pcb->mss)
              pcb->sstresh = pcb->mss;


Per-Henrik Lundblom <vajper>

 

(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 kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by vajper (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-06-18 fbernon StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2007-06-17 fbernon StatusNone Ready For Test
        Assigned toNone fbernon

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code