From 9d4fd55580a586973936d3765668b773dcab60fe Mon Sep 17 00:00:00 2001 From: Jiawei Ou Date: Thu, 6 Dec 2018 23:30:17 -0800 Subject: [PATCH] Make CONNECTION_WRITE_TIMEOUT configurable for ice connection Bug: None Change-Id: I0fd0616132705c6d15a77fc442be47080f1b81b1 Reviewed-on: https://webrtc-review.googlesource.com/c/112721 Reviewed-by: Qingsi Wang Reviewed-by: Steve Anton Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#25975} --- api/peerconnectioninterface.h | 5 +++++ p2p/base/icetransportinternal.cc | 3 +++ p2p/base/icetransportinternal.h | 8 ++++++++ p2p/base/p2pconstants.h | 4 ++-- p2p/base/p2ptransportchannel.cc | 13 ++++++++++++- p2p/base/port.cc | 8 ++++++-- p2p/base/port.h | 5 +++++ pc/peerconnection.cc | 4 ++++ 8 files changed, 45 insertions(+), 5 deletions(-) diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index 4be3d2f00e..c2fb6a3a31 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -544,6 +544,11 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // overrides the default value in the ICE implementation if set. absl::optional ice_unwritable_min_checks; + // The min time period for which a candidate pair must wait for response to + // connectivity checks it becomes inactive. This parameter overrides the + // default value in the ICE implementation if set. + absl::optional ice_inactive_timeout; + // The interval in milliseconds at which STUN candidates will resend STUN // binding requests to keep NAT bindings open. absl::optional stun_candidate_keepalive_interval; diff --git a/p2p/base/icetransportinternal.cc b/p2p/base/icetransportinternal.cc index 261e0f0e60..62bfb567e3 100644 --- a/p2p/base/icetransportinternal.cc +++ b/p2p/base/icetransportinternal.cc @@ -71,6 +71,9 @@ int IceConfig::ice_unwritable_timeout_or_default() const { int IceConfig::ice_unwritable_min_checks_or_default() const { return ice_unwritable_min_checks.value_or(CONNECTION_WRITE_CONNECT_FAILURES); } +int IceConfig::ice_inactive_timeout_or_default() const { + return ice_inactive_timeout.value_or(CONNECTION_WRITE_TIMEOUT); +} int IceConfig::stun_keepalive_interval_or_default() const { return stun_keepalive_interval.value_or(STUN_KEEPALIVE_INTERVAL); } diff --git a/p2p/base/icetransportinternal.h b/p2p/base/icetransportinternal.h index 49cc1cc32c..acc716965c 100644 --- a/p2p/base/icetransportinternal.h +++ b/p2p/base/icetransportinternal.h @@ -136,6 +136,13 @@ struct IceConfig { // overrides the default value given by |CONNECTION_WRITE_CONNECT_FAILURES| in // port.h if set, when determining the writability of a candidate pair. absl::optional ice_unwritable_min_checks; + + // The min time period for which a candidate pair must wait for response to + // connectivity checks it becomes inactive. This parameter overrides the + // default value given by |CONNECTION_WRITE_TIMEOUT| in port.h if set, when + // determining the writability of a candidate pair. + absl::optional ice_inactive_timeout; + // The interval in milliseconds at which STUN candidates will resend STUN // binding requests to keep NAT bindings open. absl::optional stun_keepalive_interval; @@ -166,6 +173,7 @@ struct IceConfig { int ice_check_min_interval_or_default() const; int ice_unwritable_timeout_or_default() const; int ice_unwritable_min_checks_or_default() const; + int ice_inactive_timeout_or_default() const; int stun_keepalive_interval_or_default() const; }; diff --git a/p2p/base/p2pconstants.h b/p2p/base/p2pconstants.h index a8462ec258..a27e90b8ed 100644 --- a/p2p/base/p2pconstants.h +++ b/p2p/base/p2pconstants.h @@ -87,6 +87,8 @@ extern const int REGATHER_ON_FAILED_NETWORKS_INTERVAL; extern const int CONNECTION_WRITE_CONNECT_TIMEOUT; // Default vaule of IceConfig.ice_unwritable_min_checks. extern const uint32_t CONNECTION_WRITE_CONNECT_FAILURES; +// Default value of IceConfig.ice_inactive_timeout; +extern const int CONNECTION_WRITE_TIMEOUT; // Default value of IceConfig.stun_keepalive_interval; extern const int STUN_KEEPALIVE_INTERVAL; @@ -98,8 +100,6 @@ extern const int WEAK_CONNECTION_RECEIVE_TIMEOUT; // A connection will be declared dead if it has not received anything for this // long. extern const int DEAD_CONNECTION_RECEIVE_TIMEOUT; -// The length of time we wait before timing out writability on a connection. -extern const int CONNECTION_WRITE_TIMEOUT; // This is the length of time that we wait for a ping response to come back. extern const int CONNECTION_RESPONSE_TIMEOUT; // The minimum time we will wait before destroying a connection after creating diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc index 0d21613ba4..6506b53917 100644 --- a/p2p/base/p2ptransportchannel.cc +++ b/p2p/base/p2ptransportchannel.cc @@ -195,6 +195,7 @@ void P2PTransportChannel::AddConnection(Connection* connection) { connection->set_receiving_timeout(config_.receiving_timeout); connection->set_unwritable_timeout(config_.ice_unwritable_timeout); connection->set_unwritable_min_checks(config_.ice_unwritable_min_checks); + connection->set_inactive_timeout(config_.ice_inactive_timeout); connection->SignalReadPacket.connect( this, &P2PTransportChannel::OnReadPacket); connection->SignalReadyToSend.connect( @@ -607,6 +608,15 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { << config_.ice_unwritable_min_checks_or_default(); } + if (config_.ice_inactive_timeout != config.ice_inactive_timeout) { + config_.ice_inactive_timeout = config.ice_inactive_timeout; + for (Connection* conn : connections_) { + conn->set_inactive_timeout(config_.ice_inactive_timeout); + } + RTC_LOG(LS_INFO) << "Set inactive timeout to " + << config_.ice_inactive_timeout_or_default(); + } + if (config_.network_preference != config.network_preference) { config_.network_preference = config.network_preference; RequestSortAndStateUpdate("network preference changed"); @@ -682,7 +692,8 @@ RTCError P2PTransportChannel::ValidateIceConfig(const IceConfig& config) { "strongly connected"); } - if (config.ice_unwritable_timeout_or_default() > CONNECTION_WRITE_TIMEOUT) { + if (config.ice_unwritable_timeout_or_default() > + config.ice_inactive_timeout_or_default()) { return RTCError(RTCErrorType::INVALID_PARAMETER, "The timeout period for the writability state to become " "UNRELIABLE is longer than that to become TIMEOUT."); diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 4d93b4f92f..835e12cc52 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -1231,6 +1231,10 @@ int Connection::unwritable_min_checks() const { return unwritable_min_checks_.value_or(CONNECTION_WRITE_CONNECT_FAILURES); } +int Connection::inactive_timeout() const { + return inactive_timeout_.value_or(CONNECTION_WRITE_TIMEOUT); +} + int Connection::receiving_timeout() const { return receiving_timeout_.value_or(WEAK_CONNECTION_RECEIVE_TIMEOUT); } @@ -1483,8 +1487,8 @@ void Connection::UpdateState(int64_t now) { } if ((write_state_ == STATE_WRITE_UNRELIABLE || write_state_ == STATE_WRITE_INIT) && - TooLongWithoutResponse(pings_since_last_response_, - CONNECTION_WRITE_TIMEOUT, now)) { + TooLongWithoutResponse(pings_since_last_response_, inactive_timeout(), + now)) { RTC_LOG(LS_INFO) << ToString() << ": Timed out after " << now - pings_since_last_response_[0].sent_time << " ms without a response, rtt=" << rtt; diff --git a/p2p/base/port.h b/p2p/base/port.h index ca4fedbbd8..320ed62ffd 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -585,6 +585,10 @@ class Connection : public CandidatePairInterface, void set_unwritable_min_checks(const absl::optional& value) { unwritable_min_checks_ = value; } + int inactive_timeout() const; + void set_inactive_timeout(const absl::optional& value) { + inactive_timeout_ = value; + } // Gets the |ConnectionInfo| stats, where |best_connection| has not been // populated (default value false). @@ -835,6 +839,7 @@ class Connection : public CandidatePairInterface, absl::optional unwritable_timeout_; absl::optional unwritable_min_checks_; + absl::optional inactive_timeout_; bool reported_; IceCandidatePairState state_; diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index d7247c4c9f..4bdaead317 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -702,6 +702,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( absl::optional ice_check_min_interval; absl::optional ice_unwritable_timeout; absl::optional ice_unwritable_min_checks; + absl::optional ice_inactive_timeout; absl::optional stun_candidate_keepalive_interval; absl::optional ice_regather_interval_range; webrtc::TurnCustomizer* turn_customizer; @@ -755,6 +756,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( ice_check_min_interval == o.ice_check_min_interval && ice_unwritable_timeout == o.ice_unwritable_timeout && ice_unwritable_min_checks == o.ice_unwritable_min_checks && + ice_inactive_timeout == o.ice_inactive_timeout && stun_candidate_keepalive_interval == o.stun_candidate_keepalive_interval && ice_regather_interval_range == o.ice_regather_interval_range && @@ -3013,6 +3015,7 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, modified_config.ice_unwritable_timeout = configuration.ice_unwritable_timeout; modified_config.ice_unwritable_min_checks = configuration.ice_unwritable_min_checks; + modified_config.ice_inactive_timeout = configuration.ice_inactive_timeout; modified_config.stun_candidate_keepalive_interval = configuration.stun_candidate_keepalive_interval; modified_config.turn_customizer = configuration.turn_customizer; @@ -5242,6 +5245,7 @@ cricket::IceConfig PeerConnection::ParseIceConfig( ice_config.ice_check_min_interval = config.ice_check_min_interval; ice_config.ice_unwritable_timeout = config.ice_unwritable_timeout; ice_config.ice_unwritable_min_checks = config.ice_unwritable_min_checks; + ice_config.ice_inactive_timeout = config.ice_inactive_timeout; ice_config.stun_keepalive_interval = config.stun_candidate_keepalive_interval; ice_config.regather_all_networks_interval_range = config.ice_regather_interval_range;