From 5107246d4bf4048ca3eb428107bfccf7d757d667 Mon Sep 17 00:00:00 2001 From: skvlad Date: Thu, 2 Feb 2017 11:50:14 -0800 Subject: [PATCH] Allow applications to limit the ICE check rate through RTCConfiguration If an application sets a non-null value in RTCConfiguration.iceCheckMinInterval, we do not sent STUN pings more often than that. This is useful for bandwidth constrained scenarios. This CL also increases the maximum STUN ping timeout to 60 seconds up from its previous value of 5 (which meant that a ping response received 5 seconds later would not be counted), and allows the RTT estimate to go up to 60 seconds from its previous limit of 3. RTTs above 3 seconds are possible on mobile links. (webrtc:7109) This CL was originally written by pthatcher@, I am just submitting it after a minor cleanup. BUG=webrtc:7082, webrtc:7109 Review-Url: https://codereview.webrtc.org/2670053002 Cr-Commit-Position: refs/heads/master@{#16421} --- webrtc/api/peerconnectioninterface.h | 3 +++ webrtc/p2p/base/jseptransport.h | 5 +++++ webrtc/p2p/base/p2ptransportchannel.cc | 14 ++++++++++---- webrtc/p2p/base/p2ptransportchannel.h | 18 ++++++++++++++++++ .../p2p/base/p2ptransportchannel_unittest.cc | 2 +- webrtc/p2p/base/port.cc | 4 ++-- webrtc/p2p/base/port.h | 5 ++++- webrtc/pc/peerconnection.cc | 4 +++- webrtc/pc/webrtcsession.cc | 1 + .../android/api/org/webrtc/PeerConnection.java | 2 ++ .../sdk/android/src/jni/peerconnection_jni.cc | 13 +++++++++++++ 11 files changed, 62 insertions(+), 9 deletions(-) diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 0280c18dbd..6cc8be7112 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -380,6 +380,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // If true, ICE role is redetermined when peerconnection sets a local // transport description that indicates an ICE restart. bool redetermine_role_on_ice_restart = true; + // If set, the min interval (max rate) at which we will send ICE checks + // (STUN pings), in milliseconds. + rtc::Optional ice_check_min_interval; // // Don't forget to update operator== if adding something. // diff --git a/webrtc/p2p/base/jseptransport.h b/webrtc/p2p/base/jseptransport.h index c7998ba0fb..45963e681f 100644 --- a/webrtc/p2p/base/jseptransport.h +++ b/webrtc/p2p/base/jseptransport.h @@ -187,6 +187,11 @@ struct IceConfig { // Default nomination mode if the remote does not support renomination. NominationMode default_nomination_mode = NominationMode::SEMI_AGGRESSIVE; + // ICE checks (STUN pings) will not be sent at higher rate (lower interval) + // than this, no matter what other settings there are. + // Measure in milliseconds. + rtc::Optional ice_check_min_interval; + IceConfig() {} IceConfig(int receiving_timeout_ms, int backup_connection_ping_interval, diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index f47f03bbd6..fc7dce2f9e 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -71,7 +71,7 @@ static const int PING_PACKET_SIZE = 60 * 8; // The next two ping intervals are at the channel level. // STRONG_PING_INTERVAL (480ms) is applied when the selected connection is both // writable and receiving. -static const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 1000; +const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 1000; // WEAK_PING_INTERVAL (48ms) is applied when the selected connection is either // not writable or not receiving. const int WEAK_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 10000; @@ -430,6 +430,12 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { LOG(LS_INFO) << "Set default nomination mode to " << static_cast(config_.default_nomination_mode); } + + if (config_.ice_check_min_interval != config.ice_check_min_interval) { + config_.ice_check_min_interval = config.ice_check_min_interval; + LOG(LS_INFO) << "Set min ping interval to " + << *config_.ice_check_min_interval; + } } const IceConfig& P2PTransportChannel::config() const { @@ -1522,8 +1528,8 @@ void P2PTransportChannel::OnCheckAndPing() { conn->num_pings_sent() < MIN_PINGS_AT_WEAK_PING_INTERVAL; }); int ping_interval = (weak() || need_more_pings_at_weak_interval) - ? weak_ping_interval_ - : STRONG_PING_INTERVAL; + ? weak_ping_interval() + : strong_ping_interval(); if (rtc::TimeMillis() >= last_ping_sent_ms_ + ping_interval) { Connection* conn = FindNextPingableConnection(); if (conn) { @@ -1607,7 +1613,7 @@ int P2PTransportChannel::CalculateActiveWritablePingInterval( // Ping each connection at a higher rate at least // MIN_PINGS_AT_WEAK_PING_INTERVAL times. if (conn->num_pings_sent() < MIN_PINGS_AT_WEAK_PING_INTERVAL) { - return weak_ping_interval_; + return weak_ping_interval(); } int stable_interval = config_.stable_writable_connection_ping_interval; diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 2327b2d477..268a7a0794 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -42,6 +42,7 @@ namespace cricket { enum class IceRestartState { CONNECTING, CONNECTED, DISCONNECTED, MAX_VALUE }; extern const int WEAK_PING_INTERVAL; +extern const int STRONG_PING_INTERVAL; extern const int WEAK_OR_STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL; extern const int STRONG_AND_STABLE_WRITABLE_CONNECTION_PING_INTERVAL; static const int MIN_PINGS_AT_WEAK_PING_INTERVAL = 3; @@ -162,6 +163,23 @@ class P2PTransportChannel : public IceTransportInternal, // A transport channel is weak if the current best connection is either // not receiving or not writable, or if there is no best connection at all. bool weak() const; + + int weak_ping_interval() const { + if (config_.ice_check_min_interval && + weak_ping_interval_ < *config_.ice_check_min_interval) { + return *config_.ice_check_min_interval; + } + return weak_ping_interval_; + } + + int strong_ping_interval() const { + if (config_.ice_check_min_interval && + STRONG_PING_INTERVAL < *config_.ice_check_min_interval) { + return *config_.ice_check_min_interval; + } + return STRONG_PING_INTERVAL; + } + // Returns true if it's possible to send packets on |connection|. bool ReadyToSend(Connection* connection) const; void UpdateConnectionStates(); diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index b35bc1bbf9..82e6c019c6 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -1217,7 +1217,7 @@ TEST_F(P2PTransportChannelTest, TestUMAIceRestartWhileDisconnected) { // Drop all packets so that both channels become not writable. fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]); - const int kWriteTimeoutDelay = 6000; + const int kWriteTimeoutDelay = 8000; EXPECT_TRUE_SIMULATED_WAIT(!ep1_ch1()->writable() && !ep2_ch1()->writable(), kWriteTimeoutDelay, clock); diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 565ddb67a6..3b78af4dd3 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -60,11 +60,11 @@ inline bool TooLongWithoutResponse( // We will restrict RTT estimates (when used for determining state) to be // within a reasonable range. const int MINIMUM_RTT = 100; // 0.1 seconds -const int MAXIMUM_RTT = 3000; // 3 seconds +const int MAXIMUM_RTT = 60000; // 60 seconds // When we don't have any RTT data, we have to pick something reasonable. We // use a large value just in case the connection is really slow. -const int DEFAULT_RTT = MAXIMUM_RTT; +const int DEFAULT_RTT = 3000; // 3 seconds // Computes our estimate of the RTT given the current estimate. inline int ConservativeRTTEstimate(int rtt) { diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 72b0e117cd..6cfbbd84c5 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -72,7 +72,10 @@ static const int CONNECTION_WRITE_TIMEOUT = 15 * 1000; // 15 seconds static const int CONNECTION_WRITE_CONNECT_TIMEOUT = 5 * 1000; // 5 seconds // This is the length of time that we wait for a ping response to come back. -static const int CONNECTION_RESPONSE_TIMEOUT = 5 * 1000; // 5 seconds +// There is no harm to keep this value high other than a small amount +// of increased memory. But in some networks (2G), +// we observe up to 60s RTTs. +static const int CONNECTION_RESPONSE_TIMEOUT = 60 * 1000; // 60 seconds // The number of pings that must fail to respond before we become unwritable. static const uint32_t CONNECTION_WRITE_CONNECT_FAILURES = 5; diff --git a/webrtc/pc/peerconnection.cc b/webrtc/pc/peerconnection.cc index a38c98601b..14d2e94572 100644 --- a/webrtc/pc/peerconnection.cc +++ b/webrtc/pc/peerconnection.cc @@ -505,6 +505,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( bool presume_writable_when_fully_relayed; bool enable_ice_renomination; bool redetermine_role_on_ice_restart; + rtc::Optional ice_check_min_interval; }; static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this), "Did you add something to RTCConfiguration and forget to " @@ -536,7 +537,8 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( presume_writable_when_fully_relayed == o.presume_writable_when_fully_relayed && enable_ice_renomination == o.enable_ice_renomination && - redetermine_role_on_ice_restart == o.redetermine_role_on_ice_restart; + redetermine_role_on_ice_restart == o.redetermine_role_on_ice_restart && + ice_check_min_interval == o.ice_check_min_interval; } bool PeerConnectionInterface::RTCConfiguration::operator!=( diff --git a/webrtc/pc/webrtcsession.cc b/webrtc/pc/webrtcsession.cc index a02aa5796a..e59466c67b 100644 --- a/webrtc/pc/webrtcsession.cc +++ b/webrtc/pc/webrtcsession.cc @@ -1189,6 +1189,7 @@ cricket::IceConfig WebRtcSession::ParseIceConfig( ice_config.continual_gathering_policy = gathering_policy; ice_config.presume_writable_when_fully_relayed = config.presume_writable_when_fully_relayed; + ice_config.ice_check_min_interval = config.ice_check_min_interval; return ice_config; } diff --git a/webrtc/sdk/android/api/org/webrtc/PeerConnection.java b/webrtc/sdk/android/api/org/webrtc/PeerConnection.java index 53184bedda..784b590c08 100644 --- a/webrtc/sdk/android/api/org/webrtc/PeerConnection.java +++ b/webrtc/sdk/android/api/org/webrtc/PeerConnection.java @@ -160,6 +160,7 @@ public class PeerConnection { public int iceCandidatePoolSize; public boolean pruneTurnPorts; public boolean presumeWritableWhenFullyRelayed; + public Integer iceCheckMinInterval; public RTCConfiguration(List iceServers) { iceTransportsType = IceTransportsType.ALL; @@ -177,6 +178,7 @@ public class PeerConnection { iceCandidatePoolSize = 0; pruneTurnPorts = false; presumeWritableWhenFullyRelayed = false; + iceCheckMinInterval = null; } }; diff --git a/webrtc/sdk/android/src/jni/peerconnection_jni.cc b/webrtc/sdk/android/src/jni/peerconnection_jni.cc index 91cf5b7880..6a6ba25c96 100644 --- a/webrtc/sdk/android/src/jni/peerconnection_jni.cc +++ b/webrtc/sdk/android/src/jni/peerconnection_jni.cc @@ -1759,6 +1759,11 @@ static void JavaRTCConfigurationToJsepRTCConfiguration( jfieldID j_prune_turn_ports_id = GetFieldID(jni, j_rtc_config_class, "pruneTurnPorts", "Z"); + jfieldID j_ice_check_min_interval_id = GetFieldID( + jni, j_rtc_config_class, "iceCheckMinInterval", "Ljava/lang/Integer;"); + jclass j_integer_class = jni->FindClass("java/lang/Integer"); + jmethodID int_value_id = GetMethodID(jni, j_integer_class, "intValue", "()I"); + rtc_config->type = JavaIceTransportsTypeToNativeType(jni, j_ice_transports_type); rtc_config->bundle_policy = @@ -1787,6 +1792,14 @@ static void JavaRTCConfigurationToJsepRTCConfiguration( GetBooleanField(jni, j_rtc_config, j_prune_turn_ports_id); rtc_config->presume_writable_when_fully_relayed = GetBooleanField( jni, j_rtc_config, j_presume_writable_when_fully_relayed_id); + jobject j_ice_check_min_interval = + GetNullableObjectField(jni, j_rtc_config, j_ice_check_min_interval_id); + if (!IsNull(jni, j_ice_check_min_interval)) { + int ice_check_min_interval_value = + jni->CallIntMethod(j_ice_check_min_interval, int_value_id); + rtc_config->ice_check_min_interval = + rtc::Optional(ice_check_min_interval_value); + } } JOW(jlong, PeerConnectionFactory_nativeCreatePeerConnection)(