diff --git a/net/dcsctp/tx/retransmission_timeout.cc b/net/dcsctp/tx/retransmission_timeout.cc index f38b94d32c..7d545a07d0 100644 --- a/net/dcsctp/tx/retransmission_timeout.cc +++ b/net/dcsctp/tx/retransmission_timeout.cc @@ -58,6 +58,11 @@ void RetransmissionTimeout::ObserveRTT(DurationMs measured_rtt) { rto_ = srtt_ + 4 * rttvar_; } + // If the RTO becomes smaller or equal to RTT, expiration timers will be + // scheduled at the same time as packets are expected. Only happens in + // extremely stable RTTs, i.e. in simulations. + rto_ = std::fmax(rto_, rtt + 1); + // Clamp RTO between min and max. rto_ = std::fmin(std::fmax(rto_, min_rto_), max_rto_); } diff --git a/net/dcsctp/tx/retransmission_timeout_test.cc b/net/dcsctp/tx/retransmission_timeout_test.cc index eb5e72e7ba..3b2e3399fe 100644 --- a/net/dcsctp/tx/retransmission_timeout_test.cc +++ b/net/dcsctp/tx/retransmission_timeout_test.cc @@ -80,29 +80,29 @@ TEST(RetransmissionTimeoutTest, WillNeverGoAboveMaximumRto) { TEST(RetransmissionTimeoutTest, CalculatesRtoForStableRtt) { RetransmissionTimeout rto_(MakeOptions()); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 372); + EXPECT_EQ(*rto_.rto(), 372); rto_.ObserveRTT(DurationMs(128)); - EXPECT_THAT(*rto_.rto(), 314); + EXPECT_EQ(*rto_.rto(), 314); rto_.ObserveRTT(DurationMs(123)); - EXPECT_THAT(*rto_.rto(), 268); + EXPECT_EQ(*rto_.rto(), 268); rto_.ObserveRTT(DurationMs(125)); - EXPECT_THAT(*rto_.rto(), 233); + EXPECT_EQ(*rto_.rto(), 233); rto_.ObserveRTT(DurationMs(127)); - EXPECT_THAT(*rto_.rto(), 208); + EXPECT_EQ(*rto_.rto(), 208); } TEST(RetransmissionTimeoutTest, CalculatesRtoForUnstableRtt) { RetransmissionTimeout rto_(MakeOptions()); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 372); + EXPECT_EQ(*rto_.rto(), 372); rto_.ObserveRTT(DurationMs(402)); - EXPECT_THAT(*rto_.rto(), 622); + EXPECT_EQ(*rto_.rto(), 622); rto_.ObserveRTT(DurationMs(728)); - EXPECT_THAT(*rto_.rto(), 800); + EXPECT_EQ(*rto_.rto(), 800); rto_.ObserveRTT(DurationMs(89)); - EXPECT_THAT(*rto_.rto(), 800); + EXPECT_EQ(*rto_.rto(), 800); rto_.ObserveRTT(DurationMs(126)); - EXPECT_THAT(*rto_.rto(), 800); + EXPECT_EQ(*rto_.rto(), 800); } TEST(RetransmissionTimeoutTest, WillStabilizeAfterAWhile) { @@ -112,25 +112,40 @@ TEST(RetransmissionTimeoutTest, WillStabilizeAfterAWhile) { rto_.ObserveRTT(DurationMs(728)); rto_.ObserveRTT(DurationMs(89)); rto_.ObserveRTT(DurationMs(126)); - EXPECT_THAT(*rto_.rto(), 800); + EXPECT_EQ(*rto_.rto(), 800); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 800); + EXPECT_EQ(*rto_.rto(), 800); rto_.ObserveRTT(DurationMs(122)); - EXPECT_THAT(*rto_.rto(), 709); + EXPECT_EQ(*rto_.rto(), 709); rto_.ObserveRTT(DurationMs(123)); - EXPECT_THAT(*rto_.rto(), 630); + EXPECT_EQ(*rto_.rto(), 630); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 561); + EXPECT_EQ(*rto_.rto(), 561); rto_.ObserveRTT(DurationMs(122)); - EXPECT_THAT(*rto_.rto(), 504); + EXPECT_EQ(*rto_.rto(), 504); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 453); + EXPECT_EQ(*rto_.rto(), 453); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 409); + EXPECT_EQ(*rto_.rto(), 409); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 372); + EXPECT_EQ(*rto_.rto(), 372); rto_.ObserveRTT(DurationMs(124)); - EXPECT_THAT(*rto_.rto(), 339); + EXPECT_EQ(*rto_.rto(), 339); } + +TEST(RetransmissionTimeoutTest, WillAlwaysStayAboveRTT) { + // In simulations, it's quite common to have a very stable RTT, and having an + // RTO at the same value will cause issues as expiry timers will be scheduled + // to be expire exactly when a packet is supposed to arrive. The RTO must be + // larger than the RTT. In non-simulated environments, this is a non-issue as + // any jitter will increase the RTO. + RetransmissionTimeout rto_(MakeOptions()); + + for (int i = 0; i < 100; ++i) { + rto_.ObserveRTT(DurationMs(124)); + } + EXPECT_GT(*rto_.rto(), 124); +} + } // namespace } // namespace dcsctp