From f8e45bf7a99dc79515eb36046e0cafc1a88873f4 Mon Sep 17 00:00:00 2001 From: Vikas Gaur Date: Tue, 9 Jul 2024 09:21:20 +0530 Subject: [PATCH] tcp: Align PRR implementation with RFC 6937 bis-08 Co-authored-by: Aniket Singh --- RELEASE_NOTES.md | 1 + src/internet/doc/tcp.rst | 36 ++++++++++++ src/internet/model/tcp-prr-recovery.cc | 56 ++++++++++--------- src/internet/model/tcp-prr-recovery.h | 12 +--- src/internet/model/tcp-recovery-ops.cc | 6 +- src/internet/model/tcp-recovery-ops.h | 6 +- src/internet/model/tcp-socket-base.cc | 6 +- .../test/tcp-classic-recovery-test.cc | 2 +- src/internet/test/tcp-prr-recovery-test.cc | 28 +++------- 9 files changed, 90 insertions(+), 63 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index fef0299ff..db97f786d 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -43,6 +43,7 @@ been tested on Linux. As of this release, the latest known version to work with - (lr-wpan) !2163 - Lr-wpan module now uses the `ns3::lrwpan` namespace in its TypeId. - (internet) !2027 - TcpSocketBase: Added TCP retransmission trace - (applications) !2027 - BulkSendApplication: Added TCP retransmission trace consuming TcpSocketBase's TCP retransmission trace +- (tcp) !2059 - Aligns PRR implementation with RFC 6937 bis-08. Added a new param `isDupAck` to `DoRecovery` method, removed `ReductionBound` attribute from `TcpPrrRecovery`. ### Bugs fixed diff --git a/src/internet/doc/tcp.rst b/src/internet/doc/tcp.rst index 36d92a623..64cb9a934 100644 --- a/src/internet/doc/tcp.rst +++ b/src/internet/doc/tcp.rst @@ -1662,6 +1662,42 @@ After limit calculation, the cWnd is updated as given below: .. math:: sndcnt = MIN (ssThresh - pipe, limit) .. math:: cWnd = pipe + sndcnt +Thanks to Neal Cardwell for providing the following documentation of |ns3| PRR model: +The |ns3| implementation of PRR has something like this (note that m_isRetransDataAcked is true exactly when retransmitted data is cumulatively ACKed, and there is no RACK-TLP or similar mechanism to detect lost retransmits as of now): + +:: + + // PRR-CRB by default + int limit = std::max(m_prrDelivered - m_prrOut, deliveredBytes); + // safeACK should be true iff ACK advances SND.UNA with no further loss indicated. + // We approximate that here (given the current lack of RACK-TLP in ns-3): + bool safeACK = tcb->m_isRetransDataAcked; // retransmit cumulatively ACKed? + if (safeACK) // PRR-SSRB when recovery makes good progress + limit += tcb->m_segmentSize; + // Attempt to catch up, as permitted + sendCount = std::min(limit, static_cast(tcb->m_ssThresh - tcb->m_bytesInFlight)); + } + // Force a fast retransmit upon entering fast recovery: + if (m_prrOut == 0 && sendCount == 0) + sendCount = tcb->m_segmentSize; + +which is close to the Linux implementation (which matches the RFC): + +:: + + int delta = tp->snd_ssthresh - tcp_packets_in_flight(tp); + ... + sndcnt = max_t(int, tp->prr_delivered - tp->prr_out, + newly_acked_sacked); + if (flag & FLAG_SND_UNA_ADVANCED && !newly_lost) + sndcnt++; + sndcnt = min(delta, sndcnt); + } + /* Force a fast retransmit upon entering fast recovery */ + sndcnt = max(sndcnt, (tp->prr_out ? 0 : 1)); + +For reference, the Linux TCP PRR implementation is entirely contained in tcp_init_cwnd_reduction(), tcp_cwnd_reduction(), tcp_end_cwnd_reduction() and the updates elsewhere to prr_out. + More information (paper): https://dl.acm.org/citation.cfm?id=2068832 More information (RFC): https://tools.ietf.org/html/rfc6937 diff --git a/src/internet/model/tcp-prr-recovery.cc b/src/internet/model/tcp-prr-recovery.cc index 04f524951..fddca640d 100644 --- a/src/internet/model/tcp-prr-recovery.cc +++ b/src/internet/model/tcp-prr-recovery.cc @@ -24,16 +24,10 @@ NS_OBJECT_ENSURE_REGISTERED(TcpPrrRecovery); TypeId TcpPrrRecovery::GetTypeId() { - static TypeId tid = - TypeId("ns3::TcpPrrRecovery") - .SetParent() - .AddConstructor() - .SetGroupName("Internet") - .AddAttribute("ReductionBound", - "Type of Reduction Bound", - EnumValue(SSRB), - MakeEnumAccessor(&TcpPrrRecovery::m_reductionBoundMode), - MakeEnumChecker(CRB, "CRB", SSRB, "SSRB")); + static TypeId tid = TypeId("ns3::TcpPrrRecovery") + .SetParent() + .AddConstructor() + .SetGroupName("Internet"); return tid; } @@ -47,8 +41,7 @@ TcpPrrRecovery::TcpPrrRecovery(const TcpPrrRecovery& recovery) : TcpClassicRecovery(recovery), m_prrDelivered(recovery.m_prrDelivered), m_prrOut(recovery.m_prrOut), - m_recoveryFlightSize(recovery.m_recoveryFlightSize), - m_reductionBoundMode(recovery.m_reductionBoundMode) + m_recoveryFlightSize(recovery.m_recoveryFlightSize) { NS_LOG_FUNCTION(this); } @@ -68,37 +61,50 @@ TcpPrrRecovery::EnterRecovery(Ptr tcb, m_prrOut = 0; m_prrDelivered = 0; - m_recoveryFlightSize = unAckDataCount; + m_recoveryFlightSize = tcb->m_bytesInFlight; // RFC 6675 pipe before recovery - DoRecovery(tcb, deliveredBytes); + DoRecovery(tcb, deliveredBytes, true); } void -TcpPrrRecovery::DoRecovery(Ptr tcb, uint32_t deliveredBytes) +TcpPrrRecovery::DoRecovery(Ptr tcb, uint32_t deliveredBytes, bool isDupAck) { NS_LOG_FUNCTION(this << tcb << deliveredBytes); + + if (isDupAck && m_prrDelivered < m_recoveryFlightSize) + { + deliveredBytes += tcb->m_segmentSize; + } + if (deliveredBytes == 0) + { + return; + } + m_prrDelivered += deliveredBytes; int sendCount; if (tcb->m_bytesInFlight > tcb->m_ssThresh) { + // Proportional Rate Reductions sendCount = std::ceil(m_prrDelivered * tcb->m_ssThresh * 1.0 / m_recoveryFlightSize) - m_prrOut; } else { - int limit = static_cast(tcb->m_ssThresh - tcb->m_bytesInFlight); - if (m_reductionBoundMode == CRB) + // PRR-CRB by default + int limit = std::max(m_prrDelivered - m_prrOut, deliveredBytes); + + // safeACK should be true iff ACK advances SND.UNA with no further loss indicated. + // We approximate that here (given the current lack of RACK-TLP in ns-3) + bool safeACK = tcb->m_isRetransDataAcked; // retransmit cumulatively ACKed? + + if (safeACK) { - limit = m_prrDelivered - m_prrOut; - } - else if (m_reductionBoundMode == SSRB) - { - if (tcb->m_isRetransDataAcked) - { - limit = std::max(m_prrDelivered - m_prrOut, deliveredBytes) + tcb->m_segmentSize; - } + // PRR-SSRB when recovery makes good progress + limit += tcb->m_segmentSize; } + + // Attempt to catch up, as permitted sendCount = std::min(limit, static_cast(tcb->m_ssThresh - tcb->m_bytesInFlight)); } diff --git a/src/internet/model/tcp-prr-recovery.h b/src/internet/model/tcp-prr-recovery.h index 9a509d02a..734a4d65c 100644 --- a/src/internet/model/tcp-prr-recovery.h +++ b/src/internet/model/tcp-prr-recovery.h @@ -50,15 +50,6 @@ class TcpPrrRecovery : public TcpClassicRecovery ~TcpPrrRecovery() override; - /** - * \brief Reduction Bound variant (CRB or SSRB) - */ - enum ReductionBound_t - { - CRB, /**< Conservative Reduction Bound */ - SSRB /**< Slow Start Reduction Bound */ - }; - std::string GetName() const override; void EnterRecovery(Ptr tcb, @@ -66,7 +57,7 @@ class TcpPrrRecovery : public TcpClassicRecovery uint32_t unAckDataCount, uint32_t deliveredBytes) override; - void DoRecovery(Ptr tcb, uint32_t deliveredBytes) override; + void DoRecovery(Ptr tcb, uint32_t deliveredBytes, bool isDupAck) override; void ExitRecovery(Ptr tcb) override; @@ -78,7 +69,6 @@ class TcpPrrRecovery : public TcpClassicRecovery uint32_t m_prrDelivered{0}; //!< total bytes delivered during recovery phase uint32_t m_prrOut{0}; //!< total bytes sent during recovery phase uint32_t m_recoveryFlightSize{0}; //!< value of bytesInFlight at the start of recovery phase - ReductionBound_t m_reductionBoundMode{SSRB}; //!< mode of Reduction Bound to be used }; } // namespace ns3 diff --git a/src/internet/model/tcp-recovery-ops.cc b/src/internet/model/tcp-recovery-ops.cc index 454c349be..1bfba96c5 100644 --- a/src/internet/model/tcp-recovery-ops.cc +++ b/src/internet/model/tcp-recovery-ops.cc @@ -94,9 +94,11 @@ TcpClassicRecovery::EnterRecovery(Ptr tcb, } void -TcpClassicRecovery::DoRecovery(Ptr tcb, uint32_t deliveredBytes [[maybe_unused]]) +TcpClassicRecovery::DoRecovery(Ptr tcb, + uint32_t deliveredBytes [[maybe_unused]], + bool isDupAck) { - NS_LOG_FUNCTION(this << tcb << deliveredBytes); + NS_LOG_FUNCTION(this << tcb << deliveredBytes << isDupAck); tcb->m_cWndInfl += tcb->m_segmentSize; } diff --git a/src/internet/model/tcp-recovery-ops.h b/src/internet/model/tcp-recovery-ops.h index 56ffbe902..48b13b31d 100644 --- a/src/internet/model/tcp-recovery-ops.h +++ b/src/internet/model/tcp-recovery-ops.h @@ -99,11 +99,13 @@ class TcpRecoveryOps : public Object * The function is called on arrival of every ack when TcpSocketState * is set to CA_RECOVERY. It performs the necessary cwnd changes * as per the recovery algorithm. + * The param `isDupAck` has been added to align PRR implementation with RFC 6937 bis-08. * * \param tcb internal congestion state * \param deliveredBytes bytes (S)ACKed in the last (S)ACK + * \param isDupAck Indicates if the last acknowledgement was duplicate. */ - virtual void DoRecovery(Ptr tcb, uint32_t deliveredBytes) = 0; + virtual void DoRecovery(Ptr tcb, uint32_t deliveredBytes, bool isDupAck) = 0; /** * \brief Performs cwnd adjustments at the end of recovery @@ -178,7 +180,7 @@ class TcpClassicRecovery : public TcpRecoveryOps uint32_t unAckDataCount, uint32_t deliveredBytes) override; - void DoRecovery(Ptr tcb, uint32_t deliveredBytes) override; + void DoRecovery(Ptr tcb, uint32_t deliveredBytes, bool isDupAck) override; void ExitRecovery(Ptr tcb) override; diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index 1fa7970ba..433d27c45 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -1779,7 +1779,7 @@ TcpSocketBase::DupAck(uint32_t currentDelivered) } if (!m_congestionControl->HasCongControl()) { - m_recoveryOps->DoRecovery(m_tcb, currentDelivered); + m_recoveryOps->DoRecovery(m_tcb, currentDelivered, true); NS_LOG_INFO(m_dupAckCount << " Dupack received in fast recovery mode." "Increase cwnd to " << m_tcb->m_cWnd); @@ -2085,7 +2085,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber, // there is available window if (!m_congestionControl->HasCongControl() && segsAcked >= 1) { - m_recoveryOps->DoRecovery(m_tcb, currentDelivered); + m_recoveryOps->DoRecovery(m_tcb, currentDelivered, false); } // If the packet is already retransmitted do not retransmit it @@ -2146,7 +2146,7 @@ TcpSocketBase::ProcessAck(const SequenceNumber32& ackNumber, // and/or packet reordering if (!m_congestionControl->HasCongControl() && segsAcked >= 1) { - m_recoveryOps->DoRecovery(m_tcb, currentDelivered); + m_recoveryOps->DoRecovery(m_tcb, currentDelivered, false); } NewAck(ackNumber, true); } diff --git a/src/internet/test/tcp-classic-recovery-test.cc b/src/internet/test/tcp-classic-recovery-test.cc index 7b1fb634c..3cf628f10 100644 --- a/src/internet/test/tcp-classic-recovery-test.cc +++ b/src/internet/test/tcp-classic-recovery-test.cc @@ -90,7 +90,7 @@ ClassicRecoveryTest::DoRun() uint32_t cWndInflPrevious = m_state->m_cWndInfl; uint32_t cWndPrevious = m_state->m_cWnd; - recovery->DoRecovery(m_state, 500); + recovery->DoRecovery(m_state, 500, false); NS_TEST_ASSERT_MSG_EQ( m_state->m_cWndInfl, (cWndInflPrevious + m_state->m_segmentSize), diff --git a/src/internet/test/tcp-prr-recovery-test.cc b/src/internet/test/tcp-prr-recovery-test.cc index bd17746b9..6c3c3ded3 100644 --- a/src/internet/test/tcp-prr-recovery-test.cc +++ b/src/internet/test/tcp-prr-recovery-test.cc @@ -36,7 +36,6 @@ class PrrRecoveryTest : public TestCase * \param bytesInFlight Current bytes in flight. * \param m_deliveredBytes Bytes SACKed on last acknowledgment. * \param bytesSent Bytes sent while in recovery phase. - * \param reductionBound Type of reduction bound to be used. * \param name Test description. */ PrrRecoveryTest(uint32_t cWnd, @@ -46,20 +45,18 @@ class PrrRecoveryTest : public TestCase uint32_t bytesInFlight, uint32_t m_deliveredBytes, uint32_t bytesSent, - const std::string& reductionBound, const std::string& name); private: void DoRun() override; - uint32_t m_cWnd; //!< Congestion window. - uint32_t m_segmentSize; //!< Segment size. - uint32_t m_ssThresh; //!< Slow Start Threshold. - uint32_t m_unAckDataCount; //!< Unacknowledged data at the start of recovery. - uint32_t m_bytesInFlight; //!< Current bytes in flight. - uint32_t m_deliveredBytes; //!< Bytes SACKed on last acknowledgment. - uint32_t m_bytesSent; //!< Bytes sent while in recovery phase. - const std::string m_reductionBound; //!< Type of reduction bound to be used. + uint32_t m_cWnd; //!< Congestion window. + uint32_t m_segmentSize; //!< Segment size. + uint32_t m_ssThresh; //!< Slow Start Threshold. + uint32_t m_unAckDataCount; //!< Unacknowledged data at the start of recovery. + uint32_t m_bytesInFlight; //!< Current bytes in flight. + uint32_t m_deliveredBytes; //!< Bytes SACKed on last acknowledgment. + uint32_t m_bytesSent; //!< Bytes sent while in recovery phase. Ptr m_state; //!< TCP socket state. }; @@ -71,7 +68,6 @@ PrrRecoveryTest::PrrRecoveryTest(uint32_t cWnd, uint32_t bytesInFlight, uint32_t deliveredBytes, uint32_t bytesSent, - const std::string& reductionBound, const std::string& name) : TestCase(name), m_cWnd(cWnd), @@ -80,8 +76,7 @@ PrrRecoveryTest::PrrRecoveryTest(uint32_t cWnd, m_unAckDataCount(unAckDataCount), m_bytesInFlight(bytesInFlight), m_deliveredBytes(deliveredBytes), - m_bytesSent(bytesSent), - m_reductionBound(reductionBound) + m_bytesSent(bytesSent) { } @@ -97,7 +92,6 @@ PrrRecoveryTest::DoRun() m_state->m_bytesInFlight = m_bytesInFlight; Ptr recovery = CreateObject(); - recovery->SetAttribute("ReductionBound", StringValue(m_reductionBound)); recovery->EnterRecovery(m_state, 3, m_unAckDataCount, 0); @@ -114,7 +108,7 @@ PrrRecoveryTest::DoRun() m_bytesInFlight += m_state->m_cWnd.Get() - m_cWnd; m_state->m_bytesInFlight = m_bytesInFlight; m_cWnd = m_state->m_cWnd.Get(); - recovery->DoRecovery(m_state, m_deliveredBytes); + recovery->DoRecovery(m_state, m_deliveredBytes, false); if (m_bytesInFlight > m_state->m_ssThresh) { @@ -151,7 +145,6 @@ class PrrRecoveryTestSuite : public TestSuite 3000, 500, 1000, - "SSRB", "Prr test on cWnd when bytesInFlight is greater than ssThresh with SSRB"), TestCase::Duration::QUICK); AddTestCase(new PrrRecoveryTest( @@ -162,7 +155,6 @@ class PrrRecoveryTestSuite : public TestSuite 1000, 500, 1000, - "SSRB", "Prr test on cWnd when bytesInFlight is lower than ssThresh with SSRB"), TestCase::Duration::QUICK); AddTestCase(new PrrRecoveryTest( @@ -173,7 +165,6 @@ class PrrRecoveryTestSuite : public TestSuite 3000, 500, 1000, - "CRB", "Prr test on cWnd when bytesInFlight is greater than ssThresh with CRB"), TestCase::Duration::QUICK); AddTestCase(new PrrRecoveryTest( @@ -184,7 +175,6 @@ class PrrRecoveryTestSuite : public TestSuite 1000, 500, 1000, - "CRB", "Prr test on cWnd when bytesInFlight is lower than ssThresh with CRB"), TestCase::Duration::QUICK); }