fw: localhop scope restriction in best-route v1, multicast, ncc strategies
canForwardToLegacy function no longer checks scope. Strategies using this
function shall use wouldViolateScope separately.
refs #3841, #1756
Change-Id: Ib1bd3f9fe19e4cadfa27e86dcc730d16cfd4b0db
diff --git a/daemon/fw/algorithm.cpp b/daemon/fw/algorithm.cpp
index 7a1f2cc..0eb9cce 100644
--- a/daemon/fw/algorithm.cpp
+++ b/daemon/fw/algorithm.cpp
@@ -99,7 +99,7 @@
return false;
}
- return !violatesScope(pitEntry, face);
+ return true;
}
int
diff --git a/daemon/fw/algorithm.hpp b/daemon/fw/algorithm.hpp
index ebf493a..88ad0b4 100644
--- a/daemon/fw/algorithm.hpp
+++ b/daemon/fw/algorithm.hpp
@@ -84,8 +84,7 @@
/** \brief decide whether Interest can be forwarded to face
*
* \return true if out-record of this face does not exist or has expired,
- * and there is an in-record not of this face,
- * and scope is not violated
+ * and there is an in-record not of this face
*
* \note This algorithm has a weakness that it does not permit consumer retransmissions
* before out-record expires. Therefore, it's not recommended to use this function
diff --git a/daemon/fw/best-route-strategy.cpp b/daemon/fw/best-route-strategy.cpp
index ebe0d08..a8877e1 100644
--- a/daemon/fw/best-route-strategy.cpp
+++ b/daemon/fw/best-route-strategy.cpp
@@ -48,16 +48,17 @@
const fib::Entry& fibEntry = this->lookupFib(*pitEntry);
const fib::NextHopList& nexthops = fibEntry.getNextHops();
- fib::NextHopList::const_iterator it = std::find_if(nexthops.begin(), nexthops.end(),
- [&pitEntry] (const fib::NextHop& nexthop) { return canForwardToLegacy(*pitEntry, nexthop.getFace()); });
- if (it == nexthops.end()) {
- this->rejectPendingInterest(pitEntry);
- return;
+ for (fib::NextHopList::const_iterator it = nexthops.begin(); it != nexthops.end(); ++it) {
+ Face& outFace = it->getFace();
+ if (!wouldViolateScope(inFace, interest, outFace) &&
+ canForwardToLegacy(*pitEntry, outFace)) {
+ this->sendInterest(pitEntry, outFace, interest);
+ return;
+ }
}
- Face& outFace = it->getFace();
- this->sendInterest(pitEntry, outFace);
+ this->rejectPendingInterest(pitEntry);
}
} // namespace fw
diff --git a/daemon/fw/multicast-strategy.cpp b/daemon/fw/multicast-strategy.cpp
index 2821d71..3d6e8b2 100644
--- a/daemon/fw/multicast-strategy.cpp
+++ b/daemon/fw/multicast-strategy.cpp
@@ -46,8 +46,9 @@
for (fib::NextHopList::const_iterator it = nexthops.begin(); it != nexthops.end(); ++it) {
Face& outFace = it->getFace();
- if (canForwardToLegacy(*pitEntry, outFace)) {
- this->sendInterest(pitEntry, outFace);
+ if (!wouldViolateScope(inFace, interest, outFace) &&
+ canForwardToLegacy(*pitEntry, outFace)) {
+ this->sendInterest(pitEntry, outFace, interest);
}
}
diff --git a/daemon/fw/ncc-strategy.cpp b/daemon/fw/ncc-strategy.cpp
index 2ae2fd0..650182c 100644
--- a/daemon/fw/ncc-strategy.cpp
+++ b/daemon/fw/ncc-strategy.cpp
@@ -67,12 +67,13 @@
shared_ptr<Face> bestFace = meInfo.getBestFace();
if (bestFace != nullptr && fibEntry.hasNextHop(*bestFace) &&
+ !wouldViolateScope(inFace, interest, *bestFace) &&
canForwardToLegacy(*pitEntry, *bestFace)) {
// TODO Should we use `randlow = 100 + nrand48(h->seed) % 4096U;` ?
deferFirst = meInfo.prediction;
deferRange = time::microseconds((deferFirst.count() + 1) / 2);
--nUpstreams;
- this->sendInterest(pitEntry, *bestFace);
+ this->sendInterest(pitEntry, *bestFace, interest);
pitEntryInfo->bestFaceTimeout = scheduler::schedule(
meInfo.prediction,
bind(&NccStrategy::timeoutOnBestFace, this, weak_ptr<pit::Entry>(pitEntry)));
@@ -80,16 +81,23 @@
else {
// use first eligible nexthop
auto firstEligibleNexthop = std::find_if(nexthops.begin(), nexthops.end(),
- [&pitEntry] (const fib::NextHop& nexthop) {
- return canForwardToLegacy(*pitEntry, nexthop.getFace());
+ [&] (const fib::NextHop& nexthop) {
+ Face& outFace = nexthop.getFace();
+ return !wouldViolateScope(inFace, interest, outFace) &&
+ canForwardToLegacy(*pitEntry, outFace);
});
if (firstEligibleNexthop != nexthops.end()) {
- this->sendInterest(pitEntry, firstEligibleNexthop->getFace());
+ this->sendInterest(pitEntry, firstEligibleNexthop->getFace(), interest);
+ }
+ else {
+ this->rejectPendingInterest(pitEntry);
+ return;
}
}
shared_ptr<Face> previousFace = meInfo.previousFace.lock();
if (previousFace != nullptr && fibEntry.hasNextHop(*previousFace) &&
+ !wouldViolateScope(inFace, interest, *previousFace) &&
canForwardToLegacy(*pitEntry, *previousFace)) {
--nUpstreams;
}
@@ -105,16 +113,25 @@
pitEntryInfo->maxInterval = deferFirst;
}
pitEntryInfo->propagateTimer = scheduler::schedule(deferFirst,
- bind(&NccStrategy::doPropagate, this, weak_ptr<pit::Entry>(pitEntry)));
+ bind(&NccStrategy::doPropagate, this, inFace.getId(), weak_ptr<pit::Entry>(pitEntry)));
}
void
-NccStrategy::doPropagate(weak_ptr<pit::Entry> pitEntryWeak)
+NccStrategy::doPropagate(FaceId inFaceId, weak_ptr<pit::Entry> pitEntryWeak)
{
+ Face* inFace = this->getFace(inFaceId);
+ if (inFace == nullptr) {
+ return;
+ }
shared_ptr<pit::Entry> pitEntry = pitEntryWeak.lock();
if (pitEntry == nullptr) {
return;
}
+ pit::InRecordCollection::const_iterator inRecord = pitEntry->getInRecord(*inFace);
+ if (inRecord == pitEntry->in_end()) {
+ return;
+ }
+ const Interest& interest = inRecord->getInterest();
const fib::Entry& fibEntry = this->lookupFib(*pitEntry);
PitEntryInfo* pitEntryInfo = pitEntry->getStrategyInfo<PitEntryInfo>();
@@ -126,17 +143,19 @@
shared_ptr<Face> previousFace = meInfo.previousFace.lock();
if (previousFace != nullptr && fibEntry.hasNextHop(*previousFace) &&
+ !wouldViolateScope(*inFace, interest, *previousFace) &&
canForwardToLegacy(*pitEntry, *previousFace)) {
- this->sendInterest(pitEntry, *previousFace);
+ this->sendInterest(pitEntry, *previousFace, interest);
}
const fib::NextHopList& nexthops = fibEntry.getNextHops();
bool isForwarded = false;
for (fib::NextHopList::const_iterator it = nexthops.begin(); it != nexthops.end(); ++it) {
Face& face = it->getFace();
- if (canForwardToLegacy(*pitEntry, face)) {
+ if (!wouldViolateScope(*inFace, interest, face) &&
+ canForwardToLegacy(*pitEntry, face)) {
isForwarded = true;
- this->sendInterest(pitEntry, face);
+ this->sendInterest(pitEntry, face, interest);
break;
}
}
@@ -145,7 +164,7 @@
std::uniform_int_distribution<time::nanoseconds::rep> dist(0, pitEntryInfo->maxInterval.count() - 1);
time::nanoseconds deferNext = time::nanoseconds(dist(getGlobalRng()));
pitEntryInfo->propagateTimer = scheduler::schedule(deferNext,
- bind(&NccStrategy::doPropagate, this, weak_ptr<pit::Entry>(pitEntry)));
+ bind(&NccStrategy::doPropagate, this, inFaceId, weak_ptr<pit::Entry>(pitEntry)));
}
}
diff --git a/daemon/fw/ncc-strategy.hpp b/daemon/fw/ncc-strategy.hpp
index 5043ee3..9403dd5 100644
--- a/daemon/fw/ncc-strategy.hpp
+++ b/daemon/fw/ncc-strategy.hpp
@@ -122,7 +122,7 @@
/// propagate to another upstream
void
- doPropagate(weak_ptr<pit::Entry> pitEntryWeak);
+ doPropagate(FaceId inFaceId, weak_ptr<pit::Entry> pitEntryWeak);
/// best face did not reply within prediction
void
diff --git a/tests/daemon/fw/strategy-scope-control.t.cpp b/tests/daemon/fw/strategy-scope-control.t.cpp
index b27fdcd..8694322 100644
--- a/tests/daemon/fw/strategy-scope-control.t.cpp
+++ b/tests/daemon/fw/strategy-scope-control.t.cpp
@@ -29,10 +29,14 @@
// Strategies implementing namespace-based scope control, sorted alphabetically.
#include "fw/access-strategy.hpp"
+#include "fw/best-route-strategy.hpp"
#include "fw/best-route-strategy2.hpp"
+#include "fw/multicast-strategy.hpp"
+#include "fw/ncc-strategy.hpp"
#include "tests/test-common.hpp"
#include "tests/limited-io.hpp"
+#include "install-strategy.hpp"
#include "strategy-tester.hpp"
#include "tests/daemon/face/dummy-face.hpp"
#include <boost/mpl/copy_if.hpp>
@@ -51,7 +55,7 @@
StrategyScopeControlFixture()
: limitedIo(this)
, nStrategyActions(0)
- , strategy(forwarder)
+ , strategy(choose<StrategyTester<S>>(forwarder))
, fib(forwarder.getFib())
, pit(forwarder.getPit())
, nonLocalFace1(make_shared<DummyFace>("dummy://1", "dummy://1", ndn::nfd::FACE_SCOPE_NON_LOCAL))
@@ -82,6 +86,9 @@
BOOST_REQUIRE_EQUAL(limitedIo.run(nExpectedActions - nStrategyActions, LimitedIo::UNLIMITED_TIME),
LimitedIo::EXCEED_OPS);
}
+ // A correctly implemented strategy is required to invoke reject pending Interest action
+ // if it decides to not forward an Interest. If a test case is stuck in an endless loop within
+ // this function, check that rejectPendingInterest is invoked under proper condition.
}
public:
@@ -89,7 +96,7 @@
int nStrategyActions;
Forwarder forwarder;
- StrategyTester<S> strategy;
+ StrategyTester<S>& strategy;
Fib& fib;
Pit& pit;
@@ -102,49 +109,35 @@
BOOST_AUTO_TEST_SUITE(Fw)
BOOST_AUTO_TEST_SUITE(TestStrategyScopeControl)
-class AccessStrategyTest
+template<typename S, bool WillSendNackNoRoute, bool CanProcessNack>
+class Test
{
public:
- using S = AccessStrategy;
+ using Strategy = S;
static bool
willSendNackNoRoute()
{
- return false;
+ return WillSendNackNoRoute;
}
static bool
canProcessNack()
{
- return false;
- }
-};
-
-class BestRouteStrategy2Test
-{
-public:
- using S = BestRouteStrategy2;
-
- static bool
- willSendNackNoRoute()
- {
- return true;
- }
-
- static bool
- canProcessNack()
- {
- return true;
+ return CanProcessNack;
}
};
using Tests = boost::mpl::vector<
- AccessStrategyTest,
- BestRouteStrategy2Test
+ Test<AccessStrategy, false, false>,
+ Test<BestRouteStrategy, false, false>,
+ Test<BestRouteStrategy2, true, true>,
+ Test<MulticastStrategy, false, false>,
+ Test<NccStrategy, false, false>
>;
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhostInterestToLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhost/A").first;
fibEntry->addNextHop(*this->localFace4, 10);
@@ -162,7 +155,7 @@
}
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhostInterestToNonLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhost/A").first;
fibEntry->addNextHop(*this->nonLocalFace2, 10);
@@ -184,7 +177,7 @@
}
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhostInterestToLocalAndNonLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhost/A").first;
fibEntry->addNextHop(*this->nonLocalFace2, 10);
@@ -204,7 +197,7 @@
}
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhopInterestToNonLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhop/A").first;
fibEntry->addNextHop(*this->nonLocalFace2, 10);
@@ -226,7 +219,7 @@
}
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhopInterestToNonLocalAndLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhop/A").first;
fibEntry->addNextHop(*this->nonLocalFace2, 10);
@@ -246,7 +239,7 @@
}
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhostNackToNonLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhost/A").first;
fibEntry->addNextHop(*this->localFace4, 10);
@@ -271,7 +264,7 @@
}
BOOST_FIXTURE_TEST_CASE_TEMPLATE(LocalhopNackToNonLocal,
- T, Tests, StrategyScopeControlFixture<typename T::S>)
+ T, Tests, StrategyScopeControlFixture<typename T::Strategy>)
{
fib::Entry* fibEntry = this->fib.insert("/localhop/A").first;
fibEntry->addNextHop(*this->localFace4, 10);