tcp (fixes #531): TcpWestwood divide-by-zero and floating point issues

- Change TcpWestwood::EstimateBW trace source from double to DataRate.
- Prevent divide-by-zero from occurring.
- Remove TcpWestwood from tcp-rto-test.cc, tcp-cong-avoid-test.cc, tcp-slow-start-test.cc.
- Add warning that currently TcpWestwood does not have unit tests.

src/internet/doc/tcp.rst

@@ -547,13 +547,15 @@ More information at: http://dl.acm.org/citation.cfm?id=2756518
 
 Westwood
 ^^^^^^^^
-Westwood and Westwood+ employ the AIAD (Additive Increase/Adaptive Decrease)·
-congestion control paradigm. When a congestion episode happens,·
+Westwood and Westwood+ employ the AIAD (Additive Increase/Adaptive Decrease)
+congestion control paradigm. When a congestion episode happens,
 instead of halving the cwnd, these protocols try to estimate the network's
-bandwidth and use the estimated value to adjust the cwnd.·
-While Westwood performs the bandwidth sampling every ACK reception,·
+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.
 
+WARNING: this TCP model lacks validation and regression tests; use with caution.
+
 More information at: http://dl.acm.org/citation.cfm?id=381704 and
 http://dl.acm.org/citation.cfm?id=2512757
 

src/internet/model/tcp-westwood.cc

@@ -59,7 +59,7 @@ TcpWestwood::GetTypeId (void)
                   MakeEnumChecker(TcpWestwood::WESTWOOD, "Westwood",TcpWestwood::WESTWOODPLUS, "WestwoodPlus"))
     .AddTraceSource("EstimatedBW", "The estimated bandwidth",
                     MakeTraceSourceAccessor(&TcpWestwood::m_currentBW),
-                    "ns3::TracedValueCallback::Double")
+                    "ns3::TracedValueCallback::DataRate")
   ;
   return tid;
 }
@@ -130,34 +130,34 @@ TcpWestwood::EstimateBW (const Time &rtt, Ptr<TcpSocketState> tcb)
 
   NS_ASSERT (!rtt.IsZero ());
 
-  m_currentBW = m_ackedSegments * tcb->m_segmentSize / rtt.GetSeconds ();
-
   if (m_pType == TcpWestwood::WESTWOOD)
     {
       Time currentAck = Simulator::Now ();
-      m_currentBW = m_ackedSegments * tcb->m_segmentSize / (currentAck - m_lastAck).GetSeconds ();
+
+      NS_ABORT_MSG_IF (currentAck == m_lastAck,
+        "This violates a model assumption and would lead to divide-by-zero; please report to ns-3 maintainers if this occurs.");
+
+      m_currentBW = DataRate (m_ackedSegments * tcb->m_segmentSize * 8.0 / (currentAck - m_lastAck).GetSeconds ());
       m_lastAck = currentAck;
     }
   else if (m_pType == TcpWestwood::WESTWOODPLUS)
     {
-      m_currentBW = m_ackedSegments * tcb->m_segmentSize / rtt.GetSeconds ();
+      m_currentBW = DataRate (m_ackedSegments * tcb->m_segmentSize * 8.0 / rtt.GetSeconds ());
       m_IsCount = false;
     }
 
   m_ackedSegments = 0;
+
   NS_LOG_LOGIC ("Estimated BW: " << m_currentBW);
 
   // Filter the BW sample
 
-  double alpha = 0.9;
+  constexpr double alpha = 0.9;
 
-  if (m_fType == TcpWestwood::NONE)
+  if (m_fType == TcpWestwood::TUSTIN)
     {
-    }
-  else if (m_fType == TcpWestwood::TUSTIN)
-    {
-      double sample_bwe = m_currentBW;
-      m_currentBW = (alpha * m_lastBW) + ((1 - alpha) * ((sample_bwe + m_lastSampleBW) / 2));
+      DataRate sample_bwe = m_currentBW;
+      m_currentBW = (m_lastBW * alpha) + (((sample_bwe + m_lastSampleBW) * 0.5) * (1 - alpha));
       m_lastSampleBW = sample_bwe;
       m_lastBW = m_currentBW;
     }
@@ -169,12 +169,13 @@ uint32_t
 TcpWestwood::GetSsThresh (Ptr<const TcpSocketState> tcb,
                           [[maybe_unused]] uint32_t bytesInFlight)
 {
-  NS_LOG_LOGIC ("CurrentBW: " << m_currentBW << " minRtt: " <<
-                tcb->m_minRtt << " ssthresh: " <<
-                m_currentBW * static_cast<double> (tcb->m_minRtt.GetSeconds ()));
+  uint32_t ssThresh = static_cast<uint32_t> ((m_currentBW * tcb->m_minRtt) / 8.0);
 
-  return std::max (2*tcb->m_segmentSize,
-                   uint32_t (m_currentBW * static_cast<double> (tcb->m_minRtt.GetSeconds ())));
+  NS_LOG_LOGIC ("CurrentBW: " << m_currentBW <<
+                " minRtt: " << tcb->m_minRtt <<
+                " ssThresh: " << ssThresh);
+
+  return std::max (2 * tcb->m_segmentSize, ssThresh);
 }
 
 Ptr<TcpCongestionOps>

src/internet/model/tcp-westwood.h

@@ -34,17 +34,14 @@
 #define TCP_WESTWOOD_H
 
 #include "tcp-congestion-ops.h"
+#include "ns3/data-rate.h"
 #include "ns3/tcp-recovery-ops.h"
-#include "ns3/sequence-number.h"
 #include "ns3/traced-value.h"
 #include "ns3/event-id.h"
 
 namespace ns3 {
 
-class Packet;
-class TcpHeader;
 class Time;
-class EventId;
 
 /**
  * \ingroup congestionOps
@@ -63,6 +60,8 @@ class EventId;
  * the number of acknowledged segments on the receipt of an ACK.
  * The EstimateBW estimates the bandwidth based on the value returned by CountAck
  * and the sampling interval (last ACK inter-arrival time for Westwood and last RTT for Westwood+).
+ *
+ * WARNING: this TCP model lacks validation and regression tests; use with caution.
  */
 class TcpWestwood : public TcpNewReno
 {
@@ -124,9 +123,9 @@ private:
   void EstimateBW (const Time& rtt, Ptr<TcpSocketState> tcb);
 
 protected:
-  TracedValue<double>    m_currentBW;              //!< Current value of the estimated BW
-  double                 m_lastSampleBW;           //!< Last bandwidth sample
-  double                 m_lastBW;                 //!< Last bandwidth sample after being filtered
+  TracedValue<DataRate>  m_currentBW;              //!< Current value of the estimated BW
+  DataRate               m_lastSampleBW;           //!< Last bandwidth sample
+  DataRate               m_lastBW;                 //!< Last bandwidth sample after being filtered
   enum ProtocolType      m_pType;                  //!< 0 for Westwood, 1 for Westwood+
   enum FilterType        m_fType;                  //!< 0 for none, 1 for Tustin
 
@@ -134,7 +133,6 @@ protected:
   bool                   m_IsCount;                //!< Start keeping track of m_ackedSegments for Westwood+ if TRUE
   EventId                m_bwEstimateEvent;        //!< The BW estimation event for Westwood+
   Time                   m_lastAck;                //!< The last ACK time
-
 };
 
 } // namespace ns3

src/internet/test/tcp-cong-avoid-test.cc

@@ -18,7 +18,6 @@
  */
 #include "ns3/log.h"
 #include "ns3/simple-channel.h"
-#include "ns3/tcp-westwood.h"
 #include "ns3/config.h"
 #include "ns3/test.h"
 #include "tcp-general-test.h"
@@ -194,7 +193,6 @@ public:
   TcpRenoCongAvoidTestSuite () : TestSuite ("tcp-cong-avoid-test", UNIT)
   {
     std::list<TypeId> types;
-    types.insert (types.begin (), TcpWestwood::GetTypeId ());
     types.insert (types.begin (), TcpNewReno::GetTypeId ());
 
     for (std::list<TypeId>::iterator it = types.begin (); it != types.end (); ++it)

src/internet/test/tcp-rto-test.cc

@@ -19,7 +19,6 @@
 
 #include "ns3/node.h"
 #include "ns3/log.h"
-#include "ns3/tcp-westwood.h"
 #include "ns3/simple-channel.h"
 #include "ns3/rtt-estimator.h"
 #include "tcp-general-test.h"
@@ -517,7 +516,6 @@ public:
   {
     std::list<TypeId> types;
     types.insert (types.begin (), TcpNewReno::GetTypeId ());
-    types.insert (types.begin (), TcpWestwood::GetTypeId ());
 
     for (std::list<TypeId>::iterator it = types.begin (); it != types.end (); ++it)
       {

src/internet/test/tcp-slow-start-test.cc

@@ -21,7 +21,6 @@
 #include "ns3/simple-channel.h"
 #include "ns3/node.h"
 #include "ns3/config.h"
-#include "ns3/tcp-westwood.h"
 #include "ns3/tcp-header.h"
 #include "tcp-general-test.h"
 
@@ -275,7 +274,6 @@ public:
     // This test have less packets to transmit than SsTh
     std::list<TypeId> types;
     types.insert (types.begin (), TcpNewReno::GetTypeId ());
-    types.insert (types.begin (), TcpWestwood::GetTypeId ());
 
     for (std::list<TypeId>::iterator it = types.begin (); it != types.end (); ++it)
       {