From 0bf177fffddbdd3085d04f957445cfe05e57734b Mon Sep 17 00:00:00 2001 From: Satyarth Ratnani Date: Wed, 9 Oct 2024 15:08:11 +0530 Subject: [PATCH] tcp: (fixes #1013) Sync TcpBbr::m_appLimited with TcpRateOps::TcpRateConnection::m_appLimited --- src/internet/doc/tcp.rst | 30 +++++++++++++++++++++++++ src/internet/model/tcp-bbr.cc | 13 +++++++---- src/internet/model/tcp-bbr.h | 6 ++--- src/internet/model/tcp-congestion-ops.h | 9 ++++++++ src/internet/model/tcp-rate-ops.cc | 10 +++++++-- src/internet/model/tcp-rate-ops.h | 11 +++++++++ src/internet/model/tcp-socket-base.cc | 5 ++++- 7 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/internet/doc/tcp.rst b/src/internet/doc/tcp.rst index 809b46641..8b9f49f06 100644 --- a/src/internet/doc/tcp.rst +++ b/src/internet/doc/tcp.rst @@ -1125,6 +1125,36 @@ please refer to: * Vivek Jain, Viyom Mittal and Mohit P. Tahiliani. "Design and Implementation of TCP BBR in ns-3." In Proceedings of the 10th Workshop on ns-3, pp. 16-22. 2018. (https://dl.acm.org/doi/abs/10.1145/3199902.3199911) +Integration with TcpRateOps +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +In the older version of the TCP BBR algorithm, the ``appLimited`` variable was handled directly inside the ``TcpBbr`` class. To make the code more modular and similar to how things are done in Linux TCP, a pointer to the ``TcpRateOps`` class called ``Ptr m_rateOps`` was introduced. This change helps manage the ``appLimited`` state through the rate operations interface instead of directly in ``TcpBbr``. + +Now, the ``appLimited`` value can be accessed with: + +.. code-block:: cpp + + m_rateOps->GetConnectionRate().m_appLimited; + +And it can be updated using: + +.. code-block:: cpp + + m_rateOps->SetAppLimited(tcb->m_bytesInFlight.Get()); + +The ``SetAppLimited`` function, which is part of the ``TcpRateLinux`` class, takes care of updating the ``appLimited`` value based on the current amount of data in flight. + +Here's an example of the ``SetRateOps`` method in ``TcpBbr``: + +.. code-block:: cpp + + void TcpBbr::SetRateOps(Ptr rateOps) + { + m_rateOps = rateOps; + } + +This setup makes the code more organized and reflects how Linux TCP handles the ``appLimited`` state, where it's managed within the TCP socket and updated by rate operations. + Support for Explicit Congestion Notification (ECN) ++++++++++++++++++++++++++++++++++++++++++++++++++ diff --git a/src/internet/model/tcp-bbr.cc b/src/internet/model/tcp-bbr.cc index 579a00825..08fbf95cd 100644 --- a/src/internet/model/tcp-bbr.cc +++ b/src/internet/model/tcp-bbr.cc @@ -119,7 +119,6 @@ TcpBbr::TcpBbr(const TcpBbr& sock) m_isInitialized(sock.m_isInitialized), m_uv(sock.m_uv), m_delivered(sock.m_delivered), - m_appLimited(sock.m_appLimited), m_extraAckedGain(sock.m_extraAckedGain), m_extraAckedWinRtt(sock.m_extraAckedWinRtt), m_extraAckedWinRttLength(sock.m_extraAckedWinRttLength), @@ -132,6 +131,12 @@ TcpBbr::TcpBbr(const TcpBbr& sock) NS_LOG_FUNCTION(this); } +void +TcpBbr::SetRateOps(Ptr rateOps) +{ + m_rateOps = rateOps; +} + const char* const TcpBbr::BbrModeName[BBR_PROBE_RTT + 1] = { "BBR_STARTUP", "BBR_DRAIN", @@ -415,8 +420,7 @@ TcpBbr::HandleProbeRTT(Ptr tcb) { NS_LOG_FUNCTION(this << tcb); - uint32_t totalBytes = m_delivered + tcb->m_bytesInFlight.Get(); - m_appLimited = (totalBytes > 0 ? totalBytes : 1); + m_rateOps->SetAppLimited(tcb->m_bytesInFlight.Get()); if (m_probeRttDoneStamp.IsZero() && tcb->m_bytesInFlight <= m_minPipeCwnd) { @@ -763,7 +767,8 @@ TcpBbr::CwndEvent(Ptr tcb, const TcpSocketState::TcpCAEvent_t ev m_packetConservation = false; RestoreCwnd(tcb); } - else if (event == TcpSocketState::CA_EVENT_TX_START && m_appLimited) + else if (event == TcpSocketState::CA_EVENT_TX_START && + m_rateOps->GetConnectionRate().m_appLimited) { NS_LOG_DEBUG("CwndEvent triggered to CA_EVENT_TX_START :: " << event); m_idleRestart = true; diff --git a/src/internet/model/tcp-bbr.h b/src/internet/model/tcp-bbr.h index 260361193..863a1e5a6 100644 --- a/src/internet/model/tcp-bbr.h +++ b/src/internet/model/tcp-bbr.h @@ -100,6 +100,7 @@ class TcpBbr : public TcpCongestionOps const TcpSocketState::TcpCongState_t newState) override; void CwndEvent(Ptr tcb, const TcpSocketState::TcpCAEvent_t event) override; uint32_t GetSsThresh(Ptr tcb, uint32_t bytesInFlight) override; + void SetRateOps(Ptr rateOps) override; Ptr Fork() override; public: @@ -375,9 +376,8 @@ class TcpBbr : public TcpCongestionOps Time m_minRttStamp; //!< The wall clock time at which the current BBR.RTProp sample was obtained bool m_isInitialized{false}; //!< Set to true after first time initialization variables Ptr m_uv{nullptr}; //!< Uniform Random Variable - uint64_t m_delivered{0}; //!< The total amount of data in bytes delivered so far - uint32_t m_appLimited{ - 0}; //!< The index of the last transmitted packet marked as application-limited + uint64_t m_delivered{0}; //!< The total amount of data in bytes delivered so far + Ptr m_rateOps; //!< Rate operations uint32_t m_extraAckedGain{1}; //!< Gain factor for adding extra ack to cwnd uint32_t m_extraAcked[2]{0, 0}; //!< Maximum excess data acked in epoch uint32_t m_extraAckedWinRtt{0}; //!< Age of extra acked in rtt diff --git a/src/internet/model/tcp-congestion-ops.h b/src/internet/model/tcp-congestion-ops.h index 76128e9a5..388933a05 100644 --- a/src/internet/model/tcp-congestion-ops.h +++ b/src/internet/model/tcp-congestion-ops.h @@ -72,6 +72,15 @@ class TcpCongestionOps : public Object { } + /** + * @brief Set rate operation required by the congestion control algorithm + * + * @param rateOps a pointer to the rate operation object used to manage rate control + */ + virtual void SetRateOps(Ptr rateOps [[maybe_unused]]) + { + } + /** * @brief Get the slow start threshold after a loss event * diff --git a/src/internet/model/tcp-rate-ops.cc b/src/internet/model/tcp-rate-ops.cc index 9334221b9..ed68aca96 100644 --- a/src/internet/model/tcp-rate-ops.cc +++ b/src/internet/model/tcp-rate-ops.cc @@ -144,8 +144,7 @@ TcpRateLinux::CalculateAppLimited(uint32_t cWnd, && in_flight < cWnd // We are not limited by CWND. && lostOut <= retransOut) // All lost packets have been retransmitted. { - m_rate.m_appLimited = std::max(m_rate.m_delivered + in_flight, 1); - m_rateTrace(m_rate); + SetAppLimited(in_flight); } // m_appLimited will be reset once in GenerateSample, if it has to be. @@ -155,6 +154,13 @@ TcpRateLinux::CalculateAppLimited(uint32_t cWnd, // } } +void +TcpRateLinux::SetAppLimited(uint32_t in_flight) +{ + m_rate.m_appLimited = std::max(m_rate.m_delivered + in_flight, 1); + m_rateTrace(m_rate); +} + void TcpRateLinux::SkbDelivered(TcpTxItem* skb) { diff --git a/src/internet/model/tcp-rate-ops.h b/src/internet/model/tcp-rate-ops.h index e4bd81c2b..542e68f87 100644 --- a/src/internet/model/tcp-rate-ops.h +++ b/src/internet/model/tcp-rate-ops.h @@ -113,6 +113,11 @@ class TcpRateOps : public Object * */ virtual const TcpRateConnection& GetConnectionRate() = 0; + /** + * @brief Updates the app-limited state based on in-flight data. + * @param in_flight In Flight size (in bytes) + */ + virtual void SetAppLimited(uint32_t in_flight) = 0; /** * @brief Rate Sample structure @@ -212,6 +217,12 @@ class TcpRateLinux : public TcpRateOps return m_rate; } + /** + * @brief Updates the app-limited state based on in-flight data. + * @param in_flight In Flight size (in bytes) + */ + void SetAppLimited(uint32_t in_flight) override; + /** * TracedCallback signature for tcp rate update events. * diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index 1b4bfd2f7..c0464d55f 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -429,10 +429,13 @@ TcpSocketBase::TcpSocketBase(const TcpSocketBase& sock) m_tcb->m_pacingRate = m_tcb->m_maxPacingRate; m_pacingTimer.SetFunction(&TcpSocketBase::NotifyPacingPerformed, this); + m_rateOps = CreateObject(); + if (sock.m_congestionControl) { m_congestionControl = sock.m_congestionControl->Fork(); m_congestionControl->Init(m_tcb); + m_congestionControl->SetRateOps(m_rateOps); } if (sock.m_recoveryOps) @@ -440,7 +443,6 @@ TcpSocketBase::TcpSocketBase(const TcpSocketBase& sock) m_recoveryOps = sock.m_recoveryOps->Fork(); } - m_rateOps = CreateObject(); if (m_tcb->m_sendEmptyPacketCallback.IsNull()) { m_tcb->m_sendEmptyPacketCallback = MakeCallback(&TcpSocketBase::SendEmptyPacket, this); @@ -4657,6 +4659,7 @@ TcpSocketBase::SetCongestionControlAlgorithm(Ptr algo) NS_LOG_FUNCTION(this << algo); m_congestionControl = algo; m_congestionControl->Init(m_tcb); + m_congestionControl->SetRateOps(m_rateOps); } void