fw: localhop scope restriction in BestRouteStrategy2

localhop scope rules are implemented as wouldViolateScope function;
the old violatesScope function is now deprecated.
Strategy::sendInterest overload without specific Interest packet is also
deprecated. BestRouteStrategy2 has switched to use the new overload.

refs #3841, #1756

Change-Id: Ic117f01926eadddf1da3ccb580b52a3903a70c89

daemon/fw/access-strategy.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "access-strategy.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 #include "core/logger.hpp"
 
 namespace nfd {

daemon/fw/algorithm.cpp

@@ -23,7 +23,7 @@
  * NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 
 namespace nfd {
 namespace scope_prefix {
@@ -33,6 +33,28 @@ const Name LOCALHOP("ndn:/localhop");
 
 namespace fw {
 
+bool
+wouldViolateScope(const Face& inFace, const Interest& interest, const Face& outFace)
+{
+  if (outFace.getScope() == ndn::nfd::FACE_SCOPE_LOCAL) {
+    // forwarding to a local face is always allowed
+    return false;
+  }
+
+  if (scope_prefix::LOCALHOST.isPrefixOf(interest.getName())) {
+    // localhost Interests cannot be forwarded to a non-local face
+    return true;
+  }
+
+  if (scope_prefix::LOCALHOP.isPrefixOf(interest.getName())) {
+    // localhop Interests can be forwarded to a non-local face only if it comes from a local face
+    return inFace.getScope() != ndn::nfd::FACE_SCOPE_LOCAL;
+  }
+
+  // Interest name is not subject to scope control
+  return false;
+}
+
 bool
 violatesScope(const pit::Entry& pitEntry, const Face& outFace)
 {

daemon/fw/algorithm.hpp

@@ -29,13 +29,13 @@
 #include "table/pit-entry.hpp"
 
 /** \file
- *  This file contains algorithms that operate on a PIT entry.
+ *  This file contains common algorithms used by forwarding strategies.
  */
 
 namespace nfd {
 
-/** \brief contain Name prefix that affects namespace-based scope control
- *  \sa http://redmine.named-data.net/projects/nfd/wiki/ScopeControl
+/** \brief contain name prefixes that affect namespace-based scope control
+ *  \sa https://redmine.named-data.net/projects/nfd/wiki/ScopeControl
  */
 namespace scope_prefix {
 
@@ -54,11 +54,9 @@ extern const Name LOCALHOST;
  *  The localhop scope limits propagation to no further than the next node.
  *
  *  Interest packets under prefix ndn:/localhop are restricted by these rules:
- *  \li Interest can come from a local face or a non-local face.
- *  \li If PIT entry has at least one in-record from a local face,
- *      it can be forwarded to local faces and non-local faces.
- *  \li If PIT entry has all in-records from non-local faces,
- *      it can only be forwarded to local faces.
+ *  \li If an Interest is received from a local face, it can be forwarded to a non-local face.
+ *  \li If an Interest is received from a non-local face, it cannot be forwarded to a non-local face.
+ *  \li In either case the Interest can be forwarded to a local face.
  *  \li PIT entry can be satisfied by Data from any source.
  *
  *  Data packets under prefix ndn:/localhop are unrestricted.
@@ -70,10 +68,18 @@ extern const Name LOCALHOP;
 namespace fw {
 
 /** \brief determine whether forwarding the Interest in \p pitEntry to \p outFace would violate scope
- *  \sa http://redmine.named-data.net/projects/nfd/wiki/ScopeControl
+ *  \sa https://redmine.named-data.net/projects/nfd/wiki/ScopeControl
  */
 bool
-violatesScope(const pit::Entry& pitEntry, const Face& outFace);
+wouldViolateScope(const Face& inFace, const Interest& interest, const Face& outFace);
+
+/** \brief determine whether forwarding the Interest in \p pitEntry to \p outFace would violate scope
+ *  \sa https://redmine.named-data.net/projects/nfd/wiki/ScopeControl
+ *  \deprecated use violatesScope(inFace, interest, outFace) instead
+ */
+DEPRECATED(
+bool
+violatesScope(const pit::Entry& pitEntry, const Face& outFace));
 
 /** \brief decide whether Interest can be forwarded to face
  *

daemon/fw/best-route-strategy.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "best-route-strategy.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 
 namespace nfd {
 namespace fw {

daemon/fw/best-route-strategy2.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "best-route-strategy2.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 #include "core/logger.hpp"
 
 namespace nfd {
@@ -47,31 +47,33 @@ BestRouteStrategy2::BestRouteStrategy2(Forwarder& forwarder, const Name& name)
 }
 
 /** \brief determines whether a NextHop is eligible
- *  \param pitEntry PIT entry
+ *  \param inFace incoming face of current Interest
+ *  \param interest incoming Interest
  *  \param nexthop next hop
- *  \param currentDownstream incoming FaceId of current Interest
+ *  \param pitEntry PIT entry
  *  \param wantUnused if true, NextHop must not have unexpired out-record
  *  \param now time::steady_clock::now(), ignored if !wantUnused
  */
 static inline bool
-predicate_NextHop_eligible(const shared_ptr<pit::Entry>& pitEntry,
-  const fib::NextHop& nexthop, FaceId currentDownstream,
-  bool wantUnused = false,
-  time::steady_clock::TimePoint now = time::steady_clock::TimePoint::min())
+isNextHopEligible(const Face& inFace, const Interest& interest,
+                  const fib::NextHop& nexthop,
+                  const shared_ptr<pit::Entry>& pitEntry,
+                  bool wantUnused = false,
+                  time::steady_clock::TimePoint now = time::steady_clock::TimePoint::min())
 {
-  Face& upstream = nexthop.getFace();
+  const Face& outFace = nexthop.getFace();
 
-  // upstream is current downstream
-  if (upstream.getId() == currentDownstream)
+  // do not forward back to the same face
+  if (&outFace == &inFace)
     return false;
 
   // forwarding would violate scope
-  if (violatesScope(*pitEntry, upstream))
+  if (wouldViolateScope(inFace, interest, outFace))
     return false;
 
   if (wantUnused) {
-    // NextHop must not have unexpired out-record
-    pit::OutRecordCollection::iterator outRecord = pitEntry->getOutRecord(upstream);
+    // nexthop must not have unexpired out-record
+    pit::OutRecordCollection::iterator outRecord = pitEntry->getOutRecord(outFace);
     if (outRecord != pitEntry->out_end() && outRecord->getExpiry() > now) {
       return false;
     }
@@ -84,14 +86,14 @@ predicate_NextHop_eligible(const shared_ptr<pit::Entry>& pitEntry,
  *  \note It is assumed that every nexthop has an out-record.
  */
 static inline fib::NextHopList::const_iterator
-findEligibleNextHopWithEarliestOutRecord(const shared_ptr<pit::Entry>& pitEntry,
+findEligibleNextHopWithEarliestOutRecord(const Face& inFace, const Interest& interest,
                                          const fib::NextHopList& nexthops,
-                                         FaceId currentDownstream)
+                                         const shared_ptr<pit::Entry>& pitEntry)
 {
   fib::NextHopList::const_iterator found = nexthops.end();
   time::steady_clock::TimePoint earliestRenewed = time::steady_clock::TimePoint::max();
   for (fib::NextHopList::const_iterator it = nexthops.begin(); it != nexthops.end(); ++it) {
-    if (!predicate_NextHop_eligible(pitEntry, *it, currentDownstream))
+    if (!isNextHopEligible(inFace, interest, *it, pitEntry))
       continue;
     pit::OutRecordCollection::iterator outRecord = pitEntry->getOutRecord(it->getFace());
     BOOST_ASSERT(outRecord != pitEntry->out_end());
@@ -121,7 +123,7 @@ BestRouteStrategy2::afterReceiveInterest(const Face& inFace, const Interest& int
   if (suppression == RetxSuppression::NEW) {
     // forward to nexthop with lowest cost except downstream
     it = std::find_if(nexthops.begin(), nexthops.end(),
-      bind(&predicate_NextHop_eligible, pitEntry, _1, inFace.getId(),
+      bind(&isNextHopEligible, cref(inFace), interest, _1, pitEntry,
            false, time::steady_clock::TimePoint::min()));
 
     if (it == nexthops.end()) {
@@ -136,7 +138,7 @@ BestRouteStrategy2::afterReceiveInterest(const Face& inFace, const Interest& int
     }
 
     Face& outFace = it->getFace();
-    this->sendInterest(pitEntry, outFace);
+    this->sendInterest(pitEntry, outFace, interest);
     NFD_LOG_DEBUG(interest << " from=" << inFace.getId()
                            << " newPitEntry-to=" << outFace.getId());
     return;
@@ -144,24 +146,24 @@ BestRouteStrategy2::afterReceiveInterest(const Face& inFace, const Interest& int
 
   // find an unused upstream with lowest cost except downstream
   it = std::find_if(nexthops.begin(), nexthops.end(),
-                    bind(&predicate_NextHop_eligible, pitEntry, _1, inFace.getId(),
+                    bind(&isNextHopEligible, cref(inFace), interest, _1, pitEntry,
                          true, time::steady_clock::now()));
   if (it != nexthops.end()) {
     Face& outFace = it->getFace();
-    this->sendInterest(pitEntry, outFace);
+    this->sendInterest(pitEntry, outFace, interest);
     NFD_LOG_DEBUG(interest << " from=" << inFace.getId()
                            << " retransmit-unused-to=" << outFace.getId());
     return;
   }
 
   // find an eligible upstream that is used earliest
-  it = findEligibleNextHopWithEarliestOutRecord(pitEntry, nexthops, inFace.getId());
+  it = findEligibleNextHopWithEarliestOutRecord(inFace, interest, nexthops, pitEntry);
   if (it == nexthops.end()) {
     NFD_LOG_DEBUG(interest << " from=" << inFace.getId() << " retransmitNoNextHop");
   }
   else {
     Face& outFace = it->getFace();
-    this->sendInterest(pitEntry, outFace);
+    this->sendInterest(pitEntry, outFace, interest);
     NFD_LOG_DEBUG(interest << " from=" << inFace.getId()
                            << " retransmit-retry-to=" << outFace.getId());
   }
@@ -226,7 +228,6 @@ BestRouteStrategy2::afterReceiveNack(const Face& inFace, const lp::Nack& nack,
     return;
   }
 
-
   NFD_LOG_DEBUG(nack.getInterest() << " nack-from=" << inFace.getId() <<
                 " nack=" << nack.getReason() <<
                 " nack-to=all out-nack=" << outNack.getReason());

daemon/fw/forwarder.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "forwarder.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 #include "core/logger.hpp"
 #include "strategy.hpp"
 #include "table/cleanup.hpp"

daemon/fw/multicast-strategy.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "multicast-strategy.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 
 namespace nfd {
 namespace fw {

daemon/fw/ncc-strategy.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "ncc-strategy.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 #include "core/random.hpp"
 
 namespace nfd {

daemon/fw/retx-suppression-exponential.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "retx-suppression-exponential.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 
 namespace nfd {
 namespace fw {

daemon/fw/retx-suppression-fixed.cpp

@@ -24,7 +24,7 @@
  */
 
 #include "retx-suppression-fixed.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 
 namespace nfd {
 namespace fw {

daemon/fw/strategy.cpp

@@ -25,7 +25,7 @@
 
 #include "strategy.hpp"
 #include "forwarder.hpp"
-#include "pit-algorithm.hpp"
+#include "algorithm.hpp"
 #include "core/logger.hpp"
 #include "core/random.hpp"
 

daemon/fw/strategy.hpp

@@ -145,10 +145,12 @@ protected: // actions
    *  \param outFace face through which to send out the Interest
    *  \param wantNewNonce if true, a new Nonce will be generated,
    *                      rather than reusing a Nonce from one of the PIT in-records
+   *  \deprecated use sendInterest(pitEntry, outFace, interest) instead
    */
+  DEPRECATED(
   void
   sendInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
-               bool wantNewNonce = false);
+               bool wantNewNonce = false));
 
   /** \brief decide that a pending Interest cannot be forwarded
    *  \param pitEntry PIT entry

docs/doxygen.conf.in

@@ -1924,11 +1924,12 @@ PREDEFINED             = DOXYGEN=1 \
                          BOOST_STATIC_ASSERT(x)= \
                          BOOST_CONCEPT_ASSERT(x)= \
                          BOOST_CONCEPT_REQUIRES(x)= \
+                         DEPRECATED(x)=x \
                          PUBLIC_WITH_TESTS_ELSE_PRIVATE=private \
                          PUBLIC_WITH_TESTS_ELSE_PROTECTED=protected \
                          PROTECTED_WITH_TESTS_ELSE_PRIVATE=private \
                          FINAL_UNLESS_WITH_TESTS=final \
-                         VIRTUAL_WITH_TESTS
+                         VIRTUAL_WITH_TESTS=
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
 # tag can be used to specify a list of macro names that should be expanded. The

tests/daemon/fw/algorithm.t.cpp

@@ -23,7 +23,7 @@
  * NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "fw/pit-algorithm.hpp"
+#include "fw/algorithm.hpp"
 
 #include "tests/test-common.hpp"
 #include "tests/daemon/face/dummy-face.hpp"
@@ -35,7 +35,7 @@ namespace tests {
 using namespace nfd::tests;
 
 BOOST_AUTO_TEST_SUITE(Fw)
-BOOST_FIXTURE_TEST_SUITE(TestPitAlgorithm, BaseFixture)
+BOOST_FIXTURE_TEST_SUITE(TestAlgorithm, BaseFixture)
 
 class ScopeControlFixture : public BaseFixture
 {
@@ -55,6 +55,40 @@ protected:
   shared_ptr<Face> localFace4;
 };
 
+BOOST_FIXTURE_TEST_SUITE(WouldViolateScope, ScopeControlFixture)
+
+BOOST_AUTO_TEST_CASE(Unrestricted)
+{
+  shared_ptr<Interest> interest = makeInterest("ndn:/ieWRzDsCu");
+
+  BOOST_CHECK_EQUAL(wouldViolateScope(*nonLocalFace1, *interest, *nonLocalFace2), false);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*nonLocalFace1, *interest, *localFace4), false);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*localFace3, *interest, *nonLocalFace2), false);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*localFace3, *interest, *localFace4), false);
+}
+
+BOOST_AUTO_TEST_CASE(Localhost)
+{
+  shared_ptr<Interest> interest = makeInterest("ndn:/localhost/5n1LzIt3");
+
+  // /localhost Interests from non-local faces should be rejected by incoming Interest pipeline,
+  // and are not tested here.
+  BOOST_CHECK_EQUAL(wouldViolateScope(*localFace3, *interest, *nonLocalFace2), true);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*localFace3, *interest, *localFace4), false);
+}
+
+BOOST_AUTO_TEST_CASE(Localhop)
+{
+  shared_ptr<Interest> interest = makeInterest("ndn:/localhop/YcIKWCRYJ");
+
+  BOOST_CHECK_EQUAL(wouldViolateScope(*nonLocalFace1, *interest, *nonLocalFace2), true);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*nonLocalFace1, *interest, *localFace4), false);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*localFace3, *interest, *nonLocalFace2), false);
+  BOOST_CHECK_EQUAL(wouldViolateScope(*localFace3, *interest, *localFace4), false);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // WouldViolateScope
+
 BOOST_FIXTURE_TEST_SUITE(ViolatesScope, ScopeControlFixture)
 
 BOOST_AUTO_TEST_CASE(Unrestricted)