dtls-in-stun: Redo handling of when ice complete before dtls
Remove the restart on OnWritableState and instead retune the timeout
(not guaranteed to work!) and let P2PTransportChannel drive retransmits
using StunPings in this scenario (something which we hope to remove!)
Add DtlsIce integration test using emulated network, which 50%
packet loss. Works fine! (except below)
Note: That testcase fails for Dtls1.3 if delay is increased from
50ms to 100ms...unless dtls in stun is enabled on both peers,
in which case it pass (yay). Investigate why!
BUG=webrtc:367395350
Change-Id: I6e15b08cf8871194b45a03f7bdf8febd09e2c407
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/380506
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44093}
diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
index 368f9ba..aa380bb 100644
--- a/p2p/BUILD.gn
+++ b/p2p/BUILD.gn
@@ -1113,15 +1113,19 @@
"../api:array_view",
"../api:async_dns_resolver",
"../api:candidate",
+ "../api:create_network_emulation_manager",
+ "../api:create_time_controller",
"../api:dtls_transport_interface",
"../api:field_trials",
"../api:field_trials_view",
"../api:ice_transport_interface",
"../api:mock_async_dns_resolver",
+ "../api:network_emulation_manager_api",
"../api:packet_socket_factory",
"../api:rtc_error",
"../api:rtc_error_matchers",
"../api:scoped_refptr",
+ "../api:simulated_network_api",
"../api/crypto:options",
"../api/environment",
"../api/environment:environment_factory",
@@ -1129,6 +1133,8 @@
"../api/task_queue:pending_task_safety_flag",
"../api/transport:enums",
"../api/transport:stun_types",
+ "../api/units:data_rate",
+ "../api/units:time_delta",
"../api/units:time_delta",
"../rtc_base:async_packet_socket",
"../rtc_base:async_packet_socket",
diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h
index f51b141..efa2813 100644
--- a/p2p/base/ice_transport_internal.h
+++ b/p2p/base/ice_transport_internal.h
@@ -44,6 +44,12 @@
namespace cricket {
+// 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.
+// TODO(webrtc:367395350): Move back into dtls_transport.cc.
+constexpr int kMinDtlsHandshakeTimeoutMs = 50;
+constexpr int kMaxDtlsHandshakeTimeoutMs = 3000;
+
struct IceTransportStats {
CandidateStatsList candidate_stats_list;
ConnectionInfos connection_infos;
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index f36ac33..55e68b9 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -37,6 +37,7 @@
#include "api/ice_transport_interface.h"
#include "api/rtc_error.h"
#include "api/sequence_checker.h"
+#include "api/task_queue/pending_task_safety_flag.h"
#include "api/transport/enums.h"
#include "api/transport/stun.h"
#include "api/units/time_delta.h"
@@ -2245,17 +2246,60 @@
}
SignalWritableState(this);
- if (writable_ && selected_connection_ &&
- !dtls_stun_piggyback_callbacks_.empty()) {
- // TODO(webrtc:367395350): Construct test that fail w/o this extra ping!
- // TODO(webrtc:367395350): Move this into DtlsTransport somehow.
- // Need to STUN ping here to get the last bit of the DTLS handshake across
- // as quickly as possible. Only done when DTLS-in-STUN is configured
- // and the data callback has not been reset due to lack of support.
- SendPingRequestInternal(selected_connection_);
+ if (config_.dtls_handshake_in_stun) {
+ // TODO (jonaso, webrtc:367395350): Switch to upcoming
+ // DTLSv1_set_timeout_duration. Remove once we can get DTLS to handle
+ // retransmission also when handshake is not complete but we become writable
+ // (e.g. by setting a good timeout).
+ SendPeriodicPingUntilDtlsConnected();
}
}
+// TODO (jonaso, webrtc:367395350): Switch to upcoming
+// DTLSv1_set_timeout_duration. Remove once we can get DTLS to handle
+// retransmission also when handshake is not complete but we become writable
+// (e.g. by setting a good timeout).
+void P2PTransportChannel::SendPeriodicPingUntilDtlsConnected() {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ if (pending_ping_until_dtls_connected_ == true) {
+ // SendPeriodicPingUntilDtlsConnected is called in two places
+ // a) Either by PostTask, in which case pending_ping_until_dtls_connected_
+ // is FALSE b) When Ice get connected, in which it is unknown if
+ // pending_ping_until_dtls_connected_.
+ return;
+ }
+ if (writable_ && config_.dtls_handshake_in_stun &&
+ !dtls_stun_piggyback_callbacks_.empty()) {
+ auto has_data_to_send =
+ dtls_stun_piggyback_callbacks_.send_data(STUN_BINDING_REQUEST);
+ if (!has_data_to_send.first && !has_data_to_send.second) {
+ // No data to send, we're done.
+ return;
+ }
+ // writable_ is TRUE => we must have a selected_connection_ ?
+ RTC_DCHECK(selected_connection_ != nullptr);
+ SendPingRequestInternal(selected_connection_);
+ RTC_LOG(LS_INFO) << ToString() << ": Sending extra ping for DTLS";
+ }
+
+ const auto rtt_ms = GetRttEstimate().value_or(100);
+ const int delay_ms =
+ std::max(kMinDtlsHandshakeTimeoutMs,
+ std::min(kMaxDtlsHandshakeTimeoutMs, 2 * rtt_ms));
+
+ // Set pending before we post task.
+ pending_ping_until_dtls_connected_ = true;
+ network_thread_->PostDelayedHighPrecisionTask(
+ webrtc::SafeTask(safety_flag_.flag(),
+ [this] {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ // Clear pending then the PostTask runs.
+ pending_ping_until_dtls_connected_ = false;
+ SendPeriodicPingUntilDtlsConnected();
+ }),
+ TimeDelta::Millis(delay_ms));
+}
+
void P2PTransportChannel::SetReceiving(bool receiving) {
RTC_DCHECK_RUN_ON(network_thread_);
if (receiving_ == receiving) {
diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h
index 334f9a0..8be7770 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/p2p/base/p2p_transport_channel.h
@@ -495,6 +495,7 @@
std::unique_ptr<StunAttribute> GoogDeltaReceived(
const StunByteStringAttribute*);
void GoogDeltaAckReceived(webrtc::RTCErrorOr<const StunUInt64Attribute*>);
+ void SendPeriodicPingUntilDtlsConnected();
// Bytes/packets sent/received on this channel.
uint64_t bytes_sent_ = 0;
@@ -521,6 +522,8 @@
StunDictionaryView stun_dict_view_;
// DTLS-STUN piggybacking callbacks.
+ bool pending_ping_until_dtls_connected_ = false;
+ webrtc::ScopedTaskSafetyDetached safety_flag_;
DtlsStunPiggybackCallbacks dtls_stun_piggyback_callbacks_;
};
diff --git a/p2p/dtls/dtls_ice_integrationtest.cc b/p2p/dtls/dtls_ice_integrationtest.cc
index 5869183..095da6f 100644
--- a/p2p/dtls/dtls_ice_integrationtest.cc
+++ b/p2p/dtls/dtls_ice_integrationtest.cc
@@ -16,7 +16,12 @@
#include "api/candidate.h"
#include "api/crypto/crypto_options.h"
#include "api/scoped_refptr.h"
+#include "api/test/create_network_emulation_manager.h"
+#include "api/test/create_time_controller.h"
+#include "api/test/network_emulation_manager.h"
#include "api/test/rtc_error_matchers.h"
+#include "api/test/simulated_network.h"
+#include "api/units/data_rate.h"
#include "api/units/time_delta.h"
#include "p2p/base/basic_packet_socket_factory.h"
#include "p2p/base/ice_transport_internal.h"
@@ -40,7 +45,7 @@
#include "test/wait_until.h"
namespace {
-constexpr int kDefaultTimeout = 10000;
+constexpr int kDefaultTimeout = 30000;
void SetRemoteFingerprintFromCert(
cricket::DtlsTransport& transport,
@@ -59,6 +64,12 @@
namespace cricket {
using ::testing::IsTrue;
+using ::webrtc::BuiltInNetworkBehaviorConfig;
+using ::webrtc::EmulatedEndpoint;
+using ::webrtc::EmulatedEndpointConfig;
+using ::webrtc::EmulatedNetworkManagerInterface;
+using ::webrtc::EmulatedNetworkNode;
+using ::webrtc::NetworkEmulationManager;
class DtlsIceIntegrationTest : public ::testing::TestWithParam<std::tuple<
/* client_piggyback= */ bool,
@@ -68,40 +79,30 @@
public sigslot::has_slots<> {
public:
void CandidateC2S(IceTransportInternal*, const Candidate& c) {
- thread_.PostTask([this, c = c]() { server_ice_->AddRemoteCandidate(c); });
+ server_thread()->PostTask(
+ [this, c = c]() { server_.ice->AddRemoteCandidate(c); });
}
void CandidateS2C(IceTransportInternal*, const Candidate& c) {
- thread_.PostTask([this, c = c]() { client_ice_->AddRemoteCandidate(c); });
+ client_thread()->PostTask(
+ [this, c = c]() { client_.ice->AddRemoteCandidate(c); });
}
+ private:
+ struct Endpoint {
+ webrtc::EmulatedNetworkManagerInterface* emulated_network_manager = nullptr;
+ std::unique_ptr<rtc::NetworkManager> network_manager;
+ std::unique_ptr<rtc::BasicPacketSocketFactory> packet_socket_factory;
+ std::unique_ptr<PortAllocator> allocator;
+ std::unique_ptr<IceTransportInternal> ice;
+ std::unique_ptr<DtlsTransport> dtls;
+ std::unique_ptr<DtlsTransport> server_dtls_;
+ };
+
protected:
DtlsIceIntegrationTest()
: ss_(std::make_unique<rtc::VirtualSocketServer>()),
socket_factory_(
std::make_unique<rtc::BasicPacketSocketFactory>(ss_.get())),
- thread_(ss_.get()),
- client_allocator_(
- std::make_unique<BasicPortAllocator>(&network_manager_,
- socket_factory_.get())),
- server_allocator_(
- std::make_unique<BasicPortAllocator>(&network_manager_,
- socket_factory_.get())),
- client_ice_(
- std::make_unique<P2PTransportChannel>("client_transport",
- 0,
- client_allocator_.get())),
- server_ice_(
- std::make_unique<P2PTransportChannel>("server_transport",
- 0,
- server_allocator_.get())),
- client_dtls_(client_ice_.get(),
- webrtc::CryptoOptions(),
- /*event_log=*/nullptr,
- std::get<2>(GetParam())),
- server_dtls_(server_ice_.get(),
- webrtc::CryptoOptions(),
- /*event_log=*/nullptr,
- std::get<2>(GetParam())),
client_ice_parameters_("c_ufrag",
"c_icepwd_something_something",
false),
@@ -109,56 +110,133 @@
"s_icepwd_something_something",
false),
client_dtls_stun_piggyback_(std::get<0>(GetParam())),
- server_dtls_stun_piggyback_(std::get<1>(GetParam())) {
- // Enable(or disable) the dtls_in_stun parameter before
- // DTLS is negotiated.
- cricket::IceConfig client_config;
- client_config.continual_gathering_policy = GATHER_CONTINUALLY;
- client_config.dtls_handshake_in_stun = client_dtls_stun_piggyback_;
- client_ice_->SetIceConfig(client_config);
+ server_dtls_stun_piggyback_(std::get<1>(GetParam())) {}
- cricket::IceConfig server_config;
- server_config.dtls_handshake_in_stun = server_dtls_stun_piggyback_;
- server_config.continual_gathering_policy = GATHER_CONTINUALLY;
- server_ice_->SetIceConfig(server_config);
+ void ConfigureEmulatedNetwork() {
+ network_emulation_manager_ = webrtc::CreateNetworkEmulationManager(
+ {.time_mode = webrtc::TimeMode::kSimulated});
- // Setup ICE.
- client_ice_->SetIceParameters(client_ice_parameters_);
- client_ice_->SetRemoteIceParameters(server_ice_parameters_);
- if (std::get<3>(GetParam())) {
- client_ice_->SetIceRole(ICEROLE_CONTROLLING);
- } else {
- client_ice_->SetIceRole(ICEROLE_CONTROLLED);
- }
- client_ice_->SignalCandidateGathered.connect(
- this, &DtlsIceIntegrationTest::CandidateC2S);
- server_ice_->SetIceParameters(server_ice_parameters_);
- server_ice_->SetRemoteIceParameters(client_ice_parameters_);
- if (std::get<3>(GetParam())) {
- server_ice_->SetIceRole(ICEROLE_CONTROLLED);
- } else {
- server_ice_->SetIceRole(ICEROLE_CONTROLLING);
- }
- server_ice_->SignalCandidateGathered.connect(
- this, &DtlsIceIntegrationTest::CandidateS2C);
+ BuiltInNetworkBehaviorConfig networkBehavior;
+ networkBehavior.link_capacity = webrtc::DataRate::KilobitsPerSec(200);
+ // TODO (webrtc:383141571) : Investigate why this testcase fails for
+ // DTLS 1.3 delay if networkBehavior.queue_delay_ms = 100ms.
+ // - unless both peers support dtls in stun, in which case it passes.
+ // - note: only for dtls1.3, it works for dtls1.2!
+ networkBehavior.queue_delay_ms = 50;
+ networkBehavior.queue_length_packets = 30;
+ networkBehavior.loss_percent = 50;
+ auto pair = network_emulation_manager_->CreateEndpointPairWithTwoWayRoutes(
+ networkBehavior);
- // Setup DTLS.
+ client_.emulated_network_manager = pair.first;
+ server_.emulated_network_manager = pair.second;
+ }
+
+ void SetupEndpoint(
+ Endpoint& ep,
+ bool client,
+ const rtc::scoped_refptr<rtc::RTCCertificate> client_certificate,
+ const rtc::scoped_refptr<rtc::RTCCertificate> server_certificate) {
+ thread(ep)->BlockingCall([&]() {
+ if (network_emulation_manager_ == nullptr) {
+ ep.allocator = std::make_unique<BasicPortAllocator>(
+ &network_manager_, socket_factory_.get());
+ } else {
+ ep.network_manager =
+ ep.emulated_network_manager->ReleaseNetworkManager();
+ ep.packet_socket_factory =
+ std::make_unique<rtc::BasicPacketSocketFactory>(
+ ep.emulated_network_manager->socket_factory());
+ ep.allocator = std::make_unique<BasicPortAllocator>(
+ ep.network_manager.get(), ep.packet_socket_factory.get());
+ }
+ ep.allocator->set_flags(ep.allocator->flags() |
+ cricket::PORTALLOCATOR_DISABLE_TCP);
+ ep.ice = std::make_unique<P2PTransportChannel>(
+ client ? "client_transport" : "server_transport", 0,
+ ep.allocator.get());
+ ep.dtls = std::make_unique<DtlsTransport>(
+ ep.ice.get(), webrtc::CryptoOptions(),
+ /*event_log=*/nullptr, std::get<2>(GetParam()));
+
+ // Enable(or disable) the dtls_in_stun parameter before
+ // DTLS is negotiated.
+ cricket::IceConfig config;
+ config.continual_gathering_policy = GATHER_CONTINUALLY;
+ config.dtls_handshake_in_stun =
+ client ? client_dtls_stun_piggyback_ : server_dtls_stun_piggyback_;
+ ep.ice->SetIceConfig(config);
+
+ // Setup ICE.
+ ep.ice->SetIceParameters(client ? client_ice_parameters_
+ : server_ice_parameters_);
+ ep.ice->SetRemoteIceParameters(client ? server_ice_parameters_
+ : client_ice_parameters_);
+ if (client) {
+ ep.ice->SetIceRole(std::get<3>(GetParam()) ? ICEROLE_CONTROLLED
+ : ICEROLE_CONTROLLING);
+ } else {
+ ep.ice->SetIceRole(std::get<3>(GetParam()) ? ICEROLE_CONTROLLING
+ : ICEROLE_CONTROLLED);
+ }
+ if (client) {
+ ep.ice->SignalCandidateGathered.connect(
+ this, &DtlsIceIntegrationTest::CandidateC2S);
+ } else {
+ ep.ice->SignalCandidateGathered.connect(
+ this, &DtlsIceIntegrationTest::CandidateS2C);
+ }
+
+ // Setup DTLS.
+ ep.dtls->SetLocalCertificate(client ? client_certificate
+ : server_certificate);
+ ep.dtls->SetDtlsRole(client ? rtc::SSL_SERVER : rtc::SSL_CLIENT);
+ SetRemoteFingerprintFromCert(
+ *ep.dtls.get(), client ? server_certificate : client_certificate);
+ });
+ }
+
+ void Prepare() {
auto client_certificate = rtc::RTCCertificate::Create(
rtc::SSLIdentity::Create("test", rtc::KT_DEFAULT));
- client_dtls_.SetLocalCertificate(client_certificate);
- client_dtls_.SetDtlsRole(rtc::SSL_SERVER);
auto server_certificate = rtc::RTCCertificate::Create(
rtc::SSLIdentity::Create("test", rtc::KT_DEFAULT));
- server_dtls_.SetLocalCertificate(server_certificate);
- server_dtls_.SetDtlsRole(rtc::SSL_CLIENT);
- SetRemoteFingerprintFromCert(server_dtls_, client_certificate);
- SetRemoteFingerprintFromCert(client_dtls_, server_certificate);
+ if (network_emulation_manager_ == nullptr) {
+ thread_ = std::make_unique<rtc::AutoSocketServerThread>(ss_.get());
+ }
+
+ client_thread()->BlockingCall([&]() {
+ SetupEndpoint(client_, /* client= */ true, client_certificate,
+ server_certificate);
+ });
+
+ server_thread()->BlockingCall([&]() {
+ SetupEndpoint(server_, /* client= */ false, client_certificate,
+ server_certificate);
+ });
// Setup the network.
- network_manager_.AddInterface(rtc::SocketAddress("192.168.1.1", 0));
- client_allocator_->Initialize();
- server_allocator_->Initialize();
+ if (network_emulation_manager_ == nullptr) {
+ network_manager_.AddInterface(rtc::SocketAddress("192.168.1.1", 0));
+ }
+
+ client_thread()->BlockingCall([&]() { client_.allocator->Initialize(); });
+ server_thread()->BlockingCall([&]() { server_.allocator->Initialize(); });
+ }
+
+ void TearDown() {
+ client_thread()->BlockingCall([&]() {
+ client_.dtls.reset();
+ client_.ice.reset();
+ client_.allocator.reset();
+ });
+
+ server_thread()->BlockingCall([&]() {
+ server_.dtls.reset();
+ server_.ice.reset();
+ server_.allocator.reset();
+ });
}
~DtlsIceIntegrationTest() = default;
@@ -175,59 +253,105 @@
return count;
}
+ webrtc::WaitUntilSettings wait_until_settings() {
+ if (network_emulation_manager_ == nullptr) {
+ return {
+ .timeout = webrtc::TimeDelta::Millis(kDefaultTimeout),
+ .clock = &fake_clock_,
+ };
+ } else {
+ return {
+ .timeout = webrtc::TimeDelta::Millis(kDefaultTimeout),
+ .clock = network_emulation_manager_->time_controller(),
+ };
+ }
+ }
+
+ rtc::Thread* thread(Endpoint& ep) {
+ if (ep.emulated_network_manager == nullptr) {
+ return thread_.get();
+ } else {
+ return ep.emulated_network_manager->network_thread();
+ }
+ }
+
+ rtc::Thread* client_thread() { return thread(client_); }
+
+ rtc::Thread* server_thread() { return thread(server_); }
+
+ rtc::ScopedFakeClock fake_clock_;
rtc::FakeNetworkManager network_manager_;
std::unique_ptr<rtc::VirtualSocketServer> ss_;
std::unique_ptr<rtc::BasicPacketSocketFactory> socket_factory_;
- rtc::AutoSocketServerThread thread_;
+ std::unique_ptr<webrtc::NetworkEmulationManager> network_emulation_manager_;
+ std::unique_ptr<rtc::AutoSocketServerThread> thread_;
- std::unique_ptr<PortAllocator> client_allocator_;
- std::unique_ptr<PortAllocator> server_allocator_;
-
- std::unique_ptr<IceTransportInternal> client_ice_;
- std::unique_ptr<IceTransportInternal> server_ice_;
-
- DtlsTransport client_dtls_;
- DtlsTransport server_dtls_;
+ Endpoint client_;
+ Endpoint server_;
IceParameters client_ice_parameters_;
IceParameters server_ice_parameters_;
bool client_dtls_stun_piggyback_;
bool server_dtls_stun_piggyback_;
-
- rtc::ScopedFakeClock fake_clock_;
};
TEST_P(DtlsIceIntegrationTest, SmokeTest) {
- client_ice_->MaybeStartGathering();
- server_ice_->MaybeStartGathering();
+ Prepare();
+ client_.ice->MaybeStartGathering();
+ server_.ice->MaybeStartGathering();
// Note: this only reaches the pending piggybacking state.
EXPECT_THAT(
webrtc::WaitUntil(
- [&] { return client_dtls_.writable() && server_dtls_.writable(); },
- IsTrue(),
- {.timeout = webrtc::TimeDelta::Millis(kDefaultTimeout),
- .clock = &fake_clock_}),
+ [&] { return client_.dtls->writable() && server_.dtls->writable(); },
+ IsTrue(), wait_until_settings()),
webrtc::IsRtcOk());
- EXPECT_EQ(client_dtls_.IsDtlsPiggybackSupportedByPeer(),
+ EXPECT_EQ(client_.dtls->IsDtlsPiggybackSupportedByPeer(),
client_dtls_stun_piggyback_ && server_dtls_stun_piggyback_);
- EXPECT_EQ(server_dtls_.IsDtlsPiggybackSupportedByPeer(),
+ EXPECT_EQ(server_.dtls->IsDtlsPiggybackSupportedByPeer(),
client_dtls_stun_piggyback_ && server_dtls_stun_piggyback_);
// Validate that we can add new Connections (that become writable).
network_manager_.AddInterface(rtc::SocketAddress("192.168.2.1", 0));
EXPECT_THAT(webrtc::WaitUntil(
[&] {
- return CountWritableConnections(client_ice_.get()) > 1 &&
- CountWritableConnections(server_ice_.get()) > 1;
+ return CountWritableConnections(client_.ice.get()) > 1 &&
+ CountWritableConnections(server_.ice.get()) > 1;
},
- IsTrue(),
- {.timeout = webrtc::TimeDelta::Millis(kDefaultTimeout),
- .clock = &fake_clock_}),
+ IsTrue(), wait_until_settings()),
webrtc::IsRtcOk());
}
+TEST_P(DtlsIceIntegrationTest, TestWithPacketLoss) {
+ ConfigureEmulatedNetwork();
+ Prepare();
+
+ client_thread()->PostTask([&]() { client_.ice->MaybeStartGathering(); });
+
+ server_thread()->PostTask([&]() { server_.ice->MaybeStartGathering(); });
+
+ EXPECT_THAT(webrtc::WaitUntil(
+ [&] {
+ return client_thread()->BlockingCall([&]() {
+ return client_.dtls->writable();
+ }) && server_thread()->BlockingCall([&]() {
+ return server_.dtls->writable();
+ });
+ },
+ IsTrue(), wait_until_settings()),
+ webrtc::IsRtcOk());
+
+ EXPECT_EQ(client_thread()->BlockingCall([&]() {
+ return client_.dtls->IsDtlsPiggybackSupportedByPeer();
+ }),
+ client_dtls_stun_piggyback_ && server_dtls_stun_piggyback_);
+ EXPECT_EQ(server_thread()->BlockingCall([&]() {
+ return server_.dtls->IsDtlsPiggybackSupportedByPeer();
+ }),
+ client_dtls_stun_piggyback_ && server_dtls_stun_piggyback_);
+}
+
// Test cases are parametrized by
// * client-piggybacking-enabled,
// * server-piggybacking-enabled,
diff --git a/p2p/dtls/dtls_transport.cc b/p2p/dtls/dtls_transport.cc
index 0cc1a6f..4b59be6 100644
--- a/p2p/dtls/dtls_transport.cc
+++ b/p2p/dtls/dtls_transport.cc
@@ -61,10 +61,6 @@
// So, temporarily increasing it to 2 to see if that makes a difference.
static const size_t kMaxPendingPackets = 2;
-// 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 kMinHandshakeTimeoutMs = 50;
-static const int kMaxHandshakeTimeoutMs = 3000;
// This effectively disables the handshake timeout.
static const int kDisabledHandshakeTimeoutMs = 3600 * 1000 * 24;
@@ -629,30 +625,22 @@
return;
}
- // The opportunistic attempt to do DTLS piggybacking failed.
- // Recreate the DTLS session. Note: this assumes we can consider
- // the previous DTLS session state beyond repair and no packet
- // reached the peer.
- if (dtls_stun_piggyback_controller_.enabled() && dtls_ &&
- !was_ever_connected_ && !IsDtlsPiggybackSupportedByPeer() &&
- (dtls_state() == webrtc::DtlsTransportState::kConnecting ||
- dtls_state() == webrtc::DtlsTransportState::kNew)) {
- RTC_LOG(LS_INFO) << "DTLS piggybacking not supported, restarting...";
- DisablePiggybackingAndRestart();
- return;
- }
-
switch (dtls_state()) {
case webrtc::DtlsTransportState::kNew:
MaybeStartDtls();
break;
case webrtc::DtlsTransportState::kConnected:
- was_ever_connected_ = true;
// Note: SignalWritableState fired by set_writable.
set_writable(ice_transport_->writable());
break;
case webrtc::DtlsTransportState::kConnecting:
// Do nothing.
+ if (dtls_stun_piggyback_controller_.enabled() && dtls_) {
+ // If DTLS piggybacking is enabled, we set the timeout
+ // on the DTLS object (which is then different from the
+ // inital kDisabledHandshakeTimeoutMs)
+ ConfigureHandshakeTimeout();
+ }
break;
case webrtc::DtlsTransportState::kFailed:
// Should not happen. Do nothing.
@@ -970,23 +958,23 @@
RTC_DCHECK(dtls_);
bool uses_dtls_in_stun = dtls_stun_piggyback_controller_.enabled();
std::optional<int> rtt_ms = ice_transport_->GetRttEstimate();
- if (uses_dtls_in_stun) {
+ if (rtt_ms) {
+ // Limit the timeout to a reasonable range in case the ICE RTT takes
+ // extreme values.
+ int initial_timeout_ms =
+ std::max(kMinDtlsHandshakeTimeoutMs,
+ std::min(kMaxDtlsHandshakeTimeoutMs, 2 * (*rtt_ms)));
+ RTC_LOG(LS_INFO) << ToString() << ": configuring DTLS handshake timeout "
+ << initial_timeout_ms << "ms based on ICE RTT " << *rtt_ms;
+
+ dtls_->SetInitialRetransmissionTimeout(initial_timeout_ms);
+ } else if (uses_dtls_in_stun) {
// Configure a very high timeout to effectively disable the DTLS timeout
// and avoid fragmented resends. This is ok since DTLS-in-STUN caches
// the handshake pacets and resends them using the pacing of ICE.
RTC_LOG(LS_INFO) << ToString() << ": configuring DTLS handshake timeout "
<< kDisabledHandshakeTimeoutMs << "ms for DTLS-in-STUN";
dtls_->SetInitialRetransmissionTimeout(kDisabledHandshakeTimeoutMs);
- } else if (rtt_ms) {
- // Limit the timeout to a reasonable range in case the ICE RTT takes
- // extreme values.
- int initial_timeout_ms =
- std::max(kMinHandshakeTimeoutMs,
- std::min(kMaxHandshakeTimeoutMs, 2 * (*rtt_ms)));
- RTC_LOG(LS_INFO) << ToString() << ": configuring DTLS handshake timeout "
- << initial_timeout_ms << "ms based on ICE RTT " << *rtt_ms;
-
- dtls_->SetInitialRetransmissionTimeout(initial_timeout_ms);
} else {
RTC_LOG(LS_INFO)
<< ToString()
diff --git a/p2p/dtls/dtls_transport.h b/p2p/dtls/dtls_transport.h
index 877145d..924278d 100644
--- a/p2p/dtls/dtls_transport.h
+++ b/p2p/dtls/dtls_transport.h
@@ -283,8 +283,6 @@
bool receiving_ = false;
bool writable_ = false;
- bool was_ever_connected_ = false;
-
webrtc::RtcEventLog* const event_log_;
// A controller for piggybacking DTLS in STUN.
diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc
index dba3e86..3db7b96 100644
--- a/rtc_base/openssl_stream_adapter.cc
+++ b/rtc_base/openssl_stream_adapter.cc
@@ -550,8 +550,14 @@
}
void OpenSSLStreamAdapter::SetInitialRetransmissionTimeout(int timeout_ms) {
- RTC_DCHECK(ssl_ctx_ == nullptr);
dtls_handshake_timeout_ms_ = timeout_ms;
+#ifdef OPENSSL_IS_BORINGSSL
+ if (ssl_ctx_ != nullptr && ssl_mode_ == SSL_MODE_DTLS) {
+ // TODO (jonaso, webrtc:367395350): Switch to upcoming
+ // DTLSv1_set_timeout_duration.
+ DTLSv1_set_initial_timeout_duration(ssl_, dtls_handshake_timeout_ms_);
+ }
+#endif
}
//
