catchunks: avoid excess window decrease in certain conditions
Refs: #5202
Change-Id: I7eeed18fbdfc3ef7a5a277da34f9de0c7068a61d
diff --git a/tests/chunks/pipeline-interests-aimd.t.cpp b/tests/chunks/pipeline-interests-aimd.t.cpp
index 0946021..12eeaa5 100644
--- a/tests/chunks/pipeline-interests-aimd.t.cpp
+++ b/tests/chunks/pipeline-interests-aimd.t.cpp
@@ -72,8 +72,6 @@
static constexpr double MARGIN = 0.001;
};
-constexpr double PipelineInterestAimdFixture::MARGIN;
-
BOOST_AUTO_TEST_SUITE(Chunks)
BOOST_FIXTURE_TEST_SUITE(TestPipelineInterestsAimd, PipelineInterestAimdFixture)
@@ -190,9 +188,7 @@
BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, 2);
BOOST_CHECK_EQUAL(pipeline->m_nRetransmitted, 2);
- BOOST_CHECK_EQUAL(pipeline->m_nTimeouts,
- pipeline->m_nRetransmitted + pipeline->m_nSkippedRetx);
-
+ BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, pipeline->m_nRetransmitted + pipeline->m_nSkippedRetx);
}
BOOST_AUTO_TEST_CASE(CongestionMarksWithCwa)
@@ -545,6 +541,58 @@
BOOST_CHECK_EQUAL(rttEstimator.getEstimatedRto(), prevRto);
}
+BOOST_AUTO_TEST_CASE(Bug5202)
+{
+ // If an interest is pending during a window decrease, it should not trigger
+ // another window decrease when it times out.
+ // This test emulates a network where RTT = 20ms and transmission time = 1ms.
+
+ // adding small sample to RTT estimator. This should set rto = 200ms
+ rttEstimator.addMeasurement(time::nanoseconds(1));
+ BOOST_REQUIRE_EQUAL(rttEstimator.getEstimatedRto(), time::milliseconds(200));
+
+ nDataSegments = 300;
+ pipeline->m_ssthresh = 0;
+ pipeline->m_cwnd = 20;
+
+ run(name);
+
+ advanceClocks(time::nanoseconds(1));
+ BOOST_CHECK_EQUAL(face.sentInterests.size(), 20);
+ advanceClocks(time::milliseconds(20));
+
+ // Segment 1 is lost. Receive segment 0 and 2-99
+ face.receive(*makeDataWithSegment(0));
+ advanceClocks(time::milliseconds(1));
+ for (uint64_t i = 2; i <= 99; ++i) {
+ face.receive(*makeDataWithSegment(i));
+ advanceClocks(time::milliseconds(1));
+ }
+
+ // Segment 100 is lost. Receive segment 100 to 181
+ for (uint64_t i = 101; i <= 181; ++i) {
+ face.receive(*makeDataWithSegment(i));
+ advanceClocks(time::milliseconds(1));
+ }
+
+ // 200ms passed after sending segment 1, check for timeout
+ BOOST_CHECK_GT(pipeline->m_cwnd, 27./2);
+ BOOST_CHECK_LT(pipeline->m_cwnd, 28./2);
+ BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, 1);
+ BOOST_CHECK_EQUAL(pipeline->m_nLossDecr, 1);
+
+ // Receive segment 182 to 300
+ for (uint64_t i = 182; i <= 300; ++i) {
+ face.receive(*makeDataWithSegment(i));
+ advanceClocks(time::milliseconds(1));
+ }
+
+ // The second packet should timeout now
+ BOOST_CHECK_EQUAL(pipeline->m_nTimeouts, 2);
+ // This timeout should NOT trigger another window decrease
+ BOOST_CHECK_EQUAL(pipeline->m_nLossDecr, 1);
+}
+
BOOST_AUTO_TEST_CASE(PrintSummaryWithNoRttMeasurements)
{
// test the console ouptut when no RTT measurement is available,
@@ -589,7 +637,6 @@
BOOST_CHECK_EQUAL(face.getNPendingInterests(), 0);
}
-
BOOST_AUTO_TEST_SUITE_END() // TestPipelineInterestsAimd
BOOST_AUTO_TEST_SUITE_END() // Chunks
diff --git a/tests/chunks/pipeline-interests-cubic.t.cpp b/tests/chunks/pipeline-interests-cubic.t.cpp
index 9934d9f..d84cce5 100644
--- a/tests/chunks/pipeline-interests-cubic.t.cpp
+++ b/tests/chunks/pipeline-interests-cubic.t.cpp
@@ -72,8 +72,6 @@
static constexpr double MARGIN = 0.001;
};
-constexpr double PipelineInterestCubicFixture::MARGIN;
-
BOOST_AUTO_TEST_SUITE(Chunks)
BOOST_FIXTURE_TEST_SUITE(TestPipelineInterestsCubic, PipelineInterestCubicFixture)
diff --git a/tools/chunks/catchunks/pipeline-interests-adaptive.cpp b/tools/chunks/catchunks/pipeline-interests-adaptive.cpp
index 3b3e0cd..891b94e 100644
--- a/tools/chunks/catchunks/pipeline-interests-adaptive.cpp
+++ b/tools/chunks/catchunks/pipeline-interests-adaptive.cpp
@@ -34,8 +34,6 @@
namespace ndn::chunks {
-constexpr double PipelineInterestsAdaptive::MIN_SSTHRESH;
-
PipelineInterestsAdaptive::PipelineInterestsAdaptive(Face& face,
RttEstimatorWithStats& rttEstimator,
const Options& opts)
@@ -83,6 +81,7 @@
return;
bool hasTimeout = false;
+ uint64_t highTimeoutSeg = 0;
for (auto& entry : m_segmentInfo) {
SegmentInfo& segInfo = entry.second;
@@ -91,13 +90,14 @@
if (timeElapsed > segInfo.rto) { // timer expired?
m_nTimeouts++;
hasTimeout = true;
+ highTimeoutSeg = std::max(highTimeoutSeg, entry.first);
enqueueForRetransmission(entry.first);
}
}
}
if (hasTimeout) {
- recordTimeout();
+ recordTimeout(highTimeoutSeg);
schedulePackets();
}
@@ -227,9 +227,7 @@
<< ", rto=" << segInfo.rto.count() / 1e6 << "ms\n";
}
- if (m_highData < recvSegNo) {
- m_highData = recvSegNo;
- }
+ m_highData = std::max(m_highData, recvSegNo);
// for segments in retx queue, we must not decrement m_nInFlight
// because it was already decremented when the segment timed out
@@ -310,7 +308,7 @@
case lp::NackReason::CONGESTION:
// treated the same as timeout for now
enqueueForRetransmission(segNo);
- recordTimeout();
+ recordTimeout(segNo);
schedulePackets();
break;
default:
@@ -327,16 +325,19 @@
return;
m_nTimeouts++;
- enqueueForRetransmission(getSegmentFromPacket(interest));
- recordTimeout();
+
+ uint64_t segNo = getSegmentFromPacket(interest);
+ enqueueForRetransmission(segNo);
+ recordTimeout(segNo);
schedulePackets();
}
void
-PipelineInterestsAdaptive::recordTimeout()
+PipelineInterestsAdaptive::recordTimeout(uint64_t segNo)
{
- if (m_options.disableCwa || m_highData > m_recPoint) {
- // react to only one timeout per RTT (conservative window adaptation)
+ if (m_options.disableCwa || segNo > m_recPoint) {
+ // interests that are still outstanding during a timeout event
+ // should not trigger another window decrease later (bug #5202)
m_recPoint = m_highInterest;
decreaseWindow();
diff --git a/tools/chunks/catchunks/pipeline-interests-adaptive.hpp b/tools/chunks/catchunks/pipeline-interests-adaptive.hpp
index 15e4dcf..7b7aad8 100644
--- a/tools/chunks/catchunks/pipeline-interests-adaptive.hpp
+++ b/tools/chunks/catchunks/pipeline-interests-adaptive.hpp
@@ -172,7 +172,7 @@
handleLifetimeExpiration(const Interest& interest);
void
- recordTimeout();
+ recordTimeout(uint64_t segNo);
void
enqueueForRetransmission(uint64_t segNo);