Bugzilla – Bug 1565
Silly window syndrome avoidance
Last modified: 2015-10-27 05:56:39 EDT
Created attachment 1494 [details] Silly window syndrome test script I recognized an incomplete implementation of silly window syndrome (SWS) avoidance in ns-3. As far as I understand SWS there are two avoidance methods: First, on the sending side. Here, you try to always send a full MSS. Thus you minimize the relation between payload and header overhead. This is called clumping. In ns-3 this feature is implemented in TcpSocketBase::SendPendingData (lines 1931ff.). Second, on the receiving side. Here, you try to avoid advertising tiny TCP flow control windows. Remedy provides Clark's algorithm (cf. rfc813). In a nutshell you set the advertised window to zero when the actual window is smaller than a MSS or the remaining free space is in the rx buffer is less than half of its max size. This feature is missing in ns-3. Thus you can run in deadlocks where a very small window is advertised, but the sender refuses to send more because of the SWS avoidance. You can produce this situation with the litte simulation script in the attachment. Actually, I think the bug-fix is quite easy. My suggestion is to edit TcpSocketBase::AdvertisedWindowSize as follows: uint16_t TcpSocketBase::AdvertisedWindowSize () { uint16_t w = std::min (m_rxBuffer.MaxBufferSize () - m_rxBuffer.Size (), (uint32_t)m_maxWinSize); return (w < m_segmentSize && w < m_rxBuffer.MaxBufferSize ()/2 ) ? 0 : w; } Feedback is highly appreciated.
Probably, the implementation of SWS avoidance on the sending side TcpSocketBase::SendPendingData is redundant since Nagle's algorithm should already cover it.
Created attachment 1495 [details] SWS patch This patch implements Clark's algorithm and straightens Nagle's algorithm/ sending side's SWS avoidance.
> > uint16_t > TcpSocketBase::AdvertisedWindowSize () > { > uint16_t w = std::min (m_rxBuffer.MaxBufferSize () - m_rxBuffer.Size (), > (uint32_t)m_maxWinSize); > return (w < m_segmentSize && w < m_rxBuffer.MaxBufferSize ()/2 ) ? 0 : w; > } > > > > Feedback is highly appreciated. I agree with this improvement, but deadlocks might still occur if the MSS are unequal between sender and receiver because there are no MSS announcements in ns-3 TCP. So, I would suggest perhaps a note here that we need to revisit this when the MSS option support is eventually added, such as: // m_segmentSize should be replaced by m_sendMssSize when sender's MSS // is eventually learned by this implementation Adrian previously started support for TCP options here: https://codereview.appspot.com/5452045/ it would be nice to push this along if someone has cycles to do this. The above code review just adds support for parsing the options but the logic to use the options is still needed.
(In reply to comment #2) > Created attachment 1495 [details] > SWS patch > > This patch implements Clark's algorithm and straightens Nagle's algorithm/ > sending side's SWS avoidance. Looks good to me; I suggest these steps: 1) make the patch conform to ns-3 coding style use of braces, and add my suggested comment about future MSS option support 2) make the sws.cc into a test case that fails without the patch and passes with it. This will require some testing of e.g. making reasonable progress on the bulk transfer. 3) merge
The roadmap sounds great to me. Thanks. In addition, I added a proposal for an enhancement to TCPs zero-window behaviour in Bug 1751. This is connected to the topic here.
ns-3 does not shrink anymore the advertised window, so the bug isn't valid anymore. Please reopen (or better, create a new bug) if you think it's still present.