Pick the DTLS handshake timeout based on the ICE RTT estimate

webrtc/p2p/base/dtlstransportchannel.cc

Index: webrtc/p2p/base/dtlstransportchannel.cc
diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc
index 476005fd54e17feb0bb77b96a3eaa2175c2aa8b0..f12b5d0b704e8e86a5b7c6056c3d1276834ec9e9 100644
--- a/webrtc/p2p/base/dtlstransportchannel.cc
+++ b/webrtc/p2p/base/dtlstransportchannel.cc
@@ -34,6 +34,11 @@ static const size_t kMinRtpPacketLen = 12;
 // after they have been written, so a capacity of "1" is sufficient.
 static const size_t kMaxPendingPackets = 1;
 
+// Minimum and maximum values for the initial DTLS handshake timeout. We'll pick
+// an initial timeout based on ICE RTT estimates, but clamp it to this range.
+static const int kMinHandshakeTimeout = 50;
+static const int kMaxHandshakeTimeout = 3000;
+
 static bool IsDtlsPacket(const char* data, size_t len) {
   const uint8_t* u = reinterpret_cast<const uint8_t*>(data);
   return (len >= kDtlsRecordHeaderLen && (u[0] > 19 && u[0] < 64));
@@ -603,6 +608,8 @@ void DtlsTransport::OnDtlsEvent(rtc::StreamInterface* dtls, int sig, int err) {
 
 void DtlsTransport::MaybeStartDtls() {
   if (dtls_ && ice_transport_->writable()) {
+    ConfigureHandshakeTimeout();
+
     if (dtls_->StartSSL()) {
       // This should never fail:
       // Because we are operating in a nonblocking mode and all
@@ -693,4 +700,24 @@ void DtlsTransport::OnDtlsHandshakeError(rtc::SSLHandshakeError error) {
   SignalDtlsHandshakeError(error);
 }
 
+void DtlsTransport::ConfigureHandshakeTimeout() {
+  RTC_DCHECK(dtls_);
+  rtc::Optional<int> rtt = ice_transport_->GetRttEstimate();

[Taylor Brandstetter, 2017/02/02 18:07:41]

If no round trips have occurred yet (which would happen with the "presume
TURN<->TURN connections will work" option), won't this return the max RTT, and
result in a configured timeout of 3 seconds?

[Taylor Brandstetter, 2017/02/02 18:13:31]

This is probably why the "EndToEndConnectionTimeWithTurnTurnPair" test is
failing:

"configuring DTLS handshake timeout 3000 based on ICE RTT 3000"

I guess that test ended up being useful after all.

[skvlad, 2017/02/02 22:19:33]

I've changed P2PTransportChannel->GetRttEstimate() to only use the RTT if there
is at least one sample.
Switching RTT to rtc::Optional() everywhere will change the existing behavior
for GetStats and many other places - which might be better done in a separate
CL.

+  if (rtt) {
+    // Limit the timeout to a reasonable range in case the ICE RTT takes
+    // extreme values.
+    int initial_timeout = std::max(kMinHandshakeTimeout,
+                                   std::min(kMaxHandshakeTimeout,
+                                            2 * (*rtt)));
+    LOG_J(LS_INFO, this) << "configuring DTLS handshake timeout "
+                         << initial_timeout << " based on ICE RTT " << *rtt;
+
+    dtls_->SetHandshakeTimeout(initial_timeout);
+  } else {
+    LOG_J(LS_INFO, this)
+        << "no RTT estimate - using default DTLS handshake timeout";
+  }
+}
+
+
 }  // namespace cricket