src: fix prefix advertise runtime error
Don't schedule Hello after receiving face changes in dataset,
let it be on its schedule. Also don't schedule routing calc (HR)
or AdjLsa (LS) let them be scheduled on Hello Data.
refs: #4215
Change-Id: I5c3881943f83b2f7683316fa3e22e6c280b6b64d
diff --git a/src/adjacency-list.cpp b/src/adjacency-list.cpp
index ccbea1b..6f83b1a 100644
--- a/src/adjacency-list.cpp
+++ b/src/adjacency-list.cpp
@@ -263,9 +263,8 @@
{
return std::find_if(m_adjList.begin(),
m_adjList.end(),
- [&faceUri] (const Adjacent& adj) {
- return faceUri == adj.getFaceUri();
- });
+ std::bind(&Adjacent::compareFaceUri,
+ _1, faceUri));
}
uint64_t
diff --git a/src/adjacency-list.hpp b/src/adjacency-list.hpp
index daceae1..dcb1539 100644
--- a/src/adjacency-list.hpp
+++ b/src/adjacency-list.hpp
@@ -1,4 +1,4 @@
- /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
* Copyright (c) 2014-2017, The University of Memphis,
* Regents of the University of California,
@@ -22,12 +22,12 @@
#ifndef NLSR_ADJACENCY_LIST_HPP
#define NLSR_ADJACENCY_LIST_HPP
+#include "adjacent.hpp"
+
#include <list>
#include <boost/cstdint.hpp>
#include <ndn-cxx/common.hpp>
-#include "adjacent.hpp"
-
namespace nlsr {
class AdjacencyList
@@ -148,6 +148,19 @@
AdjacencyList::iterator
findAdjacent(const ndn::util::FaceUri& faceUri);
+ /*! \brief Hack to stop developers from using this function
+
+ It is here so that faceUri cannot be passed in as string,
+ converted to Name and findAdjacent(Name) be used.
+ So when faceUri is passed as string this will cause a compile error
+ */
+ template <typename T = float> void
+ findAdjacent(const std::string& faceUri)
+ {
+ BOOST_STATIC_ASSERT_MSG(std::is_integral<T>::value,
+ "Don't use std::string with findAdjacent!");
+ }
+
uint64_t
getFaceId(const ndn::util::FaceUri& faceUri);
diff --git a/src/nlsr.cpp b/src/nlsr.cpp
index c4658e8..55d426a 100644
--- a/src/nlsr.cpp
+++ b/src/nlsr.cpp
@@ -91,7 +91,7 @@
void
Nlsr::registrationFailed(const ndn::Name& name)
{
- std::cerr << "ERROR: Failed to register prefix in local hub's daemon" << std::endl;
+ _LOG_ERROR("ERROR: Failed to register prefix in local hub's daemon");
BOOST_THROW_EXCEPTION(Error("Error: Prefix registration failed"));
}
@@ -430,7 +430,6 @@
}
case ndn::nfd::FACE_EVENT_CREATED: {
// Find the neighbor in our adjacency list
- _LOG_DEBUG("Face created event received.");
auto adjacent = m_adjacencyList.findAdjacent(
ndn::util::FaceUri(faceEventNotification.getRemoteUri()));
// If we have a neighbor by that FaceUri and it has no FaceId, we
@@ -442,6 +441,13 @@
adjacent->setFaceId(faceEventNotification.getFaceId());
registerAdjacencyPrefixes(*adjacent, ndn::time::milliseconds::max());
+
+ if (m_confParam.getHyperbolicState() != HYPERBOLIC_STATE_OFF) {
+ getRoutingTable().scheduleRoutingTableCalculation(*this);
+ }
+ else {
+ m_nlsrLsdb.scheduleAdjLsaBuild();
+ }
}
break;
}
@@ -463,48 +469,34 @@
void
Nlsr::processFaceDataset(const std::vector<ndn::nfd::FaceStatus>& faces)
{
- // Iterate over each neighbor listed in nlsr.conf
- bool anyFaceChanged = false;
- for (auto&& adjacency : m_adjacencyList.getAdjList()) {
+ _LOG_DEBUG("Processing face dataset");
- const std::string faceUriString = adjacency.getFaceUri().toString();
+ // Iterate over each neighbor listed in nlsr.conf
+ for (auto& adjacent : m_adjacencyList.getAdjList()) {
+
+ const std::string faceUriString = adjacent.getFaceUri().toString();
// Check the list of FaceStatus objects we got for a match
for (const ndn::nfd::FaceStatus& faceStatus : faces) {
-
// Set the adjacency FaceID if we find a URI match and it was
// previously unset. Change the boolean to true.
- if (adjacency.getFaceId() == 0 && faceUriString == faceStatus.getRemoteUri()) {
- adjacency.setFaceId(faceStatus.getFaceId());
- anyFaceChanged = true;
+ if (adjacent.getFaceId() == 0 && faceUriString == faceStatus.getRemoteUri()) {
+ _LOG_DEBUG("FaceUri: " << faceStatus.getRemoteUri() <<
+ " FaceId: "<< faceStatus.getFaceId());
+ adjacent.setFaceId(faceStatus.getFaceId());
// Register the prefixes for each neighbor
- this->registerAdjacencyPrefixes(adjacency, ndn::time::milliseconds::max());
+ this->registerAdjacencyPrefixes(adjacent, ndn::time::milliseconds::max());
}
}
// If this adjacency has no information in this dataset, then one
// of two things is happening: 1. NFD is starting slowly and this
// Face wasn't ready yet, or 2. NFD is configured
// incorrectly and this Face isn't available.
- if (adjacency.getFaceId() == 0) {
- _LOG_WARN("The adjacency " << adjacency.getName() <<
+ if (adjacent.getFaceId() == 0) {
+ _LOG_WARN("The adjacency " << adjacent.getName() <<
" has no Face information in this dataset.");
}
}
- if (anyFaceChanged) {
- // Only do these things if something has changed. Schedule an
- // adjacency LSA build to update with all of the new neighbors if
- // HR is off.
- if (m_confParam.getHyperbolicState() != HYPERBOLIC_STATE_OFF) {
- getRoutingTable().scheduleRoutingTableCalculation(*this);
- }
- else {
- m_nlsrLsdb.scheduleAdjLsaBuild();
- }
-
- // Begin the Hello Protocol loop immediately, so we don't wait.
- m_helloProtocol.sendScheduledInterest(0);
- }
-
scheduleDatasetFetch();
}
@@ -512,25 +504,25 @@
Nlsr::registerAdjacencyPrefixes(const Adjacent& adj,
const ndn::time::milliseconds& timeout)
{
- std::string faceUri = adj.getFaceUri().toString();
- double linkCost = adj.getLinkCost();
+ ndn::util::FaceUri faceUri = adj.getFaceUri();
+ double linkCost = adj.getLinkCost();
- m_fib.registerPrefix(adj.getName(), faceUri, linkCost,
- timeout, ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
+ m_fib.registerPrefix(adj.getName(), faceUri, linkCost,
+ timeout, ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
- m_fib.registerPrefix(m_confParam.getChronosyncPrefix(),
- faceUri, linkCost, timeout,
- ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
+ m_fib.registerPrefix(m_confParam.getChronosyncPrefix(),
+ faceUri, linkCost, timeout,
+ ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
- m_fib.registerPrefix(m_confParam.getLsaPrefix(),
- faceUri, linkCost, timeout,
- ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
+ m_fib.registerPrefix(m_confParam.getLsaPrefix(),
+ faceUri, linkCost, timeout,
+ ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
- ndn::Name broadcastKeyPrefix = DEFAULT_BROADCAST_PREFIX;
- broadcastKeyPrefix.append("KEYS");
- m_fib.registerPrefix(broadcastKeyPrefix,
- faceUri, linkCost, timeout,
- ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
+ ndn::Name broadcastKeyPrefix = DEFAULT_BROADCAST_PREFIX;
+ broadcastKeyPrefix.append("KEYS");
+ m_fib.registerPrefix(broadcastKeyPrefix,
+ faceUri, linkCost, timeout,
+ ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
}
void
@@ -538,6 +530,7 @@
const std::string& msg,
uint32_t nRetriesSoFar)
{
+ _LOG_DEBUG("onFaceDatasetFetchTimeout");
// If we have exceeded the maximum attempt count, do not try again.
if (nRetriesSoFar++ < m_confParam.getFaceDatasetFetchTries()) {
_LOG_DEBUG("Failed to fetch dataset: " << msg << ". Attempting retry #" << nRetriesSoFar);
@@ -559,6 +552,8 @@
void
Nlsr::scheduleDatasetFetch()
{
+ _LOG_DEBUG("Scheduling Dataset Fetch in " << m_confParam.getFaceDatasetFetchInterval()
+ << " seconds");
m_scheduler.scheduleEvent(m_confParam.getFaceDatasetFetchInterval(),
[this] {
this->initializeFaces(
diff --git a/src/route/fib.cpp b/src/route/fib.cpp
index 49ce647..0eb6d16 100644
--- a/src/route/fib.cpp
+++ b/src/route/fib.cpp
@@ -57,12 +57,6 @@
}
}
-bool
-compareFaceUri(const NextHop& hop, const std::string& faceUri)
-{
- return hop.getConnectingFaceUri() == faceUri;
-}
-
void
Fib::addNextHopsToFibEntryAndNfd(FibEntry& entry, NexthopList& hopsToAdd)
{
@@ -75,7 +69,7 @@
if (isPrefixUpdatable(name)) {
// Add nexthop to NDN-FIB
- registerPrefix(name, it->getConnectingFaceUri(),
+ registerPrefix(name, ndn::util::FaceUri(it->getConnectingFaceUri()),
it->getRouteCostAsAdjustedInteger(),
ndn::time::seconds(m_refreshTime + GRACE_PERIOD),
ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
@@ -209,11 +203,13 @@
}
void
-Fib::registerPrefix(const ndn::Name& namePrefix, const std::string& faceUri,
- uint64_t faceCost, const ndn::time::milliseconds& timeout,
+Fib::registerPrefix(const ndn::Name& namePrefix, const ndn::util::FaceUri& faceUri,
+ uint64_t faceCost,
+ const ndn::time::milliseconds& timeout,
uint64_t flags, uint8_t times)
{
uint64_t faceId = m_adjacencyList.getFaceId(ndn::util::FaceUri(faceUri));
+
if (faceId != 0) {
ndn::nfd::ControlParameters faceParameters;
faceParameters
@@ -224,30 +220,58 @@
.setExpirationPeriod(timeout)
.setOrigin(ndn::nfd::ROUTE_ORIGIN_NLSR);
- _LOG_DEBUG("Registering prefix: " << namePrefix << " Face Uri: " << faceUri
- << " Face Id: " << faceId);
- registerPrefixInNfd(faceParameters, faceUri, times);
- }
- else {
- _LOG_DEBUG("Error: No Face Id for face uri: " << faceUri);
- }
-}
-
-void
-Fib::registerPrefixInNfd(ndn::nfd::ControlParameters& parameters,
- const std::string& faceUri,
- uint8_t times)
-{
- _LOG_DEBUG("Registering prefix: " << parameters.getName() << " faceUri: " << faceUri);
- m_controller.start<ndn::nfd::RibRegisterCommand>(parameters,
+ _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,
+ this, _1,
"Failed in name registration",
- parameters,
+ faceParameters,
faceUri, times));
+ }
+ else {
+ _LOG_WARN("Error: No Face Id for face uri: " << faceUri);
+ }
+}
+
+void
+Fib::onRegistrationSuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
+ const std::string& message, const ndn::util::FaceUri& faceUri)
+{
+ _LOG_DEBUG(message << ": " << commandSuccessResult.getName() <<
+ " Face Uri: " << faceUri << " faceId: " << commandSuccessResult.getFaceId());
+
+ AdjacencyList::iterator adjacent = m_adjacencyList.findAdjacent(faceUri);
+ if (adjacent != m_adjacencyList.end()) {
+ adjacent->setFaceId(commandSuccessResult.getFaceId());
+ }
+
+ // Update the fast-access FaceMap with the new Face ID, too
+ m_faceMap.update(faceUri.toString(), commandSuccessResult.getFaceId());
+ m_faceMap.writeLog();
+}
+
+void
+Fib::onRegistrationFailure(const ndn::nfd::ControlResponse& response,
+ const std::string& message,
+ const ndn::nfd::ControlParameters& parameters,
+ const ndn::util::FaceUri& faceUri,
+ uint8_t times)
+{
+ _LOG_DEBUG(message << ": " << response.getText() << " (code: " << response.getCode() << ")");
+ _LOG_DEBUG("Prefix: " << parameters.getName() << " failed for: " << times);
+ if (times < 3) {
+ _LOG_DEBUG("Trying to register again...");
+ registerPrefix(parameters.getName(), faceUri,
+ parameters.getCost(),
+ parameters.getExpirationPeriod(),
+ parameters.getFlags(), times+1);
+ }
+ else {
+ _LOG_DEBUG("Registration trial given up");
+ }
}
void
@@ -271,6 +295,21 @@
}
void
+Fib::onUnregistrationSuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
+ const std::string& message)
+{
+ _LOG_DEBUG("Unregister successful Prefix: " << commandSuccessResult.getName() <<
+ " Face Id: " << commandSuccessResult.getFaceId());
+}
+
+void
+Fib::onUnregistrationFailure(const ndn::nfd::ControlResponse& response,
+ const std::string& message)
+{
+ _LOG_DEBUG(message << ": " << response.getText() << " (code: " << response.getCode() << ")");
+}
+
+void
Fib::setStrategy(const ndn::Name& name, const std::string& strategy, uint32_t count)
{
ndn::nfd::ControlParameters parameters;
@@ -288,56 +327,6 @@
}
void
-Fib::onRegistrationSuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
- const std::string& message, const std::string& faceUri)
-{
- _LOG_DEBUG("Register successful Prefix: " << commandSuccessResult.getName() <<
- " Face Uri: " << faceUri);
-
- // Update the adjacency list with the new Face ID
- m_adjacencyList.findAdjacent(faceUri)->setFaceId(commandSuccessResult.getFaceId());
- // Update the fast-access FaceMap with the new Face ID, too
- m_faceMap.update(faceUri, commandSuccessResult.getFaceId());
- m_faceMap.writeLog();
-}
-
-void
-Fib::onRegistrationFailure(const ndn::nfd::ControlResponse& response,
- const std::string& message,
- const ndn::nfd::ControlParameters& parameters,
- const std::string& faceUri,
- uint8_t times)
-{
- _LOG_DEBUG(message << ": " << response.getText() << " (code: " << response.getCode() << ")");
- _LOG_DEBUG("Prefix: " << parameters.getName() << " failed for: " << times);
- if (times < 3) {
- _LOG_DEBUG("Trying to register again...");
- registerPrefix(parameters.getName(), faceUri,
- parameters.getCost(),
- parameters.getExpirationPeriod(),
- parameters.getFlags(), times+1);
- }
- else {
- _LOG_DEBUG("Registration trial given up");
- }
-}
-
-void
-Fib::onUnregistrationSuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
- const std::string& message)
-{
- _LOG_DEBUG("Unregister successful Prefix: " << commandSuccessResult.getName() <<
- " Face Id: " << commandSuccessResult.getFaceId());
-}
-
-void
-Fib::onUnregistrationFailure(const ndn::nfd::ControlResponse& response,
- const std::string& message)
-{
- _LOG_DEBUG(message << ": " << response.getText() << " (code: " << response.getCode() << ")");
-}
-
-void
Fib::onSetStrategySuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
const std::string& message)
{
@@ -403,10 +392,10 @@
for (const NextHop& hop : entry) {
registerPrefix(entry.getName(),
- hop.getConnectingFaceUri(),
- hop.getRouteCostAsAdjustedInteger(),
- ndn::time::seconds(m_refreshTime + GRACE_PERIOD),
- ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
+ ndn::util::FaceUri(hop.getConnectingFaceUri()),
+ hop.getRouteCostAsAdjustedInteger(),
+ ndn::time::seconds(m_refreshTime + GRACE_PERIOD),
+ ndn::nfd::ROUTE_FLAG_CAPTURE, 0);
}
refreshCb(entry);
diff --git a/src/route/fib.hpp b/src/route/fib.hpp
index 2705e47..4f15655 100644
--- a/src/route/fib.hpp
+++ b/src/route/fib.hpp
@@ -71,7 +71,7 @@
void
registerPrefix(const ndn::Name& namePrefix,
- const std::string& faceUri,
+ const ndn::util::FaceUri& faceUri,
uint64_t faceCost,
const ndn::time::milliseconds& timeout,
uint64_t flags,
@@ -94,22 +94,17 @@
getNumberOfFacesForName(NexthopList& nextHopList);
void
- registerPrefixInNfd(ndn::nfd::ControlParameters& parameters,
- const std::string& faceUri,
- uint8_t times);
-
- void
unregisterPrefix(const ndn::Name& namePrefix, const std::string& faceUri);
void
onRegistrationSuccess(const ndn::nfd::ControlParameters& commandSuccessResult,
- const std::string& message, const std::string& faceUri);
+ const std::string& message, const ndn::util::FaceUri& faceUri);
void
onRegistrationFailure(const ndn::nfd::ControlResponse& response,
const std::string& message,
const ndn::nfd::ControlParameters& parameters,
- const std::string& faceUri,
+ const ndn::util::FaceUri& faceUri,
uint8_t times);
void
diff --git a/tests/publisher/publisher-fixture.hpp b/tests/publisher/publisher-fixture.hpp
index aa80343..c52f378 100644
--- a/tests/publisher/publisher-fixture.hpp
+++ b/tests/publisher/publisher-fixture.hpp
@@ -50,7 +50,7 @@
nlsr.getConfParameter().setNetwork("/ndn");
nlsr.getConfParameter().setRouterName("/This/Router");
nlsr.initialize();
- face.processEvents(ndn::time::milliseconds(10));
+ face.processEvents(ndn::time::milliseconds(100));
}
void
diff --git a/tests/update/test-advertise-crash.cpp b/tests/update/test-advertise-crash.cpp
new file mode 100644
index 0000000..4f59c49
--- /dev/null
+++ b/tests/update/test-advertise-crash.cpp
@@ -0,0 +1,97 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/**
+ * Copyright (c) 2014-2017, The University of Memphis,
+ * Regents of the University of California,
+ * Arizona Board of Regents.
+ *
+ * This file is part of NLSR (Named-data Link State Routing).
+ * See AUTHORS.md for complete list of NLSR authors and contributors.
+ *
+ * NLSR is free software: you can redistribute it and/or modify it under the terms
+ * of the GNU General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * NLSR is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * NLSR, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
+ **/
+
+#include "../test-common.hpp"
+#include "nlsr.hpp"
+
+namespace nlsr {
+namespace update {
+namespace test {
+
+class AdvertiseCrashFixture : public nlsr::test::UnitTestTimeFixture
+{
+public:
+ AdvertiseCrashFixture()
+ : face(g_ioService, keyChain, {true, true})
+ , nlsr(g_ioService, g_scheduler, face, g_keyChain)
+ , namePrefixList(nlsr.getNamePrefixList())
+ , updatePrefixUpdateProcessor(nlsr.getPrefixUpdateProcessor())
+ {
+ // Add an adjacency to nlsr
+ Adjacent adj("/ndn/edu/test-site-2/%C1.Router/test",
+ ndn::util::FaceUri("udp://1.0.0.2"), 10, Adjacent::STATUS_INACTIVE, 0, 0);
+ nlsr.getAdjacencyList().insert(adj);
+
+ // Create a face dataset response with the face having the same uri as
+ // the adjacent
+ ndn::nfd::FaceStatus payload1;
+ payload1.setFaceId(25401);
+ payload1.setRemoteUri("udp://1.0.0.2");
+
+ std::vector<ndn::nfd::FaceStatus> faces{payload1};
+
+ // Set the network so the LSA prefix is constructed
+ nlsr.getConfParameter().setNetwork("/ndn");
+
+ // So that NLSR starts listening on prefixes
+ nlsr.initialize();
+
+ // Simulate a callback with fake response
+ // This will trigger the undefined behavior found
+ // in fib.cpp if an operation is done on non-existent faceUri
+ nlsr.processFaceDataset(faces);
+
+ this->advanceClocks(ndn::time::milliseconds(1));
+
+ face.sentInterests.clear();
+ }
+
+public:
+ ndn::util::DummyClientFace face;
+ ndn::KeyChain keyChain;
+
+ Nlsr nlsr;
+ NamePrefixList& namePrefixList;
+ PrefixUpdateProcessor& updatePrefixUpdateProcessor;
+};
+
+BOOST_FIXTURE_TEST_CASE(TestAdvertiseCrash, AdvertiseCrashFixture)
+{
+ // Note that this test does not setup any security
+ // and the interest will be rejected by NLSR.
+ // This is because the bug triggers upon merely receiving
+ // an advertise interest so security was not needed.
+
+ // Advertise
+ ndn::nfd::ControlParameters parameters;
+ parameters.setName("/prefix/to/advertise/");
+ ndn::Name advertiseCommand("/localhost/nlsr/prefix-update/advertise");
+ advertiseCommand.append(parameters.wireEncode());
+
+ std::shared_ptr<ndn::Interest> advertiseInterest = std::make_shared<ndn::Interest>(advertiseCommand);
+
+ face.receive(*advertiseInterest);
+ this->advanceClocks(ndn::time::milliseconds(10));
+}
+
+} // namespace test
+} // namespace update
+} // namespace nlsr