rib: Add periodic invalid face clean up
refs: #1744, #1875
Change-Id: I4e6780ef6abb95b2c4ad7c1e6291897693cb551a
diff --git a/rib/rib-manager.cpp b/rib/rib-manager.cpp
index 59e6230..14dbaf3 100644
--- a/rib/rib-manager.cpp
+++ b/rib/rib-manager.cpp
@@ -71,6 +71,8 @@
const Name RibManager::LIST_COMMAND_PREFIX("/localhost/nfd/rib/list");
const size_t RibManager::LIST_COMMAND_NCOMPS = LIST_COMMAND_PREFIX.size();
+const time::seconds RibManager::ACTIVE_FACE_FETCH_INTERVAL = time::seconds(300);
+
RibManager::RibManager(ndn::Face& face)
: m_face(face)
, m_nfdController(m_face)
@@ -89,6 +91,11 @@
{
}
+RibManager::~RibManager()
+{
+ scheduler::cancel(m_activeFaceFetchEvent);
+}
+
void
RibManager::startListening(const Name& commandPrefix, const ndn::OnInterest& onRequest)
{
@@ -121,7 +128,7 @@
m_faceMonitor.onNotification += bind(&RibManager::onNotification, this, _1);
m_faceMonitor.start();
- fetchActiveFaces();
+ scheduleActiveFaceFetch(ACTIVE_FACE_FETCH_INTERVAL);
}
void
@@ -259,20 +266,6 @@
parameters.setFaceId(request->getIncomingFaceId());
}
- // Is the face valid?
- // Issue 1852: There is no need to check (and it can easily fail) for self-registrations
- if (!isSelfRegistration && activeFaces.find(parameters.getFaceId()) == activeFaces.end())
- {
- NFD_LOG_DEBUG("register result: FAIL reason: unknown faceId");
-
- if (static_cast<bool>(request))
- {
- sendResponse(request->getName(), 410, "Face not found");
- }
-
- return;
- }
-
FaceEntry faceEntry;
faceEntry.faceId = parameters.getFaceId();
faceEntry.origin = parameters.getOrigin();
@@ -303,6 +296,7 @@
NFD_LOG_TRACE("register prefix: " << faceEntry);
m_managedRib.insert(parameters.getName(), faceEntry);
+ m_registeredFaces.insert(faceEntry.faceId);
sendUpdatesToFib(request, parameters);
}
@@ -349,20 +343,6 @@
parameters.setFaceId(request->getIncomingFaceId());
}
- // Is the face valid?
- // Issue 1852: There is no need to check (and it can easily fail) for self-registrations
- if (!isSelfRegistration && activeFaces.find(parameters.getFaceId()) == activeFaces.end())
- {
- NFD_LOG_DEBUG("register result: FAIL reason: unknown faceId");
-
- if (static_cast<bool>(request))
- {
- sendResponse(request->getName(), 410, "Face not found");
- }
-
- return;
- }
-
FaceEntry faceEntry;
faceEntry.faceId = parameters.getFaceId();
faceEntry.origin = parameters.getOrigin();
@@ -598,6 +578,9 @@
{
sendErrorResponse(code, error, request);
}
+
+ // Since the FIB rejected the update, clean up the invalid face
+ scheduleActiveFaceFetch(time::seconds(1));
}
void
@@ -653,18 +636,11 @@
void
RibManager::onNotification(const FaceEventNotification& notification)
{
- /// \todo A notification can be missed, in this case check Facelist
NFD_LOG_TRACE("onNotification: " << notification);
- if (notification.getKind() == ndn::nfd::FACE_EVENT_CREATED)
- {
- NFD_LOG_DEBUG("Received notification for created faceId: " << notification.getFaceId());
- activeFaces.insert(notification.getFaceId());
- }
- else if (notification.getKind() == ndn::nfd::FACE_EVENT_DESTROYED)
+ if (notification.getKind() == ndn::nfd::FACE_EVENT_DESTROYED)
{
NFD_LOG_DEBUG("Received notification for destroyed faceId: " << notification.getFaceId());
- activeFaces.erase(notification.getFaceId());
scheduler::schedule(time::seconds(0),
bind(&RibManager::processErasureAfterNotification, this,
@@ -676,6 +652,7 @@
RibManager::processErasureAfterNotification(uint64_t faceId)
{
m_managedRib.erase(faceId);
+ m_registeredFaces.erase(faceId);
sendUpdatesToFibAfterFaceDestroyEvent();
}
@@ -786,6 +763,15 @@
}
void
+RibManager::scheduleActiveFaceFetch(const time::seconds& timeToWait)
+{
+ scheduler::cancel(m_activeFaceFetchEvent);
+
+ m_activeFaceFetchEvent = scheduler::schedule(timeToWait,
+ bind(&RibManager::fetchActiveFaces, this));
+}
+
+void
RibManager::fetchActiveFaces()
{
NFD_LOG_DEBUG("Fetching active faces");
@@ -818,19 +804,20 @@
}
else
{
- updateActiveFaces(buffer);
+ removeInvalidFaces(buffer);
}
}
void
-RibManager::updateActiveFaces(shared_ptr<ndn::OBufferStream> buffer)
+RibManager::removeInvalidFaces(shared_ptr<ndn::OBufferStream> buffer)
{
- NFD_LOG_DEBUG("Updating active faces");
+ NFD_LOG_DEBUG("Checking for invalid face registrations");
ndn::ConstBufferPtr buf = buffer->buf();
Block block;
size_t offset = 0;
+ FaceIdSet activeFaces;
while (offset < buf->size())
{
@@ -843,16 +830,30 @@
offset += block.size();
ndn::nfd::FaceStatus status(block);
-
- NFD_LOG_DEBUG("Adding faceId: " << status.getFaceId() << " to activeFaces");
activeFaces.insert(status.getFaceId());
}
+
+ // Look for face IDs that were registered but not active to find missed
+ // face destroyed events
+ for (FaceIdSet::iterator it = m_registeredFaces.begin(); it != m_registeredFaces.end(); ++it)
+ {
+ if (activeFaces.find(*it) == activeFaces.end())
+ {
+ NFD_LOG_DEBUG("Removing invalid face ID: " << *it);
+ scheduler::schedule(time::seconds(0),
+ bind(&RibManager::processErasureAfterNotification, this, *it));
+ }
+ }
+
+ // Reschedule the check for future clean up
+ scheduleActiveFaceFetch(ACTIVE_FACE_FETCH_INTERVAL);
}
void
RibManager::onFetchFaceStatusTimeout()
{
std::cerr << "Face Status Dataset request timed out" << std::endl;
+ scheduleActiveFaceFetch(ACTIVE_FACE_FETCH_INTERVAL);
}
} // namespace rib
diff --git a/rib/rib-manager.hpp b/rib/rib-manager.hpp
index dffb24a..1ca86cf 100644
--- a/rib/rib-manager.hpp
+++ b/rib/rib-manager.hpp
@@ -62,6 +62,8 @@
explicit
RibManager(ndn::Face& face);
+ ~RibManager();
+
void
registerWithNfd();
@@ -209,18 +211,24 @@
listEntries(const Interest& request);
void
+ scheduleActiveFaceFetch(const time::seconds& timeToWait);
+
+ void
fetchActiveFaces();
void
fetchSegments(const Data& data, shared_ptr<ndn::OBufferStream> buffer);
void
- updateActiveFaces(shared_ptr<ndn::OBufferStream> buffer);
-
- void
onFetchFaceStatusTimeout();
PUBLIC_WITH_TESTS_ELSE_PRIVATE:
+ /** \param buffer Face dataset contents
+ */
+ void
+ removeInvalidFaces(shared_ptr<ndn::OBufferStream> buffer);
+
+PUBLIC_WITH_TESTS_ELSE_PRIVATE:
Rib m_managedRib;
private:
@@ -286,8 +294,13 @@
static const Name FACES_LIST_DATASET_PREFIX;
-PUBLIC_WITH_TESTS_ELSE_PRIVATE:
- std::set<int> activeFaces;
+ static const time::seconds ACTIVE_FACE_FETCH_INTERVAL;
+ EventId m_activeFaceFetchEvent;
+
+ typedef std::set<uint64_t> FaceIdSet;
+ /** \brief contains FaceIds with one or more Routes in the RIB
+ */
+ FaceIdSet m_registeredFaces;
};
} // namespace rib
diff --git a/rib/rib.cpp b/rib/rib.cpp
index c76940b..0d33f24 100644
--- a/rib/rib.cpp
+++ b/rib/rib.cpp
@@ -279,6 +279,7 @@
FaceEntry copy = *faceIt;
faceIt = entry->eraseFace(faceIt);
+ m_nItems--;
const bool captureWasTurnedOff = (hadCapture && !entry->hasCapture());
createFibUpdatesForErasedFaceEntry(*entry, copy, captureWasTurnedOff);
diff --git a/tests/rib/rib-manager.cpp b/tests/rib/rib-manager.cpp
index 0d43dcb..b47e3ab 100644
--- a/tests/rib/rib-manager.cpp
+++ b/tests/rib/rib-manager.cpp
@@ -28,8 +28,11 @@
#include "tests/test-common.hpp"
#include "tests/dummy-client-face.hpp"
#include "tests/limited-io.hpp"
+
#include "rib/rib-status-publisher-common.hpp"
+#include <ndn-cxx/management/nfd-face-status.hpp>
+
namespace nfd {
namespace rib {
namespace tests {
@@ -49,8 +52,6 @@
face->processEvents(time::milliseconds(1));
face->m_sentInterests.clear();
-
- manager->activeFaces.insert(1);
}
~RibManagerFixture()
@@ -295,6 +296,55 @@
BOOST_REQUIRE_EQUAL(manager->m_managedRib.size(), 1);
}
+BOOST_FIXTURE_TEST_CASE(RemoveInvalidFaces, AuthorizedRibManager)
+{
+ // Register valid face
+ ControlParameters validParameters;
+ validParameters
+ .setName("/test")
+ .setFaceId(1);
+
+ Name validName("/localhost/nfd/rib/register");
+ receiveCommandInterest(validName, validParameters);
+
+ // Register invalid face
+ ControlParameters invalidParameters;
+ invalidParameters
+ .setName("/test")
+ .setFaceId(2);
+
+ Name invalidName("/localhost/nfd/rib/register");
+ receiveCommandInterest(invalidName, invalidParameters);
+
+ BOOST_REQUIRE_EQUAL(manager->m_managedRib.size(), 2);
+
+ // Receive status with only faceId: 1
+ ndn::nfd::FaceStatus status;
+ status.setFaceId(1);
+
+ shared_ptr<Data> data = nfd::tests::makeData("/localhost/nfd/faces/list");
+ data->setContent(status.wireEncode());
+
+ shared_ptr<ndn::OBufferStream> buffer = make_shared<ndn::OBufferStream>();
+ buffer->write(reinterpret_cast<const char*>(data->getContent().value()),
+ data->getContent().value_size());
+
+ manager->removeInvalidFaces(buffer);
+
+ // Run scheduler
+ nfd::tests::LimitedIo limitedIo;
+ limitedIo.run(nfd::tests::LimitedIo::UNLIMITED_OPS, time::seconds(1));
+
+ BOOST_REQUIRE_EQUAL(manager->m_managedRib.size(), 1);
+
+ Rib::const_iterator it = manager->m_managedRib.find("/test");
+ BOOST_REQUIRE(it != manager->m_managedRib.end());
+
+ shared_ptr<RibEntry> entry = it->second;
+ BOOST_CHECK_EQUAL(entry->hasFaceId(1), true);
+ BOOST_CHECK_EQUAL(entry->hasFaceId(2), false);
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace tests