fw: delegate sending Nack-Duplicate to forwarding strategy
This commit introduces a new strategy trigger (onInterestLoop) that
is invoked when a duplicate Interest is received. The default behavior
of the new trigger is to send a Nack as before. We also modify the
MulticastStrategy to override onInterestLoop and skip sending the Nack.
Refs: #5278
Co-authored-by: Davide Pesavento <davidepesa@gmail.com>
Change-Id: I07bb520bd31cbfc8d8f8581e05c49927f8ea35f7
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index ff21ee7..906c2f2 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-2023, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -125,7 +125,7 @@
// detect duplicate Nonce with Dead Nonce List
bool hasDuplicateNonceInDnl = m_deadNonceList.has(interest.getName(), nonce);
if (hasDuplicateNonceInDnl) {
- // goto Interest loop pipeline
+ // go to Interest loop pipeline
this->onInterestLoop(interest, ingress);
return;
}
@@ -149,7 +149,7 @@
hasDuplicateNonceInPit = hasDuplicateNonceInPit && !(dnw & fw::DUPLICATE_NONCE_IN_SAME);
}
if (hasDuplicateNonceInPit) {
- // goto Interest loop pipeline
+ // go to Interest loop pipeline
this->onInterestLoop(interest, ingress);
return;
}
@@ -176,14 +176,10 @@
}
NFD_LOG_DEBUG("onInterestLoop in=" << ingress << " interest=" << interest.getName()
- << " nonce=" << interest.getNonce() << " nack");
+ << " nonce=" << interest.getNonce());
- // send Nack with reason=DUPLICATE
- // note: Don't enter outgoing Nack pipeline because it needs an in-record.
- lp::Nack nack(interest);
- nack.setReason(lp::NackReason::DUPLICATE);
- ingress.face.sendNack(nack);
- ++m_counters.nOutNacks;
+ // leave loop handling up to the strategy (e.g., whether to reply with a Nack)
+ m_strategyChoice.findEffectiveStrategy(interest.getName()).onInterestLoop(interest, ingress);
}
void
@@ -317,7 +313,7 @@
// PIT match
pit::DataMatchResult pitMatches = m_pit.findAllDataMatches(data);
if (pitMatches.size() == 0) {
- // goto Data unsolicited pipeline
+ // go to Data unsolicited pipeline
this->onDataUnsolicited(data, ingress);
return;
}
@@ -387,7 +383,7 @@
pendingDownstream->getLinkType() != ndn::nfd::LINK_TYPE_AD_HOC) {
continue;
}
- // goto outgoing Data pipeline
+ // go to outgoing Data pipeline
this->onOutgoingData(data, *pendingDownstream);
}
}
@@ -415,7 +411,6 @@
NFD_LOG_WARN("onOutgoingData out=(invalid) data=" << data.getName());
return false;
}
- NFD_LOG_DEBUG("onOutgoingData out=" << egress.getId() << " data=" << data.getName());
// /localhost scope control
bool isViolatingLocalhost = egress.getScope() == ndn::nfd::FACE_SCOPE_NON_LOCAL &&
@@ -427,7 +422,7 @@
return false;
}
- // TODO traffic manager
+ NFD_LOG_DEBUG("onOutgoingData out=" << egress.getId() << " data=" << data.getName());
// send Data
egress.sendData(data);
@@ -486,7 +481,7 @@
this->setExpiryTimer(pitEntry, 0_ms);
}
- // trigger strategy: after receive NACK
+ // trigger strategy: after receive Nack
m_strategyChoice.findEffectiveStrategy(*pitEntry).afterReceiveNack(nack, ingress, pitEntry);
}
@@ -495,8 +490,8 @@
const shared_ptr<pit::Entry>& pitEntry)
{
if (egress.getId() == face::INVALID_FACEID) {
- NFD_LOG_WARN("onOutgoingNack out=(invalid)"
- << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason());
+ NFD_LOG_WARN("onOutgoingNack out=(invalid)" << " nack=" << pitEntry->getName()
+ << "~" << nack.getReason());
return false;
}
@@ -505,23 +500,20 @@
// if no in-record found, drop
if (inRecord == pitEntry->in_end()) {
- NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId()
- << " nack=" << pitEntry->getInterest().getName()
+ NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId() << " nack=" << pitEntry->getName()
<< "~" << nack.getReason() << " no-in-record");
return false;
}
// if multi-access or ad hoc face, drop
if (egress.getLinkType() != ndn::nfd::LINK_TYPE_POINT_TO_POINT) {
- NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId()
- << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason()
- << " link-type=" << egress.getLinkType());
+ NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId() << " nack=" << pitEntry->getName()
+ << "~" << nack.getReason() << " link-type=" << egress.getLinkType());
return false;
}
- NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId()
- << " nack=" << pitEntry->getInterest().getName()
- << "~" << nack.getReason() << " OK");
+ NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId() << " nack=" << pitEntry->getName()
+ << "~" << nack.getReason());
// create Nack packet with the Interest from in-record
lp::Nack nackPkt(inRecord->getInterest());
diff --git a/daemon/fw/multicast-strategy.cpp b/daemon/fw/multicast-strategy.cpp
index 08eda9a..85b0535 100644
--- a/daemon/fw/multicast-strategy.cpp
+++ b/daemon/fw/multicast-strategy.cpp
@@ -53,7 +53,7 @@
const Name&
MulticastStrategy::getStrategyName()
{
- static const auto strategyName = Name("/localhost/nfd/strategy/multicast").appendVersion(4);
+ static const auto strategyName = Name("/localhost/nfd/strategy/multicast").appendVersion(5);
return strategyName;
}
diff --git a/daemon/fw/multicast-strategy.hpp b/daemon/fw/multicast-strategy.hpp
index da01018..90e13fd 100644
--- a/daemon/fw/multicast-strategy.hpp
+++ b/daemon/fw/multicast-strategy.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2022, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -49,6 +49,12 @@
const shared_ptr<pit::Entry>& pitEntry) override;
void
+ onInterestLoop(const Interest& interest, const FaceEndpoint& ingress) override
+ {
+ // do nothing
+ }
+
+ void
afterNewNextHop(const fib::NextHop& nextHop, const shared_ptr<pit::Entry>& pitEntry) override;
NFD_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
diff --git a/daemon/fw/strategy.cpp b/daemon/fw/strategy.cpp
index d47df3a..10610ad 100644
--- a/daemon/fw/strategy.cpp
+++ b/daemon/fw/strategy.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2022, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -174,6 +174,16 @@
Strategy::~Strategy() = default;
void
+Strategy::onInterestLoop(const Interest& interest, const FaceEndpoint& ingress)
+{
+ NFD_LOG_DEBUG("onInterestLoop in=" << ingress << " name=" << interest.getName());
+
+ lp::Nack nack(interest);
+ nack.setReason(lp::NackReason::DUPLICATE);
+ this->sendNack(nack, ingress.face);
+}
+
+void
Strategy::afterContentStoreHit(const Data& data, const FaceEndpoint& ingress,
const shared_ptr<pit::Entry>& pitEntry)
{
diff --git a/daemon/fw/strategy.hpp b/daemon/fw/strategy.hpp
index 460fbdc..c42a968 100644
--- a/daemon/fw/strategy.hpp
+++ b/daemon/fw/strategy.hpp
@@ -135,7 +135,7 @@
* The Interest:
* - has not exceeded HopLimit
* - does not violate Scope
- * - is not looped
+ * - has not looped
* - cannot be satisfied by ContentStore
* - is under a namespace managed by this strategy
*
@@ -161,6 +161,20 @@
const shared_ptr<pit::Entry>& pitEntry) = 0;
/**
+ * \brief Trigger after an Interest loop is detected.
+ *
+ * The Interest:
+ * - has not exceeded HopLimit
+ * - does not violate Scope
+ * - has looped
+ * - is under a namespace managed by this strategy
+ *
+ * In the base class, this method sends a Nack with reason DUPLICATE to \p ingress.
+ */
+ virtual void
+ onInterestLoop(const Interest& interest, const FaceEndpoint& ingress);
+
+ /**
* \brief Trigger after a matching Data is found in the Content Store.
*
* In the base class, this method sends \p data to \p ingress.
@@ -340,6 +354,21 @@
}
/**
+ * \brief Send a Nack packet without going through the outgoing Nack pipeline.
+ *
+ * \param nack the Nack packet
+ * \param egress face through which to send out the Nack
+ * \return Whether the Nack was sent (true) or dropped (false)
+ */
+ NFD_VIRTUAL_WITH_TESTS bool
+ sendNack(const lp::Nack& nack, Face& egress)
+ {
+ egress.sendNack(nack);
+ ++m_forwarder.m_counters.nOutNacks;
+ return true;
+ }
+
+ /**
* \brief Send Nack to every face that has an in-record, except those in \p exceptFaces
* \param header the Nack header
* \param pitEntry the PIT entry
diff --git a/tests/daemon/fw/multicast-strategy.t.cpp b/tests/daemon/fw/multicast-strategy.t.cpp
index 55254da..b51b001 100644
--- a/tests/daemon/fw/multicast-strategy.t.cpp
+++ b/tests/daemon/fw/multicast-strategy.t.cpp
@@ -210,8 +210,28 @@
pitEntry->insertOrUpdateInRecord(*face1, *interest);
strategy.afterReceiveInterest(*interest, FaceEndpoint(*face1), pitEntry);
- BOOST_CHECK_EQUAL(strategy.rejectPendingInterestHistory.size(), 0);
- BOOST_CHECK_EQUAL(strategy.sendInterestHistory.size(), 0);
+ BOOST_TEST(strategy.rejectPendingInterestHistory.size() == 0);
+ BOOST_TEST(strategy.sendInterestHistory.size() == 0);
+}
+
+BOOST_AUTO_TEST_CASE(DuplicateInterest)
+{
+ fib::Entry& fibEntry = *fib.insert(Name()).first;
+ fib.addOrUpdateNextHop(fibEntry, *face3, 0);
+
+ auto interest = makeInterest("ndn:/H0D6i5fc");
+
+ // first interest
+ forwarder.onIncomingInterest(*interest, FaceEndpoint(*face1));
+ BOOST_TEST(forwarder.getCounters().nInInterests == 1);
+ BOOST_TEST(strategy.sendInterestHistory.size() == 1);
+
+ // second interest (duplicate, should enter onInterestLoop)
+ forwarder.onIncomingInterest(*interest, FaceEndpoint(*face2));
+ BOOST_TEST(forwarder.getCounters().nInInterests == 2);
+ BOOST_TEST(strategy.sendInterestHistory.size() == 1);
+ BOOST_TEST(strategy.rejectPendingInterestHistory.size() == 0);
+ BOOST_TEST(strategy.sendNackHistory.size() == 0);
}
BOOST_AUTO_TEST_CASE(RetxSuppression)
diff --git a/tests/daemon/fw/strategy-instantiation.t.cpp b/tests/daemon/fw/strategy-instantiation.t.cpp
index 535b4b0..ae18eea 100644
--- a/tests/daemon/fw/strategy-instantiation.t.cpp
+++ b/tests/daemon/fw/strategy-instantiation.t.cpp
@@ -64,7 +64,7 @@
Test<AccessStrategy, false, 1>,
Test<AsfStrategy, true, 5>,
Test<BestRouteStrategy, true, 5>,
- Test<MulticastStrategy, true, 4>,
+ Test<MulticastStrategy, true, 5>,
Test<SelfLearningStrategy, false, 1>,
Test<RandomStrategy, false, 1>
>;
diff --git a/tests/daemon/fw/strategy-tester.hpp b/tests/daemon/fw/strategy-tester.hpp
index e7dcccc..32785b8 100644
--- a/tests/daemon/fw/strategy-tester.hpp
+++ b/tests/daemon/fw/strategy-tester.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2022, Regents of the University of California,
+ * Copyright (c) 2014-2024, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -99,8 +99,7 @@
protected:
pit::OutRecord*
- sendInterest(const Interest& interest, Face& egress,
- const shared_ptr<pit::Entry>& pitEntry) override
+ sendInterest(const Interest& interest, Face& egress, const shared_ptr<pit::Entry>& pitEntry) override
{
sendInterestHistory.push_back({pitEntry->getInterest(), egress.getId(), interest});
auto it = pitEntry->insertOrUpdateOutRecord(egress, interest);
@@ -117,8 +116,7 @@
}
bool
- sendNack(const lp::NackHeader& header, Face& egress,
- const shared_ptr<pit::Entry>& pitEntry) override
+ sendNack(const lp::NackHeader& header, Face& egress, const shared_ptr<pit::Entry>& pitEntry) override
{
sendNackHistory.push_back({pitEntry->getInterest(), egress.getId(), header});
pitEntry->deleteInRecord(egress);
@@ -126,6 +124,14 @@
return true;
}
+ bool
+ sendNack(const lp::Nack& nack, Face& egress) override
+ {
+ sendNackHistory.push_back({nack.getInterest(), egress.getId(), nack.getHeader()});
+ afterAction();
+ return true;
+ }
+
public:
struct SendInterestArgs
{