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 <boivie@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35068}

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;
 

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_);

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

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