From fbd0cfb1fb336a3c2bbc1c3639541412effd34e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20D=C3=B6pmann?=
 <doepmanc@informatik.hu-berlin.de>
Date: Mon, 6 Feb 2017 13:56:19 +0100
Subject: [PATCH] tcp: Advertised Window was still incorrect

---
 CHANGES.html                                  |   1 +
 RELEASE_NOTES                                 |   1 +
 src/internet/model/tcp-socket-base.cc         |  59 ++-
 src/internet/model/tcp-socket-base.h          |   1 +
 .../test/tcp-advertised-window-test.cc        | 381 ++++++++++++++++++
 src/internet/wscript                          |   1 +
 6 files changed, 424 insertions(+), 20 deletions(-)
 create mode 100644 src/internet/test/tcp-advertised-window-test.cc

diff --git a/CHANGES.html b/CHANGES.html
index fe96b944e..c078960bb 100644
--- a/CHANGES.html
+++ b/CHANGES.html
@@ -54,6 +54,7 @@ us a note on ns-developers mailing list.</p>
 <h1>Changes from ns-3.26 to ns-3.27</h1>
 <h2>New API:</h2>
 <ul>
+<li>Added a new attribute in TcpSocketBase to track the advertised window.</li>
 <li>Included the model of <b>TCP Ledbat</b>.</li>
 <li>Included the TCP SACK-based loss recovery algorithm outlined in RFC 6675.</li>
 <li>Added <b>TCP SACK</b> and the <b>SACK emulation</b>. Added an Attribute to TcpSocketBase class,
diff --git a/RELEASE_NOTES b/RELEASE_NOTES
index 5fe5151ed..e0f342347 100644
--- a/RELEASE_NOTES
+++ b/RELEASE_NOTES
@@ -62,6 +62,7 @@ Bugs fixed
 - Bug 2547 - dead assignments on various tcp congestion controls
 - Bug 2551 - wifi preamble should be part of TXVECTOR
 - Bug 2552 - Remove code duplication between Yans and Spectrum
+- Bug 2559 - TCP advertised window still incorrect
 - Bug 2561 - TCP can not use Connect after a BindToNetDevice
 - Bug 2563 - Modify pybindgen version check to >= minimum version required
 - Bug 2564 - Simulation crashes when CtsTimeout is triggered for A-MPDU transmissions
diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc
index 77066450e..3c3fbb467 100644
--- a/src/internet/model/tcp-socket-base.cc
+++ b/src/internet/model/tcp-socket-base.cc
@@ -159,6 +159,10 @@ TcpSocketBase::GetTypeId (void)
                      "TCP Congestion machine state",
                      MakeTraceSourceAccessor (&TcpSocketBase::m_congStateTrace),
                      "ns3::TcpSocketState::TcpCongStatesTracedValueCallback")
+    .AddTraceSource ("AdvWND",
+                     "Advertised Window Size",
+                     MakeTraceSourceAccessor (&TcpSocketBase::m_advWnd),
+                     "ns3::TracedValueCallback::Uint32")
     .AddTraceSource ("RWND",
                      "Remote side's flow control window",
                      MakeTraceSourceAccessor (&TcpSocketBase::m_rWnd),
@@ -2953,8 +2957,22 @@ uint16_t
 TcpSocketBase::AdvertisedWindowSize (bool scale) const
 {
   NS_LOG_FUNCTION (this << scale);
-  uint32_t w = m_rxBuffer->MaxBufferSize ();
+  uint32_t w = (m_rxBuffer->MaxRxSequence () > m_rxBuffer->NextRxSequence ()) ?
+    m_rxBuffer->MaxRxSequence () - m_rxBuffer->NextRxSequence () : 0;
 
+  // We don't want to advertise 0 after a FIN is received. So, we just use
+  // the previous value of the advWnd.
+  if (m_rxBuffer->Finished ())
+    {
+      w = m_advWnd;
+    }
+
+  // Ugly, but we are not modifying the state, that variable
+  // is used only for tracing purpose.
+  if (w != m_advWnd)
+    {
+      const_cast<TcpSocketBase*> (this)->m_advWnd = w;
+    }
   if (scale)
     {
       w >>= m_rcvWindShift;
@@ -2983,6 +3001,26 @@ TcpSocketBase::ReceivedData (Ptr<Packet> p, const TcpHeader& tcpHeader)
       SendEmptyPacket (TcpHeader::ACK);
       return;
     }
+  // Notify app to receive if necessary
+  if (expectedSeq < m_rxBuffer->NextRxSequence ())
+    { // NextRxSeq advanced, we have something to send to the app
+      if (!m_shutdownRecv)
+        {
+          NotifyDataRecv ();
+        }
+      // Handle exceptions
+      if (m_closeNotified)
+        {
+          NS_LOG_WARN ("Why TCP " << this << " got data after close notification?");
+        }
+      // If we received FIN before and now completed all "holes" in rx buffer,
+      // invoke peer close procedure
+      if (m_rxBuffer->Finished () && (tcpHeader.GetFlags () & TcpHeader::FIN) == 0)
+        {
+          DoPeerClose ();
+          return;
+        }
+    }
   // Now send a new ACK packet acknowledging all received and delivered data
   if (m_rxBuffer->Size () > m_rxBuffer->Available () || m_rxBuffer->NextRxSequence () > expectedSeq + p->GetSize ())
     { // A gap exists in the buffer, or we filled a gap: Always ACK
@@ -3004,25 +3042,6 @@ TcpSocketBase::ReceivedData (Ptr<Packet> p, const TcpHeader& tcpHeader)
                         (Simulator::Now () + Simulator::GetDelayLeft (m_delAckEvent)).GetSeconds ());
         }
     }
-  // Notify app to receive if necessary
-  if (expectedSeq < m_rxBuffer->NextRxSequence ())
-    { // NextRxSeq advanced, we have something to send to the app
-      if (!m_shutdownRecv)
-        {
-          NotifyDataRecv ();
-        }
-      // Handle exceptions
-      if (m_closeNotified)
-        {
-          NS_LOG_WARN ("Why TCP " << this << " got data after close notification?");
-        }
-      // If we received FIN before and now completed all "holes" in rx buffer,
-      // invoke peer close procedure
-      if (m_rxBuffer->Finished () && (tcpHeader.GetFlags () & TcpHeader::FIN) == 0)
-        {
-          DoPeerClose ();
-        }
-    }
 }
 
 /**
diff --git a/src/internet/model/tcp-socket-base.h b/src/internet/model/tcp-socket-base.h
index 91d916f50..755474e80 100644
--- a/src/internet/model/tcp-socket-base.h
+++ b/src/internet/model/tcp-socket-base.h
@@ -1122,6 +1122,7 @@ protected:
   // Window management
   uint16_t              m_maxWinSize;  //!< Maximum window size to advertise
   TracedValue<uint32_t> m_rWnd;        //!< Receiver window (RCV.WND in RFC793)
+  TracedValue<uint32_t> m_advWnd;      //!< Advertised Window size
   TracedValue<SequenceNumber32> m_highRxMark;     //!< Highest seqno received
   SequenceNumber32 m_highTxAck;                   //!< Highest ack sent
   TracedValue<SequenceNumber32> m_highRxAckMark;  //!< Highest ack received
diff --git a/src/internet/test/tcp-advertised-window-test.cc b/src/internet/test/tcp-advertised-window-test.cc
new file mode 100644
index 000000000..d119a3793
--- /dev/null
+++ b/src/internet/test/tcp-advertised-window-test.cc
@@ -0,0 +1,381 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2017 Christoph Doepmann <doepmanc@informatik.hu-berlin.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation;
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include "tcp-general-test.h"
+#include "ns3/node.h"
+#include "ns3/log.h"
+#include "tcp-error-model.h"
+#include "ns3/random-variable-stream.h"
+
+using namespace ns3;
+
+NS_LOG_COMPONENT_DEFINE ("TcpAdvertisedWindowTestSuite");
+
+/**
+ * \ingroup internet-tests
+ * \ingroup tests
+ * \brief Socket that wraps every call to AdvertisedWindowSize ().
+ */
+
+class TcpSocketAdvertisedWindowProxy : public TcpSocketMsgBase
+{
+public:
+  /**
+   * \brief Get type id
+   */
+  static TypeId GetTypeId (void);
+
+  /** \brief typedef for a cb */
+  typedef Callback<void, uint16_t, uint16_t> InvalidAwndCallback;
+
+  /**
+   * \brief Constructor
+   */
+  TcpSocketAdvertisedWindowProxy () : TcpSocketMsgBase (), m_segmentSize(0)
+  {
+  }
+
+  /**
+   * \brief Copy-constructor
+   *
+   * \param other Other obj
+   */
+  TcpSocketAdvertisedWindowProxy (const TcpSocketAdvertisedWindowProxy &other)
+    : TcpSocketMsgBase (other)
+  {
+    m_segmentSize = other.m_segmentSize;
+    m_inwalidAwndCb = other.m_inwalidAwndCb;
+  }
+
+  /**
+   * \brief Set the invalid AdvWnd callback
+   *
+   * \param cb callback to set
+   */
+  void SetInvalidAwndCb (InvalidAwndCallback cb);
+
+  /**
+   * \brief Set the expected segment size
+   *
+   * \param seg Segment size
+   */
+  void SetExpectedSegmentSize (uint16_t seg) { m_segmentSize = seg; };
+
+protected:
+  virtual Ptr<TcpSocketBase> Fork ();
+  virtual uint16_t AdvertisedWindowSize (bool scale = true) const;
+
+private:
+  uint16_t OldAdvertisedWindowSize (bool scale = true) const;
+  InvalidAwndCallback m_inwalidAwndCb; //!< Callback
+
+  /**
+   * \brief Test meta-information: size of the segments that are received.
+   * 
+   * This is necessary for making sure the calculated awnd only differs by
+   * exactly that one segment that was not yet read by the application.
+   */
+  uint16_t m_segmentSize;
+};
+
+void
+TcpSocketAdvertisedWindowProxy::SetInvalidAwndCb (InvalidAwndCallback cb)
+{
+  NS_ASSERT (!cb.IsNull ());
+  m_inwalidAwndCb = cb;
+}
+
+TypeId
+TcpSocketAdvertisedWindowProxy::GetTypeId (void)
+{
+  static TypeId tid = TypeId ("ns3::TcpSocketAdvertisedWindowProxy")
+    .SetParent<TcpSocketMsgBase> ()
+    .SetGroupName ("Internet")
+    .AddConstructor<TcpSocketAdvertisedWindowProxy> ()
+  ;
+  return tid;
+}
+
+Ptr<TcpSocketBase>
+TcpSocketAdvertisedWindowProxy::Fork (void)
+{
+  return CopyObject<TcpSocketAdvertisedWindowProxy> (this);
+}
+
+uint16_t
+TcpSocketAdvertisedWindowProxy::AdvertisedWindowSize (bool scale) const
+{
+  NS_LOG_FUNCTION (this << scale);
+
+  uint16_t newAwnd = TcpSocketMsgBase::AdvertisedWindowSize (scale);
+  uint16_t oldAwnd = OldAdvertisedWindowSize (scale);
+
+  if (!m_rxBuffer->Finished ())
+    {
+      // The calculated windows will only be exactly equal if there is no data
+      // in the receive buffer yet.
+      if (newAwnd != oldAwnd)
+        {
+          uint32_t available = m_rxBuffer->Available ();
+          // If the values differ, make sure this is only due to the single segment
+          // the socket just got, which has not yet been read by the application.
+          // Therefore, the difference should be exactly the size of one segment
+          // (but taking scale and m_maxWinSize into account).
+          uint32_t newAwndKnownDifference = newAwnd;
+          if (scale)
+            {
+              newAwndKnownDifference += (available >> m_rcvWindShift);
+            }
+          else
+            {
+              newAwndKnownDifference += available;
+            }
+
+          if (newAwndKnownDifference > m_maxWinSize)
+            {
+              newAwndKnownDifference = m_maxWinSize;
+            }
+
+          if (static_cast<uint16_t> (newAwndKnownDifference) != oldAwnd)
+            {
+              if (!m_inwalidAwndCb.IsNull ())
+                {
+                  m_inwalidAwndCb(oldAwnd, newAwnd);
+                }
+
+            }
+        }
+    }
+
+
+  return newAwnd;
+}
+
+/**
+ * The legacy code used for calculating the advertised window.
+ *
+ * This was copied from tcp-socket-base.cc before changing the formula.
+ * \param scale true if should scale the window
+ * \return the old adv wnd value
+ */
+uint16_t
+TcpSocketAdvertisedWindowProxy::OldAdvertisedWindowSize (bool scale) const
+{
+  NS_LOG_FUNCTION (this << scale);
+  //NS_LOG_DEBUG ("MaxRxSequence () = " << m_rxBuffer->MaxRxSequence ());
+  //NS_LOG_DEBUG ("NextRxSequence () = " << m_rxBuffer->NextRxSequence ());
+  //NS_LOG_DEBUG ("MaxBufferSize () = " << m_rxBuffer->MaxBufferSize ());
+  //NS_LOG_DEBUG ("m_rcvWindShift = " << static_cast<uint16_t> (m_rcvWindShift));
+  //NS_LOG_DEBUG ("m_maxWinSize = " << m_maxWinSize);
+  //NS_LOG_DEBUG ("Available () = " << m_rxBuffer->Available ());
+  uint32_t w = m_rxBuffer->MaxBufferSize ();
+
+  if (scale)
+    {
+      w >>= m_rcvWindShift;
+    }
+  if (w > m_maxWinSize)
+    {
+      w = m_maxWinSize;
+      NS_LOG_WARN ("Adv window size truncated to " << m_maxWinSize << "; possibly to avoid overflow of the 16-bit integer");
+    }
+  NS_LOG_DEBUG ("Returning AdvertisedWindowSize of " << static_cast<uint16_t> (w));
+  return static_cast<uint16_t> (w);
+}
+
+NS_OBJECT_ENSURE_REGISTERED (TcpSocketAdvertisedWindowProxy);
+
+/**
+ * \ingroup internet-tests
+ * \ingroup test
+ *
+ * \brief An error model that randomly drops a given rátio of TCP segments.
+ */
+class TcpDropRatioErrorModel : public TcpGeneralErrorModel
+{
+public:
+  /**
+   * \brief Get type id
+   */
+  static TypeId GetTypeId (void);
+  /**
+   * \brief Constructor
+   * \param dropRatio the drop ratio
+   */
+  TcpDropRatioErrorModel (double dropRatio)
+    : TcpGeneralErrorModel (), m_dropRatio(dropRatio)
+  {
+    m_prng = CreateObject<UniformRandomVariable> ();
+  }
+
+protected:
+  virtual bool ShouldDrop (const Ipv4Header &ipHeader, const TcpHeader &tcpHeader,
+                           uint32_t packetSize);
+
+private:
+  virtual void DoReset (void) { };
+  double m_dropRatio; //!< Drop ratio
+  Ptr<UniformRandomVariable> m_prng; //!< Random variable
+};
+
+NS_OBJECT_ENSURE_REGISTERED (TcpDropRatioErrorModel);
+
+TypeId
+TcpDropRatioErrorModel::GetTypeId (void)
+{
+  static TypeId tid = TypeId ("ns3::TcpDropRatioErrorModel")
+    .SetParent<TcpGeneralErrorModel> ()
+  ;
+  return tid;
+}
+
+bool
+TcpDropRatioErrorModel::ShouldDrop (const Ipv4Header &ipHeader, const TcpHeader &tcpHeader,
+                              uint32_t packetSize)
+{
+  return m_prng->GetValue () < m_dropRatio; 
+}
+
+/**
+ * \ingroup internet-tests
+ * \ingroup test
+ * \brief Test the new formula for calculating TCP's advertised window size.
+ *
+ * In TcpSocketBase, the advertised window is now calculated as
+ *
+ *   m_rxBuffer->MaxRxSequence () - m_rxBuffer->NextRxSequence ()
+ * 
+ * instead ofthe previous
+ *
+ *   m_rxBuffer->MaxBufferSize ()
+ * 
+ * This change was introduced with regard to situations in which the receiviing
+ * application does not read from the socket as fast as possible (see bug 2559
+ * for details). This test ensures that no regression is introduced for other,
+ * "normal" cases.
+ *
+ * TcpGeneralTest ensures via ReceivePacket () that incoming packets are
+ * quickly consumed by the application layer, simulating a fast-reading
+ * application. We can only reasonably compare the old and the new AWND
+ * computation in this case.
+ */
+class TcpAdvertisedWindowTest : public TcpGeneralTest
+{
+public:
+  /**
+   * \brief Constructor
+   * \param desc description
+   * \param size segment size
+   * \param packets number of packets to send
+   * \param lossRatio error ratio
+   */
+  TcpAdvertisedWindowTest (const std::string &desc, uint32_t size, uint32_t packets, double lossRatio);
+
+protected:
+  virtual void ConfigureEnvironment ();
+  virtual Ptr<TcpSocketMsgBase> CreateReceiverSocket (Ptr<Node> node);
+  virtual Ptr<ErrorModel> CreateReceiverErrorModel ();
+
+private:
+  /** \brief Callback called for the update of the awnd
+   * \param oldAwnd Old advertised window
+   * \param newAwnd new value
+   */
+  void InvalidAwndCb (uint16_t oldAwnd, uint16_t newAwnd);
+  uint32_t m_pktSize; //!< Packet size
+  uint32_t m_pktCount; //!< Pkt count
+  double m_lossRatio; //!< Loss ratio
+};
+
+TcpAdvertisedWindowTest::TcpAdvertisedWindowTest (const std::string &desc,
+                                              uint32_t size, uint32_t packets, double lossRatio)
+  : TcpGeneralTest (desc),
+    m_pktSize (size),
+    m_pktCount (packets),
+    m_lossRatio (lossRatio)
+{
+}
+
+void
+TcpAdvertisedWindowTest::ConfigureEnvironment ()
+{
+  TcpGeneralTest::ConfigureEnvironment ();
+  SetAppPktCount (m_pktCount);
+  SetPropagationDelay (MilliSeconds (50));
+  SetTransmitStart (Seconds (2.0));
+  SetAppPktSize (m_pktSize);
+
+}
+
+Ptr<TcpSocketMsgBase>
+TcpAdvertisedWindowTest::CreateReceiverSocket (Ptr<Node> node)
+{
+  NS_LOG_FUNCTION (this);
+
+  Ptr<TcpSocketMsgBase> sock = CreateSocket (node, TcpSocketAdvertisedWindowProxy::GetTypeId (), m_congControlTypeId);
+  DynamicCast<TcpSocketAdvertisedWindowProxy> (sock)->SetExpectedSegmentSize (500);
+  DynamicCast<TcpSocketAdvertisedWindowProxy> (sock)->SetInvalidAwndCb (
+        MakeCallback (&TcpAdvertisedWindowTest::InvalidAwndCb, this));
+
+  return sock;
+}
+
+Ptr<ErrorModel>
+TcpAdvertisedWindowTest::CreateReceiverErrorModel ()
+{
+  return CreateObject<TcpDropRatioErrorModel> (m_lossRatio);
+}
+
+void
+TcpAdvertisedWindowTest::InvalidAwndCb (uint16_t oldAwnd, uint16_t newAwnd)
+{
+  NS_TEST_ASSERT_MSG_EQ (oldAwnd, newAwnd,
+                         "Old and new AWND calculations do not match.");
+}
+
+//-----------------------------------------------------------------------------
+
+/**
+ * \ingroup internet-tests
+ * \ingroup test
+ *
+ * \brief Test Suite for TCP adv window
+ */
+class TcpAdvertisedWindowTestSuite : public TestSuite
+{
+public:
+  TcpAdvertisedWindowTestSuite () : TestSuite ("tcp-advertised-window-test", UNIT)
+  {
+    AddTestCase (new TcpAdvertisedWindowTest ("TCP advertised window size, small seg + no loss", 500, 100, 0.0),
+                 TestCase::QUICK);
+    AddTestCase (new TcpAdvertisedWindowTest ("TCP advertised window size, small seg + loss", 500, 100, 0.1),
+                 TestCase::QUICK);
+    AddTestCase (new TcpAdvertisedWindowTest ("TCP advertised window size, large seg + no loss", 1000, 100, 0.0),
+                 TestCase::QUICK);
+    AddTestCase (new TcpAdvertisedWindowTest ("TCP advertised window size, large seg + small loss", 1000, 100, 0.1),
+                 TestCase::QUICK);
+    AddTestCase (new TcpAdvertisedWindowTest ("TCP advertised window size, large seg + big loss", 1000, 100, 0.3),
+                 TestCase::QUICK);
+    AddTestCase (new TcpAdvertisedWindowTest ("TCP advertised window size, complete loss", 1000, 100, 1.0),
+                 TestCase::QUICK);
+  }
+};
+
+static TcpAdvertisedWindowTestSuite g_tcpAdvertisedWindowTestSuite; //<! static obj for test initialization
+
diff --git a/src/internet/wscript b/src/internet/wscript
index ed6a77af2..c875a9399 100644
--- a/src/internet/wscript
+++ b/src/internet/wscript
@@ -269,6 +269,7 @@ def build(bld):
         'test/tcp-pkts-acked-test.cc',
         'test/tcp-rtt-estimation.cc',
         'test/tcp-bytes-in-flight-test.cc',
+        'test/tcp-advertised-window-test.cc',
         'test/udp-test.cc',
         'test/ipv6-address-generator-test-suite.cc',
         'test/ipv6-dual-stack-test-suite.cc',