From d42d15b50fc1ae4e4733836e83df01c27b7addf4 Mon Sep 17 00:00:00 2001 From: Natale Patriciello Date: Sun, 25 Feb 2018 11:36:52 +0100 Subject: [PATCH] tcp: Removed CraftSackOption from the API --- src/internet/doc/tcp.rst | 50 ++++------- src/internet/model/tcp-tx-buffer.cc | 93 -------------------- src/internet/model/tcp-tx-buffer.h | 11 --- src/internet/test/tcp-tx-buffer-test.cc | 107 ------------------------ 4 files changed, 15 insertions(+), 246 deletions(-) diff --git a/src/internet/doc/tcp.rst b/src/internet/doc/tcp.rst index e087de57b..a87e84821 100644 --- a/src/internet/doc/tcp.rst +++ b/src/internet/doc/tcp.rst @@ -394,7 +394,7 @@ to such value. .. math:: cWnd = (1-b(cWnd)) \cdot cWnd The lookup table for the function b() is taken from the same RFC. -More informations at: http://dl.acm.org/citation.cfm?id=2756518 +More information at: http://dl.acm.org/citation.cfm?id=2756518 Hybla ^^^^^ @@ -408,7 +408,7 @@ This coefficient is used to calculate both the slow start threshold and the congestion window when in slow start and in congestion avoidance, respectively. -More informations at: http://dl.acm.org/citation.cfm?id=2756518 +More information at: http://dl.acm.org/citation.cfm?id=2756518 Westwood ^^^^^^^^ @@ -419,7 +419,7 @@ bandwidth and use the estimated value to adjust the cwnd.· While Westwood performs the bandwidth sampling every ACK reception,· Westwood+ samples the bandwidth every RTT. -More informations at: http://dl.acm.org/citation.cfm?id=381704 and +More information at: http://dl.acm.org/citation.cfm?id=381704 and http://dl.acm.org/citation.cfm?id=2512757 Vegas @@ -446,7 +446,7 @@ Following the implementation of Vegas in Linux, we use 2, 4, and 1 as the default values of alpha, beta, and gamma, respectively, but they can be modified through the Attribute system. -More informations at: http://dx.doi.org/10.1109/49.464716 +More information at: http://dx.doi.org/10.1109/49.464716 Scalable ^^^^^^^^ @@ -465,7 +465,7 @@ the following equation: .. math:: cwnd = cwnd - ceil(0.125 \cdot cwnd) -More informations at: http://dl.acm.org/citation.cfm?id=956989 +More information at: http://dl.acm.org/citation.cfm?id=956989 Veno ^^^^ @@ -503,7 +503,7 @@ because the loss encountered is more likely a corruption-based loss than a congestion-based. Only when N is greater than beta, Veno halves its sending rate as in Reno. -More informations at: http://dx.doi.org/10.1109/JSAC.2002.807336 +More information at: http://dx.doi.org/10.1109/JSAC.2002.807336 Bic ^^^ @@ -526,7 +526,7 @@ If a loss occur in either these phases, the current window (before the loss) can be treated as the new maximum, and the reduced (with a multiplicative decrease factor Beta) window size can be used as the new minimum. -More informations at: http://ieeexplore.ieee.org/xpl/articleDetails.jsp?arnumber=1354672 +More information at: http://ieeexplore.ieee.org/xpl/articleDetails.jsp?arnumber=1354672 YeAH ^^^^ @@ -853,40 +853,20 @@ congestion window event. TCP SACK and non-SACK +++++++++++++++++++++ To avoid code duplication and the effort of maintaining two different versions -of the TCP core, namely RFC 6675 (TCP-SACK) and RFC 5681 (TCP congestion -control), we have merged RFC 6675 in the current code base. If the receiver -supports the option, the sender bases its retransmissions over the received -SACK information. However, in the absence of that option, the best it can do is -to follow the RFC 5681 specification (on Fast Retransmit/Recovery) and -employing NewReno modifications in case of partial ACKs. +of the TCP core, namely RFC 6675 (TCP-SACK) and RFC 5681 (TCP congestion control), +we have merged RFC 6675 in the current code base. If the receiver supports the +option, the sender bases its retransmissions over the received SACK information. +However, in the absence of that option, the best it can do is to follow the RFC +5681 specification (on Fast Retransmit/Recovery) and employing NewReno +modifications in case of partial ACKs. -The merge work consisted in implementing an emulation of fake SACK options in -the sender (when the receiver does not support SACK) following RFC 5681 rules. -The generation is straightforward: each duplicate ACK (following the definition -of RFC 5681) carries a new SACK option, that indicates (in increasing order) -the blocks transmitted after the SND.UNA, not including the block starting from -SND.UNA itself. - -With this emulated SACK information, the sender behaviour is unified in these -two cases. By carefully generating these SACK block, we are able to employ all -the algorithms outlined in RFC 6675 (e.g. Update(), NextSeg(), IsLost()) during -non-SACK transfers. Of course, in the case of RTO expiration, no guess about -SACK block could be made, and so they are not generated (consequently, the -implementation will re-send all segments starting from SND.UNA, even the ones -correctly received). Please note that the generated SACK option (in the case of -a non-SACK receiver) by the sender never leave the sender node itself; they are -created locally by the TCP implementation and then consumed. - -A similar concept is used in Linux with the function tcp_add_reno_sack. Our -implementation resides in the TcpTxBuffer class that implements a scoreboard +A similar concept is used in Linux with the function tcp_add_reno_sack. +Our implementation resides in the TcpTxBuffer class that implements a scoreboard through two different lists of segments. TcpSocketBase actively uses the API provided by TcpTxBuffer to query the scoreboard; please refer to the Doxygen documentation (and to in-code comments) if you want to learn more about this implementation. -When SACK attribute is enabled for the receiver socket, the sender will not -craft any SACK option, relying only on what it receives from the network. - Current limitations +++++++++++++++++++ diff --git a/src/internet/model/tcp-tx-buffer.cc b/src/internet/model/tcp-tx-buffer.cc index 08b0dfbd7..35ba3e629 100644 --- a/src/internet/model/tcp-tx-buffer.cc +++ b/src/internet/model/tcp-tx-buffer.cc @@ -1029,99 +1029,6 @@ TcpTxBuffer::IsHeadRetransmitted () const return m_sentList.front ()->m_retrans; } -Ptr -TcpTxBuffer::CraftSackOption (const SequenceNumber32 &seq, uint8_t available) const -{ - NS_LOG_FUNCTION (this); - Ptr sackBlock = 0; - SequenceNumber32 beginOfCurrentPacket = m_firstByteSeq; - Ptr current; - TcpTxItem *item; - - NS_LOG_INFO ("Crafting a SACK block, available bytes: " << (uint32_t) available << - " from seq: " << seq << " buffer starts at seq " << m_firstByteSeq); - - PacketList::const_iterator it; - if (m_highestSack.first == m_sentList.end ()) - { - it = m_sentList.begin (); - } - else - { - it = m_highestSack.first; - beginOfCurrentPacket = m_highestSack.second; - } - - while (it != m_sentList.end ()) - { - item = *it; - current = item->m_packet; - - SequenceNumber32 endOfCurrentPacket = beginOfCurrentPacket + current->GetSize (); - - // The first segment could not be sacked.. otherwise would be a - // cumulative ACK :) - if (item->m_sacked || it == m_sentList.begin ()) - { - NS_LOG_DEBUG ("Analyzing segment: [" << beginOfCurrentPacket << - ";" << endOfCurrentPacket << "], not usable, sacked=" << - item->m_sacked); - beginOfCurrentPacket += current->GetSize (); - } - else if (seq > beginOfCurrentPacket) - { - NS_LOG_DEBUG ("Analyzing segment: [" << beginOfCurrentPacket << - ";" << endOfCurrentPacket << "], not usable, sacked=" << - item->m_sacked); - beginOfCurrentPacket += current->GetSize (); - } - else - { - // RFC 2018: The first SACK block MUST specify the contiguous - // block of data containing the segment which triggered this ACK. - // Since we are hand-crafting this, select the first non-sacked block. - sackBlock = CreateObject (); - sackBlock->AddSackBlock (TcpOptionSack::SackBlock (beginOfCurrentPacket, - endOfCurrentPacket)); - NS_LOG_DEBUG ("Analyzing segment: [" << beginOfCurrentPacket << - ";" << endOfCurrentPacket << "] and found to be usable"); - - // RFC 2018: The data receiver SHOULD include as many distinct SACK - // blocks as possible in the SACK option - // The SACK option SHOULD be filled out by repeating the most - // recently reported SACK blocks that are not subsets of a SACK block - // already included - // This means go backward until we finish space and include already SACKed block - while (sackBlock->GetSerializedSize () + 8 < available) - { - --it; - - if (it == m_sentList.begin ()) - { - return sackBlock; - } - - item = *it; - current = item->m_packet; - endOfCurrentPacket = beginOfCurrentPacket; - beginOfCurrentPacket -= current->GetSize (); - sackBlock->AddSackBlock (TcpOptionSack::SackBlock (beginOfCurrentPacket, - endOfCurrentPacket)); - NS_LOG_DEBUG ("Filling the option: Adding [" << beginOfCurrentPacket << - ";" << endOfCurrentPacket << "], available space now : " << - (uint32_t) (available - sackBlock->GetSerializedSize ())); - NS_ASSERT (beginOfCurrentPacket > m_firstByteSeq); - } - - return sackBlock; - } - - ++it; - } - - return sackBlock; -} - std::ostream & operator<< (std::ostream & os, TcpTxBuffer const & tcpTxBuf) { diff --git a/src/internet/model/tcp-tx-buffer.h b/src/internet/model/tcp-tx-buffer.h index ead4a25a4..a23429fda 100644 --- a/src/internet/model/tcp-tx-buffer.h +++ b/src/internet/model/tcp-tx-buffer.h @@ -311,17 +311,6 @@ public: */ void ResetLastSegmentSent (); - /** - * \brief Craft a SACK block. Used in case the other end does not support - * SACK - * - * \param seq Look to usable block starting from this sequence number (seq will - * not be included in the block) - * \param available Space left in the header for that SACK option - * \return a SACK option that SACK the first un-SACKed segment in our sentList. - */ - Ptr CraftSackOption (const SequenceNumber32 &seq, uint8_t available) const; - private: friend std::ostream & operator<< (std::ostream & os, TcpTxBuffer const & tcpTxBuf); diff --git a/src/internet/test/tcp-tx-buffer-test.cc b/src/internet/test/tcp-tx-buffer-test.cc index 83d1a6f29..43f9b321b 100644 --- a/src/internet/test/tcp-tx-buffer-test.cc +++ b/src/internet/test/tcp-tx-buffer-test.cc @@ -47,8 +47,6 @@ private: void TestTransmittedBlock (); /** \brief Test the generation of the "next" block */ void TestNextSeg (); - /** \brief Test the scoreboard with emulated SACK */ - void TestUpdateScoreboardWithCraftedSACK (); }; TcpTxBufferTestCase::TcpTxBufferTestCase () @@ -80,8 +78,6 @@ TcpTxBufferTestCase::DoRun () &TcpTxBufferTestCase::TestTransmittedBlock, this); Simulator::Schedule (Seconds (0.0), &TcpTxBufferTestCase::TestNextSeg, this); - Simulator::Schedule (Seconds (0.0), - &TcpTxBufferTestCase::TestUpdateScoreboardWithCraftedSACK, this); Simulator::Run (); Simulator::Destroy (); @@ -287,109 +283,6 @@ TcpTxBufferTestCase::TestNewBlock () "Size is different than expected"); } -void -TcpTxBufferTestCase::TestUpdateScoreboardWithCraftedSACK () -{ - TcpTxBuffer txBuf; - SequenceNumber32 head (1); - txBuf.SetHeadSequence (head); - - // Add a single, 3000-bytes long, packet - txBuf.Add (Create (30000)); - - // Simulate sending 100 packets, 150 bytes long each, from seq 1 - for (uint32_t i=0; i<100; ++i) - { - txBuf.CopyFromSequence (150, head + (150 * i)); - } - - // Now we have 100 packets sent, 100 waiting (well, that 100 are condensed in one) - - // Suppose now we receive 99 dupacks, because the first was lost. - for (uint32_t i=0; i<99; ++i) - { - Ptr sack = txBuf.CraftSackOption (head, 32); // 3 maximum sack block - - // For iteration 0 and 1 we have 1 and 2 sack blocks, respectively. - // For all others, maximum 3 - if (i == 0) - { - NS_TEST_ASSERT_MSG_EQ (sack->GetNumSackBlocks (), 1, - "Different block number than expected"); - } - else if (i == 1) - { - NS_TEST_ASSERT_MSG_EQ (sack->GetNumSackBlocks (), 2, - "Different block number than expected"); - } - else if (i >= 2) - { - NS_TEST_ASSERT_MSG_EQ (sack->GetNumSackBlocks (), 3, - "More blocks than expected"); - } - - TcpOptionSack::SackList sackList = sack->GetSackList (); - TcpOptionSack::SackBlock block = sackList.front (); - sackList.pop_front (); - - // The first block, assuming all the other are SACKed in order (from 2nd - // onward) has seq = 1 + (150 * (i+1)) --> i+1 because the first sent - // block cannot be SACKed - NS_TEST_ASSERT_MSG_EQ (block.first, SequenceNumber32 (1 + (150*(i+1))), - "First sack block is wrong (on the left)"); - NS_TEST_ASSERT_MSG_EQ (block.second, block.first + 150, - "First sack block is wrong (on the right)"); - - SequenceNumber32 left = block.first; - for (TcpOptionSack::SackList::iterator it = sackList.begin (); it != sackList.end (); ++it) - { - block = (*it); - - // the blocks go backwards here. To understand better, an example - // of SACK option list: [1351;1501], [1201;1351], [1051;1201] - NS_TEST_ASSERT_MSG_EQ (block.first, left - 150, - "First sack block is wrong (on the left)"); - NS_TEST_ASSERT_MSG_EQ (block.second, left, - "First sack block is wrong (on the right)"); - left -= 150; - } - - txBuf.Update (sack->GetSackList ()); - - if (i == 0) - { - NS_TEST_ASSERT_MSG_EQ (false, txBuf.IsLost (SequenceNumber32 (1), 3, 150), - "SequenceNumber 1 isLost, but for RFC 6675 is not"); - } - else if (i == 1) - { - NS_TEST_ASSERT_MSG_EQ (false, txBuf.IsLost (SequenceNumber32 (1), 3, 150), - "SequenceNumber 1 isLost, but for RFC 6675 is not"); - SequenceNumber32 lost (1 + (150 * i)); - NS_TEST_ASSERT_MSG_EQ (false, txBuf.IsLost (lost, 3, 150), - "SequenceNumber " << lost << - "isLost, but for RFC 6675 is because is SACKed"); - } - else if (i >= 2) - { - NS_TEST_ASSERT_MSG_EQ (true, txBuf.IsLost (SequenceNumber32 (1), 3, 150), - "SequenceNumber 1 ! isLost, but for RFC 6675 is"); - SequenceNumber32 lost (1 + (150 * i)); - NS_TEST_ASSERT_MSG_EQ (false, txBuf.IsLost (lost, 3, 150), - "SequenceNumber " << lost << - "isLost, but for RFC 6675 is because is SACKed"); - } - } - - for (uint32_t i=0; i<100; ++i) - { - txBuf.DiscardUpTo (SequenceNumber32 (30001)); - } - - NS_TEST_ASSERT_MSG_EQ (txBuf.Size (), 0, - "Data inside the buffer"); -} - void TcpTxBufferTestCase::TestTransmittedBlock () {