fw: NccStrategy remembers only best face
refs #1961
Strategy::beforeSatisfyPendingInterest is renamed to
Strategy::beforeSatisfyInterest, so as to match the semantics of this trigger.
This is a backwards-incompatible change that requires function renaming in
all subclasses of Strategy class.
LimitedIo::afterOp gets a bugfix that allows it to be invoked out of
LimitedIo::run without leaving io_service unusable.
Change-Id: Ia4c632beb62c554724b83846a3d6358ae08779ad
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index dbfffa4..1b677be 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -274,7 +274,7 @@
}
// invoke PIT satisfy callback
- this->dispatchToStrategy(pitEntry, bind(&Strategy::beforeSatisfyPendingInterest, _1,
+ this->dispatchToStrategy(pitEntry, bind(&Strategy::beforeSatisfyInterest, _1,
pitEntry, cref(inFace), cref(data)));
// mark PIT satisfied
diff --git a/daemon/fw/ncc-strategy.cpp b/daemon/fw/ncc-strategy.cpp
index fcabe7a..925a3cf 100644
--- a/daemon/fw/ncc-strategy.cpp
+++ b/daemon/fw/ncc-strategy.cpp
@@ -183,9 +183,15 @@
}
void
-NccStrategy::beforeSatisfyPendingInterest(shared_ptr<pit::Entry> pitEntry,
- const Face& inFace, const Data& data)
+NccStrategy::beforeSatisfyInterest(shared_ptr<pit::Entry> pitEntry,
+ const Face& inFace, const Data& data)
{
+ if (pitEntry->getInRecords().empty()) {
+ // PIT entry has already been satisfied (and is now waiting for straggler timer to expire)
+ // NCC does not collect measurements for non-best face
+ return;
+ }
+
shared_ptr<measurements::Entry> measurementsEntry = this->getMeasurements().get(*pitEntry);
for (int i = 0; i < UPDATE_MEASUREMENTS_N_LEVELS; ++i) {
diff --git a/daemon/fw/ncc-strategy.hpp b/daemon/fw/ncc-strategy.hpp
index 6c284a9..0f5ddac 100644
--- a/daemon/fw/ncc-strategy.hpp
+++ b/daemon/fw/ncc-strategy.hpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#ifndef NFD_DAEMON_FW_NCC_STRATEGY_HPP
#define NFD_DAEMON_FW_NCC_STRATEGY_HPP
@@ -47,8 +48,8 @@
shared_ptr<pit::Entry> pitEntry);
virtual void
- beforeSatisfyPendingInterest(shared_ptr<pit::Entry> pitEntry,
- const Face& inFace, const Data& data);
+ beforeSatisfyInterest(shared_ptr<pit::Entry> pitEntry,
+ const Face& inFace, const Data& data);
protected:
/// StrategyInfo on measurements::Entry
diff --git a/daemon/fw/strategy.cpp b/daemon/fw/strategy.cpp
index fd6ac5f..96f132f 100644
--- a/daemon/fw/strategy.cpp
+++ b/daemon/fw/strategy.cpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#include "strategy.hpp"
#include "forwarder.hpp"
@@ -44,10 +45,10 @@
}
void
-Strategy::beforeSatisfyPendingInterest(shared_ptr<pit::Entry> pitEntry,
- const Face& inFace, const Data& data)
+Strategy::beforeSatisfyInterest(shared_ptr<pit::Entry> pitEntry,
+ const Face& inFace, const Data& data)
{
- NFD_LOG_DEBUG("beforeSatisfyPendingInterest pitEntry=" << pitEntry->getName() <<
+ NFD_LOG_DEBUG("beforeSatisfyInterest pitEntry=" << pitEntry->getName() <<
" inFace=" << inFace.getId() << " data=" << data.getName());
}
diff --git a/daemon/fw/strategy.hpp b/daemon/fw/strategy.hpp
index d690b0f..f910383 100644
--- a/daemon/fw/strategy.hpp
+++ b/daemon/fw/strategy.hpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#ifndef NFD_DAEMON_FW_STRATEGY_HPP
#define NFD_DAEMON_FW_STRATEGY_HPP
@@ -59,20 +60,32 @@
* invoke this->sendInterest one or more times, either now or shortly after
* - If strategy concludes that this Interest cannot be forwarded,
* invoke this->rejectPendingInterest so that PIT entry will be deleted shortly
+ *
+ * \note The strategy is permitted to store a weak reference to fibEntry.
+ * Do not store a shared reference, because PIT entry may be deleted at any moment.
+ * fibEntry is passed by value to allow obtaining a weak reference from it.
+ * \note The strategy is permitted to store a shared reference to pitEntry.
+ * pitEntry is passed by value to reflect this fact.
*/
virtual void
afterReceiveInterest(const Face& inFace,
const Interest& interest,
shared_ptr<fib::Entry> fibEntry,
- shared_ptr<pit::Entry> pitEntry) =0;
+ shared_ptr<pit::Entry> pitEntry) = 0;
/** \brief trigger before PIT entry is satisfied
*
+ * This trigger is invoked when an incoming Data satisfies the PIT entry.
+ * It can be invoked even if the PIT entry has already been satisfied.
+ *
* In this base class this method does nothing.
+ *
+ * \note The strategy is permitted to store a shared reference to pitEntry.
+ * pitEntry is passed by value to reflect this fact.
*/
virtual void
- beforeSatisfyPendingInterest(shared_ptr<pit::Entry> pitEntry,
- const Face& inFace, const Data& data);
+ beforeSatisfyInterest(shared_ptr<pit::Entry> pitEntry,
+ const Face& inFace, const Data& data);
/** \brief trigger before PIT entry expires
*
@@ -82,33 +95,13 @@
* This trigger is not invoked for PIT entry already satisfied.
*
* In this base class this method does nothing.
+ *
+ * \note The strategy is permitted to store a shared reference to pitEntry.
+ * pitEntry is passed by value to reflect this fact.
*/
virtual void
beforeExpirePendingInterest(shared_ptr<pit::Entry> pitEntry);
-// /** \brief trigger after FIB entry is being inserted
-// * and becomes managed by this strategy
-// *
-// * In this base class this method does nothing.
-// */
-// virtual void
-// afterAddFibEntry(shared_ptr<fib::Entry> fibEntry);
-//
-// /** \brief trigger after FIB entry being managed by this strategy is updated
-// *
-// * In this base class this method does nothing.
-// */
-// virtual void
-// afterUpdateFibEntry(shared_ptr<fib::Entry> fibEntry);
-//
-// /** \brief trigger before FIB entry ceises to be managed by this strategy
-// * or is being deleted
-// *
-// * In this base class this method does nothing.
-// */
-// virtual void
-// beforeRemoveFibEntry(shared_ptr<fib::Entry> fibEntry);
-
protected: // actions
/// send Interest to outFace
VIRTUAL_WITH_TESTS void
diff --git a/tests/daemon/fw/dummy-strategy.hpp b/tests/daemon/fw/dummy-strategy.hpp
index 67784fc..83af2fa 100644
--- a/tests/daemon/fw/dummy-strategy.hpp
+++ b/tests/daemon/fw/dummy-strategy.hpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,10 +21,10 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
-#ifndef NFD_TESTS_NFD_FW_DUMMY_STRATEGY_HPP
-#define NFD_TESTS_NFD_FW_DUMMY_STRATEGY_HPP
+#ifndef NFD_TESTS_DAEMON_FW_DUMMY_STRATEGY_HPP
+#define NFD_TESTS_DAEMON_FW_DUMMY_STRATEGY_HPP
#include "fw/strategy.hpp"
@@ -59,10 +60,10 @@
}
virtual void
- beforeSatisfyPendingInterest(shared_ptr<pit::Entry> pitEntry,
- const Face& inFace, const Data& data)
+ beforeSatisfyInterest(shared_ptr<pit::Entry> pitEntry,
+ const Face& inFace, const Data& data)
{
- ++m_beforeSatisfyPendingInterest_count;
+ ++m_beforeSatisfyInterest_count;
}
virtual void
@@ -73,7 +74,7 @@
public:
int m_afterReceiveInterest_count;
- int m_beforeSatisfyPendingInterest_count;
+ int m_beforeSatisfyInterest_count;
int m_beforeExpirePendingInterest_count;
/// outFace to use in afterReceiveInterest, nullptr to reject
@@ -83,4 +84,4 @@
} // namespace tests
} // namespace nfd
-#endif // NFD_TESTS_NFD_FW_DUMMY_STRATEGY_HPP
+#endif // NFD_TESTS_DAEMON_FW_DUMMY_STRATEGY_HPP
diff --git a/tests/daemon/fw/forwarder.cpp b/tests/daemon/fw/forwarder.cpp
index f7cea1c..6856d8f 100644
--- a/tests/daemon/fw/forwarder.cpp
+++ b/tests/daemon/fw/forwarder.cpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#include "fw/forwarder.hpp"
#include "tests/daemon/face/dummy-face.hpp"
@@ -364,14 +365,14 @@
limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(5));
shared_ptr<Data> data1 = makeData("ndn:/A/1/a");
- strategyP->m_beforeSatisfyPendingInterest_count = 0;
+ strategyP->m_beforeSatisfyInterest_count = 0;
forwarder.onData(*face2, *data1);
- BOOST_CHECK_EQUAL(strategyP->m_beforeSatisfyPendingInterest_count, 1);
+ BOOST_CHECK_EQUAL(strategyP->m_beforeSatisfyInterest_count, 1);
shared_ptr<Data> data2 = makeData("ndn:/B/2/b");
- strategyQ->m_beforeSatisfyPendingInterest_count = 0;
+ strategyQ->m_beforeSatisfyInterest_count = 0;
forwarder.onData(*face2, *data2);
- BOOST_CHECK_EQUAL(strategyQ->m_beforeSatisfyPendingInterest_count, 1);
+ BOOST_CHECK_EQUAL(strategyQ->m_beforeSatisfyInterest_count, 1);
shared_ptr<Interest> interest3 = makeInterest("ndn:/A/3");
interest3->setInterestLifetime(time::milliseconds(30));
diff --git a/tests/daemon/fw/ncc-strategy.cpp b/tests/daemon/fw/ncc-strategy.cpp
index a6229f5..d8e1ef1 100644
--- a/tests/daemon/fw/ncc-strategy.cpp
+++ b/tests/daemon/fw/ncc-strategy.cpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#include "fw/ncc-strategy.hpp"
#include "strategy-tester.hpp"
@@ -86,7 +87,7 @@
// face2 responds
shared_ptr<Data> data1p = makeData("ndn:/0Jm1ajrW/%00");
Data& data1 = *data1p;
- strategy->beforeSatisfyPendingInterest(pitEntry1, *face2, data1);
+ strategy->beforeSatisfyInterest(pitEntry1, *face2, data1);
limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(500));
// second Interest: strategy knows face2 is best
@@ -143,7 +144,7 @@
// face1 responds
shared_ptr<Data> data1 = makeData("ndn:/nztwIvHX/%00");
- strategy->beforeSatisfyPendingInterest(pitEntry1, *face1, *data1);
+ strategy->beforeSatisfyInterest(pitEntry1, *face1, *data1);
limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(500));
// second Interest: bestFace is face1, nUpstreams becomes 0,
@@ -160,6 +161,65 @@
limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(1000));// should not crash
}
+BOOST_AUTO_TEST_CASE(Bug1961)
+{
+ LimitedIo limitedIo;
+ Forwarder forwarder;
+ typedef StrategyTester<fw::NccStrategy> NccStrategyTester;
+ shared_ptr<NccStrategyTester> strategy = make_shared<NccStrategyTester>(ref(forwarder));
+ strategy->onAction += bind(&LimitedIo::afterOp, &limitedIo);
+
+ shared_ptr<DummyFace> face1 = make_shared<DummyFace>();
+ shared_ptr<DummyFace> face2 = make_shared<DummyFace>();
+ shared_ptr<DummyFace> face3 = make_shared<DummyFace>();
+ forwarder.addFace(face1);
+ forwarder.addFace(face2);
+ forwarder.addFace(face3);
+
+ Fib& fib = forwarder.getFib();
+ shared_ptr<fib::Entry> fibEntry = fib.insert(Name()).first;
+ fibEntry->addNextHop(face1, 10);
+ fibEntry->addNextHop(face2, 20);
+
+ StrategyChoice& strategyChoice = forwarder.getStrategyChoice();
+ strategyChoice.install(strategy);
+ strategyChoice.insert(Name(), strategy->getName());
+
+ Pit& pit = forwarder.getPit();
+
+ // first Interest: strategy forwards to face1 and face2
+ shared_ptr<Interest> interest1 = makeInterest("ndn:/seRMz5a6/%00");
+ interest1->setInterestLifetime(time::milliseconds(2000));
+ shared_ptr<pit::Entry> pitEntry1 = pit.insert(*interest1).first;
+
+ pitEntry1->insertOrUpdateInRecord(face3, *interest1);
+ strategy->afterReceiveInterest(*face3, *interest1, fibEntry, pitEntry1);
+ limitedIo.run(2 - strategy->m_sendInterestHistory.size(), time::milliseconds(2000));
+ BOOST_REQUIRE_EQUAL(strategy->m_sendInterestHistory.size(), 2);
+ BOOST_CHECK_EQUAL(strategy->m_sendInterestHistory[0].get<1>(), face1);
+ BOOST_CHECK_EQUAL(strategy->m_sendInterestHistory[1].get<1>(), face2);
+
+ // face1 responds
+ shared_ptr<Data> data1 = makeData("ndn:/seRMz5a6/%00");
+ strategy->beforeSatisfyInterest(pitEntry1, *face1, *data1);
+ pitEntry1->deleteInRecords();
+ limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(10));
+ // face2 also responds
+ strategy->beforeSatisfyInterest(pitEntry1, *face2, *data1);
+ limitedIo.run(LimitedIo::UNLIMITED_OPS, time::milliseconds(10));
+
+ // second Interest: bestFace should be face 1
+ shared_ptr<Interest> interest2 = makeInterest("ndn:/seRMz5a6/%01");
+ interest2->setInterestLifetime(time::milliseconds(2000));
+ shared_ptr<pit::Entry> pitEntry2 = pit.insert(*interest2).first;
+
+ pitEntry2->insertOrUpdateInRecord(face3, *interest2);
+ strategy->afterReceiveInterest(*face3, *interest2, fibEntry, pitEntry2);
+ limitedIo.run(3 - strategy->m_sendInterestHistory.size(), time::milliseconds(2000));
+
+ BOOST_REQUIRE_GE(strategy->m_sendInterestHistory.size(), 3);
+ BOOST_CHECK_EQUAL(strategy->m_sendInterestHistory[2].get<1>(), face1);
+}
BOOST_AUTO_TEST_SUITE_END()
diff --git a/tests/daemon/fw/strategy-tester.hpp b/tests/daemon/fw/strategy-tester.hpp
index 261c1a9..260a196 100644
--- a/tests/daemon/fw/strategy-tester.hpp
+++ b/tests/daemon/fw/strategy-tester.hpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#ifndef NFD_TESTS_NFD_FW_STRATEGY_TESTER_HPP
#define NFD_TESTS_NFD_FW_STRATEGY_TESTER_HPP
diff --git a/tests/limited-io.cpp b/tests/limited-io.cpp
index b22be89..9df197c 100644
--- a/tests/limited-io.cpp
+++ b/tests/limited-io.cpp
@@ -1,11 +1,12 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014 Regents of the University of California,
- * Arizona Board of Regents,
- * Colorado State University,
- * University Pierre & Marie Curie, Sorbonne University,
- * Washington University in St. Louis,
- * Beijing Institute of Technology
+ * Copyright (c) 2014, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis
*
* This file is part of NFD (Named Data Networking Forwarding Daemon).
* See AUTHORS.md for complete list of NFD authors and contributors.
@@ -20,7 +21,7 @@
*
* You should have received a copy of the GNU General Public License along with
* NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
- **/
+ */
#include "limited-io.hpp"
#include "core/logger.hpp"
@@ -43,6 +44,11 @@
LimitedIo::run(int nOpsLimit, const time::nanoseconds& nTimeLimit)
{
BOOST_ASSERT(!m_isRunning);
+
+ if (nOpsLimit <= 0) {
+ return EXCEED_OPS;
+ }
+
m_isRunning = true;
m_reason = NO_WORK;
@@ -69,6 +75,12 @@
void
LimitedIo::afterOp()
{
+ if (!m_isRunning) {
+ // Do not proceed further if .afterOp() is invoked out of .run(),
+ // because io_service.stop() without io_service.reset() would leave it unusable.
+ return;
+ }
+
--m_nOpsRemaining;
if (m_nOpsRemaining <= 0) {
m_reason = EXCEED_OPS;