From d68f18ee6e52d572e09a2f70d811f45b3429de95 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Wed, 22 Sep 2021 15:55:54 +0200 Subject: [PATCH] dcsctp: Allow specifying minimum RTT variance This is mentioned in https://datatracker.ietf.org/doc/html/rfc4960#section-6.3.1 and further described in https://datatracker.ietf.org/doc/html/rfc6298#section-4. The TCP RFCs mentioned G as the clock granularity, but in SCTP it should be set much higher, to account for the delayed ack timeout (ATO) of the peer (as that can be seen as a very high clock granularity). That one is set to 200ms by default in many clients, so a reasonable default limit could be set to 220 ms. If the measured variance is higher, it will be used instead. Bug: webrtc:12943 Change-Id: Ifc217daa390850520da8b3beb0ef214181ff8c4e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232614 Commit-Queue: Victor Boivie Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#35068} --- net/dcsctp/public/dcsctp_options.h | 15 ++++++++ net/dcsctp/tx/retransmission_timeout.cc | 11 +++--- net/dcsctp/tx/retransmission_timeout.h | 1 + net/dcsctp/tx/retransmission_timeout_test.cc | 38 ++++++++++++++++---- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/net/dcsctp/public/dcsctp_options.h b/net/dcsctp/public/dcsctp_options.h index 92bb3a3dcb..2ac2aff9bf 100644 --- a/net/dcsctp/public/dcsctp_options.h +++ b/net/dcsctp/public/dcsctp_options.h @@ -128,6 +128,21 @@ struct DcSctpOptions { // unacknowledged packet. Whatever is smallest of RTO/2 and this will be used. DurationMs delayed_ack_max_timeout = DurationMs(200); + // The minimum limit for the measured RTT variance + // + // Setting this below the expected delayed ack timeout (+ margin) of the peer + // might result in unnecessary retransmissions, as the maximum time it takes + // to ACK a DATA chunk is typically RTT + ATO (delayed ack timeout), and when + // the SCTP channel is quite idle, and heartbeats dominate the source of RTT + // measurement, the RTO would converge with the smoothed RTT (SRTT). The + // default ATO is 200ms in usrsctp, and a 20ms (10%) margin would include the + // processing time of received packets and the clock granularity when setting + // the delayed ack timer on the peer. + // + // This is described for TCP in + // https://datatracker.ietf.org/doc/html/rfc6298#section-4. + DurationMs min_rtt_variance = DurationMs(220); + // Do slow start as TCP - double cwnd instead of increasing it by MTU. bool slow_start_tcp_style = false; diff --git a/net/dcsctp/tx/retransmission_timeout.cc b/net/dcsctp/tx/retransmission_timeout.cc index ee7c48b567..aa2863f931 100644 --- a/net/dcsctp/tx/retransmission_timeout.cc +++ b/net/dcsctp/tx/retransmission_timeout.cc @@ -20,6 +20,7 @@ RetransmissionTimeout::RetransmissionTimeout(const DcSctpOptions& options) : min_rto_(*options.rto_min), max_rto_(*options.rto_max), max_rtt_(*options.rtt_max), + min_rtt_variance_(*options.min_rtt_variance), rto_(*options.rto_initial) {} void RetransmissionTimeout::ObserveRTT(DurationMs measured_rtt) { @@ -48,12 +49,12 @@ void RetransmissionTimeout::ObserveRTT(DurationMs measured_rtt) { rtt_diff -= (scaled_rtt_var_ >> kRttVarShift); scaled_rtt_var_ += rtt_diff; } - rto_ = (scaled_srtt_ >> kRttShift) + scaled_rtt_var_; - // 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::max(rto_, rtt + 1); + if (scaled_rtt_var_ < min_rtt_variance_) { + scaled_rtt_var_ = min_rtt_variance_; + } + + rto_ = (scaled_srtt_ >> kRttShift) + scaled_rtt_var_; // Clamp RTO between min and max. rto_ = std::min(std::max(rto_, min_rto_), max_rto_); diff --git a/net/dcsctp/tx/retransmission_timeout.h b/net/dcsctp/tx/retransmission_timeout.h index f3a95532d0..7cbcc6fcc9 100644 --- a/net/dcsctp/tx/retransmission_timeout.h +++ b/net/dcsctp/tx/retransmission_timeout.h @@ -44,6 +44,7 @@ class RetransmissionTimeout { const int32_t min_rto_; const int32_t max_rto_; const int32_t max_rtt_; + const int32_t min_rtt_variance_; // If this is the first measurement bool first_measurement_ = true; // Smoothed Round-Trip Time, shifted by kRttShift diff --git a/net/dcsctp/tx/retransmission_timeout_test.cc b/net/dcsctp/tx/retransmission_timeout_test.cc index d2d071948e..f3b20a86ba 100644 --- a/net/dcsctp/tx/retransmission_timeout_test.cc +++ b/net/dcsctp/tx/retransmission_timeout_test.cc @@ -20,6 +20,7 @@ constexpr DurationMs kMaxRtt = DurationMs(8'000); constexpr DurationMs kInitialRto = DurationMs(200); constexpr DurationMs kMaxRto = DurationMs(800); constexpr DurationMs kMinRto = DurationMs(120); +constexpr DurationMs kMinRttVariance = DurationMs(220); DcSctpOptions MakeOptions() { DcSctpOptions options; @@ -27,6 +28,7 @@ DcSctpOptions MakeOptions() { options.rto_initial = kInitialRto; options.rto_max = kMaxRto; options.rto_min = kMinRto; + options.min_rtt_variance = kMinRttVariance; return options; } @@ -82,13 +84,13 @@ TEST(RetransmissionTimeoutTest, CalculatesRtoForStableRtt) { rto_.ObserveRTT(DurationMs(124)); EXPECT_EQ(*rto_.rto(), 372); rto_.ObserveRTT(DurationMs(128)); - EXPECT_EQ(*rto_.rto(), 314); + EXPECT_EQ(*rto_.rto(), 344); rto_.ObserveRTT(DurationMs(123)); - EXPECT_EQ(*rto_.rto(), 268); + EXPECT_EQ(*rto_.rto(), 344); rto_.ObserveRTT(DurationMs(125)); - EXPECT_EQ(*rto_.rto(), 233); + EXPECT_EQ(*rto_.rto(), 344); rto_.ObserveRTT(DurationMs(127)); - EXPECT_EQ(*rto_.rto(), 209); + EXPECT_EQ(*rto_.rto(), 344); } TEST(RetransmissionTimeoutTest, CalculatesRtoForUnstableRtt) { @@ -130,7 +132,7 @@ TEST(RetransmissionTimeoutTest, WillStabilizeAfterAWhile) { rto_.ObserveRTT(DurationMs(124)); EXPECT_EQ(*rto_.rto(), 372); rto_.ObserveRTT(DurationMs(124)); - EXPECT_EQ(*rto_.rto(), 340); + EXPECT_EQ(*rto_.rto(), 367); } TEST(RetransmissionTimeoutTest, WillAlwaysStayAboveRTT) { @@ -141,10 +143,32 @@ TEST(RetransmissionTimeoutTest, WillAlwaysStayAboveRTT) { // any jitter will increase the RTO. RetransmissionTimeout rto_(MakeOptions()); - for (int i = 0; i < 100; ++i) { + for (int i = 0; i < 1000; ++i) { rto_.ObserveRTT(DurationMs(124)); } - EXPECT_GT(*rto_.rto(), 124); + EXPECT_EQ(*rto_.rto(), 344); +} + +TEST(RetransmissionTimeoutTest, CanSpecifySmallerMinimumRttVariance) { + DcSctpOptions options = MakeOptions(); + options.min_rtt_variance = kMinRttVariance - DurationMs(100); + RetransmissionTimeout rto_(options); + + for (int i = 0; i < 1000; ++i) { + rto_.ObserveRTT(DurationMs(124)); + } + EXPECT_EQ(*rto_.rto(), 244); +} + +TEST(RetransmissionTimeoutTest, CanSpecifyLargerMinimumRttVariance) { + DcSctpOptions options = MakeOptions(); + options.min_rtt_variance = kMinRttVariance + DurationMs(100); + RetransmissionTimeout rto_(options); + + for (int i = 0; i < 1000; ++i) { + rto_.ObserveRTT(DurationMs(124)); + } + EXPECT_EQ(*rto_.rto(), 444); } } // namespace