fib: don't refresh neighbor router's FIB entry (it is already installed w/ expiry=never)
refs: #4799
Change-Id: Ib55687655ccd8673eeae14d8418e458ad8ab00aa
diff --git a/src/hello-protocol.cpp b/src/hello-protocol.cpp
index 079bdc4..a5f935c 100644
--- a/src/hello-protocol.cpp
+++ b/src/hello-protocol.cpp
@@ -47,11 +47,15 @@
i.setMustBeFresh(true);
i.setCanBePrefix(true);
m_nlsr.getNlsrFace().expressInterest(i,
- std::bind(&HelloProtocol::onContent,
- this,
- _1, _2),
- std::bind(&HelloProtocol::processInterestTimedOut, // Nack
- this, _1),
+ std::bind(&HelloProtocol::onContent, this, _1, _2),
+ [this] (const ndn::Interest& interest,
+ const ndn::lp::Nack& nack)
+ {
+ NDN_LOG_TRACE("Received Nack with reason " <<
+ nack.getReason());
+ NDN_LOG_TRACE("Treating as timeout");
+ processInterestTimedOut(interest);
+ },
std::bind(&HelloProtocol::processInterestTimedOut,
this, _1));
diff --git a/src/route/fib-entry.cpp b/src/route/fib-entry.cpp
index a3186b3..0091245 100644
--- a/src/route/fib-entry.cpp
+++ b/src/route/fib-entry.cpp
@@ -26,7 +26,7 @@
INIT_LOGGER(route.FibEntry);
void
-FibEntry::writeLog()
+FibEntry::writeLog() const
{
NLSR_LOG_DEBUG("Name Prefix: " << m_name);
NLSR_LOG_DEBUG("Seq No: " << m_seqNo);
diff --git a/src/route/fib-entry.hpp b/src/route/fib-entry.hpp
index f43ddb1..2d653ad 100644
--- a/src/route/fib-entry.hpp
+++ b/src/route/fib-entry.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2017, The University of Memphis,
+ * Copyright (c) 2014-2018, The University of Memphis,
* Regents of the University of California
*
* This file is part of NLSR (Named-data Link State Routing).
@@ -21,10 +21,10 @@
#ifndef NLSR_FIB_ENTRY_HPP
#define NLSR_FIB_ENTRY_HPP
-#include <ndn-cxx/util/scheduler.hpp>
-
#include "nexthop-list.hpp"
+#include <ndn-cxx/util/scheduler.hpp>
+
namespace nlsr {
class FibEntry
@@ -81,7 +81,7 @@
}
void
- writeLog();
+ writeLog() const;
typedef NexthopList::const_iterator const_iterator;
diff --git a/src/route/fib.cpp b/src/route/fib.cpp
index 82978e2..a38d33b 100644
--- a/src/route/fib.cpp
+++ b/src/route/fib.cpp
@@ -35,20 +35,26 @@
const uint64_t Fib::GRACE_PERIOD = 10;
+Fib::Fib(ndn::Face& face, ndn::Scheduler& scheduler, AdjacencyList& adjacencyList,
+ ConfParameter& conf, ndn::security::v2::KeyChain& keyChain)
+ : m_scheduler(scheduler)
+ , m_refreshTime(0)
+ , m_controller(face, keyChain)
+ , m_adjacencyList(adjacencyList)
+ , m_confParameter(conf)
+{
+}
+
void
Fib::remove(const ndn::Name& name)
{
NLSR_LOG_DEBUG("Fib::remove called");
std::map<ndn::Name, FibEntry>::iterator it = m_table.find(name);
- if (it != m_table.end()) {
- for (std::set<NextHop, NextHopComparator>::iterator nhit =
- (it->second).getNexthopList().getNextHops().begin();
- nhit != (it->second).getNexthopList().getNextHops().end(); nhit++) {
- //remove entry from NDN-FIB
- // Only unregister the prefix if it ISN'T a neighbor.
- if (isPrefixUpdatable((it->second).getName())) {
- unregisterPrefix((it->second).getName(), nhit->getConnectingFaceUri());
- }
+
+ // Only unregister the prefix if it ISN'T a neighbor.
+ if (it != m_table.end() && isNotNeighbor((it->second).getName())) {
+ for (const auto& nexthop : (it->second).getNexthopList().getNextHops()) {
+ unregisterPrefix((it->second).getName(), nexthop.getConnectingFaceUri());
}
cancelEntryRefresh(it->second);
m_table.erase(it);
@@ -60,15 +66,17 @@
{
const ndn::Name& name = entry.getName();
- for (NexthopList::iterator it = hopsToAdd.cbegin(); it != hopsToAdd.cend(); ++it)
+ bool shouldRegister = isNotNeighbor(name);
+
+ for (const auto& hop : hopsToAdd.getNextHops())
{
// Add nexthop to FIB entry
- entry.getNexthopList().addNextHop(*it);
+ entry.getNexthopList().addNextHop(hop);
- if (isPrefixUpdatable(name)) {
+ if (shouldRegister) {
// Add nexthop to NDN-FIB
- registerPrefix(name, ndn::FaceUri(it->getConnectingFaceUri()),
- it->getRouteCostAsAdjustedInteger(),
+ registerPrefix(name, ndn::FaceUri(hop.getConnectingFaceUri()),
+ hop.getRouteCostAsAdjustedInteger(),
ndn::time::seconds(m_refreshTime + GRACE_PERIOD),
ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
}
@@ -80,7 +88,7 @@
{
NLSR_LOG_DEBUG("Fib::update called");
- // Get the max possible faces which is the minumum of the configuration setting and
+ // Get the max possible faces which is the minimum of the configuration setting and
// the length of the list of all next hops.
unsigned int maxFaces = getNumberOfFacesForName(allHops);
@@ -126,9 +134,9 @@
hopsToAdd.begin(), hopsToAdd.end(),
std::inserter(hopsToRemove, hopsToRemove.end()), NextHopComparator());
- bool isUpdatable = isPrefixUpdatable(entry.getName());
+ bool isUpdatable = isNotNeighbor(entry.getName());
// Remove the uninstalled next hops from NFD and FIB entry
- for (const auto& hop: hopsToRemove){
+ for (const auto& hop : hopsToRemove){
if (isUpdatable) {
unregisterPrefix(entry.getName(), hop.getConnectingFaceUri());
}
@@ -142,7 +150,9 @@
entryIt = m_table.find(name);
}
- if (entryIt != m_table.end() && !entryIt->second.getRefreshEventId()) {
+ if (entryIt != m_table.end() &&
+ !entryIt->second.getRefreshEventId() &&
+ isNotNeighbor(entryIt->second.getName())) {
scheduleEntryRefresh(entryIt->second,
[this] (FibEntry& entry) {
scheduleLoop(entry);
@@ -154,21 +164,15 @@
Fib::clean()
{
NLSR_LOG_DEBUG("Fib::clean called");
- for (std::map<ndn::Name, FibEntry>::iterator it = m_table.begin();
- it != m_table.end();
- ++it) {
- NLSR_LOG_DEBUG("Cancelling Scheduled event. Name: " << it->second.getName());
- cancelEntryRefresh(it->second);
- for (std::set<NextHop, NextHopComparator>::iterator nhit =
- (it->second).getNexthopList().getNextHops().begin();
- nhit != (it->second).getNexthopList().getNextHops().end(); nhit++) {
- //Remove entry from NDN-FIB
- unregisterPrefix((it->second).getName(), nhit->getConnectingFaceUri());
+ // can't use const ref here as getNexthopList can't be marked const
+ for (auto&& it : m_table) {
+ NLSR_LOG_DEBUG("Canceling Scheduled event. Name: " << it.second.getName());
+ cancelEntryRefresh(it.second);
+
+ for (const auto& hop : it.second.getNexthopList().getNextHops()) {
+ unregisterPrefix(it.second.getName(), hop.getConnectingFaceUri());
}
}
- if (m_table.size() > 0) {
- m_table.clear();
- }
}
unsigned int
@@ -187,12 +191,9 @@
}
bool
-Fib::isPrefixUpdatable(const ndn::Name& name) {
- if (!m_adjacencyList.isNeighbor(name)) {
- return true;
- }
-
- return false;
+Fib::isNotNeighbor(const ndn::Name& name)
+{
+ return !m_adjacencyList.isNeighbor(name);
}
void
@@ -215,14 +216,13 @@
NLSR_LOG_DEBUG("Registering prefix: " << faceParameters.getName() << " faceUri: " << faceUri);
m_controller.start<ndn::nfd::RibRegisterCommand>(faceParameters,
- std::bind(&Fib::onRegistrationSuccess, this, _1,
- "Successful in name registration",
- faceUri),
- std::bind(&Fib::onRegistrationFailure,
- this, _1,
- "Failed in name registration",
- faceParameters,
- faceUri, times));
+ std::bind(&Fib::onRegistrationSuccess, this, _1,
+ "Successful in name registration",
+ faceUri),
+ std::bind(&Fib::onRegistrationFailure, this, _1,
+ "Failed in name registration",
+ faceParameters,
+ faceUri, times));
}
else {
NLSR_LOG_WARN("Error: No Face Id for face uri: " << faceUri);
@@ -234,7 +234,7 @@
const std::string& message, const ndn::FaceUri& faceUri)
{
NLSR_LOG_DEBUG(message << ": " << commandSuccessResult.getName() <<
- " Face Uri: " << faceUri << " faceId: " << commandSuccessResult.getFaceId());
+ " Face Uri: " << faceUri << " faceId: " << commandSuccessResult.getFaceId());
AdjacencyList::iterator adjacent = m_adjacencyList.findAdjacent(faceUri);
if (adjacent != m_adjacencyList.end()) {
@@ -292,7 +292,7 @@
const std::string& message)
{
NLSR_LOG_DEBUG("Unregister successful Prefix: " << commandSuccessResult.getName() <<
- " Face Id: " << commandSuccessResult.getFaceId());
+ " Face Id: " << commandSuccessResult.getFaceId());
}
void
@@ -323,8 +323,8 @@
Fib::onSetStrategySuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
const std::string& message)
{
- NLSR_LOG_DEBUG(message << ": " << commandSuccessResult.getStrategy() << " "
- << "for name: " << commandSuccessResult.getName());
+ NLSR_LOG_DEBUG(message << ": " << commandSuccessResult.getStrategy() <<
+ " for name: " << commandSuccessResult.getName());
}
void
@@ -333,19 +333,19 @@
uint32_t count,
const std::string& message)
{
- NLSR_LOG_DEBUG(message << ": " << parameters.getStrategy() << " "
- << "for name: " << parameters.getName());
+ NLSR_LOG_DEBUG(message << ": " << parameters.getStrategy() <<
+ " for name: " << parameters.getName());
if (count < 3) {
- setStrategy(parameters.getName(), parameters.getStrategy().toUri(),count+1);
+ setStrategy(parameters.getName(), parameters.getStrategy().toUri(), count + 1);
}
}
void
Fib::scheduleEntryRefresh(FibEntry& entry, const afterRefreshCallback& refreshCallback)
{
- NLSR_LOG_DEBUG("Scheduling refresh for " << entry.getName()
- << " Seq Num: " << entry.getSeqNo()
- << " in " << m_refreshTime << " seconds");
+ NLSR_LOG_DEBUG("Scheduling refresh for " << entry.getName() <<
+ " Seq Num: " << entry.getSeqNo() <<
+ " in " << m_refreshTime << " seconds");
entry.setRefreshEventId(m_scheduler.scheduleEvent(ndn::time::seconds(m_refreshTime),
std::bind(&Fib::refreshEntry, this,
@@ -355,14 +355,13 @@
void
Fib::scheduleLoop(FibEntry& entry)
{
- scheduleEntryRefresh(entry,
- std::bind(&Fib::scheduleLoop, this, _1));
+ scheduleEntryRefresh(entry, std::bind(&Fib::scheduleLoop, this, _1));
}
void
Fib::cancelEntryRefresh(const FibEntry& entry)
{
- NLSR_LOG_DEBUG("Cancelling refresh for " << entry.getName() << " Seq Num: " << entry.getSeqNo());
+ NLSR_LOG_DEBUG("Canceling refresh for " << entry.getName() << " Seq Num: " << entry.getSeqNo());
m_scheduler.cancelEvent(entry.getRefreshEventId());
entry.getRefreshEventId().reset();
@@ -381,7 +380,7 @@
NLSR_LOG_DEBUG("Refreshing " << entry.getName() << " Seq Num: " << entry.getSeqNo());
// Increment sequence number
- entry.setSeqNo(entry.getSeqNo() + 1);
+ entry.setSeqNo(entry.getSeqNo() + 1);
for (const NextHop& hop : entry) {
registerPrefix(entry.getName(),
@@ -398,10 +397,8 @@
Fib::writeLog()
{
NLSR_LOG_DEBUG("-------------------FIB-----------------------------");
- for (std::map<ndn::Name, FibEntry>::iterator it = m_table.begin();
- it != m_table.end();
- ++it) {
- (it->second).writeLog();
+ for (const auto& entry : m_table) {
+ entry.second.writeLog();
}
}
diff --git a/src/route/fib.hpp b/src/route/fib.hpp
index 32c656d..012d61d 100644
--- a/src/route/fib.hpp
+++ b/src/route/fib.hpp
@@ -53,15 +53,8 @@
class Fib
{
public:
- Fib(ndn::Face& face, ndn::Scheduler& scheduler, AdjacencyList& adjacencyList, ConfParameter& conf,
- ndn::security::v2::KeyChain& keyChain)
- : m_scheduler(scheduler)
- , m_refreshTime(0)
- , m_controller(face, keyChain)
- , m_adjacencyList(adjacencyList)
- , m_confParameter(conf)
- {
- }
+ Fib(ndn::Face& face, ndn::Scheduler& scheduler, AdjacencyList& adjacencyList,
+ ConfParameter& conf, ndn::security::v2::KeyChain& keyChain);
/*! \brief Completely remove a name prefix from the FIB.
*
@@ -148,7 +141,7 @@
* \return Whether the name is NOT associated with a direct neighbor
*/
bool
- isPrefixUpdatable(const ndn::Name& name);
+ isNotNeighbor(const ndn::Name& name);
/*! \brief Does one half of the updating of a FibEntry with new next-hops.
*
diff --git a/src/route/name-prefix-table.cpp b/src/route/name-prefix-table.cpp
index 7fb9941..1c05c2c 100644
--- a/src/route/name-prefix-table.cpp
+++ b/src/route/name-prefix-table.cpp
@@ -117,13 +117,13 @@
}
else {
npte = *nameItr;
- NLSR_LOG_TRACE("Adding origin: " << rtpePtr->getDestination()
- << " to existing prefix: " << *nameItr);
+ NLSR_LOG_TRACE("Adding origin: " << rtpePtr->getDestination() <<
+ " to existing prefix: " << **nameItr);
(*nameItr)->addRoutingTableEntry(rtpePtr);
(*nameItr)->generateNhlfromRteList();
if ((*nameItr)->getNexthopList().size() > 0) {
- NLSR_LOG_TRACE("Updating FIB with next hops for " << (*nameItr));
+ NLSR_LOG_TRACE("Updating FIB with next hops for " << (**nameItr));
m_nlsr.getFib().update(name, (*nameItr)->getNexthopList());
}
else {
diff --git a/src/route/nexthop-list.cpp b/src/route/nexthop-list.cpp
index 698e06a..8b9366e 100644
--- a/src/route/nexthop-list.cpp
+++ b/src/route/nexthop-list.cpp
@@ -107,14 +107,13 @@
}
void
-NexthopList::writeLog()
+NexthopList::writeLog() const
{
int i = 1;
- for (std::set<NextHop, NextHopComparator>::iterator it = m_nexthopList.begin();
- it != m_nexthopList.end() ; it++, i++) {
- NLSR_LOG_DEBUG("Nexthop " << i << ": " << (*it).getConnectingFaceUri()
- << " Route Cost: " << (*it).getRouteCost());
+ for (const auto& nexthop : m_nexthopList) {
+ NLSR_LOG_DEBUG("Nexthop " << i++ << ": " << nexthop.getConnectingFaceUri() <<
+ " Route Cost: " << nexthop.getRouteCost());
}
}
diff --git a/src/route/nexthop-list.hpp b/src/route/nexthop-list.hpp
index 577165f..d9f60f4 100644
--- a/src/route/nexthop-list.hpp
+++ b/src/route/nexthop-list.hpp
@@ -121,7 +121,7 @@
}
void
- writeLog();
+ writeLog() const;
private:
std::set<NextHop, NextHopComparator> m_nexthopList;
diff --git a/tests/test-fib.cpp b/tests/test-fib.cpp
index 629f933..04b08ea 100644
--- a/tests/test-fib.cpp
+++ b/tests/test-fib.cpp
@@ -36,7 +36,7 @@
{
public:
FibFixture()
- : face(std::make_shared<ndn::util::DummyClientFace>(m_keyChain))
+ : face(std::make_shared<ndn::util::DummyClientFace>(m_ioService, m_keyChain))
, interests(face->sentInterests)
{
Adjacent neighbor1(router1Name, ndn::FaceUri(router1FaceUri), 0, Adjacent::STATUS_ACTIVE, 0, router1FaceId);
@@ -51,6 +51,7 @@
conf.setMaxFacesPerPrefix(2);
fib = std::make_shared<Fib>(*face, m_scheduler, adjacencies, conf, m_keyChain);
+ fib->setEntryRefreshTime(1);
fib->m_faceMap.update(router1FaceUri, router1FaceId);
fib->m_faceMap.update(router2FaceUri, router2FaceId);
@@ -314,6 +315,22 @@
this->advanceClocks(ndn::time::milliseconds(10), 1);
}
+BOOST_AUTO_TEST_CASE(ShouldNotRefreshNeighborRoute) // #4799
+{
+ NextHop hop1;
+ hop1.setConnectingFaceUri(router1FaceUri);
+
+ NexthopList hops;
+ hops.addNextHop(hop1);
+
+ // Simulate update for this neighbor from name prefix table
+ fib->update(router1Name, hops);
+ this->advanceClocks(ndn::time::seconds(1));
+
+ // Should not send the register interest
+ BOOST_CHECK_EQUAL(face->sentInterests.size(), 0);
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace test