Skip to content

Commit

Permalink
fib: fix route unregister bug during update
Browse files Browse the repository at this point in the history 
refs: #5179

Change-Id: I19952ebcb0c78c405f3d820694a7b0f1cf7a3fe6
  • Loading branch information
@agawande
agawande committed Sep 7, 2021
1 parent 3781c7e commit 6f0f35d
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 171 deletions.
30 changes: 16 additions & 14 deletions src/route/fib.cpp
View file
Original file line number Diff line number Diff line change
@@ -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).
Expand Down Expand Up @@ -54,24 +54,25 @@ Fib::remove(const ndn::Name& name)

// 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);
}
}

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
Expand All @@ -92,7 +93,7 @@ Fib::update(const ndn::Name& name, const NexthopList& allHops)
// 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
Expand Down Expand Up @@ -129,10 +130,11 @@ Fib::update(const ndn::Name& name, const NexthopList& allHops)
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
Expand All @@ -141,7 +143,7 @@ Fib::update(const ndn::Name& name, const NexthopList& allHops)
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
Expand All @@ -161,7 +163,7 @@ Fib::clean()
{
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());
}
}
Expand Down Expand Up @@ -339,7 +341,7 @@ Fib::refreshEntry(const ndn::Name& name, afterRefreshCallback refreshCb)

entry.seqNo += 1;

for (const NextHop& hop : entry.nexthopList) {
for (const NextHop& hop : entry.nexthopSet) {
registerPrefix(entry.name,
ndn::FaceUri(hop.getConnectingFaceUri()),
hop.getRouteCostAsAdjustedInteger(),
Expand All @@ -355,9 +357,9 @@ Fib::writeLog()
{
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);
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/route/fib.hpp
View file
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -153,7 +155,7 @@ class Fib
* \sa Fib::removeOldNextHopsFromFibEntryAndNfd
*/
void
addNextHopsToFibEntryAndNfd(FibEntry& entry, const NexthopList& hopsToAdd);
addNextHopsToFibEntryAndNfd(FibEntry& entry, const NextHopsUriSortedSet& hopsToAdd);

unsigned int
getNumberOfFacesForName(const NexthopList& nextHopList);
Expand Down
102 changes: 0 additions & 102 deletions src/route/nexthop-list.cpp
View file

This file was deleted.

89 changes: 72 additions & 17 deletions src/route/nexthop-list.hpp
View file
Original file line number Diff line number Diff line change
@@ -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).
Expand All @@ -25,6 +25,8 @@
#include "adjacent.hpp"

#include <ndn-cxx/face.hpp>
#include <ndn-cxx/util/ostream-joiner.hpp>

#include <set>

namespace nlsr {
Expand All @@ -44,10 +46,31 @@ struct NextHopComparator {
}
};

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.
Expand All @@ -57,15 +80,33 @@ class NexthopList
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.
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
Expand All @@ -79,24 +120,25 @@ class NexthopList
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();
}
Expand Down Expand Up @@ -126,20 +168,33 @@ class NexthopList
}

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

Expand Down
Loading

0 comments on commit 6f0f35d

Please sign in to comment.