From eff983b6861ff6e0880195b4cfb245a4006e91c8 Mon Sep 17 00:00:00 2001 From: Natale Patriciello Date: Wed, 24 Feb 2016 16:07:54 +0100 Subject: [PATCH] internet: (fixes #2302) check isRetransmission correctly --- src/internet/model/tcp-socket-base.cc | 5 +- src/internet/test/tcp-error-model.cc | 2 +- src/internet/test/tcp-general-test.cc | 17 ++++ src/internet/test/tcp-general-test.h | 9 +++ src/internet/test/tcp-rtt-estimation.cc | 100 ++++++++++++++++++++++-- 5 files changed, 124 insertions(+), 9 deletions(-) diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index 90a5029d3..d16173134 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -2360,8 +2360,7 @@ TcpSocketBase::SendDataPacket (SequenceNumber32 seq, uint32_t maxSize, bool with NS_LOG_FUNCTION (this << seq << maxSize << withAck); bool isRetransmission = false; - if (seq == m_txBuffer->HeadSequence () - && m_txBuffer->HeadSequence () != m_highRxAckMark) + if (seq != m_highTxMark) { isRetransmission = true; } @@ -2446,7 +2445,7 @@ TcpSocketBase::SendDataPacket (SequenceNumber32 seq, uint32_t maxSize, bool with { // Schedules retransmit timeout. If this is a retransmission, double the timer - if (seq != m_highTxMark) + if (isRetransmission) { // This is a retransmit // RFC 6298, clause 2.5 Time doubledRto = m_rto + m_rto; diff --git a/src/internet/test/tcp-error-model.cc b/src/internet/test/tcp-error-model.cc index 8af916133..15e994c4e 100644 --- a/src/internet/test/tcp-error-model.cc +++ b/src/internet/test/tcp-error-model.cc @@ -58,7 +58,7 @@ TcpGeneralErrorModel::DoCorrupt (Ptr p) bool toDrop = ShouldDrop (ipHeader, tcpHeader, p->GetSize ()); - if (toDrop) + if (toDrop && ! m_dropCallback.IsNull ()) { m_dropCallback (ipHeader, tcpHeader, p); } diff --git a/src/internet/test/tcp-general-test.cc b/src/internet/test/tcp-general-test.cc index 4186930b6..6eccf8ff1 100644 --- a/src/internet/test/tcp-general-test.cc +++ b/src/internet/test/tcp-general-test.cc @@ -597,6 +597,23 @@ TcpGeneralTest::GetSegSize (SocketWho who) } } +SequenceNumber32 +TcpGeneralTest::GetHighestTxMark (SocketWho who) +{ + if (who == SENDER) + { + return DynamicCast (m_senderSocket)->m_highTxMark; + } + else if (who == RECEIVER) + { + return DynamicCast (m_receiverSocket)->m_highTxMark; + } + else + { + NS_FATAL_ERROR ("Not defined"); + } +} + uint32_t TcpGeneralTest::GetInitialCwnd (SocketWho who) { diff --git a/src/internet/test/tcp-general-test.h b/src/internet/test/tcp-general-test.h index fd08c2391..13fb6e889 100644 --- a/src/internet/test/tcp-general-test.h +++ b/src/internet/test/tcp-general-test.h @@ -361,6 +361,15 @@ protected: */ uint32_t GetSegSize (SocketWho who); + /** + * \brief Get the highest tx mark of the node specified + * + * \param who node to get the parameter from + * + * \return mark of the specified node + */ + SequenceNumber32 GetHighestTxMark (SocketWho who); + /** * \brief Get the retransmission threshold * \param who node to get the parameter from diff --git a/src/internet/test/tcp-rtt-estimation.cc b/src/internet/test/tcp-rtt-estimation.cc index 21d79a8eb..ab47c76b0 100644 --- a/src/internet/test/tcp-rtt-estimation.cc +++ b/src/internet/test/tcp-rtt-estimation.cc @@ -57,6 +57,7 @@ private: bool m_rttChanged; SequenceNumber32 m_highestTxSeq; uint32_t m_pktCount; + uint32_t m_dataCount; }; TcpRttEstimationTest::TcpRttEstimationTest (const std::string &desc, bool enableTs, @@ -65,7 +66,8 @@ TcpRttEstimationTest::TcpRttEstimationTest (const std::string &desc, bool enable m_enableTs (enableTs), m_rttChanged (false), m_highestTxSeq (0), - m_pktCount (pktCount) + m_pktCount (pktCount), + m_dataCount (0) { } @@ -111,11 +113,12 @@ TcpRttEstimationTest::Tx (const Ptr p, const TcpHeader &h, SocketW if (m_highestTxSeq < h.GetSequenceNumber ()) { m_highestTxSeq = h.GetSequenceNumber (); + m_dataCount = 0; } Ptr rttEstimator = GetRttEstimator (SENDER); NS_ASSERT (rttEstimator != 0); - NS_LOG_DEBUG ("S Rx: seq=" << h.GetSequenceNumber () << " ack=" << h.GetAckNumber ()); + NS_LOG_DEBUG ("S Tx: seq=" << h.GetSequenceNumber () << " ack=" << h.GetAckNumber ()); NS_TEST_ASSERT_MSG_NE (rttEstimator->GetEstimate (), Seconds (1), "Default Estimate for the RTT"); } @@ -134,23 +137,33 @@ void TcpRttEstimationTest::UpdatedRttHistory (const SequenceNumber32 & seq, uint32_t sz, bool isRetransmission, SocketWho who) { + if (sz == 0) + { + return; + } + if (seq < m_highestTxSeq) { NS_TEST_ASSERT_MSG_EQ (isRetransmission, true, "A retransmission is not flagged as such"); } - else + else if (seq == m_highestTxSeq && m_dataCount == 0) { NS_TEST_ASSERT_MSG_EQ (isRetransmission, false, "Incorrectly flagging seq as retransmission"); + m_dataCount++; + } + else if (seq == m_highestTxSeq && m_dataCount > 0) + { + NS_TEST_ASSERT_MSG_EQ (isRetransmission, true, + "A retransmission is not flagged as such"); } - } void TcpRttEstimationTest::RttTrace (Time oldTime, Time newTime) { - NS_LOG_DEBUG ("Rtt changed to " << newTime); + NS_LOG_DEBUG ("Rtt changed to " << newTime.GetSeconds ()); m_rttChanged = true; } @@ -160,6 +173,45 @@ TcpRttEstimationTest::FinalChecks () NS_TEST_ASSERT_MSG_EQ (m_rttChanged, true, "Rtt was not updated"); } +//----------------------------------------------------------------------------- +class TcpRttEstimationWithLossTest : public TcpRttEstimationTest +{ +public: + TcpRttEstimationWithLossTest (const std::string &desc, bool enableTs, + uint32_t pktCount, std::vector toDrop); + +protected: + Ptr CreateReceiverErrorModel (); + +private: + std::vector m_toDrop; +}; + +TcpRttEstimationWithLossTest::TcpRttEstimationWithLossTest (const std::string &desc, + bool enableTs, + uint32_t pktCount, + std::vector toDrop) + : TcpRttEstimationTest (desc, enableTs, pktCount), + m_toDrop (toDrop) +{ + +} + +Ptr +TcpRttEstimationWithLossTest::CreateReceiverErrorModel () +{ + Ptr errorModel = CreateObject (); + + std::vector::iterator it; + + for (it = m_toDrop.begin (); it != m_toDrop.end (); ++it) + { + errorModel->AddSeqToKill (SequenceNumber32 ((*it))); + } + + return errorModel; +} + //----------------------------------------------------------------------------- static class TcpRttEstimationTestSuite : public TestSuite @@ -175,7 +227,45 @@ public: TestCase::QUICK); AddTestCase (new TcpRttEstimationTest ("RTT estimation, no ts, some data", false, 10), TestCase::QUICK); + + std::vector toDrop; + toDrop.push_back (501); + + AddTestCase (new TcpRttEstimationWithLossTest ("RTT estimation, no ts," + " some data, with retr", + false, 10, toDrop), + TestCase::QUICK); + AddTestCase (new TcpRttEstimationWithLossTest ("RTT estimation, ts," + " some data, with retr", + true, 10, toDrop), + TestCase::QUICK); + + toDrop.push_back (501); + AddTestCase (new TcpRttEstimationWithLossTest ("RTT estimation, no ts," + " some data, with retr", + false, 10, toDrop), + TestCase::QUICK); + AddTestCase (new TcpRttEstimationWithLossTest ("RTT estimation, ts," + " some data, with retr", + true, 10, toDrop), + TestCase::QUICK); + + toDrop.push_back (54001); + toDrop.push_back (58001); + toDrop.push_back (58501); + toDrop.push_back (60001); + toDrop.push_back (68501); + AddTestCase (new TcpRttEstimationWithLossTest ("RTT estimation, no ts," + " a lot of data, with retr", + false, 1000, toDrop), + TestCase::QUICK); + AddTestCase (new TcpRttEstimationWithLossTest ("RTT estimation, ts," + " a lot of data, with retr", + true, 1000, toDrop), + TestCase::QUICK); } + } g_tcpRttEstimationTestSuite; } // namespace ns3 +