fw: allow strategies to pick outgoing Interest

This commit changes outgoing Interest pipeline and Strategy API,
to give strategies an opportunity to pick an outgoing Interest that
matches the Interest table entry.
No strategy is updated in this commit.

refs #1756

Change-Id: Iffad77ef0078c437070039fca6b24a85df13bda7

daemon/fw/forwarder.cpp

@@ -26,7 +26,6 @@
 #include "forwarder.hpp"
 #include "pit-algorithm.hpp"
 #include "core/logger.hpp"
-#include "core/random.hpp"
 #include "strategy.hpp"
 #include "table/cleanup.hpp"
 #include <ndn-cxx/lp/tags.hpp>
@@ -206,8 +205,10 @@ Forwarder::onContentStoreMiss(const Face& inFace, const shared_ptr<pit::Entry>&
     // chosen NextHop face exists?
     Face* nextHopFace = m_faceTable.get(*nextHopTag);
     if (nextHopFace != nullptr) {
+      NFD_LOG_DEBUG("onContentStoreMiss interest=" << interest.getName() << " nexthop-faceid=" << nextHopFace->getId());
       // go to outgoing Interest pipeline
-      this->onOutgoingInterest(pitEntry, *nextHopFace);
+      // scope control is unnecessary, because privileged app explicitly wants to forward
+      this->onOutgoingInterest(pitEntry, *nextHopFace, interest);
     }
     return;
   }
@@ -234,50 +235,16 @@ Forwarder::onContentStoreHit(const Face& inFace, const shared_ptr<pit::Entry>& p
 }
 
 void
-Forwarder::onOutgoingInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
-                              bool wantNewNonce)
+Forwarder::onOutgoingInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace, const Interest& interest)
 {
-  if (outFace.getId() == face::INVALID_FACEID) {
-    NFD_LOG_WARN("onOutgoingInterest face=invalid interest=" << pitEntry->getName());
-    return;
-  }
   NFD_LOG_DEBUG("onOutgoingInterest face=" << outFace.getId() <<
                 " interest=" << pitEntry->getName());
 
-  // scope control
-  if (fw::violatesScope(*pitEntry, outFace)) {
-    NFD_LOG_DEBUG("onOutgoingInterest face=" << outFace.getId() <<
-                  " interest=" << pitEntry->getName() << " violates scope");
-    return;
-  }
-
-  // pick Interest
-  // The outgoing Interest picked is the last incoming Interest that does not come from outFace.
-  // If all in-records come from outFace, it's fine to pick that.
-  // This happens when there's only one in-record that comes from outFace.
-  // The legit use is for vehicular network; otherwise, strategy shouldn't send to the sole inFace.
-  pit::InRecordCollection::iterator pickedInRecord = std::max_element(
-    pitEntry->in_begin(), pitEntry->in_end(),
-    [&outFace] (const pit::InRecord& a, const pit::InRecord& b) {
-      bool isOutFaceA = &a.getFace() == &outFace;
-      bool isOutFaceB = &b.getFace() == &outFace;
-      return (isOutFaceA > isOutFaceB) ||
-             (isOutFaceA == isOutFaceB && a.getLastRenewed() < b.getLastRenewed());
-    });
-  BOOST_ASSERT(pickedInRecord != pitEntry->in_end());
-  auto interest = const_pointer_cast<Interest>(pickedInRecord->getInterest().shared_from_this());
-
-  if (wantNewNonce) {
-    interest = make_shared<Interest>(*interest);
-    static std::uniform_int_distribution<uint32_t> dist;
-    interest->setNonce(dist(getGlobalRng()));
-  }
-
   // insert out-record
-  pitEntry->insertOrUpdateOutRecord(outFace, *interest);
+  pitEntry->insertOrUpdateOutRecord(outFace, interest);
 
   // send Interest
-  outFace.sendInterest(*interest);
+  outFace.sendInterest(interest);
   ++m_counters.nOutInterests;
 }
 

daemon/fw/forwarder.hpp

@@ -187,8 +187,7 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE: // pipelines
   /** \brief Content Store miss pipeline
   */
   VIRTUAL_WITH_TESTS void
-  onContentStoreMiss(const Face& inFace, const shared_ptr<pit::Entry>& pitEntry,
-                     const Interest& interest);
+  onContentStoreMiss(const Face& inFace, const shared_ptr<pit::Entry>& pitEntry, const Interest& interest);
 
   /** \brief Content Store hit pipeline
   */
@@ -199,8 +198,7 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE: // pipelines
   /** \brief outgoing Interest pipeline
    */
   VIRTUAL_WITH_TESTS void
-  onOutgoingInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
-                     bool wantNewNonce = false);
+  onOutgoingInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace, const Interest& interest);
 
   /** \brief Interest reject pipeline
    */

daemon/fw/strategy.cpp

@@ -25,7 +25,9 @@
 
 #include "strategy.hpp"
 #include "forwarder.hpp"
+#include "pit-algorithm.hpp"
 #include "core/logger.hpp"
+#include "core/random.hpp"
 
 namespace nfd {
 namespace fw {
@@ -65,6 +67,42 @@ Strategy::afterReceiveNack(const Face& inFace, const lp::Nack& nack,
                 " pitEntry=" << pitEntry->getName());
 }
 
+void
+Strategy::sendInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
+                       bool wantNewNonce)
+{
+  // scope control
+  if (fw::violatesScope(*pitEntry, outFace)) {
+    NFD_LOG_DEBUG("sendInterestLegacy face=" << outFace.getId() <<
+                  " interest=" << pitEntry->getName() << " violates scope");
+    return;
+  }
+
+  // pick Interest
+  // The outgoing Interest picked is the last incoming Interest that does not come from outFace.
+  // If all in-records come from outFace, it's fine to pick that.
+  // This happens when there's only one in-record that comes from outFace.
+  // The legit use is for vehicular network; otherwise, strategy shouldn't send to the sole inFace.
+  pit::InRecordCollection::iterator pickedInRecord = std::max_element(
+    pitEntry->in_begin(), pitEntry->in_end(),
+    [&outFace] (const pit::InRecord& a, const pit::InRecord& b) {
+      bool isOutFaceA = &a.getFace() == &outFace;
+      bool isOutFaceB = &b.getFace() == &outFace;
+      return (isOutFaceA > isOutFaceB) ||
+             (isOutFaceA == isOutFaceB && a.getLastRenewed() < b.getLastRenewed());
+    });
+  BOOST_ASSERT(pickedInRecord != pitEntry->in_end());
+  auto interest = const_pointer_cast<Interest>(pickedInRecord->getInterest().shared_from_this());
+
+  if (wantNewNonce) {
+    interest = make_shared<Interest>(*interest);
+    static std::uniform_int_distribution<uint32_t> dist;
+    interest->setNonce(dist(getGlobalRng()));
+  }
+
+  this->sendInterest(pitEntry, outFace, *interest);
+}
+
 void
 Strategy::sendNacks(const shared_ptr<pit::Entry>& pitEntry, const lp::NackHeader& header,
                     std::initializer_list<const Face*> exceptFaces)

daemon/fw/strategy.hpp

@@ -131,16 +131,25 @@ protected: // actions
   /** \brief send Interest to outFace
    *  \param pitEntry PIT entry
    *  \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
+   *  \param interest the Interest packet
    */
   VIRTUAL_WITH_TESTS void
   sendInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
-               bool wantNewNonce = false)
+               const Interest& interest)
   {
-    m_forwarder.onOutgoingInterest(pitEntry, outFace, wantNewNonce);
+    m_forwarder.onOutgoingInterest(pitEntry, outFace, interest);
   }
 
+  /** \brief send Interest to outFace
+   *  \param pitEntry PIT entry
+   *  \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
+   */
+  void
+  sendInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
+               bool wantNewNonce = false);
+
   /** \brief decide that a pending Interest cannot be forwarded
    *  \param pitEntry PIT entry
    *

tests/daemon/fw/forwarder.t.cpp

@@ -121,53 +121,25 @@ BOOST_AUTO_TEST_CASE(OutgoingInterest)
   Forwarder forwarder;
   auto face1 = make_shared<DummyFace>();
   auto face2 = make_shared<DummyFace>();
-  auto face3 = make_shared<DummyFace>();
   forwarder.addFace(face1);
   forwarder.addFace(face2);
-  forwarder.addFace(face3);
 
   Pit& pit = forwarder.getPit();
   auto interestA1 = makeInterest("/A");
   interestA1->setNonce(8378);
   shared_ptr<pit::Entry> pitA = pit.insert(*interestA1).first;
-  pit::InRecordCollection::iterator inA1 = pitA->insertOrUpdateInRecord(*face1, *interestA1);
+  pitA->insertOrUpdateInRecord(*face1, *interestA1);
 
-  // there is only one downstream, interestA1 is used
-  forwarder.onOutgoingInterest(pitA, *face3, false);
-  BOOST_REQUIRE_EQUAL(face3->sentInterests.size(), 1);
-  BOOST_CHECK_EQUAL(face3->sentInterests.back().getNonce(), 8378);
-
-  // new Nonce is requested
-  forwarder.onOutgoingInterest(pitA, *face3, true);
-  BOOST_REQUIRE_EQUAL(face3->sentInterests.size(), 2);
-  BOOST_CHECK_NE(face3->sentInterests.back().getNonce(), 8378);
-  // Nonce on in-record Interest shouldn't be touched
-  BOOST_CHECK_EQUAL(inA1->getInterest().getNonce(), 8378);
-
-  // outgoing face is face1, interestA1 is still used because there's no other choice
-  forwarder.onOutgoingInterest(pitA, *face1, false);
-  BOOST_REQUIRE_EQUAL(face1->sentInterests.size(), 1);
-  BOOST_CHECK_EQUAL(face1->sentInterests.back().getNonce(), 8378);
-
-  this->advanceClocks(time::seconds(2));
   auto interestA2 = makeInterest("/A");
-  interestA2->setNonce(9102);
-  pitA->insertOrUpdateInRecord(*face2, *interestA2);
+  interestA2->setNonce(1698);
+  forwarder.onOutgoingInterest(pitA, *face2, *interestA2);
 
-  // there are two downstreams, prefer newer interestA2
-  forwarder.onOutgoingInterest(pitA, *face3, false);
-  BOOST_REQUIRE_EQUAL(face3->sentInterests.size(), 3);
-  BOOST_CHECK_EQUAL(face3->sentInterests.back().getNonce(), 9102);
+  pit::OutRecordCollection::iterator outA2 = pitA->getOutRecord(*face2);
+  BOOST_REQUIRE(outA2 != pitA->out_end());
+  BOOST_CHECK_EQUAL(outA2->getLastNonce(), 1698);
 
-  // outgoing face is face2, prefer interestA1 from face1 despite it's older
-  forwarder.onOutgoingInterest(pitA, *face2, false);
   BOOST_REQUIRE_EQUAL(face2->sentInterests.size(), 1);
-  BOOST_CHECK_EQUAL(face2->sentInterests.back().getNonce(), 8378);
-
-  // outgoing face is face1, prefer interestA2
-  forwarder.onOutgoingInterest(pitA, *face1, false);
-  BOOST_REQUIRE_EQUAL(face1->sentInterests.size(), 2);
-  BOOST_CHECK_EQUAL(face1->sentInterests.back().getNonce(), 9102);
+  BOOST_CHECK_EQUAL(face2->sentInterests.back().getNonce(), 1698);
 }
 
 BOOST_AUTO_TEST_CASE(NextHopFaceId)
@@ -276,134 +248,6 @@ BOOST_AUTO_TEST_CASE(ScopeLocalhostIncoming)
   BOOST_CHECK_EQUAL(forwarder.onDataUnsolicited_count, 1);
 }
 
-BOOST_AUTO_TEST_CASE(ScopeLocalhostOutgoing)
-{
-  Forwarder forwarder;
-  auto face1 = make_shared<DummyFace>("dummy://", "dummy://", ndn::nfd::FACE_SCOPE_LOCAL);
-  auto face2 = make_shared<DummyFace>();
-  auto face3 = make_shared<DummyFace>("dummy://", "dummy://", ndn::nfd::FACE_SCOPE_LOCAL);
-  forwarder.addFace(face1);
-  forwarder.addFace(face2);
-  forwarder.addFace(face3);
-  Pit& pit = forwarder.getPit();
-
-  // local face, /localhost: OK
-  shared_ptr<Interest> interestA1 = makeInterest("/localhost/A1");
-  shared_ptr<pit::Entry> pitA1 = pit.insert(*interestA1).first;
-  pitA1->insertOrUpdateInRecord(*face3, *interestA1);
-  face1->sentInterests.clear();
-  forwarder.onOutgoingInterest(pitA1, *face1);
-  BOOST_CHECK_EQUAL(face1->sentInterests.size(), 1);
-
-  // non-local face, /localhost: violate
-  shared_ptr<Interest> interestA2 = makeInterest("/localhost/A2");
-  shared_ptr<pit::Entry> pitA2 = pit.insert(*interestA2).first;
-  pitA2->insertOrUpdateInRecord(*face3, *interestA2);
-  face2->sentInterests.clear();
-  forwarder.onOutgoingInterest(pitA2, *face2);
-  BOOST_CHECK_EQUAL(face2->sentInterests.size(), 0);
-
-  // local face, non-/localhost: OK
-  shared_ptr<Interest> interestA3 = makeInterest("/A3");
-  shared_ptr<pit::Entry> pitA3 = pit.insert(*interestA3).first;
-  pitA3->insertOrUpdateInRecord(*face3, *interestA3);
-  face1->sentInterests.clear();
-  forwarder.onOutgoingInterest(pitA3, *face1);
-  BOOST_CHECK_EQUAL(face1->sentInterests.size(), 1);
-
-  // non-local face, non-/localhost: OK
-  shared_ptr<Interest> interestA4 = makeInterest("/A4");
-  shared_ptr<pit::Entry> pitA4 = pit.insert(*interestA4).first;
-  pitA4->insertOrUpdateInRecord(*face3, *interestA4);
-  face2->sentInterests.clear();
-  forwarder.onOutgoingInterest(pitA4, *face2);
-  BOOST_CHECK_EQUAL(face2->sentInterests.size(), 1);
-
-  // local face, /localhost: OK
-  face1->sentData.clear();
-  forwarder.onOutgoingData(Data("/localhost/B1"), *face1);
-  BOOST_CHECK_EQUAL(face1->sentData.size(), 1);
-
-  // non-local face, /localhost: OK
-  face2->sentData.clear();
-  forwarder.onOutgoingData(Data("/localhost/B2"), *face2);
-  BOOST_CHECK_EQUAL(face2->sentData.size(), 0);
-
-  // local face, non-/localhost: OK
-  face1->sentData.clear();
-  forwarder.onOutgoingData(Data("/B3"), *face1);
-  BOOST_CHECK_EQUAL(face1->sentData.size(), 1);
-
-  // non-local face, non-/localhost: OK
-  face2->sentData.clear();
-  forwarder.onOutgoingData(Data("/B4"), *face2);
-  BOOST_CHECK_EQUAL(face2->sentData.size(), 1);
-}
-
-BOOST_AUTO_TEST_CASE(ScopeLocalhopOutgoing)
-{
-  Forwarder forwarder;
-  auto face1 = make_shared<DummyFace>("dummy://", "dummy://", ndn::nfd::FACE_SCOPE_LOCAL);
-  auto face2 = make_shared<DummyFace>();
-  auto face3 = make_shared<DummyFace>("dummy://", "dummy://", ndn::nfd::FACE_SCOPE_LOCAL);
-  auto face4 = make_shared<DummyFace>();
-  forwarder.addFace(face1);
-  forwarder.addFace(face2);
-  forwarder.addFace(face3);
-  forwarder.addFace(face4);
-  Pit& pit = forwarder.getPit();
-
-  // from local face, to local face: OK
-  shared_ptr<Interest> interest1 = makeInterest("/localhop/1");
-  shared_ptr<pit::Entry> pit1 = pit.insert(*interest1).first;
-  pit1->insertOrUpdateInRecord(*face1, *interest1);
-  face3->sentInterests.clear();
-  forwarder.onOutgoingInterest(pit1, *face3);
-  BOOST_CHECK_EQUAL(face3->sentInterests.size(), 1);
-
-  // from non-local face, to local face: OK
-  shared_ptr<Interest> interest2 = makeInterest("/localhop/2");
-  shared_ptr<pit::Entry> pit2 = pit.insert(*interest2).first;
-  pit2->insertOrUpdateInRecord(*face2, *interest2);
-  face3->sentInterests.clear();
-  forwarder.onOutgoingInterest(pit2, *face3);
-  BOOST_CHECK_EQUAL(face3->sentInterests.size(), 1);
-
-  // from local face, to non-local face: OK
-  shared_ptr<Interest> interest3 = makeInterest("/localhop/3");
-  shared_ptr<pit::Entry> pit3 = pit.insert(*interest3).first;
-  pit3->insertOrUpdateInRecord(*face1, *interest3);
-  face4->sentInterests.clear();
-  forwarder.onOutgoingInterest(pit3, *face4);
-  BOOST_CHECK_EQUAL(face4->sentInterests.size(), 1);
-
-  // from non-local face, to non-local face: violate
-  shared_ptr<Interest> interest4 = makeInterest("/localhop/4");
-  shared_ptr<pit::Entry> pit4 = pit.insert(*interest4).first;
-  pit4->insertOrUpdateInRecord(*face2, *interest4);
-  face4->sentInterests.clear();
-  forwarder.onOutgoingInterest(pit4, *face4);
-  BOOST_CHECK_EQUAL(face4->sentInterests.size(), 0);
-
-  // from local face and non-local face, to local face: OK
-  shared_ptr<Interest> interest5 = makeInterest("/localhop/5");
-  shared_ptr<pit::Entry> pit5 = pit.insert(*interest5).first;
-  pit5->insertOrUpdateInRecord(*face1, *interest5);
-  pit5->insertOrUpdateInRecord(*face2, *interest5);
-  face3->sentInterests.clear();
-  forwarder.onOutgoingInterest(pit5, *face3);
-  BOOST_CHECK_EQUAL(face3->sentInterests.size(), 1);
-
-  // from local face and non-local face, to non-local face: OK
-  shared_ptr<Interest> interest6 = makeInterest("/localhop/6");
-  shared_ptr<pit::Entry> pit6 = pit.insert(*interest6).first;
-  pit6->insertOrUpdateInRecord(*face1, *interest6);
-  pit6->insertOrUpdateInRecord(*face2, *interest6);
-  face4->sentInterests.clear();
-  forwarder.onOutgoingInterest(pit6, *face4);
-  BOOST_CHECK_EQUAL(face4->sentInterests.size(), 1);
-}
-
 BOOST_AUTO_TEST_CASE(IncomingInterestStrategyDispatch)
 {
   Forwarder forwarder;

tests/daemon/fw/strategy-tester.hpp

@@ -52,12 +52,11 @@ public:
 
 protected:
   virtual void
-  sendInterest(const shared_ptr<pit::Entry>& pitEntry,
-               Face& outFace,
-               bool wantNewNonce = false) override
+  sendInterest(const shared_ptr<pit::Entry>& pitEntry, Face& outFace,
+               const Interest& interest) override
   {
-    sendInterestHistory.push_back({pitEntry->getInterest(), outFace.getId(), wantNewNonce});
-    pitEntry->insertOrUpdateOutRecord(outFace, pitEntry->getInterest());
+    sendInterestHistory.push_back({pitEntry->getInterest(), outFace.getId(), interest});
+    pitEntry->insertOrUpdateOutRecord(outFace, interest);
     afterAction();
   }
 
@@ -82,7 +81,7 @@ public:
   {
     Interest pitInterest;
     FaceId outFaceId;
-    bool wantNewNonce;
+    Interest interest;
   };
   std::vector<SendInterestArgs> sendInterestHistory;