From f75345fa0378bec5a7640a4dbb9fb2d441b27ebc Mon Sep 17 00:00:00 2001 From: Tom Henderson Date: Fri, 17 May 2024 14:10:18 -0700 Subject: [PATCH] tcp (fixes #966): Suppress cwnd growth when rate-limited --- CHANGES.md | 1 + RELEASE_NOTES.md | 1 + src/internet/model/tcp-cubic.cc | 7 +++++++ src/internet/model/tcp-dctcp.cc | 1 + src/internet/model/tcp-linux-reno.cc | 14 ++++++++++++++ src/internet/model/tcp-linux-reno.h | 17 +++++++++++++++++ src/internet/model/tcp-socket-base.cc | 6 ++++++ src/internet/model/tcp-socket-state.cc | 1 + src/internet/model/tcp-socket-state.h | 1 + src/test/ns3tcp/ns3tcp-cubic-test-suite.cc | 12 ++++++------ 10 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d835c885c..8bd670c1f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -55,6 +55,7 @@ Applications have a new Attribute to set the IPv4 ToS field. ### Changed behavior * Fixed the corner rebound direction in `RandomWalk2d[Outdoor]MobilityModel` and the initial direction in case of node starting from a border or corner. +* (tcp) TcpCubic and TcpLinuxReno will no longer grow their congestion window when application-limited, now matching Linux behavior Changes from ns-3.40 to ns-3.41 ------------------------------- diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e957924a0..d2823e3b8 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -54,6 +54,7 @@ topocentric coordinate systems - (bindings, core) - Introduced a helper class to manage static initialization of Time as a workaround for Cppyy3 static initialization problems. - (bindings, lte, wifi) - Relocated statically initialized variables from header files to source files for Cppyy3 compatibility. - (tests) - Enhanced error handling in test.py to avoid attempts to open non-existent XML files following early test termination by sanitizers. +- (tcp) #966 - Tcp Cubic (and LinuxReno) cwnd should not grow during application-limited phase - (tcp) #1085 - Do not reset Cubic W_max upon timeout - (wifi) #1072 - Support configuration of custom EDCA parameters via Txop attributes before device installation - (wifi) - Fix operation in 6 GHz band (added support for FILS Discovery frames and HE 6GHz Band Capabilities information element, fixed HE Operation information element, fixed NSS selection, fixed HT and VHT not supported on 6GHz links). diff --git a/src/internet/model/tcp-cubic.cc b/src/internet/model/tcp-cubic.cc index 0e6f831d8..24298a338 100644 --- a/src/internet/model/tcp-cubic.cc +++ b/src/internet/model/tcp-cubic.cc @@ -189,6 +189,13 @@ TcpCubic::IncreaseWindow(Ptr tcb, uint32_t segmentsAcked) { NS_LOG_FUNCTION(this << tcb << segmentsAcked); + if (!tcb->m_isCwndLimited) + { + NS_LOG_DEBUG("No increase because current cwnd " << tcb->m_cWnd + << " is not limiting the flow"); + return; + } + if (tcb->m_cWnd < tcb->m_ssThresh) { if (m_hystart && tcb->m_lastAckedSeq > m_endSeq) diff --git a/src/internet/model/tcp-dctcp.cc b/src/internet/model/tcp-dctcp.cc index 98b52dd80..00069103d 100644 --- a/src/internet/model/tcp-dctcp.cc +++ b/src/internet/model/tcp-dctcp.cc @@ -121,6 +121,7 @@ TcpDctcp::Init(Ptr tcb) tcb->m_useEcn = TcpSocketState::On; tcb->m_ecnMode = TcpSocketState::DctcpEcn; tcb->m_ectCodePoint = m_useEct0 ? TcpSocketState::Ect0 : TcpSocketState::Ect1; + SetSuppressIncreaseIfCwndLimited(false); m_initialized = true; } diff --git a/src/internet/model/tcp-linux-reno.cc b/src/internet/model/tcp-linux-reno.cc index f77a0c912..20a248006 100644 --- a/src/internet/model/tcp-linux-reno.cc +++ b/src/internet/model/tcp-linux-reno.cc @@ -79,6 +79,13 @@ TcpLinuxReno::CongestionAvoidance(Ptr tcb, uint32_t segmentsAcke { NS_LOG_FUNCTION(this << tcb << segmentsAcked); + if (m_suppressIncreaseIfCwndLimited && !tcb->m_isCwndLimited) + { + NS_LOG_DEBUG("No increase because current cwnd " << tcb->m_cWnd + << " is not limiting the flow"); + return; + } + uint32_t w = tcb->m_cWnd / tcb->m_segmentSize; // Floor w to 1 if w == 0 @@ -150,4 +157,11 @@ TcpLinuxReno::Fork() return CopyObject(this); } +void +TcpLinuxReno::SetSuppressIncreaseIfCwndLimited(bool value) +{ + NS_LOG_FUNCTION(this << value); + m_suppressIncreaseIfCwndLimited = value; +} + } // namespace ns3 diff --git a/src/internet/model/tcp-linux-reno.h b/src/internet/model/tcp-linux-reno.h index 792aea7d2..16066870b 100644 --- a/src/internet/model/tcp-linux-reno.h +++ b/src/internet/model/tcp-linux-reno.h @@ -76,8 +76,25 @@ class TcpLinuxReno : public TcpCongestionOps */ virtual void CongestionAvoidance(Ptr tcb, uint32_t segmentsAcked); + /** + * TcpSocketBase follows the Linux way of setting a flag 'isCwndLimited' + * when BytesInFlight() >= cwnd. This flag, if true, will suppress + * additive increase window updates for this class. However, some + * derived classes using the IncreaseWindow() method may not want this + * behavior, so this method exists to allow subclasses to set it to false. + * + * \param value Value to set whether the isCwndLimited condition + * suppresses window updates + */ + void SetSuppressIncreaseIfCwndLimited(bool value); + private: uint32_t m_cWndCnt{0}; //!< Linear increase counter + // The below flag exists for classes derived from TcpLinuxReno (such as + // TcpDctcp) that may not want to suppress cwnd increase, unlike Linux's + // tcp_reno_cong_avoid() + bool m_suppressIncreaseIfCwndLimited{ + true}; //!< Suppress window increase if TCP is not cwnd limited }; } // namespace ns3 diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index 11eb8742e..aca3f0caa 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -3238,6 +3238,12 @@ TcpSocketBase::SendDataPacket(SequenceNumber32 seq, uint32_t maxSize, bool withA << m_endPoint6->GetPeerAddress() << ". Header " << header); } + // Signal to congestion control whether the cwnd is fully used + // This is a simple version of Linux tcp_cwnd_validate() but following + // the principle implemented in Linux that limits the updating of cwnd + // (in the congestion controls) when flight size is >= cwnd + m_tcb->m_isCwndLimited = (BytesInFlight() >= m_tcb->m_cWnd); + UpdateRttHistory(seq, sz, isRetransmission); // Update bytes sent during recovery phase diff --git a/src/internet/model/tcp-socket-state.cc b/src/internet/model/tcp-socket-state.cc index 3292fa1f3..ccc20f123 100644 --- a/src/internet/model/tcp-socket-state.cc +++ b/src/internet/model/tcp-socket-state.cc @@ -119,6 +119,7 @@ TcpSocketState::TcpSocketState(const TcpSocketState& other) m_paceInitialWindow(other.m_paceInitialWindow), m_minRtt(other.m_minRtt), m_bytesInFlight(other.m_bytesInFlight), + m_isCwndLimited(other.m_isCwndLimited), m_lastRtt(other.m_lastRtt), m_ecnMode(other.m_ecnMode), m_useEcn(other.m_useEcn), diff --git a/src/internet/model/tcp-socket-state.h b/src/internet/model/tcp-socket-state.h index eb605c6ce..5f80d9073 100644 --- a/src/internet/model/tcp-socket-state.h +++ b/src/internet/model/tcp-socket-state.h @@ -207,6 +207,7 @@ class TcpSocketState : public Object Time m_minRtt{Time::Max()}; //!< Minimum RTT observed throughout the connection TracedValue m_bytesInFlight{0}; //!< Bytes in flight + bool m_isCwndLimited{false}; //!< Whether throughput is limited by cwnd TracedValue