fw: Remove NACKs from multicast strategy
Change-Id: I49b833ec0ad990dfdf547aef80915e3fc83389ec
Refs: #5146
diff --git a/daemon/fw/multicast-strategy.cpp b/daemon/fw/multicast-strategy.cpp
index 3cb351c..7c12041 100644
--- a/daemon/fw/multicast-strategy.cpp
+++ b/daemon/fw/multicast-strategy.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,
@@ -39,7 +39,6 @@
MulticastStrategy::MulticastStrategy(Forwarder& forwarder, const Name& name)
: Strategy(forwarder)
- , ProcessNackTraits(this)
, m_retxSuppression(RETX_SUPPRESSION_INITIAL,
RetxSuppressionExponential::DEFAULT_MULTIPLIER,
RETX_SUPPRESSION_MAX)
@@ -58,7 +57,7 @@
const Name&
MulticastStrategy::getStrategyName()
{
- static Name strategyName("/localhost/nfd/strategy/multicast/%FD%03");
+ static Name strategyName("/localhost/nfd/strategy/multicast/%FD%04");
return strategyName;
}
@@ -89,23 +88,6 @@
m_retxSuppression.incrementIntervalForOutRecord(*pitEntry->getOutRecord(outFace));
}
}
-
- if (!hasPendingOutRecords(*pitEntry)) {
- NFD_LOG_DEBUG(interest << " from=" << ingress << " noNextHop (removing pitEntry)");
-
- lp::NackHeader nackHeader;
- nackHeader.setReason(lp::NackReason::NO_ROUTE);
- this->sendNack(pitEntry, ingress.face, nackHeader);
-
- this->rejectPendingInterest(pitEntry);
- }
-}
-
-void
-MulticastStrategy::afterReceiveNack(const FaceEndpoint& ingress, const lp::Nack& nack,
- const shared_ptr<pit::Entry>& pitEntry)
-{
- this->processNack(ingress.face, nack, pitEntry);
}
} // namespace fw
diff --git a/daemon/fw/multicast-strategy.hpp b/daemon/fw/multicast-strategy.hpp
index 3dd9f6c..b5bfb2d 100644
--- a/daemon/fw/multicast-strategy.hpp
+++ b/daemon/fw/multicast-strategy.hpp
@@ -27,7 +27,6 @@
#define NFD_DAEMON_FW_MULTICAST_STRATEGY_HPP
#include "strategy.hpp"
-#include "process-nack-traits.hpp"
#include "retx-suppression-exponential.hpp"
namespace nfd {
@@ -36,7 +35,6 @@
/** \brief A forwarding strategy that forwards Interests to all FIB nexthops
*/
class MulticastStrategy : public Strategy
- , public ProcessNackTraits<MulticastStrategy>
{
public:
explicit
@@ -49,12 +47,7 @@
afterReceiveInterest(const FaceEndpoint& ingress, const Interest& interest,
const shared_ptr<pit::Entry>& pitEntry) override;
- void
- afterReceiveNack(const FaceEndpoint& ingress, const lp::Nack& nack,
- const shared_ptr<pit::Entry>& pitEntry) override;
-
private:
- friend ProcessNackTraits<MulticastStrategy>;
RetxSuppressionExponential m_retxSuppression;
NFD_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
diff --git a/docs/manpages/nfdc-strategy.rst b/docs/manpages/nfdc-strategy.rst
index 2a6c41a..6084c05 100644
--- a/docs/manpages/nfdc-strategy.rst
+++ b/docs/manpages/nfdc-strategy.rst
@@ -63,8 +63,8 @@
nfdc strategy set prefix / strategy /localhost/nfd/strategy/best-route
Set the default strategy to best-route, latest version.
-nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/multicast/%FD%01
- Set the strategy of the "/ndn" prefix to multicast, version 1.
+nfdc strategy set prefix /ndn strategy /localhost/nfd/strategy/multicast/%FD%04
+ Set the strategy of the "/ndn" prefix to multicast, version 4.
nfdc strategy unset prefix /ndn
Clear the strategy choice for the "/ndn" prefix.
diff --git a/tests/daemon/fw/multicast-strategy.t.cpp b/tests/daemon/fw/multicast-strategy.t.cpp
index 6871821..d9a749c 100644
--- a/tests/daemon/fw/multicast-strategy.t.cpp
+++ b/tests/daemon/fw/multicast-strategy.t.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,
@@ -195,7 +195,7 @@
retxFrom4Evt.cancel();
}
-BOOST_AUTO_TEST_CASE(RejectLoopback)
+BOOST_AUTO_TEST_CASE(LoopingInterest)
{
fib::Entry& fibEntry = *fib.insert(Name()).first;
fib.addOrUpdateNextHop(fibEntry, *face1, 0);
@@ -205,7 +205,7 @@
pitEntry->insertOrUpdateInRecord(*face1, *interest);
strategy.afterReceiveInterest(FaceEndpoint(*face1, 0), *interest, pitEntry);
- BOOST_CHECK_EQUAL(strategy.rejectPendingInterestHistory.size(), 1);
+ BOOST_CHECK_EQUAL(strategy.rejectPendingInterestHistory.size(), 0);
BOOST_CHECK_EQUAL(strategy.sendInterestHistory.size(), 0);
}
diff --git a/tests/daemon/fw/strategy-instantiation.t.cpp b/tests/daemon/fw/strategy-instantiation.t.cpp
index 312294c..4ea9bb3 100644
--- a/tests/daemon/fw/strategy-instantiation.t.cpp
+++ b/tests/daemon/fw/strategy-instantiation.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, 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,
@@ -79,7 +79,7 @@
Test<AsfStrategy, true, 3>,
Test<BestRouteStrategy, false, 1>,
Test<BestRouteStrategy2, false, 5>,
- Test<MulticastStrategy, false, 3>,
+ Test<MulticastStrategy, false, 4>,
Test<NccStrategy, false, 1>,
Test<SelfLearningStrategy, false, 1>,
Test<RandomStrategy, false, 1>
diff --git a/tests/daemon/fw/strategy-nack-return.t.cpp b/tests/daemon/fw/strategy-nack-return.t.cpp
index 870253b..a370685 100644
--- a/tests/daemon/fw/strategy-nack-return.t.cpp
+++ b/tests/daemon/fw/strategy-nack-return.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, 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,
@@ -29,7 +29,6 @@
// Strategies implementing recommended Nack processing procedure, sorted alphabetically.
#include "fw/best-route-strategy2.hpp"
-#include "fw/multicast-strategy.hpp"
#include "fw/random-strategy.hpp"
#include "choose-strategy.hpp"
@@ -88,7 +87,6 @@
using Strategies = boost::mpl::vector<
BestRouteStrategy2,
- MulticastStrategy,
RandomStrategy
>;
diff --git a/tests/daemon/fw/strategy-no-route.t.cpp b/tests/daemon/fw/strategy-no-route.t.cpp
index 26b7874..2952259 100644
--- a/tests/daemon/fw/strategy-no-route.t.cpp
+++ b/tests/daemon/fw/strategy-no-route.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, 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,
@@ -32,7 +32,6 @@
// sorted alphabetically.
#include "fw/asf-strategy.hpp"
#include "fw/best-route-strategy2.hpp"
-#include "fw/multicast-strategy.hpp"
#include "fw/random-strategy.hpp"
#include "tests/test-common.hpp"
@@ -151,10 +150,6 @@
Test<BestRouteStrategy2, NextHopIsDownstream<BestRouteStrategy2>>,
Test<BestRouteStrategy2, NextHopViolatesScope<BestRouteStrategy2>>,
- Test<MulticastStrategy, EmptyNextHopList<MulticastStrategy>>,
- Test<MulticastStrategy, NextHopIsDownstream<MulticastStrategy>>,
- Test<MulticastStrategy, NextHopViolatesScope<MulticastStrategy>>,
-
Test<RandomStrategy, EmptyNextHopList<RandomStrategy>>,
Test<RandomStrategy, NextHopIsDownstream<RandomStrategy>>,
Test<RandomStrategy, NextHopViolatesScope<RandomStrategy>>
diff --git a/tests/daemon/fw/strategy-scope-control.t.cpp b/tests/daemon/fw/strategy-scope-control.t.cpp
index 268ae67..123a4f8 100644
--- a/tests/daemon/fw/strategy-scope-control.t.cpp
+++ b/tests/daemon/fw/strategy-scope-control.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, 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,
@@ -87,13 +87,19 @@
BOOST_AUTO_TEST_SUITE(Fw)
BOOST_AUTO_TEST_SUITE(TestStrategyScopeControl)
-template<typename S, bool WillSendNackNoRoute, bool CanProcessNack>
+template<typename S, bool WillSendNackNoRoute, bool CanProcessNack, bool WillRejectPitEntry = true>
class Test
{
public:
using Strategy = S;
static bool
+ willRejectPitEntry()
+ {
+ return WillRejectPitEntry;
+ }
+
+ static bool
willSendNackNoRoute()
{
return WillSendNackNoRoute;
@@ -111,7 +117,7 @@
Test<AsfStrategy, true, false>,
Test<BestRouteStrategy, false, false>,
Test<BestRouteStrategy2, true, true>,
- Test<MulticastStrategy, true, true>,
+ Test<MulticastStrategy, false, false, false>,
Test<NccStrategy, false, false>,
Test<RandomStrategy, true, true>
>;
@@ -147,10 +153,10 @@
BOOST_REQUIRE(this->strategy.waitForAction(
[&] { this->strategy.afterReceiveInterest(FaceEndpoint(*this->localFace3, 0), *interest, pitEntry); },
- this->limitedIo, 1 + T::willSendNackNoRoute()));
+ this->limitedIo, T::willRejectPitEntry() + T::willSendNackNoRoute()));
BOOST_CHECK_EQUAL(this->strategy.sendInterestHistory.size(), 0);
- BOOST_CHECK_EQUAL(this->strategy.rejectPendingInterestHistory.size(), 1);
+ BOOST_CHECK_EQUAL(this->strategy.rejectPendingInterestHistory.size(), T::willRejectPitEntry());
if (T::willSendNackNoRoute()) {
BOOST_REQUIRE_EQUAL(this->strategy.sendNackHistory.size(), 1);
BOOST_CHECK_EQUAL(this->strategy.sendNackHistory.back().header.getReason(), lp::NackReason::NO_ROUTE);
@@ -190,10 +196,10 @@
BOOST_REQUIRE(this->strategy.waitForAction(
[&] { this->strategy.afterReceiveInterest(FaceEndpoint(*this->nonLocalFace1, 0), *interest, pitEntry); },
- this->limitedIo, 1 + T::willSendNackNoRoute()));
+ this->limitedIo, T::willRejectPitEntry() + T::willSendNackNoRoute()));
BOOST_CHECK_EQUAL(this->strategy.sendInterestHistory.size(), 0);
- BOOST_CHECK_EQUAL(this->strategy.rejectPendingInterestHistory.size(), 1);
+ BOOST_CHECK_EQUAL(this->strategy.rejectPendingInterestHistory.size(), T::willRejectPitEntry());
if (T::willSendNackNoRoute()) {
BOOST_REQUIRE_EQUAL(this->strategy.sendNackHistory.size(), 1);
BOOST_CHECK_EQUAL(this->strategy.sendNackHistory.back().header.getReason(), lp::NackReason::NO_ROUTE);
diff --git a/tests/daemon/fw/strategy-tester.hpp b/tests/daemon/fw/strategy-tester.hpp
index cc40905..eab4069 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-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,
@@ -98,9 +98,8 @@
f();
if (nActions < nExpectedActions) {
- // A correctly implemented strategy is required to invoke reject pending Interest action if it
- // decides to not forward an Interest. If a test case is stuck in the call below, check that
- // rejectPendingInterest is invoked under proper condition.
+ // If strategy doesn't forward anything (e.g., decides not to forward an Interest), the number
+ // of expected actions should be 0; otherwise the test will stuck.
return limitedIo.run(nExpectedActions - nActions, LimitedIo::UNLIMITED_TIME) == LimitedIo::EXCEED_OPS;
}
return nActions == nExpectedActions;
diff --git a/tests/tools/nfdc/strategy-choice-module.t.cpp b/tests/tools/nfdc/strategy-choice-module.t.cpp
index 6778ff6..dc4965a 100644
--- a/tests/tools/nfdc/strategy-choice-module.t.cpp
+++ b/tests/tools/nfdc/strategy-choice-module.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2018, 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,
@@ -231,7 +231,7 @@
<strategyChoice>
<namespace>/localhost</namespace>
<strategy>
- <name>/localhost/nfd/strategy/multicast/%FD%01</name>
+ <name>/localhost/nfd/strategy/multicast/%FD%04</name>
</strategy>
</strategyChoice>
</strategyChoices>
@@ -240,7 +240,7 @@
const std::string STATUS_TEXT = std::string(R"TEXT(
Strategy choices:
prefix=/ strategy=/localhost/nfd/strategy/best-route/%FD%04
- prefix=/localhost strategy=/localhost/nfd/strategy/multicast/%FD%01
+ prefix=/localhost strategy=/localhost/nfd/strategy/multicast/%FD%04
)TEXT").substr(1);
BOOST_FIXTURE_TEST_CASE(Status, StatusFixture<StrategyChoiceModule>)
@@ -251,7 +251,7 @@
.setStrategy("/localhost/nfd/strategy/best-route/%FD%04");
StrategyChoice payload2;
payload2.setName("/localhost")
- .setStrategy("/localhost/nfd/strategy/multicast/%FD%01");
+ .setStrategy("/localhost/nfd/strategy/multicast/%FD%04");
this->sendDataset("/localhost/nfd/strategy-choice/list", payload1, payload2);
this->prepareStatusOutput();