buglwIP - A Lightweight TCP/IP stack - Bugs: bug #25882, TCP hangs on MSS >...

 
 

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

bug #25882: TCP hangs on MSS > pcb->snd_wnd

Submitter:  Homyak <onlyslon>
Submitted:  Mon 16 Mar 2009 09:25:27 AM UTC
   
 
Category:  TCP Severity:  2 - Minor
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Tue 13 Mar 2012 04:30:53 PM UTC, comment #17: 

Good enough.

Kieran Mansley <kieranm>
Group Member
Sun 12 Feb 2012 01:17:53 PM UTC, comment #16: 

I've pushed a fix for this where we make sure we don't allocate segments bigger than half the max. window we ever received. That way, we always send at least 2 segments per full window and the problem stated in the summary is fixed.

There is of course still a theoretical possibility of a host not ACKing but I'm not sure we really need to fix this, can that ever happen? It would mean that the remote host as more than a half rx window of data that it could ACK. That would surely be a bug in the remote host's tcp, wouldn't it?

Simon Goldschmidt <goldsimon>
Group administrator
Sun 22 Jan 2012 09:36:25 PM UTC, comment #15: 

Oh, I didn't mean 1 and 2 to be mutually exclusive, sorry if it read like that.

Can you explain why the first wouldn't be accurate? I would have thought it should even be enough to record the initial window included in the SYN to calculate the maximum segment size...

2 is suboptimal, yes, but since 1 already takes care of splitting a full window of data into at least 2 segments, a good TCP implementation should send a delayed ACK before our timer triggers that forces us to do packet splitting. Being like that, 2 should only be a minimal code change that prevents lwIP from stalling in such a rare situation, even if it imposes extra work (i.e. packet splitting and memcpy).

Of course, a better idea is always welcome, this is only the best I came up with, so far :-)

Simon Goldschmidt <goldsimon>
Group administrator
Sun 22 Jan 2012 07:16:20 PM UTC, comment #14: 

Does it have to be one or the other?  Would adding both not be possible?

They both have disadvantages; the first requires maintaining extra state that isn't necessarily going to be accurate.  The second needs us to do packet splitting that is something of a pain.

Kieran Mansley <kieranm>
Group Member
Sun 22 Jan 2012 03:40:46 PM UTC, comment #13: 

I have been working on this and came to a solution with 2 changes:

1) track the biggest snd_wnd we received and use its half as an additional limit on mss (i.e. when we create segments in tcp_write, the maximum size is now "MIN(pcb->mss, pcb->snd_wnd_max/2)" instead of "pcb->mss"); this ensures we always need 2 segments for one window.

2) Using the persist timer in a 2nd mode: if we have a segment that starts inside the window but doesn't fit into it, start persist timer in a mode that:
a) sends what fits into the window on timeout (instead of only 1 byte) and
b) doesn't prevent the pcb from timing out

While 1) solves the original problem (and lwIP now should behave as linux does in the attached pcap trace), 2) solves the potential problem where the remote side waits for the window to grow but lwIP doesn't send because a pre-created segment is too big.

As this is (once again) the last open bug to delay the next release, please comment :-)

Code follows as soon as my development machine is running again...

Simon Goldschmidt <goldsimon>
Group administrator
Fri 02 Sep 2011 08:50:50 PM UTC, comment #12: 

This one has been idling around here for a while, but I'd like to fix it for 1.4.1. The question is, should we

a) change the segment allocation strategy to use the maximum-advertised window size as a limit to MSS or
b) use the persist timer to sent as much as the window allows in such a 'stall' scenario?

I think a) would fix the original post but would require yet another member in struct tcp_pcb. It might not solve all problems, though.

OTOH, b) would fix all scenarios where the window stays smaller then the next segment we have allocated, however at the delay of the persist timer.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 30 Aug 2011 03:30:00 PM UTC, comment #11: 

Hope this could be a way to improve performance:
--Adjusting the value of MSS like congestion avoidance.

tao tang <opensms4>
Tue 18 May 2010 01:28:54 PM UTC, comment #10: 

It would be nice to get this into 1.4.0 as it's one of the few bugs that would cause severe problems when it's encountered, but in the absence of anyone with time to do the work postponing to 1.4.1 as suggested.

Kieran Mansley <kieranm>
Group Member
Wed 21 Apr 2010 08:13:52 PM UTC, comment #9: 

Is this necessary for 1.4.0? I don't think I'll find the time to work on it in the next weeks/months, and I'd like to get 1.4.0 out in the next 2 months... so unless anyone else has the time to work on this, I'd target it as 1.4.1...

Simon Goldschmidt <goldsimon>
Group administrator
Sun 22 Nov 2009 07:14:08 PM UTC, comment #8: 


> for example keeping track of the largest window the other side has advertised


That would mean taking this value as an upper limit for segment sizes in tcp_enqueue? That would solve the problem where the server never announced a window >= MSS. Plus it's an easy chance that doesn't have a big influence on performance.

But couldn't a (badly implemented) remote TCP get into a state where the window once was larger but now doesn't grow? Could this case be solved by including code in the persist timer to send more than one byte (i.e. send as much as the window allows)?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 28 Apr 2009 12:53:59 PM UTC, comment #7: 

Or being more sophisticated, for example keeping track of the largest window the other side has advertised, and basing our decision for "should I wait for the window to grow, or send now" on that.

Kieran Mansley <kieranm>
Group Member
Tue 28 Apr 2009 12:28:06 PM UTC, comment #6: 

But we'd have to limit it to being seldom used (i.e. only if the connection would stall otherwise) to prevent that code being executed on a normal connection where the window grows big enough at a later point and we can send our pre-packaged packets.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 28 Apr 2009 07:58:15 AM UTC, comment #5: 

I think we do need to fix it, but it's not urgent.

Kieran Mansley <kieranm>
Group Member
Mon 27 Apr 2009 07:26:51 PM UTC, comment #4: 

The question is if we really have to handle this? A connection like this is likely to be very ineffective!

Starting with lwIP, I thought the way data is packed into segments is a problem, but I've come to think of it as quite handy and very performance-optimized. Moving from this implementation to being able to send data as the remote side requires from its window would be a great increasement of complexity.

And the way the original poster tried to fix it doesn't really work, either, as at the time the segment is created, you cannot tell how the send window will behave: you could later have a send window that again doesn't match the segments we have created before...

To sum it up: without the ability of re-packaging segments, we won't be able to reliably cope with this. And as this is too complex, I'd vote against it.

Plus: I think Jakob is right in that the server behaves odd!

Simon Goldschmidt <goldsimon>
Group administrator
Tue 17 Mar 2009 06:38:14 PM UTC, comment #3: 

Interesting. The server negotiates a 1360-byte MSS, but never announces more than 256 bytes of window. I think you are right that lwIP cannot handle this.

There are two problems with your proposed solution:

  • It affects normal connections. When the window closes, tcp_enqueue could create lots of small segments. It would be better to create normal MSS-sized segments and wait for the window to open.


  • It doesn't actually work if the server initially announces a large window, and then changes to a window < MSS. You could create a segment that gets stuck, even with your fix.



Jakob Stoklund Olesen <stoklund>
Group Member
Tue 17 Mar 2009 09:42:06 AM UTC, comment #2: 

(between my linux & web-server)

11:31:37.244390 IP 77.94.42.148.51045 > 77.222.40.229.80: S 1267816717:1267816717(0) win 5840 <mss 1460,sackOK,timestamp 557886541 0,nop,wscale 5>
11:31:37.603601 IP 77.222.40.229.80 > 77.94.42.148.51045: S 2899588054:2899588054(0) ack 1267816718 win 192 <mss 1360,sackOK,timestamp 2877898228 557886541,nop,wscale 7>
11:31:37.603640 IP 77.94.42.148.51045 > 77.222.40.229.80: . ack 1 win 183 <nop,nop,timestamp 557886631 2877898228>

A full session is attached to the comment.

Linux & Windows TCP stack properly handle this situation.

(file #17717)

Homyak <onlyslon>
Tue 17 Mar 2009 08:31:57 AM UTC, comment #1: 

Does the server permanently announce such a small window? Normally the window would open, and the MSS-sized segments can escape.

Does the server negotiate MSS? It is unfortunate if the maximum window size is smaller than the MSS. I am not sure what should be done in that case.

Jakob Stoklund Olesen <stoklund>
Group Member
Mon 16 Mar 2009 09:25:27 AM UTC, original submission:  

Hello.

I found a little bug when try using lwip as HTTP client. TCP session hangs if server announce small window (like 192 bytes).

The problem arose when the client (lwip) tried to write to socket more data than remote recv window.

In tcp_enqueue I found

  while (queue == NULL || left > 0) {

    /* The segment length should be the MSS if the data to be enqueued
     is larger than the MSS. /
    seglen = left > pcb->mss? pcb->mss: left;


but then seglen > snd_wnd lwip does not send this segment and this pcb loops on zero-window-probe packets

Possibly solution for this bug:

seglen = left > pcb->mss? pcb->mss: left;
+ seglen = seglen > pcb->snd_wnd ? pcb->snd_wnd : seglen;

at tcp_enqueue

In my case it works.

Homyak <onlyslon>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #17717:  dump.raw added by onlyslon (36KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by opensms4 (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by stoklund (Posted a comment)
  • -email is unavailable- added by onlyslon (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-03-13 kieranm StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2012-02-12 goldsimon StatusNeed Info In Progress
    2011-09-02 goldsimon StatusNone Need Info
        Assigned toNone goldsimon
    2010-05-18 kieranm Planned Release1.4.x 1.4.1
    2010-05-04 goldsimon Planned Release1.4.0 1.4.x
    2009-08-20 kieranm Planned Release 1.4.0
    2009-05-01 goldsimon Severity3 - Normal 2 - Minor
    2009-03-17 onlyslon Attached File- Added dump.raw, #17717

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code