From 5985a0481e5350f39dbc9c3ec92f13739a9a05ba Mon Sep 17 00:00:00 2001 From: Bjorn A Mellem Date: Fri, 28 Jun 2019 14:19:38 -0700 Subject: [PATCH] Add a field trial to control datagram transport use. First, the existing configuration parameter (use_datagram_transport) is now optional. The new field trial has two flag values: 1. Whether to enable the datagram transport (enabled) 2. Whether to use the datagram transport by default (default_value) The first is a kill-switch. It disables the datagram transport, even for applications which inject a datagram transport factory and specify use_datagram_transport = true. This allows applications which hard-code a datagram transport to switch it off via field trials. This flag defaults to true, to avoid breaking downstream projects which already inject and configure a datagram transport. It may be changed to false after updating downstream to set this field trial flag to true when required. The second provides a default value to be used in case the aforementioned use_datagram_transport parameter is unset. Applications which explicitly set use_datagram_transport will use that value. Applications which do not explicitly specify whether or not to use the datagram transport will use it (or not) according to the default_value flag. One goal of this flag is to simplify rollout in applications which already set field trials based on configuration, but require code changes for new RTCConfiguration parameters. A second goal is to provide platforms with a knob to control whether datagram transport is "opt-in" or "opt-out". This flag defaults to false, to prevent downstream projects from unintentionally enabling the datagram tranpsort. Bug: webrtc:9719 Change-Id: I521a5fa61c992e76e5081118678a1812a261d672 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144184 Reviewed-by: Steve Anton Commit-Queue: Bjorn Mellem Cr-Commit-Position: refs/heads/master@{#28435} --- api/peer_connection_interface.h | 6 +----- pc/peer_connection.cc | 32 +++++++++++++++++++++++--------- pc/peer_connection.h | 28 ++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 92b0561113..6841e14477 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -631,11 +631,7 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // informs PeerConnection that it should use the DatagramTransportInterface // for packets instead DTLS. It's invalid to set it to |true| if the // MediaTransportFactory wasn't provided. - // - // TODO(sukhanov): Once we have a working mechanism for negotiating media - // transport through SDP, we replace media transport flags in - // RTCConfiguration with field trials. - bool use_datagram_transport = false; + absl::optional use_datagram_transport; // Defines advanced optional cryptographic settings related to SRTP and // frame encryption for native WebRTC. Setting this will overwrite any diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index c46dc80cea..7ea9f50852 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -107,6 +107,10 @@ const char kDtlsSrtpSetupFailureRtcp[] = namespace { +// Field trials. +// Controls datagram transport support. +const char kDatagramTransportFieldTrial[] = "WebRTC-DatagramTransport"; + // UMA metric names. const char kSimulcastVersionApplyLocalDescription[] = "WebRTC.PeerConnection.Simulcast.ApplyLocalDescription"; @@ -764,7 +768,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( bool active_reset_srtp_params; bool use_media_transport; bool use_media_transport_for_data_channels; - bool use_datagram_transport; + absl::optional use_datagram_transport; absl::optional crypto_options; bool offer_extmap_allow_mixed; }; @@ -868,6 +872,8 @@ PeerConnection::PeerConnection(PeerConnectionFactory* factory, : factory_(factory), event_log_(std::move(event_log)), event_log_ptr_(event_log_.get()), + datagram_transport_config_( + field_trial::FindFullName(kDatagramTransportFieldTrial)), rtcp_cname_(GenerateRtcpCname()), local_streams_(StreamCollection::Create()), remote_streams_(StreamCollection::Create()), @@ -1027,8 +1033,10 @@ bool PeerConnection::Initialize( #endif config.active_reset_srtp_params = configuration.active_reset_srtp_params; - if (configuration.use_datagram_transport || - configuration.use_media_transport || + use_datagram_transport_ = datagram_transport_config_.enabled && + configuration_.use_datagram_transport.value_or( + datagram_transport_config_.default_value); + if (use_datagram_transport_ || configuration.use_media_transport || configuration.use_media_transport_for_data_channels) { if (!factory_->media_transport_factory()) { RTC_DCHECK(false) @@ -1058,7 +1066,7 @@ bool PeerConnection::Initialize( config.use_media_transport_for_media = configuration.use_media_transport; config.use_media_transport_for_data_channels = configuration.use_media_transport_for_data_channels; - config.use_datagram_transport = configuration.use_datagram_transport; + config.use_datagram_transport = use_datagram_transport_; config.media_transport_factory = factory_->media_transport_factory(); } @@ -3461,7 +3469,8 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, if (configuration.use_media_transport_for_data_channels || configuration.use_media_transport || - configuration.use_datagram_transport) { + (configuration.use_datagram_transport && + *configuration.use_datagram_transport)) { RTC_CHECK(configuration.bundle_policy == kBundlePolicyMaxBundle) << "Media transport requires MaxBundle policy."; } @@ -3496,6 +3505,7 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, modified_config.use_media_transport = configuration.use_media_transport; modified_config.use_media_transport_for_data_channels = configuration.use_media_transport_for_data_channels; + modified_config.use_datagram_transport = configuration.use_datagram_transport; if (configuration != modified_config) { RTC_LOG(LS_ERROR) << "Modifying the configuration in an unsupported way."; return SafeSetError(RTCErrorType::INVALID_MODIFICATION, error); @@ -3554,10 +3564,14 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, } transport_controller_->SetIceConfig(ParseIceConfig(modified_config)); + + use_datagram_transport_ = datagram_transport_config_.enabled && + modified_config.use_datagram_transport.value_or( + datagram_transport_config_.default_value); transport_controller_->SetMediaTransportSettings( modified_config.use_media_transport, modified_config.use_media_transport_for_data_channels, - modified_config.use_datagram_transport); + use_datagram_transport_); if (configuration_.active_reset_srtp_params != modified_config.active_reset_srtp_params) { @@ -4345,7 +4359,7 @@ void PeerConnection::GetOptionsForOffer( } // If datagram transport is in use, add opaque transport parameters. - if (configuration_.use_datagram_transport) { + if (use_datagram_transport_) { for (auto& options : session_options->media_description_options) { options.transport_options.opaque_parameters = transport_controller_->GetTransportParameters(options.mid); @@ -4651,7 +4665,7 @@ void PeerConnection::GetOptionsForAnswer( port_allocator_.get())); // If datagram transport is in use, add opaque transport parameters. - if (configuration_.use_datagram_transport) { + if (use_datagram_transport_) { for (auto& options : session_options->media_description_options) { options.transport_options.opaque_parameters = transport_controller_->GetTransportParameters(options.mid); @@ -6965,7 +6979,7 @@ bool PeerConnection::ReadyToUseRemoteCandidate( } bool PeerConnection::SrtpRequired() const { - return !configuration_.use_datagram_transport && + return !use_datagram_transport_ && (dtls_enabled_ || webrtc_session_desc_factory_->SdesPolicy() == cricket::SEC_REQUIRED); } diff --git a/pc/peer_connection.h b/pc/peer_connection.h index e7362d9f39..917ad62d63 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -309,6 +309,28 @@ class PeerConnection : public PeerConnectionInternal, uint32_t first_ssrc; }; + // Field-trial based configuration for datagram transport. + struct DatagramTransportConfig { + explicit DatagramTransportConfig(const std::string& field_trial) + : enabled("enabled", true), default_value("default_value", false) { + ParseFieldTrial({&enabled, &default_value}, field_trial); + } + + // Whether datagram transport support is enabled at all. Defaults to true, + // allowing datagram transport to be used if (a) the application provides a + // factory for it and (b) the configuration specifies its use. This flag + // provides a kill-switch to force-disable datagram transport across all + // applications, without code changes. + FieldTrialFlag enabled; + + // Whether the datagram transport is enabled or disabled by default. + // Defaults to false, meaning that applications must configure use of + // datagram transport through RTCConfiguration. If set to true, + // applications will use the datagram transport by default (but may still + // explicitly configure themselves not to use it through RTCConfiguration). + FieldTrialFlag default_value; + }; + // Implements MessageHandler. void OnMessage(rtc::Message* msg) override; @@ -1122,6 +1144,12 @@ class PeerConnection : public PeerConnectionInternal, PeerConnectionInterface::RTCConfiguration configuration_ RTC_GUARDED_BY(signaling_thread()); + // Field-trial based configuration for datagram transport. + const DatagramTransportConfig datagram_transport_config_; + + // Final, resolved value for whether datagram transport is in use. + bool use_datagram_transport_ RTC_GUARDED_BY(signaling_thread()) = false; + // Cache configuration_.use_media_transport so that we can access it from // other threads. // TODO(bugs.webrtc.org/9987): Caching just this bool and allowing the data