From 5622c5eae547dc079763865a31d7da951bb9cd76 Mon Sep 17 00:00:00 2001 From: Honghai Zhang Date: Fri, 1 Jul 2016 13:59:29 -0700 Subject: [PATCH] If continual gathering is enabled, we will periodically check if any network does not have any connection on it and if yes, attempt to re-gather on those networks. BUG= R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/2025573002 . Cr-Commit-Position: refs/heads/master@{#13367} --- webrtc/api/webrtcsession.cc | 17 +- webrtc/api/webrtcsession_unittest.cc | 4 +- webrtc/base/network.h | 11 +- webrtc/base/network_unittest.cc | 17 - webrtc/p2p/base/fakeportallocator.h | 7 +- webrtc/p2p/base/faketransportcontroller.h | 12 +- webrtc/p2p/base/p2ptransportchannel.cc | 114 +++++-- webrtc/p2p/base/p2ptransportchannel.h | 16 +- .../p2p/base/p2ptransportchannel_unittest.cc | 298 ++++++++++++++---- webrtc/p2p/base/port.cc | 6 - webrtc/p2p/base/port.h | 2 - webrtc/p2p/base/portallocator.h | 45 ++- webrtc/p2p/base/portinterface.h | 3 - webrtc/p2p/base/transport.h | 35 +- .../p2p/base/transportcontroller_unittest.cc | 12 +- webrtc/p2p/client/basicportallocator.cc | 185 ++++++++--- webrtc/p2p/client/basicportallocator.h | 26 +- 17 files changed, 594 insertions(+), 216 deletions(-) diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index 231d0ad4de..8c83dbbc62 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -1128,14 +1128,27 @@ bool WebRtcSession::RemoveRemoteIceCandidates( cricket::IceConfig WebRtcSession::ParseIceConfig( const PeerConnectionInterface::RTCConfiguration& config) const { + cricket::ContinualGatheringPolicy gathering_policy; + // TODO(honghaiz): Add the third continual gathering policy in + // PeerConnectionInterface and map it to GATHER_CONTINUALLY_AND_RECOVER. + switch (config.continual_gathering_policy) { + case PeerConnectionInterface::GATHER_ONCE: + gathering_policy = cricket::GATHER_ONCE; + break; + case PeerConnectionInterface::GATHER_CONTINUALLY: + gathering_policy = cricket::GATHER_CONTINUALLY; + break; + default: + RTC_DCHECK(false); + gathering_policy = cricket::GATHER_ONCE; + } cricket::IceConfig ice_config; ice_config.receiving_timeout = config.ice_connection_receiving_timeout; ice_config.prioritize_most_likely_candidate_pairs = config.prioritize_most_likely_ice_candidate_pairs; ice_config.backup_connection_ping_interval = config.ice_backup_candidate_pair_ping_interval; - ice_config.gather_continually = (config.continual_gathering_policy == - PeerConnectionInterface::GATHER_CONTINUALLY); + ice_config.continual_gathering_policy = gathering_policy; ice_config.presume_writable_when_fully_relayed = config.presume_writable_when_fully_relayed; return ice_config; diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index ee1d19b23a..29f94dedb4 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -2242,7 +2242,9 @@ TEST_F(WebRtcSessionTest, candidates = local_desc->candidates(kMediaContentIndex0); size_t num_local_candidates = candidates->count(); // Enable Continual Gathering - session_->SetIceConfig(cricket::IceConfig(-1, -1, true, false, -1, true)); + cricket::IceConfig config; + config.continual_gathering_policy = cricket::GATHER_CONTINUALLY; + session_->SetIceConfig(config); // Bring down the network interface to trigger candidate removals. RemoveInterface(rtc::SocketAddress(kClientAddrHost1, kClientAddrPort)); // Verify that all local candidates are removed. diff --git a/webrtc/base/network.h b/webrtc/base/network.h index fe51e4eafb..7d509c6a12 100644 --- a/webrtc/base/network.h +++ b/webrtc/base/network.h @@ -140,7 +140,7 @@ class NetworkManagerBase : public NetworkManager { NetworkManagerBase(); ~NetworkManagerBase() override; - void GetNetworks(std::vector* networks) const override; + void GetNetworks(NetworkList* networks) const override; void GetAnyAddressNetworks(NetworkList* networks) override; bool ipv6_enabled() const { return ipv6_enabled_; } void set_ipv6_enabled(bool enabled) { ipv6_enabled_ = enabled; } @@ -290,7 +290,6 @@ class Network { AdapterType type); ~Network(); - sigslot::signal1 SignalInactive; sigslot::signal1 SignalTypeChanged; const DefaultLocalAddressProvider* default_local_address_provider() { @@ -398,12 +397,8 @@ class Network { // it inactive, so that we can detect network changes properly. bool active() const { return active_; } void set_active(bool active) { - if (active_ == active) { - return; - } - active_ = active; - if (!active) { - SignalInactive(this); + if (active_ != active) { + active_ = active; } } diff --git a/webrtc/base/network_unittest.cc b/webrtc/base/network_unittest.cc index 2ddc1b03c5..42e06d08dc 100644 --- a/webrtc/base/network_unittest.cc +++ b/webrtc/base/network_unittest.cc @@ -67,16 +67,6 @@ class NetworkTest : public testing::Test, public sigslot::has_slots<> { callback_called_ = true; } - void listenToNetworkInactive(BasicNetworkManager& network_manager) { - BasicNetworkManager::NetworkList networks; - network_manager.GetNetworks(&networks); - for (Network* network : networks) { - network->SignalInactive.connect(this, &NetworkTest::OnNetworkInactive); - } - } - - void OnNetworkInactive(const Network* network) { num_networks_inactive_++; } - NetworkManager::Stats MergeNetworkList( BasicNetworkManager& network_manager, const NetworkManager::NetworkList& list, @@ -187,8 +177,6 @@ class NetworkTest : public testing::Test, public sigslot::has_slots<> { protected: bool callback_called_; - // Number of networks that become inactive. - int num_networks_inactive_ = 0; }; class TestBasicNetworkManager : public BasicNetworkManager { @@ -322,7 +310,6 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) { EXPECT_TRUE(changed); EXPECT_EQ(stats.ipv6_network_count, 0); EXPECT_EQ(stats.ipv4_network_count, 1); - listenToNetworkInactive(manager); list.clear(); manager.GetNetworks(&list); @@ -339,9 +326,7 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) { EXPECT_TRUE(changed); EXPECT_EQ(stats.ipv6_network_count, 0); EXPECT_EQ(stats.ipv4_network_count, 1); - EXPECT_EQ(1, num_networks_inactive_); list.clear(); - num_networks_inactive_ = 0; manager.GetNetworks(&list); EXPECT_EQ(1U, list.size()); @@ -359,7 +344,6 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) { EXPECT_TRUE(changed); EXPECT_EQ(stats.ipv6_network_count, 0); EXPECT_EQ(stats.ipv4_network_count, 2); - EXPECT_EQ(0, num_networks_inactive_); list.clear(); // Verify that we get previous instances of Network objects. @@ -379,7 +363,6 @@ TEST_F(NetworkTest, TestBasicMergeNetworkList) { EXPECT_FALSE(changed); EXPECT_EQ(stats.ipv6_network_count, 0); EXPECT_EQ(stats.ipv4_network_count, 2); - EXPECT_EQ(0, num_networks_inactive_); list.clear(); // Verify that we get previous instances of Network objects. diff --git a/webrtc/p2p/base/fakeportallocator.h b/webrtc/p2p/base/fakeportallocator.h index 11969e0233..15e7937694 100644 --- a/webrtc/p2p/base/fakeportallocator.h +++ b/webrtc/p2p/base/fakeportallocator.h @@ -111,7 +111,6 @@ class FakePortAllocatorSession : public PortAllocatorSession { rtc::IPAddress(in6addr_loopback), 64), port_(), - running_(false), port_config_count_(0), stun_servers_(allocator->stun_servers()), turn_servers_(allocator->turn_servers()) { @@ -124,6 +123,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { } void StartGettingPorts() override { + PortAllocatorSession::StartGettingPorts(); if (!port_) { rtc::Network& network = (rtc::HasIPv6Enabled() && (flags() & PORTALLOCATOR_ENABLE_IPV6)) @@ -137,12 +137,8 @@ class FakePortAllocatorSession : public PortAllocatorSession { AddPort(port_.get()); } ++port_config_count_; - running_ = true; } - void StopGettingPorts() override { running_ = false; } - bool IsGettingPorts() override { return running_; } - void ClearGettingPorts() override {} std::vector ReadyPorts() const override { return ready_ports_; } @@ -200,7 +196,6 @@ class FakePortAllocatorSession : public PortAllocatorSession { rtc::Network ipv4_network_; rtc::Network ipv6_network_; std::unique_ptr port_; - bool running_; int port_config_count_; std::vector candidates_; std::vector ready_ports_; diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index c96acda5d7..b8c67ecd17 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -179,13 +179,10 @@ class FakeTransportChannel : public TransportChannelImpl, void SetReceiving(bool receiving) { set_receiving(receiving); } - void SetIceConfig(const IceConfig& config) override { - receiving_timeout_ = config.receiving_timeout; - gather_continually_ = config.gather_continually; - } + void SetIceConfig(const IceConfig& config) override { ice_config_ = config; } - int receiving_timeout() const { return receiving_timeout_; } - bool gather_continually() const { return gather_continually_; } + int receiving_timeout() const { return ice_config_.receiving_timeout; } + bool gather_continually() const { return ice_config_.gather_continually(); } int SendPacket(const char* data, size_t len, @@ -318,8 +315,7 @@ class FakeTransportChannel : public TransportChannelImpl, bool do_dtls_ = false; std::vector srtp_ciphers_; int chosen_crypto_suite_ = rtc::SRTP_INVALID_CRYPTO_SUITE; - int receiving_timeout_ = -1; - bool gather_continually_ = false; + IceConfig ice_config_; IceRole role_ = ICEROLE_UNKNOWN; uint64_t tiebreaker_ = 0; std::string ice_ufrag_; diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index ebbc0eabbe..4ebd3a41c4 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -27,10 +27,14 @@ namespace { // messages for queuing up work for ourselves -enum { MSG_SORT_AND_UPDATE_STATE = 1, MSG_CHECK_AND_PING }; +enum { + MSG_SORT_AND_UPDATE_STATE = 1, + MSG_CHECK_AND_PING, + MSG_REGATHER_ON_FAILED_NETWORKS +}; // The minimum improvement in RTT that justifies a switch. -static const double kMinImprovement = 10; +const int kMinImprovement = 10; bool IsRelayRelay(const cricket::Connection* conn) { return conn->local_candidate().type() == cricket::RELAY_PORT_TYPE && @@ -76,6 +80,9 @@ const int STABLE_WRITABLE_CONNECTION_PING_INTERVAL = 2500; // ms static const int MIN_CHECK_RECEIVING_INTERVAL = 50; // ms +// We periodically check if any existing networks do not have any connection +// and regather on those networks. +static const int DEFAULT_REGATHER_ON_FAILED_NETWORKS_INTERVAL = 5 * 60 * 1000; static constexpr int a_is_better = 1; static constexpr int b_is_better = -1; @@ -101,10 +108,11 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, check_receiving_interval_(MIN_CHECK_RECEIVING_INTERVAL * 5), config_(MIN_CHECK_RECEIVING_INTERVAL * 50 /* receiving_timeout */, 0 /* backup_connection_ping_interval */, - false /* gather_continually */, + GATHER_ONCE /* continual_gathering_policy */, false /* prioritize_most_likely_candidate_pairs */, STABLE_WRITABLE_CONNECTION_PING_INTERVAL, - true /* presume_writable_when_fully_relayed */) { + true /* presume_writable_when_fully_relayed */, + DEFAULT_REGATHER_ON_FAILED_NETWORKS_INTERVAL) { uint32_t weak_ping_interval = ::strtoul( webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(), nullptr, 10); @@ -125,9 +133,13 @@ void P2PTransportChannel::AddAllocatorSession( session->set_generation(static_cast(allocator_sessions_.size())); session->SignalPortReady.connect(this, &P2PTransportChannel::OnPortReady); + session->SignalPortsRemoved.connect(this, + &P2PTransportChannel::OnPortsRemoved); session->SignalPortPruned.connect(this, &P2PTransportChannel::OnPortPruned); session->SignalCandidatesReady.connect( this, &P2PTransportChannel::OnCandidatesReady); + session->SignalCandidatesRemoved.connect( + this, &P2PTransportChannel::OnCandidatesRemoved); session->SignalCandidatesAllocationDone.connect( this, &P2PTransportChannel::OnCandidatesAllocationDone); @@ -296,8 +308,11 @@ void P2PTransportChannel::SetRemoteIceMode(IceMode mode) { } void P2PTransportChannel::SetIceConfig(const IceConfig& config) { - config_.gather_continually = config.gather_continually; - LOG(LS_INFO) << "Set gather_continually to " << config_.gather_continually; + if (config_.continual_gathering_policy != config.continual_gathering_policy) { + LOG(LS_INFO) << "Set continual_gathering_policy to " + << config_.continual_gathering_policy; + config_.continual_gathering_policy = config.continual_gathering_policy; + } if (config.backup_connection_ping_interval >= 0 && config_.backup_connection_ping_interval != @@ -347,6 +362,13 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { << config_.presume_writable_when_fully_relayed; } } + + if (config.regather_on_failed_networks_interval) { + config_.regather_on_failed_networks_interval = + config.regather_on_failed_networks_interval; + LOG(LS_INFO) << "Set regather_on_failed_networks_interval to " + << *config_.regather_on_failed_networks_interval; + } } const IceConfig& P2PTransportChannel::config() const { @@ -418,8 +440,7 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession *session, port->SignalUnknownAddress.connect( this, &P2PTransportChannel::OnUnknownAddress); port->SignalDestroyed.connect(this, &P2PTransportChannel::OnPortDestroyed); - port->SignalNetworkInactive.connect( - this, &P2PTransportChannel::OnPortNetworkInactive); + port->SignalRoleConflict.connect( this, &P2PTransportChannel::OnRoleConflict); port->SignalSentPacket.connect(this, &P2PTransportChannel::OnSentPacket); @@ -957,6 +978,9 @@ void P2PTransportChannel::MaybeStartPinging() { LOG_J(LS_INFO, this) << "Have a pingable connection for the first time; " << "starting to ping."; thread()->Post(RTC_FROM_HERE, this, MSG_CHECK_AND_PING); + thread()->PostDelayed(RTC_FROM_HERE, + *config_.regather_on_failed_networks_interval, this, + MSG_REGATHER_ON_FAILED_NETWORKS); started_pinging_ = true; } } @@ -1313,16 +1337,16 @@ void P2PTransportChannel::MaybeStopPortAllocatorSessions() { } for (const auto& session : allocator_sessions_) { - if (!session->IsGettingPorts()) { + if (session->IsStopped()) { continue; } - // If gathering continually, keep the last session running so that it - // will gather candidates if the networks change. - if (config_.gather_continually && session == allocator_sessions_.back()) { + // If gathering continually, keep the last session running so that + // it can gather candidates if the networks change. + if (config_.gather_continually() && session == allocator_sessions_.back()) { session->ClearGettingPorts(); - break; + } else { + session->StopGettingPorts(); } - session->StopGettingPorts(); } } @@ -1377,6 +1401,9 @@ void P2PTransportChannel::OnMessage(rtc::Message *pmsg) { case MSG_CHECK_AND_PING: OnCheckAndPing(); break; + case MSG_REGATHER_ON_FAILED_NETWORKS: + OnRegatherOnFailedNetworks(); + break; default: ASSERT(false); break; @@ -1606,37 +1633,62 @@ void P2PTransportChannel::OnConnectionDestroyed(Connection* connection) { } } -// When a port is destroyed remove it from our list of ports to use for +// When a port is destroyed, remove it from our list of ports to use for // connection attempts. void P2PTransportChannel::OnPortDestroyed(PortInterface* port) { ASSERT(worker_thread_ == rtc::Thread::Current()); - // Remove this port from the lists (if we didn't drop it already). ports_.erase(std::remove(ports_.begin(), ports_.end(), port), ports_.end()); removed_ports_.erase( std::remove(removed_ports_.begin(), removed_ports_.end(), port), removed_ports_.end()); - - LOG(INFO) << "Removed port because it is destroyed: " - << static_cast(ports_.size()) << " remaining"; + LOG(INFO) << "Removed port because it is destroyed: " << ports_.size() + << " remaining"; } -void P2PTransportChannel::OnPortNetworkInactive(PortInterface* port) { - // If it does not gather continually, the port will be removed from the list - // when ICE restarts. - if (!config_.gather_continually) { +void P2PTransportChannel::OnPortsRemoved( + PortAllocatorSession* session, + const std::vector& ports) { + ASSERT(worker_thread_ == rtc::Thread::Current()); + LOG(LS_INFO) << "Remove " << ports.size() << " ports"; + for (PortInterface* port : ports) { + if (RemovePort(port)) { + LOG(INFO) << "Removed port: " << port->ToString() << " " << ports_.size() + << " remaining"; + } + } +} + +void P2PTransportChannel::OnCandidatesRemoved( + PortAllocatorSession* session, + const std::vector& candidates) { + ASSERT(worker_thread_ == rtc::Thread::Current()); + // Do not signal candidate removals if continual gathering is not enabled, or + // if this is not the last session because an ICE restart would have signaled + // the remote side to remove all candidates in previous sessions. + if (!config_.gather_continually() || session != allocator_session()) { return; } - if (!RemovePort(port)) { - return; - } - LOG(INFO) << "Removed port because its network is inactive : " - << port->ToString() << " " << ports_.size() << " remaining"; - std::vector candidates = port->Candidates(); - for (Candidate& candidate : candidates) { + + std::vector candidates_to_remove; + for (Candidate candidate : candidates) { candidate.set_transport_name(transport_name()); + candidates_to_remove.push_back(candidate); } - SignalCandidatesRemoved(this, candidates); + SignalCandidatesRemoved(this, candidates_to_remove); +} + +void P2PTransportChannel::OnRegatherOnFailedNetworks() { + // Only re-gather when the current session is in the CLEARED state (i.e., not + // running or stopped). It is only possible to enter this state when we gather + // continually, so there is an implicit check on continual gathering here. + if (!allocator_sessions_.empty() && allocator_session()->IsCleared()) { + allocator_session()->RegatherOnFailedNetworks(); + } + + thread()->PostDelayed(RTC_FROM_HERE, + *config_.regather_on_failed_networks_interval, this, + MSG_REGATHER_ON_FAILED_NETWORKS); } void P2PTransportChannel::OnPortPruned(PortAllocatorSession* session, diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 3d32f31565..14cda683f5 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -268,11 +268,15 @@ class P2PTransportChannel : public TransportChannelImpl, void AddConnection(Connection* connection); void OnPortReady(PortAllocatorSession *session, PortInterface* port); + // TODO(honghaiz): Merge the two methods OnPortsRemoved and OnPortPruned but + // still log the reason of removing. + void OnPortsRemoved(PortAllocatorSession* session, + const std::vector& ports); void OnPortPruned(PortAllocatorSession* session, PortInterface* port); - // Returns true if the port is found and removed from |ports_|. - bool RemovePort(PortInterface* port); void OnCandidatesReady(PortAllocatorSession *session, const std::vector& candidates); + void OnCandidatesRemoved(PortAllocatorSession* session, + const std::vector& candidates); void OnCandidatesAllocationDone(PortAllocatorSession* session); void OnUnknownAddress(PortInterface* port, const rtc::SocketAddress& addr, @@ -280,8 +284,13 @@ class P2PTransportChannel : public TransportChannelImpl, IceMessage* stun_msg, const std::string& remote_username, bool port_muxed); + + // When a port is destroyed, remove it from both lists |ports_| + // and |removed_ports_|. void OnPortDestroyed(PortInterface* port); - void OnPortNetworkInactive(PortInterface* port); + // When removing a port, move it from |ports_| to |removed_ports_|. + // Returns true if the port is found and removed from |ports_|. + bool RemovePort(PortInterface* port); void OnRoleConflict(PortInterface* port); void OnConnectionStateChange(Connection* connection); @@ -295,6 +304,7 @@ class P2PTransportChannel : public TransportChannelImpl, void OnMessage(rtc::Message* pmsg) override; void OnCheckAndPing(); + void OnRegatherOnFailedNetworks(); // Returns true if the new_connection should be selected for transmission. bool ShouldSwitchSelectedConnection(Connection* new_connection) const; diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 0408552848..cf54fc4a27 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -105,12 +105,13 @@ static const uint64_t kHighTiebreaker = 22222; enum { MSG_ADD_CANDIDATES, MSG_REMOVE_CANDIDATES }; -cricket::IceConfig CreateIceConfig(int receiving_timeout, - bool gather_continually, - int backup_ping_interval = -1) { +cricket::IceConfig CreateIceConfig( + int receiving_timeout, + cricket::ContinualGatheringPolicy continual_gathering_policy, + int backup_ping_interval = -1) { cricket::IceConfig config; config.receiving_timeout = receiving_timeout; - config.gather_continually = gather_continually; + config.continual_gathering_policy = continual_gathering_policy; config.backup_connection_ping_interval = backup_ping_interval; return config; } @@ -412,6 +413,7 @@ class P2PTransportChannelTestBase : public testing::Test, } void RemoveAddress(int endpoint, const SocketAddress& addr) { GetEndpoint(endpoint)->network_manager_.RemoveInterface(addr); + fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, addr); } void SetProxy(int endpoint, rtc::ProxyType type) { rtc::ProxyInfo info; @@ -1543,7 +1545,7 @@ TEST_F(P2PTransportChannelTest, TestContinualGathering) { SetAllocationStepDelay(0, kDefaultStepDelay); SetAllocationStepDelay(1, kDefaultStepDelay); CreateChannels(1); - IceConfig config = CreateIceConfig(1000, true); + IceConfig config = CreateIceConfig(1000, GATHER_CONTINUALLY); ep1_ch1()->SetIceConfig(config); // By default, ep2 does not gather continually. @@ -1820,6 +1822,37 @@ TEST_F(P2PTransportChannelSameNatTest, TestConesBehindSameCone) { // In the future we will try different RTTs and configs for the different // interfaces, so that we can simulate a user with Ethernet and VPN networks. class P2PTransportChannelMultihomedTest : public P2PTransportChannelTestBase { + public: + const cricket::Connection* GetConnectionWithRemoteAddress( + cricket::P2PTransportChannel* channel, + const SocketAddress& address) { + for (cricket::Connection* conn : channel->connections()) { + if (conn->remote_candidate().address().EqualIPs(address)) { + return conn; + } + } + return nullptr; + } + + const cricket::Connection* GetConnectionWithLocalAddress( + cricket::P2PTransportChannel* channel, + const SocketAddress& address) { + for (cricket::Connection* conn : channel->connections()) { + if (conn->local_candidate().address().EqualIPs(address)) { + return conn; + } + } + return nullptr; + } + + void DestroyAllButBestConnection(cricket::P2PTransportChannel* channel) { + const cricket::Connection* best_connection = channel->best_connection(); + for (cricket::Connection* conn : channel->connections()) { + if (conn != best_connection) { + conn->Destroy(); + } + } + } }; // Test that we can establish connectivity when both peers are multihomed. @@ -1856,7 +1889,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) { RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1])); // Make the receiving timeout shorter for testing. - IceConfig config = CreateIceConfig(1000, false); + IceConfig config = CreateIceConfig(1000, GATHER_ONCE); ep1_ch1()->SetIceConfig(config); ep2_ch1()->SetIceConfig(config); @@ -1909,7 +1942,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) { RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1])); // Make the receiving timeout shorter for testing. - IceConfig config = CreateIceConfig(1000, false); + IceConfig config = CreateIceConfig(1000, GATHER_ONCE); ep1_ch1()->SetIceConfig(config); ep2_ch1()->SetIceConfig(config); @@ -1968,6 +2001,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPreferWifiToWifiConnection) { LocalCandidate(ep2_ch1())->address().EqualIPs(wifi[1]) && RemoteCandidate(ep2_ch1())->address().EqualIPs(wifi[0]), 1000); + DestroyChannels(); } // Tests that a Wifi-Cellular connection has higher precedence than @@ -1998,6 +2032,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPreferWifiOverCellularNetwork) { EXPECT_TRUE_WAIT(ep2_ch1()->selected_connection() && LocalCandidate(ep2_ch1())->address().EqualIPs(wifi[1]), 1000); + DestroyChannels(); } // Test that the backup connection is pinged at a rate no faster than @@ -2019,7 +2054,8 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPingBackupConnectionRate) { ep2_ch1()->receiving() && ep2_ch1()->writable(), 1000, 1000); int backup_ping_interval = 2000; - ep2_ch1()->SetIceConfig(CreateIceConfig(2000, false, backup_ping_interval)); + ep2_ch1()->SetIceConfig( + CreateIceConfig(2000, GATHER_ONCE, backup_ping_interval)); // After the state becomes COMPLETED, the backup connection will be pinged // once every |backup_ping_interval| milliseconds. ASSERT_TRUE_WAIT(ep2_ch1()->GetState() == STATE_COMPLETED, 1000); @@ -2034,6 +2070,8 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPingBackupConnectionRate) { backup_conn->last_ping_response_received() - last_ping_response_ms; LOG(LS_INFO) << "Time elapsed: " << time_elapsed; EXPECT_GE(time_elapsed, backup_ping_interval); + + DestroyChannels(); } TEST_F(P2PTransportChannelMultihomedTest, TestGetState) { @@ -2050,23 +2088,25 @@ TEST_F(P2PTransportChannelMultihomedTest, TestGetState) { 1000); } -// Tests that when a network interface becomes inactive, if and only if -// Continual Gathering is enabled, the ports associated with that network +// Tests that when a network interface becomes inactive, if Continual Gathering +// policy is GATHER_CONTINUALLY, the ports associated with that network // will be removed from the port list of the channel, and the respective // remote candidates on the other participant will be removed eventually. TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) { + rtc::ScopedFakeClock clock; AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); // Create channels and let them go writable, as usual. CreateChannels(1); - ep1_ch1()->SetIceConfig(CreateIceConfig(2000, true)); - ep2_ch1()->SetIceConfig(CreateIceConfig(2000, false)); + ep1_ch1()->SetIceConfig(CreateIceConfig(2000, GATHER_CONTINUALLY)); + ep2_ch1()->SetIceConfig(CreateIceConfig(2000, GATHER_ONCE)); SetAllocatorFlags(0, kOnlyLocalPorts); SetAllocatorFlags(1, kOnlyLocalPorts); - EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() && - ep2_ch1()->receiving() && ep2_ch1()->writable(), - 1000, 1000); + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + kDefaultTimeout, clock); // More than one port has been created. EXPECT_LE(1U, ep1_ch1()->ports().size()); // Endpoint 1 enabled continual gathering; the port will be removed @@ -2074,29 +2114,76 @@ TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) { RemoveAddress(0, kPublicAddrs[0]); EXPECT_TRUE(ep1_ch1()->ports().empty()); // The remote candidates will be removed eventually. - EXPECT_TRUE_WAIT(ep2_ch1()->remote_candidates().empty(), 1000); + EXPECT_TRUE_SIMULATED_WAIT(ep2_ch1()->remote_candidates().empty(), 1000, + clock); size_t num_ports = ep2_ch1()->ports().size(); EXPECT_LE(1U, num_ports); size_t num_remote_candidates = ep1_ch1()->remote_candidates().size(); - // Endpoint 2 did not enable continual gathering; the port will not be removed - // when the interface is removed and neither the remote candidates on the - // other participant. + // Endpoint 2 did not enable continual gathering; the local port will still be + // removed when the interface is removed but the remote candidates on the + // other participant will not be removed. RemoveAddress(1, kPublicAddrs[1]); - rtc::Thread::Current()->ProcessMessages(500); - EXPECT_EQ(num_ports, ep2_ch1()->ports().size()); + + EXPECT_EQ_SIMULATED_WAIT(0U, ep2_ch1()->ports().size(), kDefaultTimeout, + clock); + SIMULATED_WAIT(0U == ep1_ch1()->remote_candidates().size(), 500, clock); EXPECT_EQ(num_remote_candidates, ep1_ch1()->remote_candidates().size()); + + DestroyChannels(); } -/* +// Tests that continual gathering will create new connections when a new +// interface is added. +TEST_F(P2PTransportChannelMultihomedTest, + TestContinualGatheringOnNewInterface) { + auto& wifi = kAlternateAddrs; + auto& cellular = kPublicAddrs; + AddAddress(0, wifi[0], "test_wifi0", rtc::ADAPTER_TYPE_WIFI); + AddAddress(1, cellular[1], "test_cell1", rtc::ADAPTER_TYPE_CELLULAR); + CreateChannels(1); + // Set continual gathering policy. + ep1_ch1()->SetIceConfig(CreateIceConfig(1000, GATHER_CONTINUALLY)); + ep2_ch1()->SetIceConfig(CreateIceConfig(1000, GATHER_CONTINUALLY)); + SetAllocatorFlags(0, kOnlyLocalPorts); + SetAllocatorFlags(1, kOnlyLocalPorts); + EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && ep2_ch1()->writable(), + kDefaultTimeout, kDefaultTimeout); -TODO(pthatcher): Once have a way to handle network interfaces changes -without signalling an ICE restart, put a test like this back. In the -mean time, this test only worked for GICE. With ICE, it's currently -not possible without an ICE restart. + // Add a new wifi interface on end point 2. We should expect a new connection + // to be created and the new one will be the best connection. + AddAddress(1, wifi[1], "test_wifi1", rtc::ADAPTER_TYPE_WIFI); + const cricket::Connection* conn; + EXPECT_TRUE_WAIT((conn = ep1_ch1()->best_connection()) != nullptr && + conn->remote_candidate().address().EqualIPs(wifi[1]), + kDefaultTimeout); + EXPECT_TRUE_WAIT((conn = ep2_ch1()->best_connection()) != nullptr && + conn->local_candidate().address().EqualIPs(wifi[1]), + kDefaultTimeout); -// Test that we can switch links in a coordinated fashion. -TEST_F(P2PTransportChannelMultihomedTest, TestDrain) { + // Add a new cellular interface on end point 1, we should expect a new + // backup connection created using this new interface. + AddAddress(0, cellular[0], "test_cellular0", rtc::ADAPTER_TYPE_CELLULAR); + EXPECT_TRUE_WAIT(ep1_ch1()->GetState() == cricket::STATE_COMPLETED && + (conn = GetConnectionWithLocalAddress( + ep1_ch1(), cellular[0])) != nullptr && + conn != ep1_ch1()->best_connection() && conn->writable(), + kDefaultTimeout); + EXPECT_TRUE_WAIT( + ep2_ch1()->GetState() == cricket::STATE_COMPLETED && + (conn = GetConnectionWithRemoteAddress(ep2_ch1(), cellular[0])) != + nullptr && + conn != ep2_ch1()->best_connection() && conn->receiving(), + kDefaultTimeout); + + DestroyChannels(); +} + +// Tests that we can switch links via continual gathering. +TEST_F(P2PTransportChannelMultihomedTest, + TestSwitchLinksViaContinualGathering) { + rtc::ScopedFakeClock clock; AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); // Use only local ports for simplicity. @@ -2105,36 +2192,134 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) { // Create channels and let them go writable, as usual. CreateChannels(1); - EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && - ep2_ch1()->receiving() && ep2_ch1()->writable(), - 1000); + // Set continual gathering policy. + ep1_ch1()->SetIceConfig(CreateIceConfig(1000, GATHER_CONTINUALLY)); + ep2_ch1()->SetIceConfig(CreateIceConfig(1000, GATHER_CONTINUALLY)); + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + 3000, clock); EXPECT_TRUE( ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() && LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1])); - - // Remove the public interface, add the alternate interface, and allocate - // a new generation of candidates for the new interface (via - // MaybeStartGathering()). + // Add the new address first and then remove the other one. LOG(LS_INFO) << "Draining..."; AddAddress(1, kAlternateAddrs[1]); RemoveAddress(1, kPublicAddrs[1]); - ep2_ch1()->MaybeStartGathering(); - - // We should switch over to use the alternate address after - // an exchange of pings. - EXPECT_TRUE_WAIT( + // We should switch to use the alternate address after an exchange of pings. + EXPECT_TRUE_SIMULATED_WAIT( ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() && - LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && - RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]), - 3000); + LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && + RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]), + 3000, clock); + + // Remove one address first and then add another address. + LOG(LS_INFO) << "Draining again..."; + RemoveAddress(1, kAlternateAddrs[1]); + AddAddress(1, kAlternateAddrs[0]); + EXPECT_TRUE_SIMULATED_WAIT( + ep1_ch1()->best_connection() && ep2_ch1()->best_connection() && + LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) && + RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[0]), + 3000, clock); DestroyChannels(); } +/* +TODO(honghaiz) Once continual gathering fully supports +GATHER_CONTINUALLY_AND_RECOVER, put this test back. + +// Tests that if the backup connections are lost and then the interface with the +// selected connection is gone, continual gathering will restore the +// connectivity. +TEST_F(P2PTransportChannelMultihomedTest, + TestBackupConnectionLostThenInterfaceGone) { + rtc::ScopedFakeClock clock; + auto& wifi = kAlternateAddrs; + auto& cellular = kPublicAddrs; + AddAddress(0, wifi[0], "test_wifi0", rtc::ADAPTER_TYPE_WIFI); + AddAddress(0, cellular[0], "test_cell0", rtc::ADAPTER_TYPE_CELLULAR); + AddAddress(1, wifi[1], "test_wifi1", rtc::ADAPTER_TYPE_WIFI); + AddAddress(1, cellular[1], "test_cell1", rtc::ADAPTER_TYPE_CELLULAR); + // Use only local ports for simplicity. + SetAllocatorFlags(0, kOnlyLocalPorts); + SetAllocatorFlags(1, kOnlyLocalPorts); + + // Create channels and let them go writable, as usual. + CreateChannels(1); + // Set continual gathering policy. + IceConfig config = CreateIceConfig(1000, GATHER_CONTINUALLY_AND_RECOVER); + ep1_ch1()->SetIceConfig(config); + ep2_ch1()->SetIceConfig(config); + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && ep2_ch1()->writable(), + 3000, clock); + EXPECT_TRUE(ep1_ch1()->best_connection() && ep2_ch1()->best_connection() && + LocalCandidate(ep1_ch1())->address().EqualIPs(wifi[0]) && + RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1])); + + // First destroy all backup connection. + DestroyAllButBestConnection(ep1_ch1()); + + SIMULATED_WAIT(false, 10, clock); + // Then the interface of the best connection goes away. + RemoveAddress(0, wifi[0]); + EXPECT_TRUE_SIMULATED_WAIT( + ep1_ch1()->best_connection() && ep2_ch1()->best_connection() && + LocalCandidate(ep1_ch1())->address().EqualIPs(cellular[0]) && + RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1]), + 3000, clock); + + DestroyChannels(); +} */ +// Tests that the backup connection will be restored after it is destroyed. +TEST_F(P2PTransportChannelMultihomedTest, TestRestoreBackupConnection) { + rtc::ScopedFakeClock clock; + auto& wifi = kAlternateAddrs; + auto& cellular = kPublicAddrs; + AddAddress(0, wifi[0], "test_wifi0", rtc::ADAPTER_TYPE_WIFI); + AddAddress(0, cellular[0], "test_cell0", rtc::ADAPTER_TYPE_CELLULAR); + AddAddress(1, wifi[1], "test_wifi1", rtc::ADAPTER_TYPE_WIFI); + AddAddress(1, cellular[1], "test_cell1", rtc::ADAPTER_TYPE_CELLULAR); + // Use only local ports for simplicity. + SetAllocatorFlags(0, kOnlyLocalPorts); + SetAllocatorFlags(1, kOnlyLocalPorts); + + // Create channels and let them go writable, as usual. + CreateChannels(1); + IceConfig config = CreateIceConfig(1000, GATHER_CONTINUALLY); + config.regather_on_failed_networks_interval = rtc::Optional(2000); + ep1_ch1()->SetIceConfig(config); + ep2_ch1()->SetIceConfig(config); + EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && + ep2_ch1()->receiving() && + ep2_ch1()->writable(), + 3000, clock); + EXPECT_TRUE(ep1_ch1()->best_connection() && ep2_ch1()->best_connection() && + LocalCandidate(ep1_ch1())->address().EqualIPs(wifi[0]) && + RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1])); + + // Destroy all backup connections. + DestroyAllButBestConnection(ep1_ch1()); + // Ensure the backup connection is removed first. + EXPECT_TRUE_SIMULATED_WAIT( + GetConnectionWithLocalAddress(ep1_ch1(), cellular[0]) == nullptr, + kDefaultTimeout, clock); + const cricket::Connection* conn; + EXPECT_TRUE_SIMULATED_WAIT( + (conn = GetConnectionWithLocalAddress(ep1_ch1(), cellular[0])) != + nullptr && + conn != ep1_ch1()->best_connection() && conn->writable(), + 5000, clock); + + DestroyChannels(); +} + // A collection of tests which tests a single P2PTransportChannel by sending // pings. class P2PTransportChannelPingTest : public testing::Test, @@ -2624,7 +2809,7 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) { // small. EXPECT_LE(1000, ch.receiving_timeout()); EXPECT_LE(200, ch.check_receiving_interval()); - ch.SetIceConfig(CreateIceConfig(500, false)); + ch.SetIceConfig(CreateIceConfig(500, GATHER_ONCE)); EXPECT_EQ(500, ch.receiving_timeout()); EXPECT_EQ(50, ch.check_receiving_interval()); ch.MaybeStartGathering(); @@ -3038,7 +3223,7 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) { conn2->SignalNominated(conn2); EXPECT_TRUE_WAIT(conn1->pruned(), 3000); - ch.SetIceConfig(CreateIceConfig(500, false)); + ch.SetIceConfig(CreateIceConfig(500, GATHER_ONCE)); // Wait until conn2 becomes not receiving. EXPECT_TRUE_WAIT(!conn2->receiving(), 3000); @@ -3104,7 +3289,7 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) { FakePortAllocator pa(rtc::Thread::Current(), nullptr); P2PTransportChannel ch("test channel", 1, &pa); PrepareChannel(&ch); - ch.SetIceConfig(CreateIceConfig(1000, false)); + ch.SetIceConfig(CreateIceConfig(1000, GATHER_ONCE)); ch.MaybeStartGathering(); ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100)); Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1); @@ -3178,7 +3363,7 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { FakePortAllocator pa(rtc::Thread::Current(), nullptr); P2PTransportChannel ch("test channel", 1, &pa); PrepareChannel(&ch); - ch.SetIceConfig(CreateIceConfig(2000, false)); + ch.SetIceConfig(CreateIceConfig(2000, GATHER_ONCE)); ch.MaybeStartGathering(); ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100)); Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1); @@ -3204,17 +3389,15 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts()); } -// Test that the ICE role is updated even on ports with inactive networks when -// doing continual gathering. These ports may still have connections that need -// a correct role, in case the network becomes active before the connection is -// destroyed. -TEST_F(P2PTransportChannelPingTest, - TestIceRoleUpdatedOnPortAfterSignalNetworkInactive) { +// Test that the ICE role is updated even on ports that has been removed. +// These ports may still have connections that need a correct role, in case that +// the connections on it may still receive stun pings. +TEST_F(P2PTransportChannelPingTest, TestIceRoleUpdatedOnRemovedPort) { FakePortAllocator pa(rtc::Thread::Current(), nullptr); P2PTransportChannel ch("test channel", ICE_CANDIDATE_COMPONENT_DEFAULT, &pa); // Starts with ICEROLE_CONTROLLING. PrepareChannel(&ch); - IceConfig config = CreateIceConfig(1000, true); + IceConfig config = CreateIceConfig(1000, GATHER_CONTINUALLY); ch.SetIceConfig(config); ch.MaybeStartGathering(); ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1)); @@ -3222,9 +3405,10 @@ TEST_F(P2PTransportChannelPingTest, Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1); ASSERT_TRUE(conn != nullptr); - // Make the fake port signal that its network is inactive, then change the - // ICE role and expect it to be updated. - conn->port()->SignalNetworkInactive(conn->port()); + // Make a fake signal to remove the ports in the p2ptransportchannel. then + // change the ICE role and expect it to be updated. + std::vector ports(1, conn->port()); + ch.allocator_session()->SignalPortsRemoved(ch.allocator_session(), ports); ch.SetIceRole(ICEROLE_CONTROLLED); EXPECT_EQ(ICEROLE_CONTROLLED, conn->port()->GetIceRole()); } diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 71a9e6b2b1..903f13d7a6 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -195,7 +195,6 @@ void Port::Construct() { ice_username_fragment_ = rtc::CreateRandomString(ICE_UFRAG_LENGTH); password_ = rtc::CreateRandomString(ICE_PWD_LENGTH); } - network_->SignalInactive.connect(this, &Port::OnNetworkInactive); network_->SignalTypeChanged.connect(this, &Port::OnNetworkTypeChanged); network_cost_ = network_->GetCost(); @@ -652,11 +651,6 @@ void Port::OnMessage(rtc::Message *pmsg) { } } -void Port::OnNetworkInactive(const rtc::Network* network) { - ASSERT(network == network_); - SignalNetworkInactive(this); -} - void Port::OnNetworkTypeChanged(const rtc::Network* network) { ASSERT(network == network_); diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 2cd264e09e..2c3b61b718 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -363,8 +363,6 @@ class Port : public PortInterface, public rtc::MessageHandler, return ice_role_ == ICEROLE_CONTROLLED && connections_.empty(); } - void OnNetworkInactive(const rtc::Network* network); - void OnNetworkTypeChanged(const rtc::Network* network); rtc::Thread* thread_; diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h index 9b4465d3de..dc1961db73 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h @@ -88,6 +88,14 @@ enum { CF_ALL = 0x7, }; +enum class SessionState { + GATHERING, // Actively allocating ports and gathering candidates. + CLEARED, // Current allocation process has been stopped but may start + // new ones. + STOPPED // This session has completely stopped, no new allocation + // process will be started. +}; + // TODO(deadbeef): Rename to TurnCredentials (and username to ufrag). struct RelayCredentials { RelayCredentials() {} @@ -159,12 +167,27 @@ class PortAllocatorSession : public sigslot::has_slots<> { virtual void SetCandidateFilter(uint32_t filter) = 0; // Starts gathering STUN and Relay configurations. - virtual void StartGettingPorts() = 0; - virtual void StopGettingPorts() = 0; - // Only stop the existing gathering process but may start new ones if needed. - virtual void ClearGettingPorts() = 0; - // Whether the process of getting ports has been stopped. - virtual bool IsGettingPorts() = 0; + virtual void StartGettingPorts() { state_ = SessionState::GATHERING; } + // Completely stops the gathering process and will not start new ones. + virtual void StopGettingPorts() { state_ = SessionState::STOPPED; } + // Only stops the existing gathering process but may start new ones if needed. + virtual void ClearGettingPorts() { state_ = SessionState::CLEARED; } + // Whether the session is actively getting ports. + bool IsGettingPorts() { return state_ == SessionState::GATHERING; } + // Whether it is in the state where the existing gathering process is stopped, + // but new ones may be started (basically after calling ClearGettingPorts). + bool IsCleared() { return state_ == SessionState::CLEARED; } + // Whether the session has completely stopped. + bool IsStopped() { return state_ == SessionState::STOPPED; } + // Re-gathers candidates on networks that do not have any connections. More + // precisely, a network interface may have more than one IP addresses (e.g., + // IPv4 and IPv6 addresses). Each address subnet will be used to create a + // network. Only if all networks of an interface have no connection, the + // implementation should start re-gathering on all networks of that interface. + virtual void RegatherOnFailedNetworks() {} + // Re-gathers candidates on all networks. + // TODO(honghaiz): Implement this in BasicPortAllocator. + virtual void RegatherOnAllNetworks() {} // Another way of getting the information provided by the signals below. // @@ -175,8 +198,17 @@ class PortAllocatorSession : public sigslot::has_slots<> { virtual bool CandidatesAllocationDone() const = 0; sigslot::signal2 SignalPortReady; + // Ports should be signaled to be removed when the networks of the ports + // failed (either because the interface is down, or because there is no + // connection on the interface). + sigslot::signal2&> + SignalPortsRemoved; sigslot::signal2&> SignalCandidatesReady; + // Candidates should be signaled to be removed when the port that generated + // the candidates is removed. + sigslot::signal2&> + SignalCandidatesRemoved; sigslot::signal1 SignalCandidatesAllocationDone; // A TURN port is pruned if a higher-priority TURN port becomes ready // (pairable). When it is pruned, it will not be used for creating @@ -220,6 +252,7 @@ class PortAllocatorSession : public sigslot::has_slots<> { int component_; std::string ice_ufrag_; std::string ice_pwd_; + SessionState state_ = SessionState::CLEARED; // SetIceParameters is an implementation detail which only PortAllocator // should be able to call. diff --git a/webrtc/p2p/base/portinterface.h b/webrtc/p2p/base/portinterface.h index 987c3fc1ee..38945f9e13 100644 --- a/webrtc/p2p/base/portinterface.h +++ b/webrtc/p2p/base/portinterface.h @@ -106,9 +106,6 @@ class PortInterface { // any usefulness. sigslot::signal1 SignalDestroyed; - // Signaled when the network used by this port becomes inactive. - sigslot::signal1 SignalNetworkInactive; - // Signaled when Port discovers ice role conflict with the peer. sigslot::signal1 SignalRoleConflict; diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index 9269891577..cf8223851c 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -31,6 +31,7 @@ #include #include "webrtc/base/constructormagic.h" +#include "webrtc/base/optional.h" #include "webrtc/p2p/base/candidate.h" #include "webrtc/p2p/base/p2pconstants.h" #include "webrtc/p2p/base/sessiondescription.h" @@ -81,6 +82,16 @@ enum IceGatheringState { kIceGatheringComplete, }; +enum ContinualGatheringPolicy { + // All port allocator sessions will stop after a writable connection is found. + GATHER_ONCE = 0, + // The most recent port allocator session will keep on running. + GATHER_CONTINUALLY, + // The most recent port allocator session will keep on running, and it will + // try to recover connectivity if the channel becomes disconnected. + GATHER_CONTINUALLY_AND_RECOVER, +}; + // Stats that we can return about the connections for a transport channel. // TODO(hta): Rename to ConnectionStats struct ConnectionInfo { @@ -160,8 +171,13 @@ struct IceConfig { // Time interval in milliseconds to ping a backup connection when the ICE // channel is strongly connected. int backup_connection_ping_interval = -1; - // If true, the most recent port allocator session will keep on running. - bool gather_continually = false; + + ContinualGatheringPolicy continual_gathering_policy = GATHER_ONCE; + + bool gather_continually() const { + return continual_gathering_policy == GATHER_CONTINUALLY || + continual_gathering_policy == GATHER_CONTINUALLY_AND_RECOVER; + } // Whether we should prioritize Relay/Relay candidate when nothing // is writable yet. @@ -174,22 +190,29 @@ struct IceConfig { // candidate pairs will succeed, even before a binding response is received. bool presume_writable_when_fully_relayed = false; + // Interval to check on all networks and to perform ICE regathering on any + // active network having no connection on it. + rtc::Optional regather_on_failed_networks_interval; + IceConfig() {} IceConfig(int receiving_timeout_ms, int backup_connection_ping_interval, - bool gather_continually, + ContinualGatheringPolicy gathering_policy, bool prioritize_most_likely_candidate_pairs, int stable_writable_connection_ping_interval_ms, - bool presume_writable_when_fully_relayed) + bool presume_writable_when_fully_relayed, + int regather_on_failed_networks_interval_ms) : receiving_timeout(receiving_timeout_ms), backup_connection_ping_interval(backup_connection_ping_interval), - gather_continually(gather_continually), + continual_gathering_policy(gathering_policy), prioritize_most_likely_candidate_pairs( prioritize_most_likely_candidate_pairs), stable_writable_connection_ping_interval( stable_writable_connection_ping_interval_ms), presume_writable_when_fully_relayed( - presume_writable_when_fully_relayed) {} + presume_writable_when_fully_relayed), + regather_on_failed_networks_interval( + regather_on_failed_networks_interval_ms) {} }; bool BadTransportDescription(const std::string& desc, std::string* err_desc); diff --git a/webrtc/p2p/base/transportcontroller_unittest.cc b/webrtc/p2p/base/transportcontroller_unittest.cc index 68b66cf994..f068bb52db 100644 --- a/webrtc/p2p/base/transportcontroller_unittest.cc +++ b/webrtc/p2p/base/transportcontroller_unittest.cc @@ -137,11 +137,12 @@ class TransportControllerTest : public testing::Test, channel2->SetConnectionCount(1); } - cricket::IceConfig CreateIceConfig(int receiving_timeout, - bool gather_continually) { + cricket::IceConfig CreateIceConfig( + int receiving_timeout, + cricket::ContinualGatheringPolicy continual_gathering_policy) { cricket::IceConfig config; config.receiving_timeout = receiving_timeout; - config.gather_continually = gather_continually; + config.continual_gathering_policy = continual_gathering_policy; return config; } @@ -204,10 +205,13 @@ TEST_F(TransportControllerTest, TestSetIceConfig) { FakeTransportChannel* channel1 = CreateChannel("audio", 1); ASSERT_NE(nullptr, channel1); - transport_controller_->SetIceConfig(CreateIceConfig(1000, true)); + transport_controller_->SetIceConfig( + CreateIceConfig(1000, cricket::GATHER_CONTINUALLY)); EXPECT_EQ(1000, channel1->receiving_timeout()); EXPECT_TRUE(channel1->gather_continually()); + transport_controller_->SetIceConfig( + CreateIceConfig(1000, cricket::GATHER_CONTINUALLY_AND_RECOVER)); // Test that value stored in controller is applied to new channels. FakeTransportChannel* channel2 = CreateChannel("video", 1); ASSERT_NE(nullptr, channel2); diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index edd5fed9cf..3a7aa5a333 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -184,7 +184,6 @@ BasicPortAllocatorSession::BasicPortAllocatorSession( socket_factory_(allocator->socket_factory()), allocation_started_(false), network_manager_started_(false), - running_(false), allocation_sequences_created_(false), prune_turn_ports_(allocator->prune_turn_ports()) { allocator_->network_manager()->SignalNetworksChanged.connect( @@ -239,27 +238,84 @@ void BasicPortAllocatorSession::SetCandidateFilter(uint32_t filter) { void BasicPortAllocatorSession::StartGettingPorts() { network_thread_ = rtc::Thread::Current(); + PortAllocatorSession::StartGettingPorts(); if (!socket_factory_) { owned_socket_factory_.reset( new rtc::BasicPacketSocketFactory(network_thread_)); socket_factory_ = owned_socket_factory_.get(); } - running_ = true; network_thread_->Post(RTC_FROM_HERE, this, MSG_CONFIG_START); } void BasicPortAllocatorSession::StopGettingPorts() { ASSERT(rtc::Thread::Current() == network_thread_); - running_ = false; network_thread_->Post(RTC_FROM_HERE, this, MSG_CONFIG_STOP); ClearGettingPorts(); + // Note: this must be called after ClearGettingPorts because both may set the + // session state and we should set the state to STOPPED. + PortAllocatorSession::StopGettingPorts(); } void BasicPortAllocatorSession::ClearGettingPorts() { + ASSERT(rtc::Thread::Current() == network_thread_); network_thread_->Clear(this, MSG_ALLOCATE); - for (uint32_t i = 0; i < sequences_.size(); ++i) + for (uint32_t i = 0; i < sequences_.size(); ++i) { sequences_[i]->Stop(); + } + PortAllocatorSession::ClearGettingPorts(); +} + +std::vector BasicPortAllocatorSession::GetFailedNetworks() { + std::vector networks = GetNetworks(); + + // A network interface may have both IPv4 and IPv6 networks. Only if + // neither of the networks has any connections, the network interface + // is considered failed and need to be regathered on. + std::set networks_with_connection; + for (const PortData& data : ports_) { + Port* port = data.port(); + if (!port->connections().empty()) { + networks_with_connection.insert(port->Network()->name()); + } + } + + networks.erase( + std::remove_if(networks.begin(), networks.end(), + [networks_with_connection](rtc::Network* network) { + // If a network does not have any connection, it is + // considered failed. + return networks_with_connection.find(network->name()) != + networks_with_connection.end(); + }), + networks.end()); + return networks; +} + +void BasicPortAllocatorSession::RegatherOnFailedNetworks() { + // Find the list of networks that have no connection. + std::vector failed_networks = GetFailedNetworks(); + if (failed_networks.empty()) { + return; + } + + // Mark a sequence as "network failed" if its network is in the list of failed + // networks, so that it won't be considered as equivalent when the session + // regathers ports and candidates. + for (AllocationSequence* sequence : sequences_) { + if (!sequence->network_failed() && + std::find(failed_networks.begin(), failed_networks.end(), + sequence->network()) != failed_networks.end()) { + sequence->set_network_failed(); + } + } + // Remove ports from being used locally and send signaling to remove + // the candidates on the remote side. + RemovePortsAndCandidates(failed_networks); + + if (allocation_started_ && network_manager_started_) { + DoAllocate(); + } } std::vector BasicPortAllocatorSession::ReadyPorts() const { @@ -278,22 +334,28 @@ std::vector BasicPortAllocatorSession::ReadyCandidates() const { if (!data.ready()) { continue; } - - for (const Candidate& candidate : data.port()->Candidates()) { - if (!CheckCandidateFilter(candidate)) { - continue; - } - ProtocolType pvalue; - if (!StringToProto(candidate.protocol().c_str(), &pvalue) || - !data.sequence()->ProtocolEnabled(pvalue)) { - continue; - } - candidates.push_back(SanitizeRelatedAddress(candidate)); - } + GetCandidatesFromPort(data, &candidates); } return candidates; } +void BasicPortAllocatorSession::GetCandidatesFromPort( + const PortData& data, + std::vector* candidates) const { + RTC_CHECK(candidates != nullptr); + for (const Candidate& candidate : data.port()->Candidates()) { + if (!CheckCandidateFilter(candidate)) { + continue; + } + ProtocolType pvalue; + if (!StringToProto(candidate.protocol().c_str(), &pvalue) || + !data.sequence()->ProtocolEnabled(pvalue)) { + continue; + } + candidates->push_back(SanitizeRelatedAddress(candidate)); + } +} + Candidate BasicPortAllocatorSession::SanitizeRelatedAddress( const Candidate& c) const { Candidate copy = c; @@ -437,9 +499,8 @@ void BasicPortAllocatorSession::OnAllocate() { allocation_started_ = true; } -void BasicPortAllocatorSession::GetNetworks( - std::vector* networks) { - networks->clear(); +std::vector BasicPortAllocatorSession::GetNetworks() { + std::vector networks; rtc::NetworkManager* network_manager = allocator_->network_manager(); ASSERT(network_manager != nullptr); // If the network permission state is BLOCKED, we just act as if the flag has @@ -453,37 +514,37 @@ void BasicPortAllocatorSession::GetNetworks( // traffic by OS is also used here to avoid any local or public IP leakage // during stun process. if (flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) { - network_manager->GetAnyAddressNetworks(networks); + network_manager->GetAnyAddressNetworks(&networks); } else { - network_manager->GetNetworks(networks); + network_manager->GetNetworks(&networks); } - networks->erase(std::remove_if(networks->begin(), networks->end(), - [this](rtc::Network* network) { - return allocator_->network_ignore_mask() & - network->type(); - }), - networks->end()); + networks.erase(std::remove_if(networks.begin(), networks.end(), + [this](rtc::Network* network) { + return allocator_->network_ignore_mask() & + network->type(); + }), + networks.end()); if (flags() & PORTALLOCATOR_DISABLE_COSTLY_NETWORKS) { uint16_t lowest_cost = rtc::kNetworkCostMax; - for (rtc::Network* network : *networks) { + for (rtc::Network* network : networks) { lowest_cost = std::min(lowest_cost, network->GetCost()); } - networks->erase(std::remove_if(networks->begin(), networks->end(), - [lowest_cost](rtc::Network* network) { - return network->GetCost() > - lowest_cost + rtc::kNetworkCostLow; - }), - networks->end()); + networks.erase(std::remove_if(networks.begin(), networks.end(), + [lowest_cost](rtc::Network* network) { + return network->GetCost() > + lowest_cost + rtc::kNetworkCostLow; + }), + networks.end()); } + return networks; } // For each network, see if we have a sequence that covers it already. If not, // create a new sequence to create the appropriate ports. void BasicPortAllocatorSession::DoAllocate() { bool done_signal_needed = false; - std::vector networks; - GetNetworks(&networks); + std::vector networks = GetNetworks(); if (networks.empty()) { LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated"; @@ -528,8 +589,9 @@ void BasicPortAllocatorSession::DoAllocate() { done_signal_needed = true; sequence->SignalPortAllocationComplete.connect( this, &BasicPortAllocatorSession::OnPortAllocationComplete); - if (running_) + if (!IsStopped()) { sequence->Start(); + } sequences_.push_back(sequence); } } @@ -539,17 +601,19 @@ void BasicPortAllocatorSession::DoAllocate() { } void BasicPortAllocatorSession::OnNetworksChanged() { - std::vector networks; - GetNetworks(&networks); + std::vector networks = GetNetworks(); + std::vector failed_networks; for (AllocationSequence* sequence : sequences_) { - // Remove the network from the allocation sequence if it is not in + // Mark the sequence as "network failed" if its network is not in // |networks|. - if (!sequence->network_removed() && + if (!sequence->network_failed() && std::find(networks.begin(), networks.end(), sequence->network()) == networks.end()) { - sequence->OnNetworkRemoved(); + sequence->OnNetworkFailed(); + failed_networks.push_back(sequence->network()); } } + RemovePortsAndCandidates(failed_networks); network_manager_started_ = true; if (allocation_started_) @@ -847,6 +911,32 @@ BasicPortAllocatorSession::PortData* BasicPortAllocatorSession::FindPort( return NULL; } +// Removes ports and candidates created on a given list of networks. +void BasicPortAllocatorSession::RemovePortsAndCandidates( + const std::vector& networks) { + std::vector ports_to_remove; + std::vector candidates_to_remove; + for (PortData& data : ports_) { + if (std::find(networks.begin(), networks.end(), + data.sequence()->network()) == networks.end()) { + continue; + } + ports_to_remove.push_back(data.port()); + if (data.has_pairable_candidate()) { + GetCandidatesFromPort(data, &candidates_to_remove); + // Mark the port as having no pairable candidates so that its candidates + // won't be removed multiple times. + data.set_has_pairable_candidate(false); + } + } + if (!ports_to_remove.empty()) { + SignalPortsRemoved(this, ports_to_remove); + } + if (!candidates_to_remove.empty()) { + SignalCandidatesRemoved(this, candidates_to_remove); + } +} + // AllocationSequence AllocationSequence::AllocationSequence(BasicPortAllocatorSession* session, @@ -884,10 +974,11 @@ void AllocationSequence::Clear() { turn_ports_.clear(); } -void AllocationSequence::OnNetworkRemoved() { - // Stop the allocation sequence if its network is gone. +void AllocationSequence::OnNetworkFailed() { + RTC_DCHECK(!network_failed_); + network_failed_ = true; + // Stop the allocation sequence if its network failed. Stop(); - network_removed_ = true; } AllocationSequence::~AllocationSequence() { @@ -896,8 +987,8 @@ AllocationSequence::~AllocationSequence() { void AllocationSequence::DisableEquivalentPhases(rtc::Network* network, PortConfiguration* config, uint32_t* flags) { - if (network_removed_) { - // If the network of this allocation sequence has ever gone away, + if (network_failed_) { + // If the network of this allocation sequence has ever become failed, // it won't be equivalent to the new network. return; } diff --git a/webrtc/p2p/client/basicportallocator.h b/webrtc/p2p/client/basicportallocator.h index bddb3967e5..f133bdfc7e 100644 --- a/webrtc/p2p/client/basicportallocator.h +++ b/webrtc/p2p/client/basicportallocator.h @@ -47,7 +47,7 @@ class BasicPortAllocator : public PortAllocator { int network_ignore_mask() const { return network_ignore_mask_; } - rtc::NetworkManager* network_manager() { return network_manager_; } + rtc::NetworkManager* network_manager() const { return network_manager_; } // If socket_factory() is set to NULL each PortAllocatorSession // creates its own socket factory. @@ -92,11 +92,11 @@ class BasicPortAllocatorSession : public PortAllocatorSession, void StartGettingPorts() override; void StopGettingPorts() override; void ClearGettingPorts() override; - bool IsGettingPorts() override { return running_; } // These will all be cricket::Ports. std::vector ReadyPorts() const override; std::vector ReadyCandidates() const override; bool CandidatesAllocationDone() const override; + void RegatherOnFailedNetworks() override; protected: void UpdateIceParametersInternal() override; @@ -156,8 +156,8 @@ class BasicPortAllocatorSession : public PortAllocatorSession, }; Port* port_ = nullptr; AllocationSequence* sequence_ = nullptr; - State state_ = STATE_INPROGRESS; bool has_pairable_candidate_ = false; + State state_ = STATE_INPROGRESS; }; void OnConfigReady(PortConfiguration* config); @@ -180,7 +180,8 @@ class BasicPortAllocatorSession : public PortAllocatorSession, void MaybeSignalCandidatesAllocationDone(); void OnPortAllocationComplete(AllocationSequence* seq); PortData* FindPort(Port* port); - void GetNetworks(std::vector* networks); + std::vector GetNetworks(); + std::vector GetFailedNetworks(); bool CheckCandidateFilter(const Candidate& c) const; bool CandidatePairable(const Candidate& c, const Port* port) const; @@ -188,6 +189,12 @@ class BasicPortAllocatorSession : public PortAllocatorSession, // in order to avoid leaking any information. Candidate SanitizeRelatedAddress(const Candidate& c) const; + // Removes the ports and candidates on given networks. + void RemovePortsAndCandidates(const std::vector& networks); + // Gets filtered and sanitized candidates generated from a port and + // append to |candidates|. + void GetCandidatesFromPort(const PortData& data, + std::vector* candidates) const; Port* GetBestTurnPortForNetwork(const std::string& network_name) const; // Returns true if at least one TURN port is pruned. bool PruneTurnPorts(Port* newly_pairable_turn_port); @@ -198,7 +205,6 @@ class BasicPortAllocatorSession : public PortAllocatorSession, rtc::PacketSocketFactory* socket_factory_; bool allocation_started_; bool network_manager_started_; - bool running_; // set when StartGetAllPorts is called bool allocation_sequences_created_; std::vector configs_; std::vector sequences_; @@ -271,11 +277,13 @@ class AllocationSequence : public rtc::MessageHandler, ~AllocationSequence(); bool Init(); void Clear(); - void OnNetworkRemoved(); + void OnNetworkFailed(); State state() const { return state_; } - const rtc::Network* network() const { return network_; } - bool network_removed() const { return network_removed_; } + rtc::Network* network() const { return network_; } + + bool network_failed() const { return network_failed_; } + void set_network_failed() { network_failed_ = true; } // Disables the phases for a new sequence that this one already covers for an // equivalent network setup. @@ -325,7 +333,7 @@ class AllocationSequence : public rtc::MessageHandler, void OnPortDestroyed(PortInterface* port); BasicPortAllocatorSession* session_; - bool network_removed_ = false; + bool network_failed_ = false; rtc::Network* network_; rtc::IPAddress ip_; PortConfiguration* config_;