fib: fix route unregister bug during update
refs: #5179
Change-Id: I19952ebcb0c78c405f3d820694a7b0f1cf7a3fe6
diff --git a/src/route/fib.cpp b/src/route/fib.cpp
index 1004889..2d34a36 100644
--- a/src/route/fib.cpp
+++ b/src/route/fib.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2020, The University of Memphis,
+ * Copyright (c) 2014-2021, The University of Memphis,
* Regents of the University of California
*
* This file is part of NLSR (Named-data Link State Routing).
@@ -54,7 +54,7 @@
// Only unregister the prefix if it ISN'T a neighbor.
if (it != m_table.end() && isNotNeighbor((it->second).name)) {
- for (const auto& nexthop : (it->second).nexthopList.getNextHops()) {
+ for (const auto& nexthop : (it->second).nexthopSet) {
unregisterPrefix((it->second).name, nexthop.getConnectingFaceUri());
}
m_table.erase(it);
@@ -62,16 +62,17 @@
}
void
-Fib::addNextHopsToFibEntryAndNfd(FibEntry& entry, const NexthopList& hopsToAdd)
+Fib::addNextHopsToFibEntryAndNfd(FibEntry& entry, const NextHopsUriSortedSet& hopsToAdd)
{
const ndn::Name& name = entry.name;
bool shouldRegister = isNotNeighbor(name);
- for (const auto& hop : hopsToAdd.getNextHops())
+ for (const auto& hop : hopsToAdd)
{
// Add nexthop to FIB entry
- entry.nexthopList.addNextHop(hop);
+ NLSR_LOG_DEBUG("Adding " << hop.getConnectingFaceUri() << " to " << entry.name);
+ entry.nexthopSet.addNextHop(hop);
if (shouldRegister) {
// Add nexthop to NDN-FIB
@@ -92,7 +93,7 @@
// the length of the list of all next hops.
unsigned int maxFaces = getNumberOfFacesForName(allHops);
- NexthopList hopsToAdd;
+ NextHopsUriSortedSet hopsToAdd;
unsigned int nFaces = 0;
// Create a list of next hops to be installed with length == maxFaces
@@ -129,10 +130,11 @@
FibEntry& entry = (entryIt->second);
addNextHopsToFibEntryAndNfd(entry, hopsToAdd);
- std::set<NextHop, NextHopComparator> hopsToRemove;
- std::set_difference(entry.nexthopList.begin(), entry.nexthopList.end(),
+ std::set<NextHop, NextHopUriSortedComparator> hopsToRemove;
+ std::set_difference(entry.nexthopSet.begin(), entry.nexthopSet.end(),
hopsToAdd.begin(), hopsToAdd.end(),
- std::inserter(hopsToRemove, hopsToRemove.end()), NextHopComparator());
+ std::inserter(hopsToRemove, hopsToRemove.begin()),
+ NextHopUriSortedComparator());
bool isUpdatable = isNotNeighbor(entry.name);
// Remove the uninstalled next hops from NFD and FIB entry
@@ -141,7 +143,7 @@
unregisterPrefix(entry.name, hop.getConnectingFaceUri());
}
NLSR_LOG_DEBUG("Removing " << hop.getConnectingFaceUri() << " from " << entry.name);
- entry.nexthopList.removeNextHop(hop);
+ entry.nexthopSet.removeNextHop(hop);
}
// Increment sequence number
@@ -161,7 +163,7 @@
{
NLSR_LOG_DEBUG("Clean called");
for (const auto& it : m_table) {
- for (const auto& hop : it.second.nexthopList.getNextHops()) {
+ for (const auto& hop : it.second.nexthopSet) {
unregisterPrefix(it.second.name, hop.getConnectingFaceUri());
}
}
@@ -339,7 +341,7 @@
entry.seqNo += 1;
- for (const NextHop& hop : entry.nexthopList) {
+ for (const NextHop& hop : entry.nexthopSet) {
registerPrefix(entry.name,
ndn::FaceUri(hop.getConnectingFaceUri()),
hop.getRouteCostAsAdjustedInteger(),
@@ -355,9 +357,9 @@
{
NLSR_LOG_DEBUG("-------------------FIB-----------------------------");
for (const auto& entry : m_table) {
- NLSR_LOG_DEBUG("Name Prefix: " << entry.second.name);
+ NLSR_LOG_DEBUG("Name prefix: " << entry.first);
NLSR_LOG_DEBUG("Seq No: " << entry.second.seqNo);
- NLSR_LOG_DEBUG(entry.second.nexthopList);
+ NLSR_LOG_DEBUG("Nexthop List: \n" << entry.second.nexthopSet);
}
}
diff --git a/src/route/fib.hpp b/src/route/fib.hpp
index e8f8a40..25b5956 100644
--- a/src/route/fib.hpp
+++ b/src/route/fib.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2020, The University of Memphis,
+ * Copyright (c) 2014-2021, The University of Memphis,
* Regents of the University of California,
* Arizona Board of Regents.
*
@@ -31,11 +31,13 @@
namespace nlsr {
+typedef NexthopListT<NextHopUriSortedComparator> NextHopsUriSortedSet;
+
struct FibEntry {
ndn::Name name;
ndn::scheduler::ScopedEventId refreshEventId;
int32_t seqNo = 1;
- NexthopList nexthopList;
+ NextHopsUriSortedSet nexthopSet;
};
typedef std::function<void(FibEntry&)> afterRefreshCallback;
@@ -153,7 +155,7 @@
* \sa Fib::removeOldNextHopsFromFibEntryAndNfd
*/
void
- addNextHopsToFibEntryAndNfd(FibEntry& entry, const NexthopList& hopsToAdd);
+ addNextHopsToFibEntryAndNfd(FibEntry& entry, const NextHopsUriSortedSet& hopsToAdd);
unsigned int
getNumberOfFacesForName(const NexthopList& nextHopList);
diff --git a/src/route/nexthop-list.cpp b/src/route/nexthop-list.cpp
deleted file mode 100644
index 7ab1352..0000000
--- a/src/route/nexthop-list.cpp
+++ /dev/null
@@ -1,102 +0,0 @@
-/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/*
- * Copyright (c) 2014-2020, 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 "nexthop-list.hpp"
-#include "common.hpp"
-#include "nexthop.hpp"
-
-#include <ndn-cxx/util/ostream-joiner.hpp>
-
-namespace nlsr {
-
-static bool
-nexthopAddCompare(const NextHop& nh1, const NextHop& nh2)
-{
- return nh1.getConnectingFaceUri() == nh2.getConnectingFaceUri();
-}
-
-static bool
-nexthopRemoveCompare(const NextHop& nh1, const NextHop& nh2)
-{
- return (nh1.getConnectingFaceUri() == nh2.getConnectingFaceUri() &&
- nh1.getRouteCostAsAdjustedInteger() == nh2.getRouteCostAsAdjustedInteger()) ;
-}
-
-bool
-operator==(const NexthopList& lhs, const NexthopList& rhs)
-{
- if (lhs.size() != rhs.size()) {
- return false;
- }
-
- NexthopList slhs = lhs;
- NexthopList srhs = rhs;
-
- for (struct {std::set<NextHop>::iterator lItr;
- std::set<NextHop>::iterator rItr;} pair = {slhs.begin(), srhs.begin()};
- (pair.lItr != slhs.end() || pair.rItr != srhs.end());
- pair.rItr++, pair.lItr++) {
- if (!((*pair.lItr) == (*pair.rItr))) {
- return false;
- }
- }
- return true;
-}
-
-bool
-operator!=(const NexthopList& lhs, const NexthopList& rhs)
-{
- return !(lhs == rhs);
-}
-
-std::ostream&
-operator<<(std::ostream& os, const NexthopList& nhl)
-{
- os << " ";
- std::copy(nhl.cbegin(), nhl.cend(), ndn::make_ostream_joiner(os, "\n "));
- return os;
-}
-
-void
-NexthopList::addNextHop(const NextHop& nh)
-{
- auto it = std::find_if(m_nexthopList.begin(), m_nexthopList.end(),
- std::bind(&nexthopAddCompare, _1, nh));
- if (it == m_nexthopList.end()) {
- m_nexthopList.insert(nh);
- }
- else if (it->getRouteCost() > nh.getRouteCost()) {
- removeNextHop(*it);
- m_nexthopList.insert(nh);
- }
-}
-
-void
-NexthopList::removeNextHop(const NextHop& nh)
-{
- auto it = std::find_if(m_nexthopList.begin(), m_nexthopList.end(),
- std::bind(&nexthopRemoveCompare, _1, nh));
- if (it != m_nexthopList.end()) {
- m_nexthopList.erase(it);
- }
-}
-
-} // namespace nlsr
diff --git a/src/route/nexthop-list.hpp b/src/route/nexthop-list.hpp
index 537448e..98e5760 100644
--- a/src/route/nexthop-list.hpp
+++ b/src/route/nexthop-list.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2020, The University of Memphis,
+ * Copyright (c) 2014-2021, The University of Memphis,
* Regents of the University of California
*
* This file is part of NLSR (Named-data Link State Routing).
@@ -25,6 +25,8 @@
#include "adjacent.hpp"
#include <ndn-cxx/face.hpp>
+#include <ndn-cxx/util/ostream-joiner.hpp>
+
#include <set>
namespace nlsr {
@@ -44,10 +46,31 @@
}
};
-class NexthopList
+struct NextHopUriSortedComparator {
+ bool
+ operator() (const NextHop& nh1, const NextHop& nh2) const {
+ return nh1.getConnectingFaceUri() < nh2.getConnectingFaceUri();
+ }
+};
+
+static inline bool
+nexthopAddCompare(const NextHop& nh1, const NextHop& nh2)
+{
+ return nh1.getConnectingFaceUri() == nh2.getConnectingFaceUri();
+}
+
+static inline bool
+nexthopRemoveCompare(const NextHop& nh1, const NextHop& nh2)
+{
+ return (nh1.getConnectingFaceUri() == nh2.getConnectingFaceUri() &&
+ nh1.getRouteCostAsAdjustedInteger() == nh2.getRouteCostAsAdjustedInteger()) ;
+}
+
+template<typename T = NextHopComparator>
+class NexthopListT
{
public:
- NexthopList() = default;
+ NexthopListT() = default;
/*! \brief Adds a next hop to the list.
\param nh The next hop.
@@ -57,7 +80,18 @@
hop's route cost is updated.
*/
void
- addNextHop(const NextHop& nh);
+ addNextHop(const NextHop& nh)
+ {
+ auto it = std::find_if(m_nexthopList.begin(), m_nexthopList.end(),
+ std::bind(&nexthopAddCompare, _1, nh));
+ if (it == m_nexthopList.end()) {
+ m_nexthopList.insert(nh);
+ }
+ else if (it->getRouteCost() > nh.getRouteCost()) {
+ removeNextHop(*it);
+ m_nexthopList.insert(nh);
+ }
+ }
/*! \brief Remove a next hop from the Next Hop list
\param nh The NextHop we want to remove.
@@ -65,7 +99,14 @@
The next hop gets removed only if both next hop face and route cost are same.
*/
void
- removeNextHop(const NextHop& nh);
+ removeNextHop(const NextHop& nh)
+ {
+ auto it = std::find_if(m_nexthopList.begin(), m_nexthopList.end(),
+ std::bind(&nexthopRemoveCompare, _1, nh));
+ if (it != m_nexthopList.end()) {
+ m_nexthopList.erase(it);
+ }
+ }
size_t
size() const
@@ -79,24 +120,25 @@
m_nexthopList.clear();
}
- const std::set<NextHop, NextHopComparator>&
+ const std::set<NextHop, T>&
getNextHops() const
{
return m_nexthopList;
}
- typedef std::set<NextHop, NextHopComparator>::iterator iterator;
- typedef std::set<NextHop, NextHopComparator>::const_iterator const_iterator;
- typedef std::set<NextHop, NextHopComparator>::reverse_iterator reverse_iterator;
+ typedef T value_type;
+ typedef typename std::set<NextHop, T>::iterator iterator;
+ typedef typename std::set<NextHop, T>::const_iterator const_iterator;
+ typedef typename std::set<NextHop, T>::reverse_iterator reverse_iterator;
iterator
- begin()
+ begin() const
{
return m_nexthopList.begin();
}
iterator
- end()
+ end() const
{
return m_nexthopList.end();
}
@@ -126,20 +168,33 @@
}
private:
- std::set<NextHop, NextHopComparator> m_nexthopList;
+ std::set<NextHop, T> m_nexthopList;
};
-bool
-operator==(NexthopList& lhs, NexthopList& rhs);
+typedef NexthopListT<> NexthopList;
+template<typename T>
bool
-operator==(const NexthopList& lhs, const NexthopList& rhs);
+operator==(const NexthopListT<T>& lhs, const NexthopListT<T>& rhs)
+{
+ return lhs.getNextHops() == rhs.getNextHops();
+}
+template<typename T>
bool
-operator!=(const NexthopList& lhs, const NexthopList& rhs);
+operator!=(const NexthopListT<T>& lhs, const NexthopListT<T>& rhs)
+{
+ return !(lhs == rhs);
+}
+template<typename T>
std::ostream&
-operator<<(std::ostream& os, const NexthopList& nhl);
+operator<<(std::ostream& os, const NexthopListT<T>& nhl)
+{
+ os << " ";
+ std::copy(nhl.cbegin(), nhl.cend(), ndn::make_ostream_joiner(os, "\n "));
+ return os;
+}
} // namespace nlsr