fw: outgoing Interest pipeline prefers newest in-record
refs #3642
Change-Id: I591a9a9c2efdcb4607b69f2c3be584110f9e14aa
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 612b3e2..3578bb7 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -257,32 +257,6 @@
this->onOutgoingData(data, *const_pointer_cast<Face>(inFace.shared_from_this()));
}
-/** \brief compare two in-records for picking outgoing Interest
- * \return true if b is preferred over a
- *
- * This function should be passed to std::max_element over InRecordCollection.
- * The outgoing Interest picked is the last incoming Interest
- * that does not come from outFace.
- * If all in-records come from outFace, it's fine to pick that. This happens when
- * there's only one in-record that comes from outFace. The legit use is for
- * vehicular network; otherwise, strategy shouldn't send to the sole inFace.
- */
-static inline bool
-compare_pickInterest(const pit::InRecord& a, const pit::InRecord& b, const Face* outFace)
-{
- bool isOutFaceA = a.getFace().get() == outFace;
- bool isOutFaceB = b.getFace().get() == outFace;
-
- if (!isOutFaceA && isOutFaceB) {
- return false;
- }
- if (isOutFaceA && !isOutFaceB) {
- return true;
- }
-
- return a.getLastRenewed() > b.getLastRenewed();
-}
-
void
Forwarder::onOutgoingInterest(shared_ptr<pit::Entry> pitEntry, Face& outFace,
bool wantNewNonce)
@@ -302,8 +276,18 @@
}
// pick Interest
+ // The outgoing Interest picked is the last incoming Interest that does not come from outFace.
+ // If all in-records come from outFace, it's fine to pick that.
+ // This happens when there's only one in-record that comes from outFace.
+ // The legit use is for vehicular network; otherwise, strategy shouldn't send to the sole inFace.
pit::InRecordCollection::iterator pickedInRecord = std::max_element(
- pitEntry->in_begin(), pitEntry->in_end(), bind(&compare_pickInterest, _1, _2, &outFace));
+ pitEntry->in_begin(), pitEntry->in_end(),
+ [&outFace] (const pit::InRecord& a, const pit::InRecord& b) {
+ bool isOutFaceA = a.getFace().get() == &outFace;
+ bool isOutFaceB = b.getFace().get() == &outFace;
+ return (isOutFaceA > isOutFaceB) ||
+ (isOutFaceA == isOutFaceB && a.getLastRenewed() < b.getLastRenewed());
+ });
BOOST_ASSERT(pickedInRecord != pitEntry->in_end());
auto interest = const_pointer_cast<Interest>(pickedInRecord->getInterest().shared_from_this());
diff --git a/tests/daemon/fw/forwarder.t.cpp b/tests/daemon/fw/forwarder.t.cpp
index 6bbad77..d716988 100644
--- a/tests/daemon/fw/forwarder.t.cpp
+++ b/tests/daemon/fw/forwarder.t.cpp
@@ -119,6 +119,60 @@
BOOST_CHECK_EQUAL(pit.size(), 0);
}
+BOOST_AUTO_TEST_CASE(OutgoingInterest)
+{
+ Forwarder forwarder;
+ auto face1 = make_shared<DummyFace>();
+ auto face2 = make_shared<DummyFace>();
+ auto face3 = make_shared<DummyFace>();
+ forwarder.addFace(face1);
+ forwarder.addFace(face2);
+ forwarder.addFace(face3);
+
+ Pit& pit = forwarder.getPit();
+ auto interestA1 = makeInterest("ndn:/A");
+ interestA1->setNonce(8378);
+ shared_ptr<pit::Entry> pitA = pit.insert(*interestA1).first;
+ pit::InRecordCollection::iterator inA1 = pitA->insertOrUpdateInRecord(face1, *interestA1);
+
+ // there is only one downstream, interestA1 is used
+ forwarder.onOutgoingInterest(pitA, *face3, false);
+ BOOST_REQUIRE_EQUAL(face3->sentInterests.size(), 1);
+ BOOST_CHECK_EQUAL(face3->sentInterests.back().getNonce(), 8378);
+
+ // new Nonce is requested
+ forwarder.onOutgoingInterest(pitA, *face3, true);
+ BOOST_REQUIRE_EQUAL(face3->sentInterests.size(), 2);
+ BOOST_CHECK_NE(face3->sentInterests.back().getNonce(), 8378);
+ // Nonce on in-record Interest shouldn't be touched
+ BOOST_CHECK_EQUAL(inA1->getInterest().getNonce(), 8378);
+
+ // outgoing face is face1, interestA1 is still used because there's no other choice
+ forwarder.onOutgoingInterest(pitA, *face1, false);
+ BOOST_REQUIRE_EQUAL(face1->sentInterests.size(), 1);
+ BOOST_CHECK_EQUAL(face1->sentInterests.back().getNonce(), 8378);
+
+ this->advanceClocks(time::seconds(2));
+ auto interestA2 = makeInterest("ndn:/A");
+ interestA2->setNonce(9102);
+ pitA->insertOrUpdateInRecord(face2, *interestA2);
+
+ // there are two downstreams, prefer newer interestA2
+ forwarder.onOutgoingInterest(pitA, *face3, false);
+ BOOST_REQUIRE_EQUAL(face3->sentInterests.size(), 3);
+ BOOST_CHECK_EQUAL(face3->sentInterests.back().getNonce(), 9102);
+
+ // outgoing face is face2, prefer interestA1 from face1 despite it's older
+ forwarder.onOutgoingInterest(pitA, *face2, false);
+ BOOST_REQUIRE_EQUAL(face2->sentInterests.size(), 1);
+ BOOST_CHECK_EQUAL(face2->sentInterests.back().getNonce(), 8378);
+
+ // outgoing face is face1, prefer interestA2
+ forwarder.onOutgoingInterest(pitA, *face1, false);
+ BOOST_REQUIRE_EQUAL(face1->sentInterests.size(), 2);
+ BOOST_CHECK_EQUAL(face1->sentInterests.back().getNonce(), 9102);
+}
+
class ScopeLocalhostIncomingTestForwarder : public Forwarder
{
public: