fw: various code simplifications in AsfStrategy

Change-Id: Ie006680b2469fa7dc5d9b19665320b2686564f2c
diff --git a/daemon/fw/asf-strategy.cpp b/daemon/fw/asf-strategy.cpp
index 4e26817..62336eb 100644
--- a/daemon/fw/asf-strategy.cpp
+++ b/daemon/fw/asf-strategy.cpp
@@ -42,7 +42,6 @@
   : Strategy(forwarder)
   , m_measurements(getMeasurements())
   , m_probing(m_measurements)
-  , m_maxSilentTimeouts(0)
   , m_retxSuppression(RETX_SUPPRESSION_INITIAL,
                       RetxSuppressionExponential::DEFAULT_MULTIPLIER,
                       RETX_SUPPRESSION_MAX)
@@ -58,8 +57,8 @@
   }
   this->setInstanceName(makeInstanceName(name, getStrategyName()));
 
-  NFD_LOG_DEBUG("Probing interval=" << m_probing.getProbingInterval()
-                << ", Num silent timeouts=" << m_maxSilentTimeouts);
+  NFD_LOG_DEBUG("probing-interval=" << m_probing.getProbingInterval()
+                << " n-silent-timeouts=" << m_maxSilentTimeouts);
 }
 
 const Name&
@@ -69,6 +68,20 @@
   return strategyName;
 }
 
+static uint64_t
+getParamValue(const std::string& param, const std::string& value)
+{
+  try {
+    if (!value.empty() && value[0] == '-')
+      NDN_THROW(boost::bad_lexical_cast());
+
+    return boost::lexical_cast<uint64_t>(value);
+  }
+  catch (const boost::bad_lexical_cast&) {
+    NDN_THROW(std::invalid_argument("Value of " + param + " must be a non-negative integer"));
+  }
+}
+
 void
 AsfStrategy::processParams(const PartialName& parsed)
 {
@@ -93,40 +106,14 @@
   }
 }
 
-uint64_t
-AsfStrategy::getParamValue(const std::string& param, const std::string& value)
-{
-  try {
-    if (!value.empty() && value[0] == '-')
-      NDN_THROW(boost::bad_lexical_cast());
-
-    return boost::lexical_cast<uint64_t>(value);
-  }
-  catch (const boost::bad_lexical_cast&) {
-    NDN_THROW(std::invalid_argument("Value of " + param + " must be a non-negative integer"));
-  }
-}
-
-void
-AsfStrategy::sendAsfProbe(const FaceEndpoint& ingress, const Interest& interest,
-                          const shared_ptr<pit::Entry>& pitEntry, const Face& faceToUse,
-                          const fib::Entry& fibEntry)
-{
-  Face* faceToProbe = m_probing.getFaceToProbe(ingress.face, interest, fibEntry, faceToUse);
-  if (faceToProbe != nullptr) {
-    forwardInterest(interest, fibEntry, pitEntry, *faceToProbe, true);
-    m_probing.afterForwardingProbe(fibEntry, interest);
-  }
-}
-
 void
 AsfStrategy::afterReceiveInterest(const FaceEndpoint& ingress, const Interest& interest,
                                   const shared_ptr<pit::Entry>& pitEntry)
 {
   // Should the Interest be suppressed?
-  RetxSuppressionResult suppressResult = m_retxSuppression.decidePerPitEntry(*pitEntry);
+  auto suppressResult = m_retxSuppression.decidePerPitEntry(*pitEntry);
   if (suppressResult == RetxSuppressionResult::SUPPRESS) {
-    NFD_LOG_DEBUG(interest << " from=" << ingress << " suppressed");
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " suppressed");
     return;
   }
 
@@ -135,36 +122,31 @@
 
   if (suppressResult == RetxSuppressionResult::NEW) {
     if (nexthops.size() == 0) {
-      // send noRouteNack if nexthop is not available
-      sendNoRouteNack(ingress, interest, pitEntry);
-      this->rejectPendingInterest(pitEntry);
+      NFD_LOG_DEBUG(interest << " new-interest from=" << ingress << " no-nexthop");
+      sendNoRouteNack(ingress, pitEntry);
       return;
     }
 
-    Face* faceToUse = getBestFaceForForwarding(fibEntry, interest, ingress.face, pitEntry);
+    Face* faceToUse = getBestFaceForForwarding(interest, ingress.face, fibEntry, pitEntry);
+    if (faceToUse != nullptr) {
+      NFD_LOG_DEBUG(interest << " new-interest from=" << ingress << " forward-to=" << faceToUse->getId());
+      forwardInterest(interest, *faceToUse, fibEntry, pitEntry);
 
-    if (faceToUse == nullptr) {
-      sendNoRouteNack(ingress, interest, pitEntry);
-      this->rejectPendingInterest(pitEntry);
-      return;
+      // If necessary, send probe
+      sendProbe(interest, ingress, *faceToUse, fibEntry, pitEntry);
     }
-
-    NFD_LOG_TRACE("Forwarding interest to face: " << faceToUse->getId());
-    forwardInterest(interest, fibEntry, pitEntry, *faceToUse);
-
-    // If necessary, send probe
-    if (m_probing.isProbingNeeded(fibEntry, interest)) {
-      sendAsfProbe(ingress, interest, pitEntry, *faceToUse, fibEntry);
+    else {
+      NFD_LOG_DEBUG(interest << " new-interest from=" << ingress << " no-nexthop");
+      sendNoRouteNack(ingress, pitEntry);
     }
     return;
   }
 
-  Face* faceToUse = getBestFaceForForwarding(fibEntry, interest, ingress.face, pitEntry, false);
+  Face* faceToUse = getBestFaceForForwarding(interest, ingress.face, fibEntry, pitEntry, false);
   // if unused face not found, select nexthop with earliest out record
   if (faceToUse != nullptr) {
-
-    NFD_LOG_TRACE("Forwarding interest to face: " << faceToUse->getId());
-    forwardInterest(interest, fibEntry, pitEntry, *faceToUse);
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " forward-to=" << faceToUse->getId());
+    forwardInterest(interest, *faceToUse, fibEntry, pitEntry);
     // avoid probing in case of forwarding
     return;
   }
@@ -173,11 +155,11 @@
   auto it = nexthops.end();
   it = findEligibleNextHopWithEarliestOutRecord(ingress.face, interest, nexthops, pitEntry);
   if (it == nexthops.end()) {
-    NFD_LOG_DEBUG(interest << " from=" << ingress << " retransmitNoNextHop");
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " no-nexthop");
   }
   else {
     auto egress = FaceEndpoint(it->getFace(), 0);
-    NFD_LOG_DEBUG(interest << " from=" << ingress << " retransmit-retry-to=" << egress);
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " retry-to=" << egress);
     this->sendInterest(pitEntry, egress, interest);
   }
 }
@@ -187,52 +169,52 @@
                                    const FaceEndpoint& ingress, const Data& data)
 {
   NamespaceInfo* namespaceInfo = m_measurements.getNamespaceInfo(pitEntry->getName());
-
   if (namespaceInfo == nullptr) {
-    NFD_LOG_TRACE("Could not find measurements entry for " << pitEntry->getName());
+    NFD_LOG_DEBUG(pitEntry->getName() << " data from=" << ingress << " no-measurements");
     return;
   }
 
   // Record the RTT between the Interest out to Data in
-  FaceInfo* faceInfo = namespaceInfo->get(ingress.face.getId());
+  FaceInfo* faceInfo = namespaceInfo->getFaceInfo(ingress.face.getId());
   if (faceInfo == nullptr) {
+    NFD_LOG_DEBUG(pitEntry->getName() << " data from=" << ingress << " no-face-info");
     return;
   }
-  faceInfo->recordRtt(pitEntry, ingress.face);
+
+  auto outRecord = pitEntry->getOutRecord(ingress.face, 0);
+  if (outRecord == pitEntry->out_end()) {
+    NFD_LOG_DEBUG(pitEntry->getName() << " data from=" << ingress << " no-out-record");
+  }
+  else {
+    faceInfo->recordRtt(time::steady_clock::now() - outRecord->getLastRenewed());
+    NFD_LOG_DEBUG(pitEntry->getName() << " data from=" << ingress
+                  << " rtt=" << faceInfo->getLastRtt() << " srtt=" << faceInfo->getSrtt());
+  }
 
   // Extend lifetime for measurements associated with Face
   namespaceInfo->extendFaceInfoLifetime(*faceInfo, ingress.face.getId());
 
-  if (faceInfo->isTimeoutScheduled()) {
-    faceInfo->cancelTimeoutEvent(data.getName());
-  }
+  faceInfo->cancelTimeout(data.getName());
 }
 
 void
 AsfStrategy::afterReceiveNack(const FaceEndpoint& ingress, const lp::Nack& nack,
                               const shared_ptr<pit::Entry>& pitEntry)
 {
-  NFD_LOG_DEBUG("Nack for " << nack.getInterest() << " from=" << ingress << ": reason=" << nack.getReason());
+  NFD_LOG_DEBUG(nack.getInterest() << " nack from=" << ingress << " reason=" << nack.getReason());
   onTimeout(pitEntry->getName(), ingress.face.getId());
 }
 
-////////////////////////////////////////////////////////////////////////////////
-////////////////////////////////////////////////////////////////////////////////
-
 void
-AsfStrategy::forwardInterest(const Interest& interest,
-                             const fib::Entry& fibEntry,
-                             const shared_ptr<pit::Entry>& pitEntry,
-                             Face& outFace,
-                             bool wantNewNonce)
+AsfStrategy::forwardInterest(const Interest& interest, Face& outFace, const fib::Entry& fibEntry,
+                             const shared_ptr<pit::Entry>& pitEntry, bool wantNewNonce)
 {
   auto egress = FaceEndpoint(outFace, 0);
   if (wantNewNonce) {
-    //Send probe: interest with new Nonce
+    // Send probe: interest with new Nonce
     Interest probeInterest(interest);
     probeInterest.refreshNonce();
-    NFD_LOG_TRACE("Sending probe for " << probeInterest << probeInterest.getNonce()
-                  << " to: " << egress);
+    NFD_LOG_TRACE("Sending probe for " << probeInterest << " to=" << egress);
     this->sendInterest(pitEntry, egress, probeInterest);
   }
   else {
@@ -246,18 +228,30 @@
   namespaceInfo.extendFaceInfoLifetime(faceInfo, egress.face.getId());
 
   if (!faceInfo.isTimeoutScheduled()) {
-    // Estimate and schedule timeout
-    auto timeout = faceInfo.computeRto();
-
-    NFD_LOG_TRACE("Scheduling timeout for " << fibEntry.getPrefix() << " to: " << egress
+    auto timeout = faceInfo.scheduleTimeout(interest.getName(),
+      [this, name = interest.getName(), faceId = egress.face.getId()] {
+        onTimeout(name, faceId);
+      });
+    NFD_LOG_TRACE("Scheduled timeout for " << fibEntry.getPrefix() << " to=" << egress
                   << " in " << time::duration_cast<time::milliseconds>(timeout) << " ms");
-
-    auto id = getScheduler().schedule(timeout, bind(&AsfStrategy::onTimeout, this,
-                                                    interest.getName(), egress.face.getId()));
-    faceInfo.setTimeoutEvent(id, interest.getName());
   }
 }
 
+void
+AsfStrategy::sendProbe(const Interest& interest, const FaceEndpoint& ingress, const Face& faceToUse,
+                       const fib::Entry& fibEntry, const shared_ptr<pit::Entry>& pitEntry)
+{
+  if (!m_probing.isProbingNeeded(fibEntry, interest))
+    return;
+
+  Face* faceToProbe = m_probing.getFaceToProbe(ingress.face, interest, fibEntry, faceToUse);
+  if (faceToProbe == nullptr)
+    return;
+
+  forwardInterest(interest, *faceToProbe, fibEntry, pitEntry, true);
+  m_probing.afterForwardingProbe(fibEntry, interest);
+}
+
 struct FaceStats
 {
   Face* face;
@@ -266,131 +260,101 @@
   uint64_t cost;
 };
 
-time::nanoseconds
-getValueForSorting(const FaceStats& stats)
+struct FaceStatsCompare
 {
-  // These values allow faces with no measurements to be ranked better than timeouts
-  // srtt < RTT_NO_MEASUREMENT < RTT_TIMEOUT
-  static const time::nanoseconds SORTING_RTT_TIMEOUT = time::nanoseconds::max();
-  static const time::nanoseconds SORTING_RTT_NO_MEASUREMENT = SORTING_RTT_TIMEOUT / 2;
+  bool
+  operator()(const FaceStats& lhs, const FaceStats& rhs) const
+  {
+    time::nanoseconds lhsValue = getValueForSorting(lhs);
+    time::nanoseconds rhsValue = getValueForSorting(rhs);
 
-  if (stats.rtt == RttStats::RTT_TIMEOUT) {
-    return SORTING_RTT_TIMEOUT;
+    // Sort by RTT and then by cost
+    return std::tie(lhsValue, lhs.cost) < std::tie(rhsValue, rhs.cost);
   }
-  else if (stats.rtt == RttStats::RTT_NO_MEASUREMENT) {
-    return SORTING_RTT_NO_MEASUREMENT;
+
+private:
+  static time::nanoseconds
+  getValueForSorting(const FaceStats& stats)
+  {
+    // These values allow faces with no measurements to be ranked better than timeouts
+    // srtt < RTT_NO_MEASUREMENT < RTT_TIMEOUT
+    if (stats.rtt == FaceInfo::RTT_TIMEOUT) {
+      return time::nanoseconds::max();
+    }
+    else if (stats.rtt == FaceInfo::RTT_NO_MEASUREMENT) {
+      return time::nanoseconds::max() / 2;
+    }
+    else {
+      return stats.srtt;
+    }
   }
-  else {
-    return stats.srtt;
-  }
-}
+};
 
 Face*
-AsfStrategy::getBestFaceForForwarding(const fib::Entry& fibEntry, const Interest& interest,
-                                      const Face& inFace, const shared_ptr<pit::Entry>& pitEntry,
+AsfStrategy::getBestFaceForForwarding(const Interest& interest, const Face& inFace,
+                                      const fib::Entry& fibEntry, const shared_ptr<pit::Entry>& pitEntry,
                                       bool isInterestNew)
 {
-  NFD_LOG_TRACE("Looking for best face for " << fibEntry.getPrefix());
-
-  typedef std::function<bool(const FaceStats&, const FaceStats&)> FaceStatsPredicate;
-  typedef std::set<FaceStats, FaceStatsPredicate> FaceStatsSet;
-
-  FaceStatsSet rankedFaces(
-    [] (const FaceStats& lhs, const FaceStats& rhs) -> bool {
-      // Sort by RTT and then by cost
-      time::nanoseconds lhsValue = getValueForSorting(lhs);
-      time::nanoseconds rhsValue = getValueForSorting(rhs);
-
-      if (lhsValue < rhsValue) {
-        return true;
-      }
-      else if (lhsValue == rhsValue) {
-        return lhs.cost < rhs.cost;
-      }
-      else {
-        return false;
-      }
-  });
+  std::set<FaceStats, FaceStatsCompare> rankedFaces;
 
   auto now = time::steady_clock::now();
-  for (const fib::NextHop& hop : fibEntry.getNextHops()) {
-    Face& hopFace = hop.getFace();
-
-    if (!isNextHopEligible(inFace, interest, hop, pitEntry, !isInterestNew, now)) {
+  for (const auto& nh : fibEntry.getNextHops()) {
+    if (!isNextHopEligible(inFace, interest, nh, pitEntry, !isInterestNew, now)) {
       continue;
     }
 
-    FaceInfo* info = m_measurements.getFaceInfo(fibEntry, interest, hopFace.getId());
-
+    FaceInfo* info = m_measurements.getFaceInfo(fibEntry, interest, nh.getFace().getId());
     if (info == nullptr) {
-      FaceStats stats = {&hopFace,
-                         RttStats::RTT_NO_MEASUREMENT,
-                         RttStats::RTT_NO_MEASUREMENT,
-                         hop.getCost()};
-
-      rankedFaces.insert(stats);
+      rankedFaces.insert({&nh.getFace(), FaceInfo::RTT_NO_MEASUREMENT,
+                          FaceInfo::RTT_NO_MEASUREMENT, nh.getCost()});
     }
     else {
-      FaceStats stats = {&hopFace, info->getRtt(), info->getSrtt(), hop.getCost()};
-      rankedFaces.insert(stats);
+      rankedFaces.insert({&nh.getFace(), info->getLastRtt(), info->getSrtt(), nh.getCost()});
     }
   }
 
-  FaceStatsSet::iterator it = rankedFaces.begin();
-
-  if (it != rankedFaces.end()) {
-    return it->face;
-  }
-  else {
-    return nullptr;
-  }
+  auto it = rankedFaces.begin();
+  return it != rankedFaces.end() ? it->face : nullptr;
 }
 
 void
-AsfStrategy::onTimeout(const Name& interestName, const face::FaceId faceId)
+AsfStrategy::onTimeout(const Name& interestName, FaceId faceId)
 {
   NamespaceInfo* namespaceInfo = m_measurements.getNamespaceInfo(interestName);
-
   if (namespaceInfo == nullptr) {
-    NFD_LOG_TRACE("FibEntry for " << interestName << " has been removed since timeout scheduling");
+    NFD_LOG_TRACE(interestName << " FibEntry has been removed since timeout scheduling");
     return;
   }
 
-  FaceInfoTable::iterator it = namespaceInfo->find(faceId);
-
-  if (it == namespaceInfo->end()) {
-    it = namespaceInfo->insert(faceId);
+  FaceInfo* fiPtr = namespaceInfo->getFaceInfo(faceId);
+  if (fiPtr == nullptr) {
+    NFD_LOG_TRACE(interestName << " FaceInfo id=" << faceId << " has been removed since timeout scheduling");
+    return;
   }
 
-  FaceInfo& faceInfo = it->second;
+  auto& faceInfo = *fiPtr;
+  size_t nTimeouts = faceInfo.getNSilentTimeouts() + 1;
+  faceInfo.setNSilentTimeouts(nTimeouts);
 
-  faceInfo.setNSilentTimeouts(faceInfo.getNSilentTimeouts() + 1);
-
-  if (faceInfo.getNSilentTimeouts() <= m_maxSilentTimeouts) {
-    NFD_LOG_TRACE("FaceId " << faceId << " for " << interestName << " has timed-out "
-                  << faceInfo.getNSilentTimeouts() << " time(s), ignoring");
+  if (nTimeouts <= m_maxSilentTimeouts) {
+    NFD_LOG_TRACE(interestName << " face=" << faceId << " timeout-count=" << nTimeouts << " ignoring");
     // Extend lifetime for measurements associated with Face
     namespaceInfo->extendFaceInfoLifetime(faceInfo, faceId);
-
-    if (faceInfo.isTimeoutScheduled()) {
-      faceInfo.cancelTimeoutEvent(interestName);
-    }
+    faceInfo.cancelTimeout(interestName);
   }
   else {
-    NFD_LOG_TRACE("FaceId " << faceId << " for " << interestName << " has timed-out");
+    NFD_LOG_TRACE(interestName << " face=" << faceId << " timeout-count=" << nTimeouts);
     faceInfo.recordTimeout(interestName);
   }
 }
 
 void
-AsfStrategy::sendNoRouteNack(const FaceEndpoint& ingress, const Interest& interest,
-                             const shared_ptr<pit::Entry>& pitEntry)
+AsfStrategy::sendNoRouteNack(const FaceEndpoint& ingress, const shared_ptr<pit::Entry>& pitEntry)
 {
-  NFD_LOG_DEBUG(interest << " from=" << ingress << " noNextHop");
-
   lp::NackHeader nackHeader;
   nackHeader.setReason(lp::NackReason::NO_ROUTE);
   this->sendNack(pitEntry, ingress, nackHeader);
+  this->rejectPendingInterest(pitEntry);
 }
 
 } // namespace asf