fw: use per-upstream suppression in ASF strategy

Refs: #5140
Change-Id: I49b27ce6b55d01724d0eafa792785d081c6009f7

daemon/fw/asf-measurements.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2019,  Regents of the University of California,
+ * Copyright (c) 2014-2021,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -92,22 +92,21 @@ AsfMeasurements::AsfMeasurements(MeasurementsAccessor& measurements)
 }
 
 FaceInfo*
-AsfMeasurements::getFaceInfo(const fib::Entry& fibEntry, const Interest& interest, FaceId faceId)
+AsfMeasurements::getFaceInfo(const fib::Entry& fibEntry, const Name& interestName, FaceId faceId)
 {
-  return getOrCreateNamespaceInfo(fibEntry, interest).getFaceInfo(faceId);
+  return getOrCreateNamespaceInfo(fibEntry, interestName).getFaceInfo(faceId);
 }
 
 FaceInfo&
-AsfMeasurements::getOrCreateFaceInfo(const fib::Entry& fibEntry, const Interest& interest,
-                                     FaceId faceId)
+AsfMeasurements::getOrCreateFaceInfo(const fib::Entry& fibEntry, const Name& interestName, FaceId faceId)
 {
-  return getOrCreateNamespaceInfo(fibEntry, interest).getOrCreateFaceInfo(faceId);
+  return getOrCreateNamespaceInfo(fibEntry, interestName).getOrCreateFaceInfo(faceId);
 }
 
 NamespaceInfo*
 AsfMeasurements::getNamespaceInfo(const Name& prefix)
 {
-  measurements::Entry* me = m_measurements.findLongestPrefixMatch(prefix);
+  auto* me = m_measurements.findLongestPrefixMatch(prefix);
   if (me == nullptr) {
     return nullptr;
   }
@@ -121,15 +120,16 @@ AsfMeasurements::getNamespaceInfo(const Name& prefix)
 }
 
 NamespaceInfo&
-AsfMeasurements::getOrCreateNamespaceInfo(const fib::Entry& fibEntry, const Interest& interest)
+AsfMeasurements::getOrCreateNamespaceInfo(const fib::Entry& fibEntry, const Name& prefix)
 {
-  measurements::Entry* me = m_measurements.get(fibEntry);
+  auto* me = m_measurements.get(fibEntry);
 
   // If the FIB entry is not under the strategy's namespace, find a part of the prefix
   // that falls under the strategy's namespace
   for (size_t prefixLen = fibEntry.getPrefix().size() + 1;
-       me == nullptr && prefixLen <= interest.getName().size(); ++prefixLen) {
-    me = m_measurements.get(interest.getName().getPrefix(prefixLen));
+       me == nullptr && prefixLen <= prefix.size();
+       ++prefixLen) {
+    me = m_measurements.get(prefix.getPrefix(prefixLen));
   }
 
   // Either the FIB entry or the Interest's name must be under this strategy's namespace

daemon/fw/asf-measurements.hpp

@@ -192,16 +192,16 @@ public:
   AsfMeasurements(MeasurementsAccessor& measurements);
 
   FaceInfo*
-  getFaceInfo(const fib::Entry& fibEntry, const Interest& interest, FaceId faceId);
+  getFaceInfo(const fib::Entry& fibEntry, const Name& interestName, FaceId faceId);
 
   FaceInfo&
-  getOrCreateFaceInfo(const fib::Entry& fibEntry, const Interest& interest, FaceId faceId);
+  getOrCreateFaceInfo(const fib::Entry& fibEntry, const Name& interestName, FaceId faceId);
 
   NamespaceInfo*
   getNamespaceInfo(const Name& prefix);
 
   NamespaceInfo&
-  getOrCreateNamespaceInfo(const fib::Entry& fibEntry, const Interest& interest);
+  getOrCreateNamespaceInfo(const fib::Entry& fibEntry, const Name& prefix);
 
 private:
   void

daemon/fw/asf-probing-module.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2019,  Regents of the University of California,
+ * Copyright (c) 2014-2021,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -81,7 +81,7 @@ ProbingModule::getFaceToProbe(const Face& inFace, const Interest& interest,
       continue;
     }
 
-    FaceInfo* info = m_measurements.getFaceInfo(fibEntry, interest, hopFace.getId());
+    FaceInfo* info = m_measurements.getFaceInfo(fibEntry, interest.getName(), hopFace.getId());
     // If no RTT has been recorded, probe this face
     if (info == nullptr || info->getLastRtt() == FaceInfo::RTT_NO_MEASUREMENT) {
       return &hopFace;
@@ -100,10 +100,10 @@ ProbingModule::getFaceToProbe(const Face& inFace, const Interest& interest,
 }
 
 bool
-ProbingModule::isProbingNeeded(const fib::Entry& fibEntry, const Interest& interest)
+ProbingModule::isProbingNeeded(const fib::Entry& fibEntry, const Name& interestName)
 {
   // Return the probing status flag for a namespace
-  NamespaceInfo& info = m_measurements.getOrCreateNamespaceInfo(fibEntry, interest);
+  NamespaceInfo& info = m_measurements.getOrCreateNamespaceInfo(fibEntry, interestName);
 
   // If a first probe has not been scheduled for a namespace
   if (!info.isFirstProbeScheduled()) {
@@ -118,11 +118,11 @@ ProbingModule::isProbingNeeded(const fib::Entry& fibEntry, const Interest& inter
 }
 
 void
-ProbingModule::afterForwardingProbe(const fib::Entry& fibEntry, const Interest& interest)
+ProbingModule::afterForwardingProbe(const fib::Entry& fibEntry, const Name& interestName)
 {
   // After probing is done, need to set probing flag to false and
   // schedule another future probe
-  NamespaceInfo& info = m_measurements.getOrCreateNamespaceInfo(fibEntry, interest);
+  NamespaceInfo& info = m_measurements.getOrCreateNamespaceInfo(fibEntry, interestName);
   info.setIsProbingDue(false);
 
   scheduleProbe(fibEntry, m_probingInterval);

daemon/fw/asf-probing-module.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2019,  Regents of the University of California,
+ * Copyright (c) 2014-2021,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -48,10 +48,10 @@ public:
                  const fib::Entry& fibEntry, const Face& faceUsed);
 
   bool
-  isProbingNeeded(const fib::Entry& fibEntry, const Interest& interest);
+  isProbingNeeded(const fib::Entry& fibEntry, const Name& interestName);
 
   void
-  afterForwardingProbe(const fib::Entry& fibEntry, const Interest& interest);
+  afterForwardingProbe(const fib::Entry& fibEntry, const Name& interestName);
 
   void
   setProbingInterval(size_t probingInterval);

daemon/fw/asf-strategy.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2020,  Regents of the University of California,
+ * Copyright (c) 2014-2021,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -64,7 +64,7 @@ AsfStrategy::AsfStrategy(Forwarder& forwarder, const Name& name)
 const Name&
 AsfStrategy::getStrategyName()
 {
-  static Name strategyName("/localhost/nfd/strategy/asf/%FD%03");
+  static Name strategyName("/localhost/nfd/strategy/asf/%FD%04");
   return strategyName;
 }
 
@@ -110,56 +110,65 @@ void
 AsfStrategy::afterReceiveInterest(const FaceEndpoint& ingress, const Interest& interest,
                                   const shared_ptr<pit::Entry>& pitEntry)
 {
-  // Should the Interest be suppressed?
-  auto suppressResult = m_retxSuppression.decidePerPitEntry(*pitEntry);
-  if (suppressResult == RetxSuppressionResult::SUPPRESS) {
-    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " suppressed");
-    return;
-  }
+  const auto& fibEntry = this->lookupFib(*pitEntry);
 
-  const fib::Entry& fibEntry = this->lookupFib(*pitEntry);
-  const fib::NextHopList& nexthops = fibEntry.getNextHops();
-
-  if (suppressResult == RetxSuppressionResult::NEW) {
-    if (nexthops.size() == 0) {
+  // Check if the interest is new and, if so, skip the retx suppression check
+  if (!hasPendingOutRecords(*pitEntry)) {
+    auto* faceToUse = getBestFaceForForwarding(interest, ingress.face, fibEntry, pitEntry);
+    if (faceToUse == nullptr) {
       NFD_LOG_DEBUG(interest << " new-interest from=" << ingress << " no-nexthop");
       sendNoRouteNack(ingress.face, pitEntry);
-      return;
-    }
-
-    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 necessary, send probe
-      sendProbe(interest, ingress, *faceToUse, fibEntry, pitEntry);
     }
     else {
-      NFD_LOG_DEBUG(interest << " new-interest from=" << ingress << " no-nexthop");
-      sendNoRouteNack(ingress.face, pitEntry);
+      NFD_LOG_DEBUG(interest << " new-interest from=" << ingress << " forward-to=" << faceToUse->getId());
+      forwardInterest(interest, *faceToUse, fibEntry, pitEntry);
+      sendProbe(interest, ingress, *faceToUse, fibEntry, pitEntry);
     }
     return;
   }
 
-  Face* faceToUse = getBestFaceForForwarding(interest, ingress.face, fibEntry, pitEntry, false);
-  // if unused face not found, select nexthop with earliest out record
+  auto* faceToUse = getBestFaceForForwarding(interest, ingress.face, fibEntry, pitEntry, false);
   if (faceToUse != nullptr) {
-    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " forward-to=" << faceToUse->getId());
-    forwardInterest(interest, *faceToUse, fibEntry, pitEntry);
-    // avoid probing in case of forwarding
+    auto suppressResult = m_retxSuppression.decidePerUpstream(*pitEntry, *faceToUse);
+    if (suppressResult == RetxSuppressionResult::SUPPRESS) {
+      // Cannot be sent on this face, interest was received within the suppression window
+      NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress
+                    << " forward-to=" << faceToUse->getId() << " suppressed");
+    }
+    else {
+      // The retx arrived after the suppression period: forward it but don't probe, because
+      // probing was done earlier for this interest when it was newly received
+      NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " forward-to=" << faceToUse->getId());
+      auto* outRecord = forwardInterest(interest, *faceToUse, fibEntry, pitEntry);
+      if (outRecord && suppressResult == RetxSuppressionResult::FORWARD) {
+        m_retxSuppression.incrementIntervalForOutRecord(*outRecord);
+      }
+    }
     return;
   }
 
-  // find an eligible upstream that is used earliest
-  auto it = nexthops.end();
-  it = findEligibleNextHopWithEarliestOutRecord(ingress.face, interest, nexthops, pitEntry);
+  // If all eligible faces have been used (i.e., they all have a pending out-record),
+  // choose the nexthop with the earliest out-record
+  const auto& nexthops = fibEntry.getNextHops();
+  auto it = findEligibleNextHopWithEarliestOutRecord(ingress.face, interest, nexthops, pitEntry);
   if (it == nexthops.end()) {
-    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " no-nexthop");
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " no eligible nexthop");
+    return;
+  }
+  auto& outFace = it->getFace();
+  auto suppressResult = m_retxSuppression.decidePerUpstream(*pitEntry, outFace);
+  if (suppressResult == RetxSuppressionResult::SUPPRESS) {
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress
+                  << " retry-to=" << outFace.getId() << " suppressed");
   }
   else {
-    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " retry-to=" << it->getFace().getId());
-    this->sendInterest(pitEntry, it->getFace(), interest);
+    NFD_LOG_DEBUG(interest << " retx-interest from=" << ingress << " retry-to=" << outFace.getId());
+    // sendInterest() is used here instead of forwardInterest() because the measurements info
+    // were already attached to this face in the previous forwarding
+    auto* outRecord = sendInterest(pitEntry, outFace, interest);
+    if (outRecord && suppressResult == RetxSuppressionResult::FORWARD) {
+      m_retxSuppression.incrementIntervalForOutRecord(*outRecord);
+    }
   }
 }
 
@@ -205,52 +214,50 @@ AsfStrategy::afterReceiveNack(const FaceEndpoint& ingress, const lp::Nack& nack,
   onTimeoutOrNack(pitEntry->getName(), ingress.face.getId(), true);
 }
 
-void
+pit::OutRecord*
 AsfStrategy::forwardInterest(const Interest& interest, Face& outFace, const fib::Entry& fibEntry,
-                             const shared_ptr<pit::Entry>& pitEntry, bool wantNewNonce)
+                             const shared_ptr<pit::Entry>& pitEntry)
 {
+  const auto& interestName = interest.getName();
   auto faceId = outFace.getId();
 
-  if (wantNewNonce) {
-    // Send probe: interest with new Nonce
-    Interest probeInterest(interest);
-    probeInterest.refreshNonce();
-    NFD_LOG_TRACE("Sending probe for " << probeInterest << " to=" << faceId);
-    this->sendInterest(pitEntry, outFace, probeInterest);
-  }
-  else {
-    this->sendInterest(pitEntry, outFace, interest);
-  }
+  auto* outRecord = sendInterest(pitEntry, outFace, interest);
 
-  FaceInfo& faceInfo = m_measurements.getOrCreateFaceInfo(fibEntry, interest, faceId);
+  FaceInfo& faceInfo = m_measurements.getOrCreateFaceInfo(fibEntry, interestName, faceId);
 
   // Refresh measurements since Face is being used for forwarding
-  NamespaceInfo& namespaceInfo = m_measurements.getOrCreateNamespaceInfo(fibEntry, interest);
+  NamespaceInfo& namespaceInfo = m_measurements.getOrCreateNamespaceInfo(fibEntry, interestName);
   namespaceInfo.extendFaceInfoLifetime(faceInfo, faceId);
 
   if (!faceInfo.isTimeoutScheduled()) {
-    auto timeout = faceInfo.scheduleTimeout(interest.getName(),
-      [this, name = interest.getName(), faceId] {
-        onTimeoutOrNack(name, faceId, false);
-      });
+    auto timeout = faceInfo.scheduleTimeout(interestName,
+                                            [this, name = interestName, faceId] {
+                                              onTimeoutOrNack(name, faceId, false);
+                                            });
     NFD_LOG_TRACE("Scheduled timeout for " << fibEntry.getPrefix() << " to=" << faceId
-                  << " in " << time::duration_cast<time::milliseconds>(timeout) << " ms");
+                  << " in " << time::duration_cast<time::milliseconds>(timeout));
   }
+
+  return outRecord;
 }
 
 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))
+  if (!m_probing.isProbingNeeded(fibEntry, interest.getName()))
     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);
+  Interest probeInterest(interest);
+  probeInterest.refreshNonce();
+  NFD_LOG_TRACE("Sending probe " << probeInterest << " to=" << faceToProbe->getId());
+  forwardInterest(probeInterest, *faceToProbe, fibEntry, pitEntry);
+
+  m_probing.afterForwardingProbe(fibEntry, interest.getName());
 }
 
 struct FaceStats
@@ -304,7 +311,7 @@ AsfStrategy::getBestFaceForForwarding(const Interest& interest, const Face& inFa
       continue;
     }
 
-    FaceInfo* info = m_measurements.getFaceInfo(fibEntry, interest, nh.getFace().getId());
+    const FaceInfo* info = m_measurements.getFaceInfo(fibEntry, interest.getName(), nh.getFace().getId());
     if (info == nullptr) {
       rankedFaces.insert({&nh.getFace(), FaceInfo::RTT_NO_MEASUREMENT,
                           FaceInfo::RTT_NO_MEASUREMENT, nh.getCost()});

daemon/fw/asf-strategy.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2020,  Regents of the University of California,
+ * Copyright (c) 2014-2021,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -37,9 +37,10 @@ namespace asf {
 
 /** \brief Adaptive SRTT-based Forwarding Strategy
  *
- *  \see Vince Lehman, Ashlesh Gawande, Rodrigo Aldecoa, Dmitri Krioukov, Beichuan Zhang, Lixia Zhang, and Lan Wang,
- *       "An Experimental Investigation of Hyperbolic Routing with a Smart Forwarding Plane in NDN,"
- *       NDN Technical Report NDN-0042, 2016. http://named-data.net/techreports.html
+ *  \see Vince Lehman, Ashlesh Gawande, Rodrigo Aldecoa, Dmitri Krioukov, Beichuan Zhang,
+ *       Lixia Zhang, and Lan Wang, "An Experimental Investigation of Hyperbolic Routing
+ *       with a Smart Forwarding Plane in NDN", NDN Technical Report NDN-0042, 2016.
+ *       https://named-data.net/publications/techreports/ndn-0042-1-asf/
  */
 class AsfStrategy : public Strategy
 {
@@ -67,9 +68,9 @@ private:
   void
   processParams(const PartialName& parsed);
 
-  void
+  pit::OutRecord*
   forwardInterest(const Interest& interest, Face& outFace, const fib::Entry& fibEntry,
-                  const shared_ptr<pit::Entry>& pitEntry, bool wantNewNonce = false);
+                  const shared_ptr<pit::Entry>& pitEntry);
 
   void
   sendProbe(const Interest& interest, const FaceEndpoint& ingress, const Face& faceToUse,

tests/daemon/fw/asf-strategy.t.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2020,  Regents of the University of California,
+ * Copyright (c) 2014-2021,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -46,7 +46,8 @@ class AsfGridFixture : public GlobalIoTimeFixture
 {
 protected:
   explicit
-  AsfGridFixture(const Name& params = AsfStrategy::getStrategyName(), time::nanoseconds replyDelay = 0_ms)
+  AsfGridFixture(const Name& params = AsfStrategy::getStrategyName(),
+                 time::nanoseconds replyDelay = 0_ns)
     : parameters(params)
   {
     /*
@@ -70,10 +71,10 @@ protected:
     nodeC = topo.addForwarder("C");
     nodeD = topo.addForwarder("D");
 
-    topo.setStrategy<AsfStrategy>(nodeA, Name("ndn:/"), parameters);
-    topo.setStrategy<AsfStrategy>(nodeB, Name("ndn:/"), parameters);
-    topo.setStrategy<AsfStrategy>(nodeC, Name("ndn:/"), parameters);
-    topo.setStrategy<AsfStrategy>(nodeD, Name("ndn:/"), parameters);
+    topo.setStrategy<AsfStrategy>(nodeA, Name("/"), parameters);
+    topo.setStrategy<AsfStrategy>(nodeB, Name("/"), parameters);
+    topo.setStrategy<AsfStrategy>(nodeC, Name("/"), parameters);
+    topo.setStrategy<AsfStrategy>(nodeD, Name("/"), parameters);
 
     linkAB = topo.addLink("AB", 10_ms, {nodeA, nodeB});
     linkAD = topo.addLink("AD", 100_ms, {nodeA, nodeD});
@@ -91,7 +92,7 @@ protected:
   }
 
   void
-  runConsumer(int numInterests = 30)
+  runConsumer(size_t numInterests = 30)
   {
     topo.addIntervalConsumer(consumer->getClientFace(), PRODUCER_PREFIX, 1_s, numInterests);
     this->advanceClocks(10_ms, time::seconds(numInterests));
@@ -117,6 +118,8 @@ protected:
   static const Name PRODUCER_PREFIX;
 };
 
+const Name AsfGridFixture::PRODUCER_PREFIX("/hr/C");
+
 class AsfStrategyParametersGridFixture : public AsfGridFixture
 {
 protected:
@@ -128,8 +131,6 @@ protected:
   }
 };
 
-const Name AsfGridFixture::PRODUCER_PREFIX = Name("ndn:/hr/C");
-
 BOOST_FIXTURE_TEST_CASE(Basic, AsfGridFixture)
 {
   // Both nodeB and nodeD have FIB entries to reach the producer
@@ -210,10 +211,9 @@ protected:
 
 BOOST_FIXTURE_TEST_CASE(InterestForwarding, AsfStrategyDelayedDataFixture)
 {
-
   Name name(PRODUCER_PREFIX);
   name.appendTimestamp();
-  shared_ptr<Interest> interest = makeInterest(name, true);
+  auto interest = makeInterest(name);
 
   topo.registerPrefix(nodeB, linkBC->getFace(nodeB), PRODUCER_PREFIX);
   topo.registerPrefix(nodeD, linkCD->getFace(nodeD), PRODUCER_PREFIX);
@@ -240,43 +240,40 @@ BOOST_FIXTURE_TEST_CASE(InterestForwarding, AsfStrategyDelayedDataFixture)
   BOOST_CHECK_EQUAL(linkAD->getFace(nodeA).getCounters().nInData, 1);
   BOOST_CHECK_EQUAL(linkAB->getFace(nodeA).getCounters().nInData, 1);
   BOOST_CHECK_EQUAL(consumer->getForwarderFace().getCounters().nOutData, 1);
-
 }
 
-BOOST_AUTO_TEST_CASE(Retransmission)
+BOOST_AUTO_TEST_CASE(Retransmission) // Bug #4874
 {
-  // This is a unit test written to recreate the following issue
-  // https://redmine.named-data.net/issues/4874
-  /*
-   *                  +---------+   10ms   +---------+
-   *                  |  nodeB  | ------>  |  nodeC  |
-   *                  +---------+          +---------+
-   */
   // Avoid clearing pit entry for those incoming interest that have pit entry but no next hops
+  /*
+   *        +---------+   10ms   +---------+
+   *        |  nodeB  | ------>  |  nodeC  |
+   *        +---------+          +---------+
+   */
 
-  static const Name PRODUCER_PREFIX = "ndn:/pnr/C";
-  Name parameters = AsfStrategy::getStrategyName();
+  const Name PRODUCER_PREFIX = "/pnr/C";
   TopologyTester topo;
 
   TopologyNode nodeB = topo.addForwarder("B"),
                nodeC = topo.addForwarder("C");
 
-  topo.setStrategy<AsfStrategy>(nodeB, Name("ndn:/"), parameters);
-  topo.setStrategy<AsfStrategy>(nodeC, Name("ndn:/"), parameters);
+  for (auto node : {nodeB, nodeC}) {
+    topo.setStrategy<AsfStrategy>(node);
+  }
 
-  shared_ptr<TopologyLink> linkBC = topo.addLink("BC", time::milliseconds(10), {nodeB, nodeC});
+  auto linkBC = topo.addLink("BC", time::milliseconds(10), {nodeB, nodeC});
 
-  shared_ptr<TopologyAppLink> consumer = topo.addAppFace("c", nodeB),
-                              producer = topo.addAppFace("p", nodeC, PRODUCER_PREFIX);
+  auto consumer = topo.addAppFace("c", nodeB),
+       producer = topo.addAppFace("p", nodeC, PRODUCER_PREFIX);
 
   topo.addEchoProducer(producer->getClientFace(), PRODUCER_PREFIX, 100_ms);
 
   Name name(PRODUCER_PREFIX);
   name.appendTimestamp();
-  auto interest = makeInterest(name, true);
+  auto interest = makeInterest(name);
 
-  nfd::pit::Pit& pit = topo.getForwarder(nodeB).getPit();
-  shared_ptr<pit::Entry> pitEntry = pit.insert(*interest).first;
+  auto& pit = topo.getForwarder(nodeB).getPit();
+  auto pitEntry = pit.insert(*interest).first;
 
   topo.getForwarder(nodeB).onOutgoingInterest(pitEntry, linkBC->getFace(nodeB), *interest);
   this->advanceClocks(time::milliseconds(100));
@@ -285,7 +282,7 @@ BOOST_AUTO_TEST_CASE(Retransmission)
   consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
   this->advanceClocks(time::milliseconds(100));
 
-  pit::OutRecordCollection::const_iterator outRecord = pitEntry->getOutRecord(linkBC->getFace(nodeB));
+  auto outRecord = pitEntry->getOutRecord(linkBC->getFace(nodeB));
   BOOST_CHECK(outRecord != pitEntry->out_end());
 
   this->advanceClocks(time::milliseconds(100));
@@ -293,47 +290,144 @@ BOOST_AUTO_TEST_CASE(Retransmission)
   BOOST_CHECK_EQUAL(linkBC->getFace(nodeB).getCounters().nInData, 1);
 }
 
+BOOST_AUTO_TEST_CASE(PerUpstreamSuppression)
+{
+  /*
+   *                          +---------+
+   *                     +----|  nodeB  |----+
+   *                     |    +---------+    |
+   *                50ms |                   | 50ms
+   *                     |                   |
+   *                +---------+   50ms  +---------+
+   *                |  nodeA  | <-----> |  nodeP  |
+   *                +---------+         +---------+
+   */
+
+  const Name PRODUCER_PREFIX = "/suppress/me";
+  TopologyTester topo;
+
+  TopologyNode nodeA = topo.addForwarder("A"),
+               nodeB = topo.addForwarder("B"),
+               nodeP = topo.addForwarder("P");
+
+  for (auto node : {nodeA, nodeB, nodeP}) {
+    topo.setStrategy<AsfStrategy>(node);
+  }
+
+  auto linkAB = topo.addLink("AB", 50_ms, {nodeA, nodeB});
+  auto linkAP = topo.addLink("AP", 50_ms, {nodeA, nodeP});
+  auto linkBP = topo.addLink("BP", 50_ms, {nodeB, nodeP});
+
+  auto consumer = topo.addAppFace("cons", nodeA),
+       producer = topo.addAppFace("prod", nodeP, PRODUCER_PREFIX);
+
+  topo.addEchoProducer(producer->getClientFace(), PRODUCER_PREFIX);
+
+  topo.registerPrefix(nodeA, linkAP->getFace(nodeA), PRODUCER_PREFIX, 10);
+  topo.registerPrefix(nodeA, linkAB->getFace(nodeA), PRODUCER_PREFIX, 1);
+  topo.registerPrefix(nodeB, linkBP->getFace(nodeB), PRODUCER_PREFIX, 1);
+
+  auto& faceA2B = linkAB->getFace(nodeA);
+  auto& faceA2P = linkAP->getFace(nodeA);
+
+  Name name(PRODUCER_PREFIX);
+  name.appendTimestamp();
+  // very short lifetime to make it expire within the initial retx suppression period (10ms)
+  auto interest = makeInterest(name, false, 5_ms);
+
+  // 1st interest should be sent to B
+  consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
+  this->advanceClocks(1_ms);
+  BOOST_TEST(faceA2B.getCounters().nOutInterests == 1);
+  BOOST_TEST(faceA2P.getCounters().nOutInterests == 0);
+
+  // 2nd interest should be sent to P and NOT suppressed
+  interest->setInterestLifetime(100_ms);
+  interest->refreshNonce();
+  consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
+  this->advanceClocks(1_ms);
+  BOOST_TEST(faceA2B.getCounters().nOutInterests == 1);
+  BOOST_TEST(faceA2P.getCounters().nOutInterests == 1);
+
+  this->advanceClocks(1_ms);
+
+  // 3rd interest should be suppressed
+  // without suppression, it would have been sent again to B as that's the earliest out-record
+  interest->refreshNonce();
+  consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
+  this->advanceClocks(1_ms);
+  BOOST_TEST(faceA2B.getCounters().nOutInterests == 1);
+  BOOST_TEST(faceA2P.getCounters().nOutInterests == 1);
+
+  this->advanceClocks(2_ms); // 1st interest should expire now
+
+  // 4th interest should be suppressed
+  // without suppression, it would have been sent again to B because the out-record expired
+  interest->refreshNonce();
+  consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
+  this->advanceClocks(1_ms);
+  BOOST_TEST(faceA2B.getCounters().nOutInterests == 1);
+  BOOST_TEST(faceA2P.getCounters().nOutInterests == 1);
+
+  this->advanceClocks(5_ms); // suppression window ends
+
+  // 5th interest is sent to B and is outside the suppression window
+  interest->refreshNonce();
+  consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
+  this->advanceClocks(1_ms);
+  BOOST_TEST(faceA2B.getCounters().nOutInterests == 2);
+  BOOST_TEST(faceA2P.getCounters().nOutInterests == 1);
+
+  this->advanceClocks(10_ms);
+
+  // 6th interest is sent to P and is outside the suppression window
+  interest->refreshNonce();
+  consumer->getClientFace().expressInterest(*interest, nullptr, nullptr, nullptr);
+  this->advanceClocks(1_ms);
+  BOOST_TEST(faceA2B.getCounters().nOutInterests == 2);
+  BOOST_TEST(faceA2P.getCounters().nOutInterests == 2);
+}
+
 BOOST_AUTO_TEST_CASE(NoPitOutRecordAndProbeInterestNewNonce)
 {
-  /*                 +---------+
-  *                  |  nodeD  |
-  *                  +---------+
-  *                       |
-  *                       | 80ms
-  *                       |
-  *                       |
-  *                  +---------+
-  *           +----->|  nodeB  |<------+
-  *           |      +---------+       |
-  *      15ms |                        | 16ms
-  *           v                        v
-  *      +---------+              +---------+
-  *      |  nodeA  |--------------|  nodeC  |
-  *      +---------+     14ms      +---------+
-  */
+  /*                  +---------+
+   *                  |  nodeD  |
+   *                  +---------+
+   *                       |
+   *                       | 80ms
+   *                       |
+   *                       |
+   *                  +---------+
+   *           +----->|  nodeB  |<------+
+   *           |      +---------+       |
+   *      15ms |                        | 16ms
+   *           v                        v
+   *      +---------+              +---------+
+   *      |  nodeA  |--------------|  nodeC  |
+   *      +---------+     14ms      +---------+
+   */
 
   const Name PRODUCER_PREFIX = "/ndn/edu/nodeD/ping";
-
   TopologyTester topo;
+
   TopologyNode nodeA = topo.addForwarder("A"),
                nodeB = topo.addForwarder("B"),
                nodeC = topo.addForwarder("C"),
                nodeD = topo.addForwarder("D");
 
-  for (TopologyNode node : {nodeA, nodeB, nodeC, nodeD}) {
+  for (auto node : {nodeA, nodeB, nodeC, nodeD}) {
     topo.setStrategy<AsfStrategy>(node);
   }
 
-  shared_ptr<TopologyLink> linkAB = topo.addLink("AB", 15_ms, {nodeA, nodeB}),
-                           linkAC = topo.addLink("AC", 14_ms, {nodeA, nodeC}),
-                           linkBC = topo.addLink("BC", 16_ms, {nodeB, nodeC}),
-                           linkBD = topo.addLink("BD", 80_ms, {nodeB, nodeD});
+  auto linkAB = topo.addLink("AB", 15_ms, {nodeA, nodeB}),
+       linkAC = topo.addLink("AC", 14_ms, {nodeA, nodeC}),
+       linkBC = topo.addLink("BC", 16_ms, {nodeB, nodeC}),
+       linkBD = topo.addLink("BD", 80_ms, {nodeB, nodeD});
 
-  shared_ptr<TopologyAppLink> ping = topo.addAppFace("c", nodeA),
-                        pingServer = topo.addAppFace("p", nodeD, PRODUCER_PREFIX);
+  auto ping = topo.addAppFace("c", nodeA);
+  auto pingServer = topo.addAppFace("p", nodeD, PRODUCER_PREFIX);
   topo.addEchoProducer(pingServer->getClientFace());
 
-  // Register prefixes
   topo.registerPrefix(nodeA, linkAB->getFace(nodeA), PRODUCER_PREFIX, 15);
   topo.registerPrefix(nodeA, linkAC->getFace(nodeA), PRODUCER_PREFIX, 14);
   topo.registerPrefix(nodeC, linkBC->getFace(nodeC), PRODUCER_PREFIX, 16);
@@ -410,7 +504,7 @@ BOOST_FIXTURE_TEST_CASE(IgnoreTimeouts, AsfStrategyParametersGridFixture)
   // Send 15 interests let it change to use the 10 ms link
   runConsumer(15);
 
-  int outInterestsBeforeFailure = linkAD->getFace(nodeA).getCounters().nOutInterests;
+  uint64_t outInterestsBeforeFailure = linkAD->getFace(nodeA).getCounters().nOutInterests;
 
   // Bring down 10 ms link
   linkAB->fail();
@@ -420,12 +514,12 @@ BOOST_FIXTURE_TEST_CASE(IgnoreTimeouts, AsfStrategyParametersGridFixture)
   runConsumer(6);
 
   // Check that link has not been switched to 100 ms because n-silent-timeouts = 5
-  BOOST_CHECK_EQUAL(linkAD->getFace(nodeA).getCounters().nOutInterests - outInterestsBeforeFailure, 0);
+  BOOST_CHECK_EQUAL(linkAD->getFace(nodeA).getCounters().nOutInterests, outInterestsBeforeFailure);
 
   // Send 5 interests, check that 100 ms link is used
   runConsumer(5);
 
-  BOOST_CHECK_EQUAL(linkAD->getFace(nodeA).getCounters().nOutInterests - outInterestsBeforeFailure, 5);
+  BOOST_CHECK_EQUAL(linkAD->getFace(nodeA).getCounters().nOutInterests, outInterestsBeforeFailure + 5);
 }
 
 BOOST_FIXTURE_TEST_CASE(ProbingInterval, AsfStrategyParametersGridFixture)
@@ -437,7 +531,7 @@ BOOST_FIXTURE_TEST_CASE(ProbingInterval, AsfStrategyParametersGridFixture)
   // Send 6 interests let it change to use the 10 ms link
   runConsumer(6);
 
-  shared_ptr<TopologyLink> linkAC = topo.addLink("AC", 5_ms, {nodeA, nodeD});
+  auto linkAC = topo.addLink("AC", 5_ms, {nodeA, nodeD});
   topo.registerPrefix(nodeA, linkAC->getFace(nodeA), PRODUCER_PREFIX, 1);
 
   BOOST_CHECK_EQUAL(linkAC->getFace(nodeA).getCounters().nOutInterests, 0);
@@ -448,28 +542,21 @@ BOOST_FIXTURE_TEST_CASE(ProbingInterval, AsfStrategyParametersGridFixture)
   BOOST_CHECK_EQUAL(linkAC->getFace(nodeA).getCounters().nOutInterests, 1);
 }
 
-class ParametersFixture
+BOOST_AUTO_TEST_CASE(InstantiationWithParameters)
 {
-public:
-  void
-  checkValidity(std::string parameters, bool isCorrect)
-  {
-    Name strategyName(Name(AsfStrategy::getStrategyName()).append(parameters));
+  FaceTable faceTable;
+  Forwarder forwarder{faceTable};
+
+  auto checkValidity = [&] (std::string parameters, bool isCorrect) {
+    Name strategyName(Name(AsfStrategy::getStrategyName()).append(std::move(parameters)));
     if (isCorrect) {
       BOOST_CHECK_NO_THROW(make_unique<AsfStrategy>(forwarder, strategyName));
     }
     else {
       BOOST_CHECK_THROW(make_unique<AsfStrategy>(forwarder, strategyName), std::invalid_argument);
     }
-  }
+  };
 
-protected:
-  FaceTable faceTable;
-  Forwarder forwarder{faceTable};
-};
-
-BOOST_FIXTURE_TEST_CASE(InstantiationTest, ParametersFixture)
-{
   checkValidity("/probing-interval~30000/n-silent-timeouts~5", true);
   checkValidity("/n-silent-timeouts~5/probing-interval~30000", true);
   checkValidity("/probing-interval~30000", true);

tests/daemon/fw/strategy-instantiation.t.cpp

@@ -73,7 +73,7 @@ public:
 
 using Tests = boost::mpl::vector<
   Test<AccessStrategy, false, 1>,
-  Test<AsfStrategy, true, 3>,
+  Test<AsfStrategy, true, 4>,
   Test<BestRouteStrategy, false, 5>,
   Test<MulticastStrategy, false, 4>,
   Test<SelfLearningStrategy, false, 1>,