fw: always create PIT in-records with EndpointId=0 for now
Also improve test coverage
Refs: #4849
Change-Id: I0f3aba98ec7460f0dbf58b6dc00414e1f722c036
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 7a85daf..17751bf 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -160,12 +160,6 @@
ingress.face.sendNack(nack);
}
-static inline bool
-compare_InRecord_expiry(const pit::InRecord& a, const pit::InRecord& b)
-{
- return a.getExpiry() < b.getExpiry();
-}
-
void
Forwarder::onContentStoreMiss(const FaceEndpoint& ingress,
const shared_ptr<pit::Entry>& pitEntry, const Interest& interest)
@@ -174,20 +168,27 @@
++m_counters.nCsMisses;
// insert in-record
- pitEntry->insertOrUpdateInRecord(ingress.face, ingress.endpoint, interest);
+ // FIXME Strategies are not prepared to handle non-zero EndpointIds, so always insert
+ // the in-record with EndpointId=0 for now. Eventually, this pipeline will need
+ // to be refactored so that strategies can control the in-record insertion.
+ pitEntry->insertOrUpdateInRecord(ingress.face, 0, interest);
// set PIT expiry timer to the time that the last PIT in-record expires
- auto lastExpiring = std::max_element(pitEntry->in_begin(), pitEntry->in_end(), &compare_InRecord_expiry);
+ auto lastExpiring = std::max_element(pitEntry->in_begin(), pitEntry->in_end(),
+ [] (const auto& a, const auto& b) {
+ return a.getExpiry() < b.getExpiry();
+ });
auto lastExpiryFromNow = lastExpiring->getExpiry() - time::steady_clock::now();
this->setExpiryTimer(pitEntry, time::duration_cast<time::milliseconds>(lastExpiryFromNow));
// has NextHopFaceId?
- shared_ptr<lp::NextHopFaceIdTag> nextHopTag = interest.getTag<lp::NextHopFaceIdTag>();
+ auto nextHopTag = interest.getTag<lp::NextHopFaceIdTag>();
if (nextHopTag != nullptr) {
// chosen NextHop face exists?
Face* nextHopFace = m_faceTable.get(*nextHopTag);
if (nextHopFace != nullptr) {
- NFD_LOG_DEBUG("onContentStoreMiss interest=" << interest.getName() << " nexthop-faceid=" << nextHopFace->getId());
+ NFD_LOG_DEBUG("onContentStoreMiss interest=" << interest.getName()
+ << " nexthop-faceid=" << nextHopFace->getId());
// go to outgoing Interest pipeline
// scope control is unnecessary, because privileged app explicitly wants to forward
this->onOutgoingInterest(pitEntry, FaceEndpoint(*nextHopFace, 0), interest);
@@ -208,7 +209,7 @@
++m_counters.nCsHits;
data.setTag(make_shared<lp::IncomingFaceIdTag>(face::FACEID_CONTENT_STORE));
- // XXX should we lookup PIT for other Interests that also match csMatch?
+ // FIXME Should we lookup PIT for other Interests that also match the data?
pitEntry->isSatisfied = true;
pitEntry->dataFreshnessPeriod = data.getFreshnessPeriod();
@@ -314,7 +315,7 @@
std::set<std::pair<Face*, EndpointId>> pendingDownstreams;
auto now = time::steady_clock::now();
- for (const shared_ptr<pit::Entry>& pitEntry : pitMatches) {
+ for (const auto& pitEntry : pitMatches) {
NFD_LOG_DEBUG("onIncomingData matching=" << pitEntry->getName());
// remember pending downstreams
@@ -403,8 +404,9 @@
// if multi-access or ad hoc face, drop
if (ingress.face.getLinkType() != ndn::nfd::LINK_TYPE_POINT_TO_POINT) {
- NFD_LOG_DEBUG("onIncomingNack in=" << ingress << " nack=" << nack.getInterest().getName()
- << "~" << nack.getReason() << " face-is-multi-access");
+ NFD_LOG_DEBUG("onIncomingNack in=" << ingress
+ << " nack=" << nack.getInterest().getName() << "~" << nack.getReason()
+ << " link-type=" << ingress.face.getLinkType());
return;
}
@@ -418,7 +420,7 @@
}
// has out-record?
- pit::OutRecordCollection::iterator outRecord = pitEntry->getOutRecord(ingress.face, ingress.endpoint);
+ auto outRecord = pitEntry->getOutRecord(ingress.face, ingress.endpoint);
// if no out-record found, drop
if (outRecord == pitEntry->out_end()) {
NFD_LOG_DEBUG("onIncomingNack in=" << ingress << " nack=" << nack.getInterest().getName()
@@ -456,13 +458,12 @@
{
if (egress.face.getId() == face::INVALID_FACEID) {
NFD_LOG_WARN("onOutgoingNack out=(invalid)"
- << " nack=" << pitEntry->getInterest().getName()
- << "~" << nack.getReason() << " no-in-record");
+ << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason());
return;
}
// has in-record?
- pit::InRecordCollection::iterator inRecord = pitEntry->getInRecord(egress.face, egress.endpoint);
+ auto inRecord = pitEntry->getInRecord(egress.face, egress.endpoint);
// if no in-record found, drop
if (inRecord == pitEntry->in_end()) {
@@ -475,8 +476,8 @@
// if multi-access or ad hoc face, drop
if (egress.face.getLinkType() != ndn::nfd::LINK_TYPE_POINT_TO_POINT) {
NFD_LOG_DEBUG("onOutgoingNack out=" << egress
- << " nack=" << pitEntry->getInterest().getName()
- << "~" << nack.getReason() << " face-is-multi-access");
+ << " nack=" << pitEntry->getInterest().getName() << "~" << nack.getReason()
+ << " link-type=" << egress.face.getLinkType());
return;
}
diff --git a/tests/daemon/fw/broadcast-medium.t.cpp b/tests/daemon/fw/broadcast-medium.t.cpp
new file mode 100644
index 0000000..15999e2
--- /dev/null
+++ b/tests/daemon/fw/broadcast-medium.t.cpp
@@ -0,0 +1,122 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2014-2019, Regents of the University of California,
+ * Arizona Board of Regents,
+ * Colorado State University,
+ * University Pierre & Marie Curie, Sorbonne University,
+ * Washington University in St. Louis,
+ * Beijing Institute of Technology,
+ * The University of Memphis.
+ *
+ * This file is part of NFD (Named Data Networking Forwarding Daemon).
+ * See AUTHORS.md for complete list of NFD authors and contributors.
+ *
+ * NFD is free software: you can redistribute it and/or modify it under the terms
+ * of the GNU General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * NFD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * NFD, e.g., in COPYING.md file. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+// Strategies that can correctly handle multi-access faces on a broadcast medium,
+// sorted alphabetically.
+#include "fw/asf-strategy.hpp"
+#include "fw/best-route-strategy2.hpp"
+#include "fw/multicast-strategy.hpp"
+
+#include "tests/daemon/global-io-fixture.hpp"
+#include "topology-tester.hpp"
+
+#include <boost/mpl/vector.hpp>
+
+namespace nfd {
+namespace fw {
+namespace tests {
+
+template<typename S>
+class BroadcastMediumFixture : public GlobalIoTimeFixture
+{
+protected:
+ BroadcastMediumFixture()
+ {
+ nodeC = topo.addForwarder("C");
+ nodeD = topo.addForwarder("D");
+ nodeP = topo.addForwarder("P");
+
+ for (TopologyNode node : {nodeC, nodeD, nodeP}) {
+ topo.setStrategy<S>(node);
+ }
+
+ auto w = topo.addLink("W", 10_ms, {nodeC, nodeD, nodeP}, ndn::nfd::LINK_TYPE_MULTI_ACCESS);
+ faceC = &w->getFace(nodeC);
+ faceD = &w->getFace(nodeD);
+ faceP = &w->getFace(nodeP);
+
+ appC = topo.addAppFace("consumer1", nodeC);
+ topo.registerPrefix(nodeC, *faceC, "/P");
+ topo.addIntervalConsumer(appC->getClientFace(), "/P", 0_ms, 1, 1);
+ appD = topo.addAppFace("consumer2", nodeD);
+ topo.registerPrefix(nodeD, *faceD, "/P");
+ topo.addIntervalConsumer(appD->getClientFace(), "/P", 0_ms, 1, 1);
+ appP = topo.addAppFace("producer", nodeP, "/P");
+ topo.addEchoProducer(appP->getClientFace(), "/P", 100_ms);
+ }
+
+protected:
+ TopologyTester topo;
+ TopologyNode nodeC;
+ TopologyNode nodeD;
+ TopologyNode nodeP;
+ Face* faceC;
+ Face* faceD;
+ Face* faceP;
+ shared_ptr<TopologyAppLink> appC;
+ shared_ptr<TopologyAppLink> appD;
+ shared_ptr<TopologyAppLink> appP;
+};
+
+BOOST_AUTO_TEST_SUITE(Fw)
+BOOST_AUTO_TEST_SUITE(TestBroadcastMedium)
+
+using Strategies = boost::mpl::vector<
+ AsfStrategy,
+ BestRouteStrategy2,
+ MulticastStrategy
+>;
+
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(SameFaceDifferentEndpoint, S, Strategies, BroadcastMediumFixture<S>)
+{
+ // C D P
+ // | | |
+ // --+---+---+--
+ //
+ // C and D are consumers. P is the producer. They communicate with each other
+ // through a multi-access face over a (wired or wireless) broadcast medium.
+
+ this->advanceClocks(10_ms, 50_ms);
+
+ BOOST_CHECK_EQUAL(this->faceC->getCounters().nOutInterests, 1);
+ BOOST_CHECK_EQUAL(this->faceD->getCounters().nOutInterests, 1);
+ BOOST_CHECK_EQUAL(this->faceP->getCounters().nInInterests, 2);
+ BOOST_CHECK_EQUAL(this->faceC->getCounters().nInData, 0);
+ BOOST_CHECK_EQUAL(this->faceD->getCounters().nInData, 0);
+ BOOST_CHECK_EQUAL(this->faceP->getCounters().nOutData, 0);
+
+ this->advanceClocks(10_ms, 200_ms);
+
+ BOOST_CHECK_EQUAL(this->faceC->getCounters().nInData, 1);
+ BOOST_CHECK_EQUAL(this->faceD->getCounters().nInData, 1);
+ BOOST_CHECK_EQUAL(this->faceP->getCounters().nOutData, 1);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // TestBroadcastMedium
+BOOST_AUTO_TEST_SUITE_END() // Fw
+
+} // namespace tests
+} // namespace fw
+} // namespace nfd
diff --git a/tests/daemon/fw/forwarder.t.cpp b/tests/daemon/fw/forwarder.t.cpp
index 598935c..d07d160 100644
--- a/tests/daemon/fw/forwarder.t.cpp
+++ b/tests/daemon/fw/forwarder.t.cpp
@@ -180,26 +180,22 @@
class ScopeLocalhostIncomingTestForwarder : public Forwarder
{
public:
- ScopeLocalhostIncomingTestForwarder()
- {
- }
-
void
- onDataUnsolicited(const FaceEndpoint& ingress, const Data& data) override
+ onDataUnsolicited(const FaceEndpoint&, const Data&) final
{
++onDataUnsolicited_count;
}
protected:
void
- dispatchToStrategy(pit::Entry&, std::function<void(fw::Strategy&)>) override
+ dispatchToStrategy(pit::Entry&, std::function<void(fw::Strategy&)>) final
{
++dispatchToStrategy_count;
}
public:
- int dispatchToStrategy_count;
- int onDataUnsolicited_count;
+ int dispatchToStrategy_count = 0;
+ int onDataUnsolicited_count = 0;
};
BOOST_AUTO_TEST_CASE(ScopeLocalhostIncoming)
diff --git a/tests/daemon/table/pit-entry.t.cpp b/tests/daemon/table/pit-entry.t.cpp
index f46aa38..ac6709b 100644
--- a/tests/daemon/table/pit-entry.t.cpp
+++ b/tests/daemon/table/pit-entry.t.cpp
@@ -66,20 +66,21 @@
BOOST_AUTO_TEST_CASE(InOutRecords)
{
- shared_ptr<Face> face1 = make_shared<DummyFace>();
- shared_ptr<Face> face2 = make_shared<DummyFace>();
+ auto face1 = make_shared<DummyFace>();
+ auto face2 = make_shared<DummyFace>();
+
Name name("ndn:/KuYfjtRq");
- shared_ptr<Interest> interest = makeInterest(name);
- shared_ptr<Interest> interest1 = makeInterest(name);
+ auto interest = makeInterest(name);
+ auto interest1 = makeInterest(name);
interest1->setInterestLifetime(2528_ms);
interest1->setNonce(25559);
- shared_ptr<Interest> interest2 = makeInterest(name);
+ auto interest2 = makeInterest(name);
interest2->setInterestLifetime(6464_ms);
interest2->setNonce(19004);
- shared_ptr<Interest> interest3 = makeInterest(name);
+ auto interest3 = makeInterest(name);
interest3->setInterestLifetime(3585_ms);
interest3->setNonce(24216);
- shared_ptr<Interest> interest4 = makeInterest(name);
+ auto interest4 = makeInterest(name);
interest4->setInterestLifetime(8795_ms);
interest4->setNonce(17365);
@@ -94,9 +95,9 @@
BOOST_CHECK_EQUAL(outRecords1.size(), 0);
// insert in-record
- time::steady_clock::TimePoint before1 = time::steady_clock::now();
+ auto before1 = time::steady_clock::now();
InRecordCollection::iterator in1 = entry.insertOrUpdateInRecord(*face1, 0, *interest1);
- time::steady_clock::TimePoint after1 = time::steady_clock::now();
+ auto after1 = time::steady_clock::now();
const InRecordCollection& inRecords2 = entry.getInRecords();
BOOST_CHECK_EQUAL(inRecords2.size(), 1);
BOOST_CHECK(in1 == inRecords2.begin());
@@ -104,15 +105,14 @@
BOOST_CHECK_EQUAL(in1->getLastNonce(), interest1->getNonce());
BOOST_CHECK_GE(in1->getLastRenewed(), before1);
BOOST_CHECK_LE(in1->getLastRenewed(), after1);
- BOOST_CHECK_LE(in1->getExpiry() - in1->getLastRenewed()
- - interest1->getInterestLifetime(),
- (after1 - before1));
+ BOOST_CHECK_LE(in1->getExpiry() - in1->getLastRenewed() - interest1->getInterestLifetime(),
+ after1 - before1);
BOOST_CHECK(in1 == entry.getInRecord(*face1, 0));
// insert out-record
- time::steady_clock::TimePoint before2 = time::steady_clock::now();
+ auto before2 = time::steady_clock::now();
OutRecordCollection::iterator out1 = entry.insertOrUpdateOutRecord(*face1, 0, *interest1);
- time::steady_clock::TimePoint after2 = time::steady_clock::now();
+ auto after2 = time::steady_clock::now();
const OutRecordCollection& outRecords2 = entry.getOutRecords();
BOOST_CHECK_EQUAL(outRecords2.size(), 1);
BOOST_CHECK(out1 == outRecords2.begin());
@@ -120,44 +120,61 @@
BOOST_CHECK_EQUAL(out1->getLastNonce(), interest1->getNonce());
BOOST_CHECK_GE(out1->getLastRenewed(), before2);
BOOST_CHECK_LE(out1->getLastRenewed(), after2);
- BOOST_CHECK_LE(out1->getExpiry() - out1->getLastRenewed()
- - interest1->getInterestLifetime(),
- (after2 - before2));
+ BOOST_CHECK_LE(out1->getExpiry() - out1->getLastRenewed() - interest1->getInterestLifetime(),
+ after2 - before2);
BOOST_CHECK(out1 == entry.getOutRecord(*face1, 0));
// update in-record
- time::steady_clock::TimePoint before3 = time::steady_clock::now();
+ auto before3 = time::steady_clock::now();
InRecordCollection::iterator in2 = entry.insertOrUpdateInRecord(*face1, 0, *interest2);
- time::steady_clock::TimePoint after3 = time::steady_clock::now();
+ auto after3 = time::steady_clock::now();
const InRecordCollection& inRecords3 = entry.getInRecords();
BOOST_CHECK_EQUAL(inRecords3.size(), 1);
BOOST_CHECK(in2 == inRecords3.begin());
BOOST_CHECK_EQUAL(&in2->getFace(), face1.get());
BOOST_CHECK_EQUAL(in2->getLastNonce(), interest2->getNonce());
- BOOST_CHECK_LE(in2->getExpiry() - in2->getLastRenewed()
- - interest2->getInterestLifetime(),
- (after3 - before3));
+ BOOST_CHECK_LE(in2->getExpiry() - in2->getLastRenewed() - interest2->getInterestLifetime(),
+ after3 - before3);
- // insert another in-record
+ // insert another in-record (different face)
InRecordCollection::iterator in3 = entry.insertOrUpdateInRecord(*face2, 0, *interest3);
const InRecordCollection& inRecords4 = entry.getInRecords();
BOOST_CHECK_EQUAL(inRecords4.size(), 2);
BOOST_CHECK_EQUAL(&in3->getFace(), face2.get());
+ BOOST_CHECK_EQUAL(in3->getEndpointId(), 0);
+
+ // insert another in-record (different endpoint)
+ InRecordCollection::iterator in4 = entry.insertOrUpdateInRecord(*face1, 42, *interest4);
+ const InRecordCollection& inRecords5 = entry.getInRecords();
+ BOOST_CHECK_EQUAL(inRecords5.size(), 3);
+ BOOST_CHECK_EQUAL(&in4->getFace(), face1.get());
+ BOOST_CHECK_EQUAL(in4->getEndpointId(), 42);
// get in-record
- InRecordCollection::iterator in4 = entry.getInRecord(*face1, 0);
- BOOST_REQUIRE(in4 != entry.in_end());
- BOOST_CHECK_EQUAL(&in4->getFace(), face1.get());
+ InRecordCollection::iterator in5 = entry.getInRecord(*face1, 0);
+ BOOST_REQUIRE(in5 != entry.in_end());
+ BOOST_CHECK_EQUAL(&in5->getFace(), face1.get());
+ BOOST_CHECK_EQUAL(in5->getEndpointId(), 0);
+ InRecordCollection::iterator in6 = entry.getInRecord(*face1, 42);
+ BOOST_REQUIRE(in6 != entry.in_end());
+ BOOST_CHECK_EQUAL(&in6->getFace(), face1.get());
+ BOOST_CHECK_EQUAL(in6->getEndpointId(), 42);
+ BOOST_CHECK(in5 != in6);
+
+ // delete in-record
+ entry.deleteInRecord(*face1, 0);
+ BOOST_CHECK_EQUAL(entry.getInRecords().size(), 2);
+ BOOST_CHECK(entry.getInRecord(*face1, 0) == entry.in_end());
+ BOOST_CHECK(entry.getInRecord(*face1, 42) != entry.in_end());
// clear in-records
entry.clearInRecords();
- const InRecordCollection& inRecords5 = entry.getInRecords();
- BOOST_CHECK_EQUAL(inRecords5.size(), 0);
+ BOOST_CHECK_EQUAL(entry.getInRecords().size(), 0);
BOOST_CHECK(entry.getInRecord(*face1, 0) == entry.in_end());
+ BOOST_CHECK(entry.getInRecord(*face1, 42) == entry.in_end());
// insert another out-record
- OutRecordCollection::iterator out2 =
- entry.insertOrUpdateOutRecord(*face2, 0, *interest4);
+ OutRecordCollection::iterator out2 = entry.insertOrUpdateOutRecord(*face2, 0, *interest4);
const OutRecordCollection& outRecords3 = entry.getOutRecords();
BOOST_CHECK_EQUAL(outRecords3.size(), 2);
BOOST_CHECK_EQUAL(&out2->getFace(), face2.get());
@@ -177,30 +194,29 @@
BOOST_AUTO_TEST_CASE(Lifetime)
{
- shared_ptr<Interest> interest = makeInterest("ndn:/7oIEurbgy6");
-
- shared_ptr<Face> face = make_shared<DummyFace>();
+ auto interest = makeInterest("ndn:/7oIEurbgy6");
+ auto face = make_shared<DummyFace>();
Entry entry(*interest);
- InRecordCollection::iterator inIt = entry.insertOrUpdateInRecord(*face, 0, *interest);
+ auto inIt = entry.insertOrUpdateInRecord(*face, 0, *interest);
BOOST_CHECK_GT(inIt->getExpiry(), time::steady_clock::now());
- OutRecordCollection::iterator outIt = entry.insertOrUpdateOutRecord(*face, 0, *interest);
+ auto outIt = entry.insertOrUpdateOutRecord(*face, 0, *interest);
BOOST_CHECK_GT(outIt->getExpiry(), time::steady_clock::now());
}
BOOST_AUTO_TEST_CASE(OutRecordNack)
{
- shared_ptr<Face> face1 = make_shared<DummyFace>();
+ auto face1 = make_shared<DummyFace>();
OutRecord outR(*face1, 0);
BOOST_CHECK(outR.getIncomingNack() == nullptr);
- shared_ptr<Interest> interest1 = makeInterest("ndn:/uWiapGjYL");
+ auto interest1 = makeInterest("ndn:/uWiapGjYL");
interest1->setNonce(165);
outR.update(*interest1);
BOOST_CHECK(outR.getIncomingNack() == nullptr);
- shared_ptr<Interest> interest2 = makeInterest("ndn:/uWiapGjYL");
+ auto interest2 = makeInterest("ndn:/uWiapGjYL");
interest2->setNonce(996);
lp::Nack nack2(*interest2);
nack2.setReason(lp::NackReason::CONGESTION);