Bugzilla – Bug 579
TCP congestion window is not updated whent segment size chages
Last modified: 2009-06-30 01:10:07 UTC
Behavior. when using non-standard TCP segment size > 536 bytes first data packet is 3 seconds delayed. Explanation. In the code below: socket = socketFactory->CreateSocket(); socket->SetAttribute ("SegmentSize", UintegerValue (1400)); TCP congestion window size is set to default value of 536 bytes in the first line (CreateSocket()). SetAttribute() call changes segment size but NOT CW size. As a result one has socket with segment size = 1400 and CW size = 536. Trying to send, say, 1000 bytes one will finish in if (w < m_segmentSize && m_pendingData->Size () > w) { break; // No more } of TcpSocketImpl::SendPendingData (tcp-socket-impl.cc:1038), which in turn will cause retry in 3 seconds. Ugly solution. Now I just set TCP segment size globally, e.g. from command line as --ns3::TcpSocket::SegmentSize=1400. This works, because segment size is set BEFORE CreateSocket (). Better solution. I guess, that CW size must be set to max (CW size, new segment size) when segment size is changed. Don't have a patch & test for this solution, sorry.
Could you produce a patch for this ?
Created attachment 498 [details] Solution
I took a look at this and it is a bit more complicated, I think. There are two Attribute setters that interact when setting a third (initial) congestion window variable. We have to worry about consistency of: m_cWnd, m_initialCWnd and m_segmentSize. The only time that the segment size can be changed is from (socket) object creation up to the SYN of a connection which can provide an MSS option. TCP specs say no, can't do it afterward. Ns-3 TCP seems to ignore the MSS in the SYN, so as it stands we just have to deal with the static-initialization-by attribute problem. To make this work, to a first approximation all we have to do is to add: m_cWnd = m_initialCWnd * m_segmentSize; to the Attribute setters for both "InitialCwnd" and "SegmentSize." Then setting either attribute will set the initial m_cWnd correctly. However, the TCP protocol is in charge of changing m_cWnd and it isn't going to expect some other entity to mess with it, so I think we must not allow changes to m_segmentSize or m_initialCWnd after a connection leaves the CLOSED state unless TCP does it (modifies m_Cwnd according to the protocol, or sets m_segmentSize according to the MSS in a SYN). So I think the right thing to do is to also change the attribute setters to restrict the times (states) during which they can be used to preclude having a user shoot him or herself in the foot: void TcpSocketImpl::SetSegSize (uint32_t size) { m_segmentSize = size; NS_ABORT_MSG_UNLESS (m_state == CLOSED, "TcpSocketImpl::SetSegSize(): Cannot change segment size dynamically."); m_cWnd = m_initialCWnd * m_segmentSize; } and void TcpSocketImpl::SetInitialCwnd (uint32_t cwnd) { m_initialCWnd = cwnd; NS_ABORT_MSG_UNLESS (m_state == CLOSED, "TcpSocketImpl::SetInitialCwnd(): Cannot change initial cwnd dynamically."); m_cWnd = m_initialCWnd * m_segmentSize; } If we ever teach ns-3 TCP to repect an initial MSS, it will need to do: m_segmentSize = segmentSizeFromSynOption; m_cWnd = m_initialCWnd * m_segmentSize; outside of the Attribute setters. I think this covers all of the bases. Does this sound right? As it stands now, you can set m_segmentSize to anything you want at any time you want.
> void > TcpSocketImpl::SetSegSize (uint32_t size) > { > m_segmentSize = size; > NS_ABORT_MSG_UNLESS (m_state == CLOSED, > "TcpSocketImpl::SetSegSize(): Cannot change segment size dynamically."); > m_cWnd = m_initialCWnd * m_segmentSize; > } > > and > > void > TcpSocketImpl::SetInitialCwnd (uint32_t cwnd) > { > m_initialCWnd = cwnd; > NS_ABORT_MSG_UNLESS (m_state == CLOSED, > "TcpSocketImpl::SetInitialCwnd(): Cannot change initial cwnd > dynamically."); > m_cWnd = m_initialCWnd * m_segmentSize; > } > > If we ever teach ns-3 TCP to repect an initial MSS, it will need to do: > > m_segmentSize = segmentSizeFromSynOption; > m_cWnd = m_initialCWnd * m_segmentSize; > > outside of the Attribute setters. I think this covers all of the bases. > > Does this sound right? I agree that this patch is more robust and support checking it now in if it tests well. In the future, someone could add the MSS option support, and allow TCP to set the cwnd to the outbound interface MTU - 40 bytes when the initial SYN is sent. But for now, this should solve Pavel's problem.
(In reply to comment #3) Solution by Craig looks pretty good so I close this bug as fixed. Thank you. > I took a look at this and it is a bit more complicated, I think. > > There are two Attribute setters that interact when setting a third (initial) > congestion window variable. We have to worry about consistency of: m_cWnd, > m_initialCWnd and m_segmentSize. > > The only time that the segment size can be changed is from (socket) object > creation up to the SYN of a connection which can provide an MSS option. TCP > specs say no, can't do it afterward. Ns-3 TCP seems to ignore the MSS in the > SYN, so as it stands we just have to deal with the static-initialization-by > attribute problem. > > To make this work, to a first approximation all we have to do is to add: > > m_cWnd = m_initialCWnd * m_segmentSize; > > to the Attribute setters for both "InitialCwnd" and "SegmentSize." Then > setting either attribute will set the initial m_cWnd correctly. > > However, the TCP protocol is in charge of changing m_cWnd and it isn't going to > expect some other entity to mess with it, so I think we must not allow changes > to m_segmentSize or m_initialCWnd after a connection leaves the CLOSED state > unless TCP does it (modifies m_Cwnd according to the protocol, or sets > m_segmentSize according to the MSS in a SYN). > > So I think the right thing to do is to also change the attribute setters to > restrict the times (states) during which they can be used to preclude having a > user shoot him or herself in the foot: > > void > TcpSocketImpl::SetSegSize (uint32_t size) > { > m_segmentSize = size; > NS_ABORT_MSG_UNLESS (m_state == CLOSED, > "TcpSocketImpl::SetSegSize(): Cannot change segment size dynamically."); > m_cWnd = m_initialCWnd * m_segmentSize; > } > > and > > void > TcpSocketImpl::SetInitialCwnd (uint32_t cwnd) > { > m_initialCWnd = cwnd; > NS_ABORT_MSG_UNLESS (m_state == CLOSED, > "TcpSocketImpl::SetInitialCwnd(): Cannot change initial cwnd > dynamically."); > m_cWnd = m_initialCWnd * m_segmentSize; > } > > If we ever teach ns-3 TCP to repect an initial MSS, it will need to do: > > m_segmentSize = segmentSizeFromSynOption; > m_cWnd = m_initialCWnd * m_segmentSize; > > outside of the Attribute setters. I think this covers all of the bases. > > Does this sound right? > > > > > > > > > > > As it stands now, you can set m_segmentSize to anything you want at any time > you want. >
By the way, I have one more confusing 200 ms delay between the first and the second packets in TCP session. This time the reason is delayed ack -- receiver doesn't reply on the first packet waiting for a) the second one OR b) 200 ms (delayed ack timeout). But the sender has CWND = (segment size) by default and doesn't send the second segment waiting for an ack. So both wait for 200 ms in vain. Setting ns3::TcpSocket::InitialCwnd attribute to "2" solves this problem. Should we set this as default (sorry, I don't know what TCP spec says about this)?