From 195b78a53396b9169d1798c61f384f75b1befb8e Mon Sep 17 00:00:00 2001 From: Mark Theeranantachai Date: Wed, 14 Feb 2024 20:54:44 -0500 Subject: [PATCH] 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 Change-Id: I07bb520bd31cbfc8d8f8581e05c49927f8ea35f7 --- daemon/fw/forwarder.cpp | 42 ++++++++------------ daemon/fw/multicast-strategy.cpp | 2 +- daemon/fw/multicast-strategy.hpp | 8 +++- daemon/fw/strategy.cpp | 12 +++++- daemon/fw/strategy.hpp | 31 ++++++++++++++- tests/daemon/fw/multicast-strategy.t.cpp | 24 ++++++++++- tests/daemon/fw/strategy-instantiation.t.cpp | 2 +- tests/daemon/fw/strategy-tester.hpp | 16 +++++--- 8 files changed, 100 insertions(+), 37 deletions(-) diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp index ff21ee7c..906c2f23 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 @@ Forwarder::onIncomingInterest(const Interest& interest, const FaceEndpoint& ingr // 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 @@ Forwarder::onIncomingInterest(const Interest& interest, const FaceEndpoint& ingr 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 @@ Forwarder::onInterestLoop(const Interest& interest, const FaceEndpoint& ingress) } 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 @@ Forwarder::onIncomingData(const Data& data, const FaceEndpoint& ingress) // 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 @@ Forwarder::onIncomingData(const Data& data, const FaceEndpoint& ingress) 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 @@ Forwarder::onOutgoingData(const Data& data, Face& egress) 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 @@ Forwarder::onOutgoingData(const Data& data, Face& egress) return false; } - // TODO traffic manager + NFD_LOG_DEBUG("onOutgoingData out=" << egress.getId() << " data=" << data.getName()); // send Data egress.sendData(data); @@ -486,7 +481,7 @@ Forwarder::onIncomingNack(const lp::Nack& nack, const FaceEndpoint& ingress) 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 @@ Forwarder::onOutgoingNack(const lp::NackHeader& nack, Face& egress, const shared_ptr& 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 @@ Forwarder::onOutgoingNack(const lp::NackHeader& nack, Face& egress, // 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 08eda9ac..85b05354 100644 --- a/daemon/fw/multicast-strategy.cpp +++ b/daemon/fw/multicast-strategy.cpp @@ -53,7 +53,7 @@ MulticastStrategy::MulticastStrategy(Forwarder& forwarder, const Name& name) 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 da010184..90e13fd2 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, @@ -48,6 +48,12 @@ class MulticastStrategy : public Strategy afterReceiveInterest(const Interest& interest, const FaceEndpoint& ingress, const shared_ptr& pitEntry) override; + void + onInterestLoop(const Interest& interest, const FaceEndpoint& ingress) override + { + // do nothing + } + void afterNewNextHop(const fib::NextHop& nextHop, const shared_ptr& pitEntry) override; diff --git a/daemon/fw/strategy.cpp b/daemon/fw/strategy.cpp index d47df3a9..10610ad1 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, @@ -173,6 +173,16 @@ Strategy::Strategy(Forwarder& forwarder) 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& pitEntry) diff --git a/daemon/fw/strategy.hpp b/daemon/fw/strategy.hpp index 460fbdc9..c42a968c 100644 --- a/daemon/fw/strategy.hpp +++ b/daemon/fw/strategy.hpp @@ -135,7 +135,7 @@ class Strategy : noncopyable * 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 * @@ -160,6 +160,20 @@ class Strategy : noncopyable afterReceiveInterest(const Interest& interest, const FaceEndpoint& ingress, const shared_ptr& 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. * @@ -339,6 +353,21 @@ class Strategy : noncopyable return m_forwarder.onOutgoingNack(header, egress, pitEntry); } + /** + * \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 diff --git a/tests/daemon/fw/multicast-strategy.t.cpp b/tests/daemon/fw/multicast-strategy.t.cpp index 55254daf..b51b001e 100644 --- a/tests/daemon/fw/multicast-strategy.t.cpp +++ b/tests/daemon/fw/multicast-strategy.t.cpp @@ -210,8 +210,28 @@ BOOST_AUTO_TEST_CASE(LoopingInterest) 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 535b4b02..ae18eeaf 100644 --- a/tests/daemon/fw/strategy-instantiation.t.cpp +++ b/tests/daemon/fw/strategy-instantiation.t.cpp @@ -64,7 +64,7 @@ using Tests = boost::mp11::mp_list< Test, Test, Test, - Test, + Test, Test, Test >; diff --git a/tests/daemon/fw/strategy-tester.hpp b/tests/daemon/fw/strategy-tester.hpp index e7dcccc0..32785b84 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 @@ class StrategyTester : public S protected: pit::OutRecord* - sendInterest(const Interest& interest, Face& egress, - const shared_ptr& pitEntry) override + sendInterest(const Interest& interest, Face& egress, const shared_ptr& pitEntry) override { sendInterestHistory.push_back({pitEntry->getInterest(), egress.getId(), interest}); auto it = pitEntry->insertOrUpdateOutRecord(egress, interest); @@ -117,8 +116,7 @@ class StrategyTester : public S } bool - sendNack(const lp::NackHeader& header, Face& egress, - const shared_ptr& pitEntry) override + sendNack(const lp::NackHeader& header, Face& egress, const shared_ptr& pitEntry) override { sendNackHistory.push_back({pitEntry->getInterest(), egress.getId(), header}); pitEntry->deleteInRecord(egress); @@ -126,6 +124,14 @@ class StrategyTester : public S 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 {