fw: fix and simplify enumeration logic in Forwarder::onNewNextHop()
Also remove Strategy::enableNewNextHopTrigger(). Strategies can already
opt-in by overriding afterNewNextHop(), there is no need for a separate
enablement flag.
Refs: #4931
Change-Id: Ifcbdbc7e954bd20d9969fd99882a2a9aebbc8fcc
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 671c536..8301014 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2020, Regents of the University of California,
+ * Copyright (c) 2014-2021, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -76,8 +76,8 @@
cleanupOnFaceRemoval(m_nameTree, m_fib, m_pit, face);
});
- m_fib.afterNewNextHop.connect([&] (const Name& prefix, const fib::NextHop& nextHop) {
- this->startProcessNewNextHop(prefix, nextHop);
+ m_fib.afterNewNextHop.connect([this] (const Name& prefix, const fib::NextHop& nextHop) {
+ this->onNewNextHop(prefix, nextHop);
});
m_strategyChoice.setDefaultStrategy(getDefaultStrategyName());
@@ -213,7 +213,7 @@
// dispatch to strategy: after incoming Interest
this->dispatchToStrategy(*pitEntry,
- [&] (fw::Strategy& strategy) {
+ [&] (auto& strategy) {
strategy.afterReceiveInterest(FaceEndpoint(ingress.face, 0), interest, pitEntry);
});
}
@@ -237,7 +237,7 @@
// dispatch to strategy: after Content Store hit
this->dispatchToStrategy(*pitEntry,
- [&] (fw::Strategy& strategy) { strategy.afterContentStoreHit(pitEntry, ingress, data); });
+ [&] (auto& strategy) { strategy.afterContentStoreHit(pitEntry, ingress, data); });
}
pit::OutRecord*
@@ -325,7 +325,7 @@
// trigger strategy: after receive Data
this->dispatchToStrategy(*pitEntry,
- [&] (fw::Strategy& strategy) { strategy.afterReceiveData(pitEntry, ingress, data); });
+ [&] (auto& strategy) { strategy.afterReceiveData(pitEntry, ingress, data); });
// mark PIT satisfied
pitEntry->isSatisfied = true;
@@ -358,7 +358,7 @@
// invoke PIT satisfy callback
this->dispatchToStrategy(*pitEntry,
- [&] (fw::Strategy& strategy) { strategy.beforeSatisfyInterest(pitEntry, ingress, data); });
+ [&] (auto& strategy) { strategy.beforeSatisfyInterest(pitEntry, ingress, data); });
// mark PIT satisfied
pitEntry->isSatisfied = true;
@@ -481,7 +481,7 @@
// trigger strategy: after receive NACK
this->dispatchToStrategy(*pitEntry,
- [&] (fw::Strategy& strategy) { strategy.afterReceiveNack(ingress, nack, pitEntry); });
+ [&] (auto& strategy) { strategy.afterReceiveNack(ingress, nack, pitEntry); });
}
bool
@@ -542,37 +542,21 @@
{
const auto affectedEntries = this->getNameTree().partialEnumerate(prefix,
[&] (const name_tree::Entry& nte) -> std::pair<bool, bool> {
- const fib::Entry* fibEntry = nte.getFibEntry();
- const fw::Strategy* strategy = nullptr;
- if (nte.getStrategyChoiceEntry() != nullptr) {
- strategy = &nte.getStrategyChoiceEntry()->getStrategy();
- }
- // current nte has buffered Interests but no fibEntry (except for the root nte) and the strategy
- // enables new nexthop behavior, we enumerate the current nte and keep visiting its children.
- if (nte.getName().size() == 0 ||
- (strategy != nullptr && strategy->wantNewNextHopTrigger() &&
- fibEntry == nullptr && nte.hasPitEntries())) {
- return {true, true};
- }
- // we don't need the current nte (no pitEntry or strategy doesn't support new nexthop), but
- // if the current nte has no fibEntry, it's still possible that its children are affected by
- // the new nexthop.
- else if (fibEntry == nullptr) {
- return {false, true};
- }
- // if the current nte has a fibEntry, we ignore the current nte and don't visit its
- // children because they are already covered by the current nte's fibEntry.
- else {
+ // we ignore an NTE and skip visiting its descendants if that NTE has an
+ // associated FIB entry (1st condition), since in that case the new nexthop
+ // won't affect any PIT entries anywhere in that subtree, *unless* this is
+ // the initial NTE from which the enumeration started (2nd condition), which
+ // must always be considered
+ if (nte.getFibEntry() != nullptr && nte.getName().size() > prefix.size()) {
return {false, false};
}
+ return {nte.hasPitEntries(), true};
});
for (const auto& nte : affectedEntries) {
for (const auto& pitEntry : nte.getPitEntries()) {
this->dispatchToStrategy(*pitEntry,
- [&] (fw::Strategy& strategy) {
- strategy.afterNewNextHop(nextHop, pitEntry);
- });
+ [&] (auto& strategy) { strategy.afterNewNextHop(nextHop, pitEntry); });
}
}
}
diff --git a/daemon/fw/forwarder.hpp b/daemon/fw/forwarder.hpp
index 4bdd42a..aaef44c 100644
--- a/daemon/fw/forwarder.hpp
+++ b/daemon/fw/forwarder.hpp
@@ -107,16 +107,6 @@
this->onIncomingNack(ingress, nack);
}
- /** \brief start new nexthop processing
- * \param prefix the prefix of the FibEntry containing the new nexthop
- * \param nextHop the new NextHop
- */
- void
- startProcessNewNextHop(const Name& prefix, const fib::NextHop& nextHop)
- {
- this->onNewNextHop(prefix, nextHop);
- }
-
NameTree&
getNameTree()
{
diff --git a/daemon/fw/strategy.hpp b/daemon/fw/strategy.hpp
index 571d7d1..3c6536e 100644
--- a/daemon/fw/strategy.hpp
+++ b/daemon/fw/strategy.hpp
@@ -114,14 +114,6 @@
return m_name;
}
- /** \return Whether the afterNewNextHop trigger should be invoked for this strategy.
- */
- bool
- wantNewNextHopTrigger() const
- {
- return m_wantNewNextHopTrigger;
- }
-
public: // triggers
/** \brief Trigger after Interest is received
*
@@ -388,15 +380,6 @@
m_name = name;
}
-NFD_PUBLIC_WITH_TESTS_ELSE_PROTECTED: // setter
- /** \brief Set whether the afterNewNextHop trigger should be invoked for this strategy
- */
- void
- enableNewNextHopTrigger(bool enabled)
- {
- m_wantNewNextHopTrigger = enabled;
- }
-
private: // registry
typedef std::function<unique_ptr<Strategy>(Forwarder& forwarder, const Name& strategyName)> CreateFunc;
typedef std::map<Name, CreateFunc> Registry; // indexed by strategy name
@@ -421,8 +404,6 @@
Forwarder& m_forwarder;
MeasurementsAccessor m_measurements;
-
- bool m_wantNewNextHopTrigger = false;
};
} // namespace fw