From ee70f4502b6fe59e8d2b9f0a84b6418a6e303b31 Mon Sep 17 00:00:00 2001
From: Xiuchao Wu <xiuchao@amazon.com>
Date: Thu, 13 Aug 2020 14:33:18 +0100
Subject: [PATCH] tcp: (merges !367) Stop to merge items whose m_lost values
 are different

---
 src/internet/model/tcp-tx-buffer.cc     |  4 ++--
 src/internet/test/tcp-tx-buffer-test.cc | 31 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/internet/model/tcp-tx-buffer.cc b/src/internet/model/tcp-tx-buffer.cc
index df96dd4e3..7e897b656 100644
--- a/src/internet/model/tcp-tx-buffer.cc
+++ b/src/internet/model/tcp-tx-buffer.cc
@@ -312,8 +312,8 @@ TcpTxBuffer::GetTransmittedSegment (uint32_t numBytes, const SequenceNumber32 &s
           next++;
           if (next != m_sentList.end ())
             {
-              // Next is not sacked... there is the possibility to merge
-              if (! (*next)->m_sacked)
+              // Next is not sacked and have the same value for m_lost ... there is the possibility to merge
+              if ((! (*next)->m_sacked) && ((*it)->m_lost == (*next)->m_lost))
                 {
                   s = std::min(s, (*it)->m_packet->GetSize () + (*next)->m_packet->GetSize ());
                 }
diff --git a/src/internet/test/tcp-tx-buffer-test.cc b/src/internet/test/tcp-tx-buffer-test.cc
index f65b848e6..9823b0776 100644
--- a/src/internet/test/tcp-tx-buffer-test.cc
+++ b/src/internet/test/tcp-tx-buffer-test.cc
@@ -50,6 +50,9 @@ private:
   void TestTransmittedBlock ();
   /** \brief Test the generation of the "next" block */
   void TestNextSeg ();
+  /** \brief Test the logic of merging items in GetTransmittedSegment()
+   * which is triggered by CopyFromSequence()*/
+  void TestMergeItemsWhenGetTransmittedSegment ();
   /** \brief Callback to provide a value of receiver window */
   uint32_t GetRWnd (void) const;
 };
@@ -85,6 +88,16 @@ TcpTxBufferTestCase::DoRun ()
   Simulator::Schedule (Seconds (0.0),
                        &TcpTxBufferTestCase::TestNextSeg, this);
 
+  /*
+   * Case for transmitted block:
+   *  -> transmitted packets are marked differently for m_lost under some scenarios
+   *  -> packets could be small than MSS when socket buffer is not a multiple of MSS.
+   *  -> during retransmission, the sender tries to send a full segment but it
+   *     should stop to merge items when they have different values for m_lost.
+   */
+  Simulator::Schedule (Seconds (0.0),
+                         &TcpTxBufferTestCase::TestMergeItemsWhenGetTransmittedSegment, this);
+
   Simulator::Run ();
   Simulator::Destroy ();
 }
@@ -351,6 +364,24 @@ TcpTxBufferTestCase::TestNewBlock ()
                          "Size is different than expected");
 }
 
+void
+TcpTxBufferTestCase::TestMergeItemsWhenGetTransmittedSegment ()
+{
+  TcpTxBuffer txBuf;
+  SequenceNumber32 head (1);
+  txBuf.SetHeadSequence (head);
+  txBuf.SetSegmentSize (2000);
+
+  txBuf.Add(Create<Packet> (2000));
+  txBuf.CopyFromSequence (1000, SequenceNumber32(1));
+  txBuf.CopyFromSequence (1000, SequenceNumber32(1001));
+  txBuf.MarkHeadAsLost();
+
+  // GetTransmittedSegment() will be called and handle the case that two items
+  // have different m_lost value.
+  txBuf.CopyFromSequence (2000, SequenceNumber32(1));
+}
+
 void
 TcpTxBufferTestCase::TestTransmittedBlock ()
 {