From 2377ada11a162aa98d0dc199111ff38b83035c75 Mon Sep 17 00:00:00 2001 From: Eric Newberry Date: Mon, 28 Sep 2020 22:40:14 -0700 Subject: [PATCH] fw: handle dropped packets in strategies refs #5128 Change-Id: Ic7f7e61b2dde66004d0291bb41e008961fc7b252 --- daemon/fw/access-strategy.cpp | 9 ++- daemon/fw/algorithm.hpp | 6 +- daemon/fw/forwarder.cpp | 26 ++++--- daemon/fw/forwarder.hpp | 9 ++- daemon/fw/multicast-strategy.cpp | 5 +- daemon/fw/self-learning-strategy.cpp | 30 +++++--- daemon/fw/strategy.cpp | 14 ++-- daemon/fw/strategy.hpp | 11 ++- tests/daemon/fw/forwarder.t.cpp | 108 +++++++++++++++++++-------- tests/daemon/fw/strategy-tester.hpp | 9 ++- tests/daemon/fw/strategy.t.cpp | 6 +- 11 files changed, 149 insertions(+), 84 deletions(-) diff --git a/daemon/fw/access-strategy.cpp b/daemon/fw/access-strategy.cpp index ee2a7a22..c40b12e6 100644 --- a/daemon/fw/access-strategy.cpp +++ b/daemon/fw/access-strategy.cpp @@ -144,7 +144,9 @@ AccessStrategy::sendToLastNexthop(const FaceEndpoint& ingress, const Interest& i NFD_LOG_DEBUG(pitEntry->getInterest() << " interestTo " << mi.lastNexthop << " last-nexthop rto=" << time::duration_cast(rto).count()); - this->sendInterest(pitEntry, *outFace, interest); + if (!this->sendInterest(pitEntry, *outFace, interest)) { + return false; + } // schedule RTO timeout PitInfo* pi = pitEntry->insertStrategyInfo().first; @@ -198,8 +200,9 @@ AccessStrategy::multicast(const Face& inFace, const Interest& interest, continue; } NFD_LOG_DEBUG(pitEntry->getInterest() << " interestTo " << outFace.getId() << " multicast"); - this->sendInterest(pitEntry, outFace, interest); - ++nSent; + if (this->sendInterest(pitEntry, outFace, interest)) { + ++nSent; + } } return nSent; } diff --git a/daemon/fw/algorithm.hpp b/daemon/fw/algorithm.hpp index 68d9f14b..8a6b849f 100644 --- a/daemon/fw/algorithm.hpp +++ b/daemon/fw/algorithm.hpp @@ -23,8 +23,8 @@ * NFD, e.g., in COPYING.md file. If not, see . */ -#ifndef NFD_DAEMON_FW_PIT_ALGORITHM_HPP -#define NFD_DAEMON_FW_PIT_ALGORITHM_HPP +#ifndef NFD_DAEMON_FW_ALGORITHM_HPP +#define NFD_DAEMON_FW_ALGORITHM_HPP #include "fw/scope-prefix.hpp" #include "table/fib.hpp" @@ -110,4 +110,4 @@ isNextHopEligible(const Face& inFace, const Interest& interest, } // namespace fw } // namespace nfd -#endif // NFD_DAEMON_FW_PIT_ALGORITHM_HPP +#endif // NFD_DAEMON_FW_ALGORITHM_HPP diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp index bd4d6927..671c536c 100644 --- a/daemon/fw/forwarder.cpp +++ b/daemon/fw/forwarder.cpp @@ -240,7 +240,7 @@ Forwarder::onContentStoreHit(const FaceEndpoint& ingress, const shared_ptr& pitEntry, Face& egress, const Interest& interest) { @@ -249,17 +249,19 @@ Forwarder::onOutgoingInterest(const shared_ptr& pitEntry, NFD_LOG_DEBUG("onOutgoingInterest out=" << egress.getId() << " interest=" << pitEntry->getName() << " non-local hop-limit=0"); ++const_cast(egress.getCounters().nOutHopLimitZero); - return; + return nullptr; } NFD_LOG_DEBUG("onOutgoingInterest out=" << egress.getId() << " interest=" << pitEntry->getName()); // insert out-record - pitEntry->insertOrUpdateOutRecord(egress, interest); + auto it = pitEntry->insertOrUpdateOutRecord(egress, interest); + BOOST_ASSERT(it != pitEntry->out_end()); // send Interest egress.sendInterest(interest); ++m_counters.nOutInterests; + return &*it; } void @@ -397,12 +399,12 @@ Forwarder::onDataUnsolicited(const FaceEndpoint& ingress, const Data& data) ++m_counters.nUnsolicitedData; } -void +bool Forwarder::onOutgoingData(const Data& data, Face& egress) { if (egress.getId() == face::INVALID_FACEID) { NFD_LOG_WARN("onOutgoingData out=(invalid) data=" << data.getName()); - return; + return false; } NFD_LOG_DEBUG("onOutgoingData out=" << egress.getId() << " data=" << data.getName()); @@ -413,7 +415,7 @@ Forwarder::onOutgoingData(const Data& data, Face& egress) NFD_LOG_DEBUG("onOutgoingData out=" << egress.getId() << " data=" << data.getName() << " violates /localhost"); // (drop) - return; + return false; } // TODO traffic manager @@ -421,6 +423,8 @@ Forwarder::onOutgoingData(const Data& data, Face& egress) // send Data egress.sendData(data); ++m_counters.nOutData; + + return true; } void @@ -480,14 +484,14 @@ Forwarder::onIncomingNack(const FaceEndpoint& ingress, const lp::Nack& nack) [&] (fw::Strategy& strategy) { strategy.afterReceiveNack(ingress, nack, pitEntry); }); } -void +bool Forwarder::onOutgoingNack(const shared_ptr& pitEntry, Face& egress, const lp::NackHeader& nack) { if (egress.getId() == face::INVALID_FACEID) { NFD_LOG_WARN("onOutgoingNack out=(invalid)" << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason()); - return; + return false; } // has in-record? @@ -498,7 +502,7 @@ Forwarder::onOutgoingNack(const shared_ptr& pitEntry, NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId() << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason() << " no-in-record"); - return; + return false; } // if multi-access or ad hoc face, drop @@ -506,7 +510,7 @@ Forwarder::onOutgoingNack(const shared_ptr& pitEntry, NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId() << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason() << " link-type=" << egress.getLinkType()); - return; + return false; } NFD_LOG_DEBUG("onOutgoingNack out=" << egress.getId() @@ -523,6 +527,8 @@ Forwarder::onOutgoingNack(const shared_ptr& pitEntry, // send Nack on face egress.sendNack(nackPkt); ++m_counters.nOutNacks; + + return true; } void diff --git a/daemon/fw/forwarder.hpp b/daemon/fw/forwarder.hpp index a3d4771d..f736e0f9 100644 --- a/daemon/fw/forwarder.hpp +++ b/daemon/fw/forwarder.hpp @@ -189,8 +189,9 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE: // pipelines const Interest& interest, const Data& data); /** \brief outgoing Interest pipeline + * \return A pointer to the out-record created or nullptr if the Interest was dropped */ - VIRTUAL_WITH_TESTS void + VIRTUAL_WITH_TESTS pit::OutRecord* onOutgoingInterest(const shared_ptr& pitEntry, Face& egress, const Interest& interest); @@ -210,8 +211,9 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE: // pipelines onDataUnsolicited(const FaceEndpoint& ingress, const Data& data); /** \brief outgoing Data pipeline + * \return Whether the Data was transmitted (true) or dropped (false) */ - VIRTUAL_WITH_TESTS void + VIRTUAL_WITH_TESTS bool onOutgoingData(const Data& data, Face& egress); /** \brief incoming Nack pipeline @@ -220,8 +222,9 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE: // pipelines onIncomingNack(const FaceEndpoint& ingress, const lp::Nack& nack); /** \brief outgoing Nack pipeline + * \return Whether the Nack was transmitted (true) or dropped (false) */ - VIRTUAL_WITH_TESTS void + VIRTUAL_WITH_TESTS bool onOutgoingNack(const shared_ptr& pitEntry, Face& egress, const lp::NackHeader& nack); diff --git a/daemon/fw/multicast-strategy.cpp b/daemon/fw/multicast-strategy.cpp index ee921c37..3cb351cf 100644 --- a/daemon/fw/multicast-strategy.cpp +++ b/daemon/fw/multicast-strategy.cpp @@ -83,10 +83,9 @@ MulticastStrategy::afterReceiveInterest(const FaceEndpoint& ingress, const Inter continue; } - this->sendInterest(pitEntry, outFace, interest); NFD_LOG_DEBUG(interest << " from=" << ingress << " pitEntry-to=" << outFace.getId()); - - if (suppressResult == RetxSuppressionResult::FORWARD) { + bool wasSent = this->sendInterest(pitEntry, outFace, interest) != nullptr; + if (wasSent && suppressResult == RetxSuppressionResult::FORWARD) { m_retxSuppression.incrementIntervalForOutRecord(*pitEntry->getOutRecord(outFace)); } } diff --git a/daemon/fw/self-learning-strategy.cpp b/daemon/fw/self-learning-strategy.cpp index ea32709e..59a3f8f7 100644 --- a/daemon/fw/self-learning-strategy.cpp +++ b/daemon/fw/self-learning-strategy.cpp @@ -149,13 +149,17 @@ SelfLearningStrategy::broadcastInterest(const Interest& interest, const Face& in { for (auto& outFace : this->getFaceTable() | boost::adaptors::reversed) { if ((outFace.getId() == inFace.getId() && outFace.getLinkType() != ndn::nfd::LINK_TYPE_AD_HOC) || - wouldViolateScope(inFace, interest, outFace) || outFace.getScope() == ndn::nfd::FACE_SCOPE_LOCAL) { + wouldViolateScope(inFace, interest, outFace) || + outFace.getScope() == ndn::nfd::FACE_SCOPE_LOCAL) { continue; } - this->sendInterest(pitEntry, outFace, interest); - pitEntry->getOutRecord(outFace)->insertStrategyInfo().first->isNonDiscoveryInterest = false; - NFD_LOG_DEBUG("send discovery Interest=" << interest << " from=" - << inFace.getId() << " to=" << outFace.getId()); + + NFD_LOG_DEBUG("send discovery Interest=" << interest << " from=" << inFace.getId() << + " to=" << outFace.getId()); + auto outRecord = this->sendInterest(pitEntry, outFace, interest); + if (outRecord != nullptr) { + outRecord->insertStrategyInfo().first->isNonDiscoveryInterest = false; + } } } @@ -165,15 +169,17 @@ SelfLearningStrategy::multicastInterest(const Interest& interest, const Face& in const fib::NextHopList& nexthops) { for (const auto& nexthop : nexthops) { - Face& outFace = nexthop.getFace(); - if ((outFace.getId() == inFace.getId() && outFace.getLinkType() != ndn::nfd::LINK_TYPE_AD_HOC) || - wouldViolateScope(inFace, interest, outFace)) { + if (!isNextHopEligible(inFace, interest, nexthop, pitEntry)) { continue; } - this->sendInterest(pitEntry, outFace, interest); - pitEntry->getOutRecord(outFace)->insertStrategyInfo().first->isNonDiscoveryInterest = true; - NFD_LOG_DEBUG("send non-discovery Interest=" << interest << " from=" - << inFace.getId() << " to=" << outFace.getId()); + + Face& outFace = nexthop.getFace(); + NFD_LOG_DEBUG("send non-discovery Interest=" << interest << " from=" << inFace.getId() << + " to=" << outFace.getId()); + auto outRecord = this->sendInterest(pitEntry, outFace, interest); + if (outRecord != nullptr) { + outRecord->insertStrategyInfo().first->isNonDiscoveryInterest = true; + } } } diff --git a/daemon/fw/strategy.cpp b/daemon/fw/strategy.cpp index 785cc8b0..3dc9abd4 100644 --- a/daemon/fw/strategy.cpp +++ b/daemon/fw/strategy.cpp @@ -193,16 +193,15 @@ Strategy::onDroppedInterest(const Face& egress, const Interest& interest) NFD_LOG_DEBUG("onDroppedInterest out=" << egress.getId() << " name=" << interest.getName()); } -void +pit::OutRecord* Strategy::sendInterest(const shared_ptr& pitEntry, Face& egress, const Interest& interest) { if (interest.getTag() != nullptr) { Interest interest2 = interest; // make a copy to preserve tag on original packet interest2.removeTag(); - m_forwarder.onOutgoingInterest(pitEntry, egress, interest2); - return; + return m_forwarder.onOutgoingInterest(pitEntry, egress, interest2); } - m_forwarder.onOutgoingInterest(pitEntry, egress, interest); + return m_forwarder.onOutgoingInterest(pitEntry, egress, interest); } void @@ -212,7 +211,7 @@ Strategy::afterNewNextHop(const fib::NextHop& nextHop, const shared_ptr pitA = pit.insert(*interestA1).first; + auto pitA = pit.insert(*interestA1).first; pitA->insertOrUpdateInRecord(*face1, *interestA1); auto interestA2 = makeInterest("/A", false, nullopt, 1698); - forwarder.onOutgoingInterest(pitA, *face2, *interestA2); - - auto outA2 = pitA->getOutRecord(*face2); - BOOST_REQUIRE(outA2 != pitA->out_end()); + auto outA2 = forwarder.onOutgoingInterest(pitA, *face2, *interestA2); + BOOST_REQUIRE(outA2 != nullptr); BOOST_CHECK_EQUAL(outA2->getLastNonce(), 1698); + // This packet will be dropped because HopLimit=0 + auto interestA3 = makeInterest("/A", false, nullopt, 9876); + interestA3->setHopLimit(0); + auto outA3 = forwarder.onOutgoingInterest(pitA, *face2, *interestA3); + BOOST_CHECK(outA3 == nullptr); + BOOST_REQUIRE_EQUAL(face2->sentInterests.size(), 1); BOOST_CHECK_EQUAL(face2->sentInterests.back().getNonce(), 1698); } @@ -339,8 +343,8 @@ BOOST_AUTO_TEST_CASE(IncomingInterestStrategyDispatch) auto face1 = addFace(); auto face2 = addFace(); - DummyStrategy& strategyA = choose(forwarder, "/", DummyStrategy::getStrategyName()); - DummyStrategy& strategyB = choose(forwarder, "/B", DummyStrategy::getStrategyName()); + auto& strategyA = choose(forwarder, "/", DummyStrategy::getStrategyName()); + auto& strategyB = choose(forwarder, "/B", DummyStrategy::getStrategyName()); auto interest1 = makeInterest("/A/1"); strategyA.afterReceiveInterest_count = 0; @@ -381,14 +385,14 @@ BOOST_AUTO_TEST_CASE(IncomingData) Pit& pit = forwarder.getPit(); auto interestD = makeInterest("/A/B/C/D"); - shared_ptr pitD = pit.insert(*interestD).first; + auto pitD = pit.insert(*interestD).first; pitD->insertOrUpdateInRecord(*face1, *interestD); auto interestA = makeInterest("/A", true); - shared_ptr pitA = pit.insert(*interestA).first; + auto pitA = pit.insert(*interestA).first; pitA->insertOrUpdateInRecord(*face2, *interestA); pitA->insertOrUpdateInRecord(*face3, *interestA); auto interestC = makeInterest("/A/B/C", true); - shared_ptr pitC = pit.insert(*interestC).first; + auto pitC = pit.insert(*interestC).first; pitC->insertOrUpdateInRecord(*face3, *interestC); pitC->insertOrUpdateInRecord(*face4, *interestC); @@ -406,6 +410,36 @@ BOOST_AUTO_TEST_CASE(IncomingData) BOOST_CHECK_EQUAL(forwarder.getCounters().nUnsolicitedData, 0); } +BOOST_AUTO_TEST_CASE(OutgoingData) +{ + auto face1 = addFace("dummy://", "dummy://", ndn::nfd::FACE_SCOPE_LOCAL); + auto face2 = addFace("dummy://", "dummy://", ndn::nfd::FACE_SCOPE_NON_LOCAL); + auto face3 = addFace(); + face3->setId(face::INVALID_FACEID); + + auto data = makeData("/QkzAWU6K"); + auto localData = makeData("/localhost/YH8bqnbv"); + + face1->sentData.clear(); + BOOST_CHECK(forwarder.onOutgoingData(*data, *face1)); + BOOST_REQUIRE_EQUAL(face1->sentData.size(), 1); + BOOST_CHECK_EQUAL(face1->sentData.back().getName(), data->getName()); + + // scope control + face1->sentData.clear(); + face2->sentData.clear(); + BOOST_CHECK(!forwarder.onOutgoingData(*localData, *face2)); + BOOST_CHECK_EQUAL(face2->sentData.size(), 0); + BOOST_CHECK(forwarder.onOutgoingData(*localData, *face1)); + BOOST_REQUIRE_EQUAL(face1->sentData.size(), 1); + BOOST_CHECK_EQUAL(face1->sentData.back().getName(), localData->getName()); + + // face with invalid ID + face3->sentData.clear(); + BOOST_CHECK(!forwarder.onOutgoingData(*data, *face3)); + BOOST_CHECK_EQUAL(face3->sentData.size(), 0); +} + BOOST_AUTO_TEST_CASE(IncomingNack) { auto face1 = addFace(); @@ -415,27 +449,27 @@ BOOST_AUTO_TEST_CASE(IncomingNack) ndn::nfd::FACE_PERSISTENCY_PERSISTENT, ndn::nfd::LINK_TYPE_MULTI_ACCESS); - DummyStrategy& strategyA = choose(forwarder, "/", DummyStrategy::getStrategyName()); - DummyStrategy& strategyB = choose(forwarder, "/B", DummyStrategy::getStrategyName()); + auto& strategyA = choose(forwarder, "/", DummyStrategy::getStrategyName()); + auto& strategyB = choose(forwarder, "/B", DummyStrategy::getStrategyName()); Pit& pit = forwarder.getPit(); // dispatch to the correct strategy auto interest1 = makeInterest("/A/AYJqayrzF", false, nullopt, 562); - shared_ptr pit1 = pit.insert(*interest1).first; + auto pit1 = pit.insert(*interest1).first; pit1->insertOrUpdateOutRecord(*face1, *interest1); auto interest2 = makeInterest("/B/EVyP73ru", false, nullopt, 221); - shared_ptr pit2 = pit.insert(*interest2).first; + auto pit2 = pit.insert(*interest2).first; pit2->insertOrUpdateOutRecord(*face1, *interest2); - lp::Nack nack1 = makeNack(*interest1, lp::NackReason::CONGESTION); + auto nack1 = makeNack(*interest1, lp::NackReason::CONGESTION); strategyA.afterReceiveNack_count = 0; strategyB.afterReceiveNack_count = 0; forwarder.onIncomingNack(FaceEndpoint(*face1, 0), nack1); BOOST_CHECK_EQUAL(strategyA.afterReceiveNack_count, 1); BOOST_CHECK_EQUAL(strategyB.afterReceiveNack_count, 0); - lp::Nack nack2 = makeNack(*interest2, lp::NackReason::CONGESTION); + auto nack2 = makeNack(*interest2, lp::NackReason::CONGESTION); strategyA.afterReceiveNack_count = 0; strategyB.afterReceiveNack_count = 0; forwarder.onIncomingNack(FaceEndpoint(*face1, 0), nack2); @@ -449,7 +483,7 @@ BOOST_AUTO_TEST_CASE(IncomingNack) BOOST_CHECK_EQUAL(outRecord1->getIncomingNack()->getReason(), lp::NackReason::CONGESTION); // drop if no PIT entry - lp::Nack nack3 = makeNack(*makeInterest("/yEcw5HhdM", false, nullopt, 243), lp::NackReason::CONGESTION); + auto nack3 = makeNack(*makeInterest("/yEcw5HhdM", false, nullopt, 243), lp::NackReason::CONGESTION); strategyA.afterReceiveNack_count = 0; strategyB.afterReceiveNack_count = 0; forwarder.onIncomingNack(FaceEndpoint(*face1, 0), nack3); @@ -458,10 +492,10 @@ BOOST_AUTO_TEST_CASE(IncomingNack) // drop if no out-record auto interest4 = makeInterest("/Etab4KpY", false, nullopt, 157); - shared_ptr pit4 = pit.insert(*interest4).first; + auto pit4 = pit.insert(*interest4).first; pit4->insertOrUpdateOutRecord(*face1, *interest4); - lp::Nack nack4a = makeNack(*interest4, lp::NackReason::CONGESTION); + auto nack4a = makeNack(*interest4, lp::NackReason::CONGESTION); strategyA.afterReceiveNack_count = 0; strategyB.afterReceiveNack_count = 0; forwarder.onIncomingNack(FaceEndpoint(*face2, 0), nack4a); @@ -469,7 +503,7 @@ BOOST_AUTO_TEST_CASE(IncomingNack) BOOST_CHECK_EQUAL(strategyB.afterReceiveNack_count, 0); // drop if Nonce does not match out-record - lp::Nack nack4b = makeNack(*makeInterest("/Etab4KpY", false, nullopt, 294), lp::NackReason::CONGESTION); + auto nack4b = makeNack(*makeInterest("/Etab4KpY", false, nullopt, 294), lp::NackReason::CONGESTION); strategyA.afterReceiveNack_count = 0; strategyB.afterReceiveNack_count = 0; forwarder.onIncomingNack(FaceEndpoint(*face1, 0), nack4b); @@ -493,6 +527,8 @@ BOOST_AUTO_TEST_CASE(OutgoingNack) ndn::nfd::FACE_SCOPE_NON_LOCAL, ndn::nfd::FACE_PERSISTENCY_PERSISTENT, ndn::nfd::LINK_TYPE_MULTI_ACCESS); + auto face4 = addFace(); + face4->setId(face::INVALID_FACEID); Pit& pit = forwarder.getPit(); @@ -501,39 +537,41 @@ BOOST_AUTO_TEST_CASE(OutgoingNack) // don't send Nack if there's no in-record auto interest1 = makeInterest("/fM5IVEtC", false, nullopt, 719); - shared_ptr pit1 = pit.insert(*interest1).first; + auto pit1 = pit.insert(*interest1).first; pit1->insertOrUpdateInRecord(*face1, *interest1); face2->sentNacks.clear(); - forwarder.onOutgoingNack(pit1, *face2, nackHeader); + BOOST_CHECK(!forwarder.onOutgoingNack(pit1, *face2, nackHeader)); BOOST_CHECK_EQUAL(face2->sentNacks.size(), 0); // send Nack with correct Nonce auto interest2a = makeInterest("/Vi8tRm9MG3", false, nullopt, 152); - shared_ptr pit2 = pit.insert(*interest2a).first; + auto pit2 = pit.insert(*interest2a).first; pit2->insertOrUpdateInRecord(*face1, *interest2a); auto interest2b = makeInterest("/Vi8tRm9MG3", false, nullopt, 808); pit2->insertOrUpdateInRecord(*face2, *interest2b); - face1->sentNacks.clear(); - forwarder.onOutgoingNack(pit2, *face1, nackHeader); + face2->sentNacks.clear(); + + BOOST_CHECK(forwarder.onOutgoingNack(pit2, *face1, nackHeader)); BOOST_REQUIRE_EQUAL(face1->sentNacks.size(), 1); BOOST_CHECK_EQUAL(face1->sentNacks.back().getReason(), lp::NackReason::CONGESTION); BOOST_CHECK_EQUAL(face1->sentNacks.back().getInterest().getNonce(), 152); + BOOST_CHECK_EQUAL(face2->sentNacks.size(), 0); - // erase in-record + // in-record is erased auto inRecord2a = pit2->getInRecord(*face1); BOOST_CHECK(inRecord2a == pit2->in_end()); // send Nack with correct Nonce - face2->sentNacks.clear(); - forwarder.onOutgoingNack(pit2, *face2, nackHeader); + BOOST_CHECK(forwarder.onOutgoingNack(pit2, *face2, nackHeader)); + BOOST_CHECK_EQUAL(face1->sentNacks.size(), 1); BOOST_REQUIRE_EQUAL(face2->sentNacks.size(), 1); BOOST_CHECK_EQUAL(face2->sentNacks.back().getReason(), lp::NackReason::CONGESTION); BOOST_CHECK_EQUAL(face2->sentNacks.back().getInterest().getNonce(), 808); - // erase in-record - auto inRecord2b = pit2->getInRecord(*face1); + // in-record is erased + auto inRecord2b = pit2->getInRecord(*face2); BOOST_CHECK(inRecord2b == pit2->in_end()); // don't send Nack to multi-access face @@ -541,8 +579,16 @@ BOOST_AUTO_TEST_CASE(OutgoingNack) pit2->insertOrUpdateInRecord(*face3, *interest2c); face3->sentNacks.clear(); - forwarder.onOutgoingNack(pit1, *face3, nackHeader); + BOOST_CHECK(!forwarder.onOutgoingNack(pit2, *face3, nackHeader)); BOOST_CHECK_EQUAL(face3->sentNacks.size(), 0); + + // don't send Nack to face with invalid ID + auto interest1b = makeInterest("/fM5IVEtC", false, nullopt, 553); + pit1->insertOrUpdateInRecord(*face4, *interest1b); + + face4->sentNacks.clear(); + BOOST_CHECK(!forwarder.onOutgoingNack(pit1, *face4, nackHeader)); + BOOST_CHECK_EQUAL(face4->sentNacks.size(), 0); } BOOST_AUTO_TEST_CASE(InterestLoopNack) diff --git a/tests/daemon/fw/strategy-tester.hpp b/tests/daemon/fw/strategy-tester.hpp index 17806ecc..cc40905c 100644 --- a/tests/daemon/fw/strategy-tester.hpp +++ b/tests/daemon/fw/strategy-tester.hpp @@ -107,13 +107,15 @@ public: } protected: - void + pit::OutRecord* sendInterest(const shared_ptr& pitEntry, Face& egress, const Interest& interest) override { sendInterestHistory.push_back({pitEntry->getInterest(), egress.getId(), interest}); - pitEntry->insertOrUpdateOutRecord(egress, interest); + auto it = pitEntry->insertOrUpdateOutRecord(egress, interest); + BOOST_ASSERT(it != pitEntry->out_end()); afterAction(); + return &*it; } void @@ -123,13 +125,14 @@ protected: afterAction(); } - void + bool sendNack(const shared_ptr& pitEntry, Face& egress, const lp::NackHeader& header) override { sendNackHistory.push_back({pitEntry->getInterest(), egress.getId(), header}); pitEntry->deleteInRecord(egress); afterAction(); + return true; } public: diff --git a/tests/daemon/fw/strategy.t.cpp b/tests/daemon/fw/strategy.t.cpp index 8b27986c..6f8e09b9 100644 --- a/tests/daemon/fw/strategy.t.cpp +++ b/tests/daemon/fw/strategy.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-2020, Regents of the University of California, * Arizona Board of Regents, * Colorado State University, * University Pierre & Marie Curie, Sorbonne University, @@ -43,7 +43,7 @@ using namespace nfd::tests; BOOST_AUTO_TEST_SUITE(Fw) BOOST_FIXTURE_TEST_SUITE(TestStrategy, GlobalIoFixture) -// Strategy registry is tested in table/strategy-choice.t.cpp and strategy-instantiation.t.cpp +// Strategy registry is tested in strategy-choice.t.cpp and strategy-instantiation.t.cpp class FaceTableAccessTestStrategy : public DummyStrategy { @@ -103,8 +103,6 @@ BOOST_AUTO_TEST_CASE(FaceTableAccess) BOOST_CHECK((strategy.removedFaces == std::vector{id2, id1})); } -// LookupFib is tested in Fw/TestLinkForwarding test suite. - BOOST_AUTO_TEST_SUITE_END() // TestStrategy BOOST_AUTO_TEST_SUITE_END() // Fw