Don't build Adjacency LSA when destroyed face belongs to INACTIVE node
refs: #2733
Change-Id: I2a53bce876486980b380e9d8226b410bc754b034
diff --git a/src/lsdb.hpp b/src/lsdb.hpp
index 7ab54e5..3cf74c6 100644
--- a/src/lsdb.hpp
+++ b/src/lsdb.hpp
@@ -172,10 +172,12 @@
void
exprireOrRefreshNameLsa(const ndn::Name& lsaKey, uint64_t seqNo);
+PUBLIC_WITH_TESTS_ELSE_PRIVATE:
ndn::EventId
scheduleAdjLsaExpiration(const ndn::Name& key, int seqNo,
const seconds& expTime);
+private:
void
exprireOrRefreshAdjLsa(const ndn::Name& lsaKey, uint64_t seqNo);
diff --git a/src/nlsr.cpp b/src/nlsr.cpp
index 5d40002..2d58b19 100644
--- a/src/nlsr.cpp
+++ b/src/nlsr.cpp
@@ -296,13 +296,28 @@
_LOG_DEBUG("Face to " << adjacent->getName() << " with face id: " << faceId << " destroyed");
adjacent->setFaceId(0);
- adjacent->setStatus(Adjacent::STATUS_INACTIVE);
- // A new adjacency LSA cannot be built until the neighbor is marked INACTIVE and
- // has met the HELLO retry threshold
- adjacent->setInterestTimedOutNo(m_confParam.getInterestRetryNumber());
+ // Only trigger an Adjacency LSA build if this node is changing from ACTIVE to INACTIVE
+ // since this rebuild will effectively cancel the previous Adjacency LSA refresh event
+ // and schedule a new one further in the future.
+ //
+ // Continuously scheduling the refresh in the future will block the router from refreshing
+ // its Adjacency LSA. Since other routers' Name prefixes' expiration times are updated
+ // when this router refreshes its Adjacency LSA, the other routers' prefixes will expire
+ // and be removed from the RIB.
+ //
+ // This check is required to fix Bug #2733 for now. This check would be unnecessary
+ // to fix Bug #2733 when Issue #2732 is completed, but the check also helps with
+ // optimization so it can remain even when Issue #2732 is implemented.
+ if (adjacent->getStatus() == Adjacent::STATUS_ACTIVE) {
+ adjacent->setStatus(Adjacent::STATUS_INACTIVE);
- m_nlsrLsdb.scheduleAdjLsaBuild();
+ // A new adjacency LSA cannot be built until the neighbor is marked INACTIVE and
+ // has met the HELLO retry threshold
+ adjacent->setInterestTimedOutNo(m_confParam.getInterestRetryNumber());
+
+ m_nlsrLsdb.scheduleAdjLsaBuild();
+ }
}
}
}
diff --git a/tests/test-nlsr.cpp b/tests/test-nlsr.cpp
index 74a3f47..ed41d9b 100644
--- a/tests/test-nlsr.cpp
+++ b/tests/test-nlsr.cpp
@@ -21,6 +21,7 @@
**/
#include "test-common.hpp"
+#include "control-commands.hpp"
#include "dummy-face.hpp"
#include "nlsr.hpp"
@@ -183,6 +184,101 @@
BOOST_CHECK(rtEntry == nullptr);
}
+// Bug #2733
+// This test checks that when a face for an inactive node is destroyed, an
+// Adjacency LSA build does not postpone the LSA refresh and cause RIB
+// entries for other nodes' name prefixes to not be refreshed.
+//
+// This test is invalid when Issue #2732 is implemented since an Adjacency LSA
+// refresh will not cause RIB entries for other nodes' name prefixes to be refreshed.
+BOOST_FIXTURE_TEST_CASE(FaceDestroyEventInactive, UnitTestTimeFixture)
+{
+ shared_ptr<ndn::util::DummyClientFace> face = ndn::util::makeDummyClientFace(g_ioService);
+ Nlsr nlsr(g_ioService, g_scheduler, ndn::ref(*face));
+ Lsdb& lsdb = nlsr.getLsdb();
+
+ // Simulate loading configuration file
+ ConfParameter& conf = nlsr.getConfParameter();
+ conf.setNetwork("/ndn");
+ conf.setSiteName("/site");
+ conf.setRouterName("/%C1.router/this-router");
+ conf.setFirstHelloInterval(0);
+ conf.setAdjLsaBuildInterval(0);
+ conf.setRoutingCalcInterval(0);
+
+ // Add neighbors
+ AdjacencyList& neighbors = nlsr.getAdjacencyList();
+
+ uint64_t destroyFaceId = 128;
+
+ // Create an inactive neighbor whose Face will be destroyed
+ Adjacent failNeighbor("/ndn/neighborA", "uri://faceA", 10, Adjacent::STATUS_INACTIVE, 3,
+ destroyFaceId);
+ neighbors.insert(failNeighbor);
+
+ // Create an additional active neighbor so an adjacency LSA can be built
+ Adjacent otherNeighbor("/ndn/neighborB", "uri://faceB", 25, Adjacent::STATUS_ACTIVE, 0, 256);
+ neighbors.insert(otherNeighbor);
+
+ // Add a name for the neighbor to advertise
+ NamePrefixList nameList;
+ ndn::Name nameToAdvertise("/ndn/neighborB/name");
+ nameList.insert(nameToAdvertise);
+
+ NameLsa nameLsa("/ndn/neighborB", "name", 25, ndn::time::system_clock::now(), nameList);
+ lsdb.installNameLsa(nameLsa);
+
+ nlsr.initialize();
+
+ // Simulate successful HELLO responses from neighbor B
+ lsdb.scheduleAdjLsaBuild();
+
+ // Run the scheduler to build an adjacency LSA
+ this->advanceClocks(ndn::time::milliseconds(1));
+
+ ndn::Name key = ndn::Name(nlsr.getConfParameter().getRouterPrefix()).append(AdjLsa::TYPE_STRING);
+ AdjLsa* lsa = lsdb.findAdjLsa(key);
+ BOOST_REQUIRE(lsa != nullptr);
+
+ // Cancel previous LSA expiration event
+ g_scheduler.cancelEvent(lsa->getExpiringEventId());
+
+ // Set expiration time for own Adjacency LSA to earlier value for unit-test
+ //
+ // Expiration time is negative to offset the GRACE_PERIOD (10 seconds) automatically applied
+ // to expiration times
+ ndn::EventId id = lsdb.scheduleAdjLsaExpiration(key, lsa->getLsSeqNo(), -ndn::time::seconds(9));
+ lsa->setExpiringEventId(id);
+
+ // Generate a FaceEventDestroyed notification
+ ndn::nfd::FaceEventNotification event;
+ event.setKind(ndn::nfd::FACE_EVENT_DESTROYED)
+ .setFaceId(destroyFaceId);
+
+ shared_ptr<ndn::Data> data = make_shared<ndn::Data>("/localhost/nfd/faces/events/%FE%00");
+ data->setContent(event.wireEncode());
+ nlsr.getKeyChain().sign(*data);
+
+ // Receive the FaceEventDestroyed notification
+ face->receive(*data);
+
+ // Run the scheduler to expire the Adjacency LSA. The expiration should refresh the RIB
+ // entries associated with Neighbor B's advertised prefix.
+ face->sentInterests.clear();
+ this->advanceClocks(ndn::time::seconds(1));
+
+ // The Face should have two sent Interests: the face event and a RIB registration
+ BOOST_REQUIRE(face->sentInterests.size() > 0);
+ const ndn::Interest& interest = face->sentInterests.back();
+
+ ndn::nfd::ControlParameters parameters;
+ ndn::Name::Component verb;
+ BOOST_REQUIRE_NO_THROW(extractRibCommandParameters(interest, verb, parameters));
+
+ BOOST_CHECK_EQUAL(verb, ndn::Name::Component("register"));
+ BOOST_CHECK_EQUAL(parameters.getName(), nameToAdvertise);
+}
+
BOOST_AUTO_TEST_SUITE_END()
} //namespace test