fw: rename n-silent-timeouts to max-timeouts in AsfStrategy

Change-Id: Ie2790a4e1fd545aacf8b297d7ec33b204f2cdbfc
diff --git a/daemon/fw/asf-measurements.hpp b/daemon/fw/asf-measurements.hpp
index 02f4553..aede00b 100644
--- a/daemon/fw/asf-measurements.hpp
+++ b/daemon/fw/asf-measurements.hpp
@@ -91,15 +91,15 @@
   }
 
   size_t
-  getNSilentTimeouts() const
+  getNTimeouts() const
   {
-    return m_nSilentTimeouts;
+    return m_nTimeouts;
   }
 
   void
-  setNSilentTimeouts(size_t nSilentTimeouts)
+  setNTimeouts(size_t nTimeouts)
   {
-    m_nSilentTimeouts = nSilentTimeouts;
+    m_nTimeouts = nTimeouts;
   }
 
 public:
@@ -110,7 +110,7 @@
   ndn::util::RttEstimator m_rttEstimator;
   time::nanoseconds m_lastRtt = RTT_NO_MEASUREMENT;
   Name m_lastInterestName;
-  size_t m_nSilentTimeouts = 0;
+  size_t m_nTimeouts = 0;
 
   // Timeout associated with measurement
   scheduler::ScopedEventId m_measurementExpiration;
diff --git a/daemon/fw/asf-strategy.cpp b/daemon/fw/asf-strategy.cpp
index f337a27..df38099 100644
--- a/daemon/fw/asf-strategy.cpp
+++ b/daemon/fw/asf-strategy.cpp
@@ -58,7 +58,7 @@
   this->setInstanceName(makeInstanceName(name, getStrategyName()));
 
   NFD_LOG_DEBUG("probing-interval=" << m_probing.getProbingInterval()
-                << " n-silent-timeouts=" << m_nMaxSilentTimeouts);
+                << " max-timeouts=" << m_nMaxTimeouts);
 }
 
 const Name&
@@ -97,11 +97,13 @@
     if (f == "probing-interval") {
       m_probing.setProbingInterval(getParamValue(f, s));
     }
-    else if (f == "n-silent-timeouts") {
-      m_nMaxSilentTimeouts = getParamValue(f, s);
+    else if (f == "max-timeouts") {
+      m_nMaxTimeouts = getParamValue(f, s);
+      if (m_nMaxTimeouts <= 0)
+        NDN_THROW(std::invalid_argument("max-timeouts should be greater than 0"));
     }
     else {
-      NDN_THROW(std::invalid_argument("Parameter should be probing-interval or n-silent-timeouts"));
+      NDN_THROW(std::invalid_argument("Parameter should be probing-interval or max-timeouts"));
     }
   }
 }
@@ -341,10 +343,10 @@
   }
 
   auto& faceInfo = *fiPtr;
-  size_t nTimeouts = faceInfo.getNSilentTimeouts() + 1;
-  faceInfo.setNSilentTimeouts(nTimeouts);
+  size_t nTimeouts = faceInfo.getNTimeouts() + 1;
+  faceInfo.setNTimeouts(nTimeouts);
 
-  if (nTimeouts <= m_nMaxSilentTimeouts && !isNack) {
+  if (nTimeouts < m_nMaxTimeouts && !isNack) {
     NFD_LOG_TRACE(interestName << " face=" << faceId << " timeout-count=" << nTimeouts << " ignoring");
     // Extend lifetime for measurements associated with Face
     namespaceInfo->extendFaceInfoLifetime(faceInfo, faceId);
diff --git a/daemon/fw/asf-strategy.hpp b/daemon/fw/asf-strategy.hpp
index 2e0ba24..af3e4b5 100644
--- a/daemon/fw/asf-strategy.hpp
+++ b/daemon/fw/asf-strategy.hpp
@@ -91,7 +91,7 @@
   AsfMeasurements m_measurements;
   ProbingModule m_probing;
   RetxSuppressionExponential m_retxSuppression;
-  size_t m_nMaxSilentTimeouts = 3;
+  size_t m_nMaxTimeouts = 3;
 
   static const time::milliseconds RETX_SUPPRESSION_INITIAL;
   static const time::milliseconds RETX_SUPPRESSION_MAX;
diff --git a/docs/manpages/nfd-asf-strategy.rst b/docs/manpages/nfd-asf-strategy.rst
index 7d84607..d168b2a 100644
--- a/docs/manpages/nfd-asf-strategy.rst
+++ b/docs/manpages/nfd-asf-strategy.rst
@@ -1,46 +1,54 @@
 nfd-asf-strategy
 ================
 
-SYNOPSIS
+Synopsis
 --------
-| nfdc strategy set prefix <PREFIX> strategy /localhost/nfd/strategy/asf[/v=4][/probing-interval~<PROBING-INTERVAL>][/n-silent-timeouts~<N-SILENT-TIMEOUTS>]
 
-DESCRIPTION
+**nfdc strategy set** **prefix** *NAME* **strategy**
+/localhost/nfd/strategy/asf[/v=4][/**probing-interval**\ ~\ *INTERVAL*][/**max-timeouts**\ ~\ *TIMEOUTS*]
+
+Description
 -----------
 
-ASF is an Adaptive Smoothed RTT-based Forwarding Strategy that chooses the best next hop based on SRTT measurement, and also periodically probes other next hops to learn their RTTs.
+**ASF** is an Adaptive Smoothed RTT-based Forwarding Strategy that chooses the
+best next hop based on SRTT measurements, and also periodically probes other
+next hops to learn their RTTs.
 
-OPTIONS
+Options
 -------
-<PROBING-INTERVAL>
-    Tells ASF how often to send a probe to determine alternative paths.
-    The value is specified in milliseconds (non-negative integer)
-    Lower value means high overhead but faster reaction.
-    Default value is 1 minute and minimum value is 1 second.
-    It is optional to specify probing-interval.
 
-<N-SILENT-TIMEOUTS>
-    ASF switches immediately to another appropriate face (if available) upon timeout.
-    This behavior may be too sensitive for application use and appropriate only for link
-    failures and not transient timeouts. So this parameter makes ASF switch paths
-    only after it has encountered the specified number of timeouts (non-negative integer).
-    Default and minimum value is 0, i.e., switch immediately.
-    It is optional to specify n-silent-timeouts.
+.. option:: probing-interval
 
-EXAMPLES
+    This optional parameter tells ASF how often to send a probe to determine
+    alternative paths. The value is specified in milliseconds (non-negative
+    integer). Smaller values will result in higher overhead but faster reaction.
+    The default value is 1 minute and the minimum value is 1 second.
+
+.. option:: max-timeouts
+
+    This optional parameter makes ASF switch to another appropriate face (if available)
+    after it has encountered the specified number of timeouts. The value is a positive
+    integer and defaults to 3, i.e., switch to another face after 3 timeouts. Smaller
+    values make ASF more sensitive to timeouts and will switch paths more frequently,
+    which should provide a faster reaction to link failures. Larger values may be better
+    suited when transient timeouts are common and for certain application uses.
+
+Examples
 --------
+
 nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/asf
-    Use the default values.
+    Use the default values for all parameters.
 
 nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/asf/v=4/probing-interval~30000
-    Set probing interval as 30 seconds.
+    Set probing interval to 30 seconds.
 
-nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/asf/v=4/n-silent-timeouts~5
-    Set n-silent-timeouts as 5.
+nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/asf/v=4/max-timeouts~5
+    Set max timeouts to 5.
 
-nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/asf/v=4/probing-interval~30000/n-silent-timeouts~5
-    Set probing interval as 30 seconds and n-silent-timeouts as 5.
+nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/asf/v=4/probing-interval~30000/max-timeouts~2
+    Set probing interval to 30 seconds and max timeouts to 2.
 
-SEE ALSO
+See also
 --------
+
 nfdc(1), nfdc-strategy(1)
diff --git a/tests/daemon/fw/asf-strategy.t.cpp b/tests/daemon/fw/asf-strategy.t.cpp
index afe009f..1336b3e 100644
--- a/tests/daemon/fw/asf-strategy.t.cpp
+++ b/tests/daemon/fw/asf-strategy.t.cpp
@@ -71,10 +71,9 @@
     nodeC = topo.addForwarder("C");
     nodeD = topo.addForwarder("D");
 
-    topo.setStrategy<AsfStrategy>(nodeA, Name("/"), parameters);
-    topo.setStrategy<AsfStrategy>(nodeB, Name("/"), parameters);
-    topo.setStrategy<AsfStrategy>(nodeC, Name("/"), parameters);
-    topo.setStrategy<AsfStrategy>(nodeD, Name("/"), parameters);
+    for (auto node : {nodeA, nodeB, nodeC, nodeD}) {
+      topo.setStrategy<AsfStrategy>(node, Name("/"), parameters);
+    }
 
     linkAB = topo.addLink("AB", 10_ms, {nodeA, nodeB});
     linkAD = topo.addLink("AD", 100_ms, {nodeA, nodeD});
@@ -126,7 +125,7 @@
   AsfStrategyParametersGridFixture()
     : AsfGridFixture(Name(AsfStrategy::getStrategyName())
                      .append("probing-interval~30000")
-                     .append("n-silent-timeouts~5"))
+                     .append("max-timeouts~5"))
   {
   }
 };
@@ -153,8 +152,8 @@
   linkAB->fail();
 
   runConsumer();
-  // We experience 3 silent timeouts before marking AB as timed out on the fourth interest.
-  BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 56);
+  // We experience 3 timeouts and marked AB as timed out
+  BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 57);
   BOOST_CHECK_LE(linkAB->getFace(nodeA).getCounters().nOutInterests, 36);
   BOOST_CHECK_GE(linkAD->getFace(nodeA).getCounters().nOutInterests, 24);
 
@@ -166,7 +165,7 @@
   this->advanceClocks(10_ms, 10_s);
 
   runConsumer();
-  BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 86);
+  BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 87);
   BOOST_CHECK_GE(linkAB->getFace(nodeA).getCounters().nOutInterests, 50);
   BOOST_CHECK_LE(linkAD->getFace(nodeA).getCounters().nOutInterests, 40);
 
@@ -176,7 +175,7 @@
 
   runConsumer();
 
-  BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 86);
+  BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 87);
   BOOST_CHECK_LE(linkAB->getFace(nodeA).getCounters().nOutInterests, 65); // FIXME #3830
   BOOST_CHECK_GE(linkAD->getFace(nodeA).getCounters().nOutInterests, 57); // FIXME #3830
 }
@@ -509,11 +508,11 @@
   // Bring down 10 ms link
   linkAB->fail();
 
-  // Send 6 interests, first 5 will be ignored and on the 6th it will record the timeout
+  // Send 5 interests, after the last one it will record the timeout
   // ready to switch for the next interest
-  runConsumer(6);
+  runConsumer(5);
 
-  // Check that link has not been switched to 100 ms because n-silent-timeouts = 5
+  // Check that link has not been switched to 100 ms because max-timeouts = 5
   BOOST_CHECK_EQUAL(linkAD->getFace(nodeA).getCounters().nOutInterests, outInterestsBeforeFailure);
 
   // Send 5 interests, check that 100 ms link is used
@@ -557,21 +556,24 @@
     }
   };
 
-  checkValidity("/probing-interval~30000/n-silent-timeouts~5", true);
-  checkValidity("/n-silent-timeouts~5/probing-interval~30000", true);
+  checkValidity("/probing-interval~30000/max-timeouts~5", true);
+  checkValidity("/max-timeouts~5/probing-interval~30000", true);
   checkValidity("/probing-interval~30000", true);
-  checkValidity("/n-silent-timeouts~5", true);
+  checkValidity("/max-timeouts~5", true);
   checkValidity("", true);
 
   checkValidity("/probing-interval~500", false); // At least 1 seconds
   checkValidity("/probing-interval~-5000", false);
-  checkValidity("/n-silent-timeouts~-5", false);
-  checkValidity("/n-silent-timeouts~-5/probing-interval~-30000", false);
-  checkValidity("/n-silent-timeouts", false);
+  checkValidity("/max-timeouts~0", false);
+  checkValidity("/max-timeouts~-5", false);
+  checkValidity("/max-timeouts~-5/probing-interval~-30000", false);
+  checkValidity("/max-timeouts", false);
   checkValidity("/probing-interval~", false);
   checkValidity("/~1000", false);
   checkValidity("/probing-interval~foo", false);
-  checkValidity("/n-silent-timeouts~1~2", false);
+  checkValidity("/max-timeouts~1~2", false);
+  checkValidity("/foo", false);
+  checkValidity("/foo~42", false);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestAsfStrategy