diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index cc33aa88ce..e0b4fca48d 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -2212,8 +2212,7 @@ bool PeerConnection::InitializePortAllocator_n( // Call this last since it may create pooled allocator sessions using the // properties set above. port_allocator_->SetConfiguration(stun_servers, turn_servers, - configuration.ice_candidate_pool_size, - configuration.prune_turn_ports); + configuration.ice_candidate_pool_size); return true; } @@ -2229,8 +2228,7 @@ bool PeerConnection::ReconfigurePortAllocator_n( // Call this last since it may create pooled allocator sessions using the // candidate filter set above. port_allocator_->SetConfiguration(stun_servers, turn_servers, - configuration.ice_candidate_pool_size, - configuration.prune_turn_ports); + configuration.ice_candidate_pool_size); return true; } diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 3757dad8bb..0ac37e1564 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -300,7 +300,6 @@ class PeerConnectionInterface : public rtc::RefCountInterface { rtc::Optional combined_audio_video_bwe; rtc::Optional enable_dtls_srtp; int ice_candidate_pool_size = 0; - bool prune_turn_ports = false; }; struct RTCOfferAnswerOptions { diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index ebbc0eabbe..b5d6755cee 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -125,7 +125,6 @@ void P2PTransportChannel::AddAllocatorSession( session->set_generation(static_cast(allocator_sessions_.size())); session->SignalPortReady.connect(this, &P2PTransportChannel::OnPortReady); - session->SignalPortPruned.connect(this, &P2PTransportChannel::OnPortPruned); session->SignalCandidatesReady.connect( this, &P2PTransportChannel::OnCandidatesReady); session->SignalCandidatesAllocationDone.connect( @@ -1617,7 +1616,7 @@ void P2PTransportChannel::OnPortDestroyed(PortInterface* port) { std::remove(removed_ports_.begin(), removed_ports_.end(), port), removed_ports_.end()); - LOG(INFO) << "Removed port because it is destroyed: " + LOG(INFO) << "Removed port from p2p socket: " << static_cast(ports_.size()) << " remaining"; } @@ -1627,11 +1626,15 @@ void P2PTransportChannel::OnPortNetworkInactive(PortInterface* port) { if (!config_.gather_continually) { return; } - if (!RemovePort(port)) { + auto it = std::find(ports_.begin(), ports_.end(), port); + // Don't need to do anything if the port has been deleted from the port list. + if (it == ports_.end()) { return; } - LOG(INFO) << "Removed port because its network is inactive : " - << port->ToString() << " " << ports_.size() << " remaining"; + removed_ports_.push_back(*it); + ports_.erase(it); + LOG(INFO) << "Removed port due to inactive networks: " << ports_.size() + << " remaining"; std::vector candidates = port->Candidates(); for (Candidate& candidate : candidates) { candidate.set_transport_name(transport_name()); @@ -1639,25 +1642,6 @@ void P2PTransportChannel::OnPortNetworkInactive(PortInterface* port) { SignalCandidatesRemoved(this, candidates); } -void P2PTransportChannel::OnPortPruned(PortAllocatorSession* session, - PortInterface* port) { - if (RemovePort(port)) { - LOG(INFO) << "Removed port because it is pruned: " << port->ToString() - << " " << ports_.size() << " remaining"; - } -} - -bool P2PTransportChannel::RemovePort(PortInterface* port) { - auto it = std::find(ports_.begin(), ports_.end(), port); - // Don't need to do anything if the port has been deleted from the port list. - if (it == ports_.end()) { - return false; - } - ports_.erase(it); - removed_ports_.push_back(port); - return true; -} - // We data is available, let listeners know void P2PTransportChannel::OnReadPacket(Connection* connection, const char* data, diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 3d32f31565..d90cc61ebc 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -268,9 +268,6 @@ class P2PTransportChannel : public TransportChannelImpl, void AddConnection(Connection* connection); void OnPortReady(PortAllocatorSession *session, PortInterface* port); - 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 OnCandidatesAllocationDone(PortAllocatorSession* session); diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 0408552848..b5645007c3 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -1572,9 +1572,9 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionBeforeDoneGathering) { auto& allocator_2 = GetEndpoint(1)->allocator_; int pool_size = 1; allocator_1->SetConfiguration(allocator_1->stun_servers(), - allocator_1->turn_servers(), pool_size, false); + allocator_1->turn_servers(), pool_size); allocator_2->SetConfiguration(allocator_2->stun_servers(), - allocator_2->turn_servers(), pool_size, false); + allocator_2->turn_servers(), pool_size); const PortAllocatorSession* pooled_session_1 = allocator_1->GetPooledSession(); const PortAllocatorSession* pooled_session_2 = @@ -1615,9 +1615,9 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) { auto& allocator_2 = GetEndpoint(1)->allocator_; int pool_size = 1; allocator_1->SetConfiguration(allocator_1->stun_servers(), - allocator_1->turn_servers(), pool_size, false); + allocator_1->turn_servers(), pool_size); allocator_2->SetConfiguration(allocator_2->stun_servers(), - allocator_2->turn_servers(), pool_size, false); + allocator_2->turn_servers(), pool_size); const PortAllocatorSession* pooled_session_1 = allocator_1->GetPooledSession(); const PortAllocatorSession* pooled_session_2 = diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index e156e017fe..743cddacdb 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -152,8 +152,6 @@ class TestPort : public Port { return true; } - virtual ProtocolType GetProtocol() const { return PROTO_UDP; } - // Exposed for testing candidate building. void AddCandidateAddress(const rtc::SocketAddress& addr) { AddAddress(addr, addr, rtc::SocketAddress(), "udp", "", "", Type(), diff --git a/webrtc/p2p/base/portallocator.cc b/webrtc/p2p/base/portallocator.cc index 9ee08a1476..f9f87b007f 100644 --- a/webrtc/p2p/base/portallocator.cc +++ b/webrtc/p2p/base/portallocator.cc @@ -32,13 +32,11 @@ PortAllocatorSession::PortAllocatorSession(const std::string& content_name, void PortAllocator::SetConfiguration( const ServerAddresses& stun_servers, const std::vector& turn_servers, - int candidate_pool_size, - bool prune_turn_ports) { + int candidate_pool_size) { bool ice_servers_changed = (stun_servers != stun_servers_ || turn_servers != turn_servers_); stun_servers_ = stun_servers; turn_servers_ = turn_servers; - prune_turn_ports_ = prune_turn_ports; // If ICE servers changed, throw away any existing pooled sessions and create // new ones. diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h index 9b4465d3de..6a32b9668f 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h @@ -178,11 +178,6 @@ class PortAllocatorSession : public sigslot::has_slots<> { sigslot::signal2&> SignalCandidatesReady; 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 - // connections and its candidates will not be sent to the remote side - // if they have not been sent. - sigslot::signal2 SignalPortPruned; virtual uint32_t generation() { return generation_; } virtual void set_generation(uint32_t generation) { generation_ = generation; } @@ -258,8 +253,7 @@ class PortAllocator : public sigslot::has_slots<> { // pooled sessions will be either created or destroyed as necessary. void SetConfiguration(const ServerAddresses& stun_servers, const std::vector& turn_servers, - int candidate_pool_size, - bool prune_turn_ports); + int candidate_pool_size); const ServerAddresses& stun_servers() const { return stun_servers_; } @@ -333,8 +327,6 @@ class PortAllocator : public sigslot::has_slots<> { candidate_filter_ = filter; } - bool prune_turn_ports() const { return prune_turn_ports_; } - // Gets/Sets the Origin value used for WebRTC STUN requests. const std::string& origin() const { return origin_; } void set_origin(const std::string& origin) { origin_ = origin; } @@ -365,7 +357,6 @@ class PortAllocator : public sigslot::has_slots<> { // both owned by this class and taken by TakePooledSession. int allocated_pooled_session_count_ = 0; std::deque> pooled_sessions_; - bool prune_turn_ports_ = false; }; } // namespace cricket diff --git a/webrtc/p2p/base/portallocator_unittest.cc b/webrtc/p2p/base/portallocator_unittest.cc index 06abaad2a0..6a34a25c13 100644 --- a/webrtc/p2p/base/portallocator_unittest.cc +++ b/webrtc/p2p/base/portallocator_unittest.cc @@ -35,7 +35,7 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { void SetConfigurationWithPoolSize(int candidate_pool_size) { allocator_->SetConfiguration(cricket::ServerAddresses(), std::vector(), - candidate_pool_size, false); + candidate_pool_size); } std::unique_ptr CreateSession( @@ -107,14 +107,14 @@ TEST_F(PortAllocatorTest, CreateSession) { TEST_F(PortAllocatorTest, SetConfigurationUpdatesIceServers) { cricket::ServerAddresses stun_servers_1 = {stun_server_1}; std::vector turn_servers_1 = {turn_server_1}; - allocator_->SetConfiguration(stun_servers_1, turn_servers_1, 0, false); + allocator_->SetConfiguration(stun_servers_1, turn_servers_1, 0); EXPECT_EQ(stun_servers_1, allocator_->stun_servers()); EXPECT_EQ(turn_servers_1, allocator_->turn_servers()); // Update with a different set of servers. cricket::ServerAddresses stun_servers_2 = {stun_server_2}; std::vector turn_servers_2 = {turn_server_2}; - allocator_->SetConfiguration(stun_servers_2, turn_servers_2, 0, false); + allocator_->SetConfiguration(stun_servers_2, turn_servers_2, 0); EXPECT_EQ(stun_servers_2, allocator_->stun_servers()); EXPECT_EQ(turn_servers_2, allocator_->turn_servers()); } @@ -182,14 +182,14 @@ TEST_F(PortAllocatorTest, SetConfigurationRecreatesPooledSessionsWhenIceServersChange) { cricket::ServerAddresses stun_servers_1 = {stun_server_1}; std::vector turn_servers_1 = {turn_server_1}; - allocator_->SetConfiguration(stun_servers_1, turn_servers_1, 1, false); + allocator_->SetConfiguration(stun_servers_1, turn_servers_1, 1); EXPECT_EQ(stun_servers_1, allocator_->stun_servers()); EXPECT_EQ(turn_servers_1, allocator_->turn_servers()); // Update with a different set of servers (and also change pool size). cricket::ServerAddresses stun_servers_2 = {stun_server_2}; std::vector turn_servers_2 = {turn_server_2}; - allocator_->SetConfiguration(stun_servers_2, turn_servers_2, 2, false); + allocator_->SetConfiguration(stun_servers_2, turn_servers_2, 2); EXPECT_EQ(stun_servers_2, allocator_->stun_servers()); EXPECT_EQ(turn_servers_2, allocator_->turn_servers()); auto session_1 = TakePooledSession(); diff --git a/webrtc/p2p/base/portinterface.h b/webrtc/p2p/base/portinterface.h index 987c3fc1ee..e73861965e 100644 --- a/webrtc/p2p/base/portinterface.h +++ b/webrtc/p2p/base/portinterface.h @@ -76,8 +76,6 @@ class PortInterface { virtual int GetOption(rtc::Socket::Option opt, int* value) = 0; virtual int GetError() = 0; - virtual ProtocolType GetProtocol() const = 0; - virtual const std::vector& Candidates() const = 0; // Sends the given packet to the given address, provided that the address is diff --git a/webrtc/p2p/base/relayport.h b/webrtc/p2p/base/relayport.h index 8fa2235043..402736c34d 100644 --- a/webrtc/p2p/base/relayport.h +++ b/webrtc/p2p/base/relayport.h @@ -68,12 +68,6 @@ class RelayPort : public Port { const ProtocolAddress * ServerAddress(size_t index) const; bool IsReady() { return ready_; } - ProtocolType GetProtocol() const override { - // We shouldn't be using RelayPort, but we need to provide an - // implementation here. - return PROTO_UDP; - } - // Used for testing. sigslot::signal1 SignalConnectFailure; sigslot::signal1 SignalSoftTimeout; diff --git a/webrtc/p2p/base/stunport.h b/webrtc/p2p/base/stunport.h index 1bbe25f0e8..82be6fae2a 100644 --- a/webrtc/p2p/base/stunport.h +++ b/webrtc/p2p/base/stunport.h @@ -80,7 +80,8 @@ class UDPPort : public Port { const ServerAddresses& server_addresses() const { return server_addresses_; } - void set_server_addresses(const ServerAddresses& addresses) { + void + set_server_addresses(const ServerAddresses& addresses) { server_addresses_ = addresses; } @@ -104,8 +105,6 @@ class UDPPort : public Port { return protocol == UDP_PROTOCOL_NAME; } - virtual ProtocolType GetProtocol() const { return PROTO_UDP; } - void set_stun_keepalive_delay(int delay) { stun_keepalive_delay_ = delay; } diff --git a/webrtc/p2p/base/tcpport.h b/webrtc/p2p/base/tcpport.h index 76a73f8f47..77bbd09eab 100644 --- a/webrtc/p2p/base/tcpport.h +++ b/webrtc/p2p/base/tcpport.h @@ -61,8 +61,6 @@ class TCPPort : public Port { return protocol == TCP_PROTOCOL_NAME || protocol == SSLTCP_PROTOCOL_NAME; } - ProtocolType GetProtocol() const override { return PROTO_TCP; } - protected: TCPPort(rtc::Thread* thread, rtc::PacketSocketFactory* factory, diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h index 3bb09bc062..6e528aaf69 100644 --- a/webrtc/p2p/base/turnport.h +++ b/webrtc/p2p/base/turnport.h @@ -85,8 +85,6 @@ class TurnPort : public Port { } const RelayCredentials& credentials() const { return credentials_; } - virtual ProtocolType GetProtocol() const { return server_address_.proto; } - virtual void PrepareAddress(); virtual Connection* CreateConnection( const Candidate& c, PortInterface::CandidateOrigin origin); diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index 19f23b3f00..e39a44013c 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -47,36 +47,6 @@ const int PHASE_SSLTCP = 3; const int kNumPhases = 4; -// Gets address family priority: IPv6 > IPv4 > Unspecified. -int GetAddressFamilyPriority(int ip_family) { - switch (ip_family) { - case AF_INET6: - return 2; - case AF_INET: - return 1; - default: - RTC_DCHECK(false); - return 0; - } -} - -// Returns positive if a is better, negative if b is better, and 0 otherwise. -int ComparePort(const cricket::Port* a, const cricket::Port* b) { - static constexpr int a_is_better = 1; - static constexpr int b_is_better = -1; - // Protocol type is defined as UDP = 0, TCP = 1, SSLTCP = 2. - if (a->GetProtocol() < b->GetProtocol()) { - return a_is_better; - } - if (a->GetProtocol() > b->GetProtocol()) { - return b_is_better; - } - - int a_family = GetAddressFamilyPriority(a->Network()->GetBestIP().family()); - int b_family = GetAddressFamilyPriority(b->Network()->GetBestIP().family()); - return a_family - b_family; -} - } // namespace namespace cricket { @@ -104,7 +74,7 @@ BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager, const ServerAddresses& stun_servers) : network_manager_(network_manager), socket_factory_(socket_factory) { ASSERT(socket_factory_ != NULL); - SetConfiguration(stun_servers, std::vector(), 0, false); + SetConfiguration(stun_servers, std::vector(), 0); Construct(); } @@ -131,7 +101,7 @@ BasicPortAllocator::BasicPortAllocator( turn_servers.push_back(config); } - SetConfiguration(stun_servers, turn_servers, 0, false); + SetConfiguration(stun_servers, turn_servers, 0); Construct(); } @@ -152,30 +122,24 @@ PortAllocatorSession* BasicPortAllocator::CreateSessionInternal( void BasicPortAllocator::AddTurnServer(const RelayServerConfig& turn_server) { std::vector new_turn_servers = turn_servers(); new_turn_servers.push_back(turn_server); - SetConfiguration(stun_servers(), new_turn_servers, candidate_pool_size(), - prune_turn_ports()); + SetConfiguration(stun_servers(), new_turn_servers, candidate_pool_size()); } // BasicPortAllocatorSession BasicPortAllocatorSession::BasicPortAllocatorSession( - BasicPortAllocator* allocator, + BasicPortAllocator *allocator, const std::string& content_name, int component, const std::string& ice_ufrag, const std::string& ice_pwd) - : PortAllocatorSession(content_name, - component, - ice_ufrag, - ice_pwd, - allocator->flags()), - allocator_(allocator), - network_thread_(NULL), + : PortAllocatorSession(content_name, component, + ice_ufrag, ice_pwd, allocator->flags()), + allocator_(allocator), network_thread_(NULL), 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()) { + allocation_sequences_created_(false) { allocator_->network_manager()->SignalNetworksChanged.connect( this, &BasicPortAllocatorSession::OnNetworksChanged); allocator_->network_manager()->StartUpdating(); @@ -253,9 +217,9 @@ void BasicPortAllocatorSession::ClearGettingPorts() { std::vector BasicPortAllocatorSession::ReadyPorts() const { std::vector ret; - for (const PortData& data : ports_) { - if (data.ready()) { - ret.push_back(data.port()); + for (const PortData& port : ports_) { + if (port.has_pairable_candidate() && !port.error()) { + ret.push_back(port.port()); } } return ret; @@ -264,10 +228,6 @@ std::vector BasicPortAllocatorSession::ReadyPorts() const { std::vector BasicPortAllocatorSession::ReadyCandidates() const { std::vector candidates; for (const PortData& data : ports_) { - if (!data.ready()) { - continue; - } - for (const Candidate& candidate : data.port()->Candidates()) { if (!CheckCandidateFilter(candidate)) { continue; @@ -318,11 +278,16 @@ bool BasicPortAllocatorSession::CandidatesAllocationDone() const { return false; } - // If all allocated ports are no longer gathering, session must have got all + // If all allocated ports are in complete state, session must have got all // expected candidates. Session will trigger candidates allocation complete // signal. - return std::none_of(ports_.begin(), ports_.end(), - [](const PortData& port) { return port.inprogress(); }); + if (!std::all_of(ports_.begin(), ports_.end(), [](const PortData& port) { + return (port.complete() || port.error()); + })) { + return false; + } + + return true; } void BasicPortAllocatorSession::OnMessage(rtc::Message *message) { @@ -392,7 +357,7 @@ void BasicPortAllocatorSession::OnConfigStop() { bool send_signal = false; for (std::vector::iterator it = ports_.begin(); it != ports_.end(); ++it) { - if (it->inprogress()) { + if (!it->complete() && !it->error()) { // Updating port state to error, which didn't finish allocating candidates // yet. it->set_error(); @@ -478,8 +443,11 @@ void BasicPortAllocatorSession::DoAllocate() { LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated"; done_signal_needed = true; } else { - PortConfiguration* config = configs_.empty() ? nullptr : configs_.back(); for (uint32_t i = 0; i < networks.size(); ++i) { + PortConfiguration* config = NULL; + if (configs_.size() > 0) + config = configs_.back(); + uint32_t sequence_flags = flags(); if ((sequence_flags & DISABLE_ALL_PHASES) == DISABLE_ALL_PHASES) { // If all the ports are disabled we should just fire the allocation @@ -600,85 +568,37 @@ void BasicPortAllocatorSession::OnCandidateReady( PortData* data = FindPort(port); ASSERT(data != NULL); // Discarding any candidate signal if port allocation status is - // already done with gathering. - if (!data->inprogress()) { + // already in completed state. + if (data->complete() || data->error()) { return; } - // Mark that the port has a pairable candidate, either because we have a - // usable candidate from the port, or simply because the port is bound to the - // any address and therefore has no host candidate. This will trigger the port - // to start creating candidate pairs (connections) and issue connectivity - // checks. If port has already been marked as having a pairable candidate, - // do nothing here. - // Note: We should check whether any candidates may become ready after this - // because there we will check whether the candidate is generated by the ready - // ports, which may include this port. - bool pruned_port = false; - if (CandidatePairable(c, port) && !data->has_pairable_candidate()) { - data->set_has_pairable_candidate(true); - - if (prune_turn_ports_ && port->Type() == RELAY_PORT_TYPE) { - pruned_port = PruneTurnPorts(port); - } - // If the current port is not pruned yet, SignalPortReady. - if (!data->pruned()) { - SignalPortReady(this, port); - } - } - ProtocolType pvalue; bool candidate_protocol_enabled = StringToProto(c.protocol().c_str(), &pvalue) && data->sequence()->ProtocolEnabled(pvalue); - if (data->ready() && CheckCandidateFilter(c) && candidate_protocol_enabled) { + if (CheckCandidateFilter(c) && candidate_protocol_enabled) { std::vector candidates; candidates.push_back(SanitizeRelatedAddress(c)); SignalCandidatesReady(this, candidates); } - // If we have pruned any port, maybe need to signal port allocation done. - if (pruned_port) { - MaybeSignalCandidatesAllocationDone(); + // Port has already been marked as having a pairable candidate. + // Nothing to do here. + if (data->has_pairable_candidate()) { + return; } -} -Port* BasicPortAllocatorSession::GetBestTurnPortForNetwork( - const std::string& network_name) const { - Port* best_turn_port = nullptr; - for (const PortData& data : ports_) { - if (data.port()->Network()->name() == network_name && - data.port()->Type() == RELAY_PORT_TYPE && data.ready() && - (!best_turn_port || ComparePort(data.port(), best_turn_port) > 0)) { - best_turn_port = data.port(); - } + // Mark that the port has a pairable candidate, either because we have a + // usable candidate from the port, or simply because the port is bound to the + // any address and therefore has no host candidate. This will trigger the port + // to start creating candidate pairs (connections) and issue connectivity + // checks. + if (CandidatePairable(c, port)) { + data->set_has_pairable_candidate(true); + SignalPortReady(this, port); } - return best_turn_port; -} - -bool BasicPortAllocatorSession::PruneTurnPorts(Port* newly_pairable_turn_port) { - bool pruned_port = false; - // Note: We determine the same network based only on their network names. So - // if an IPv4 address and an IPv6 address have the same network name, they - // are considered the same network here. - const std::string& network_name = newly_pairable_turn_port->Network()->name(); - Port* best_turn_port = GetBestTurnPortForNetwork(network_name); - // |port| is already in the list of ports, so the best port cannot be nullptr. - RTC_CHECK(best_turn_port != nullptr); - - for (PortData& data : ports_) { - if (data.port()->Network()->name() == network_name && - data.port()->Type() == RELAY_PORT_TYPE && !data.pruned() && - ComparePort(data.port(), best_turn_port) < 0) { - data.set_pruned(); - pruned_port = true; - if (data.port() != newly_pairable_turn_port) { - SignalPortPruned(this, data.port()); - } - } - } - return pruned_port; } void BasicPortAllocatorSession::OnPortComplete(Port* port) { @@ -687,7 +607,7 @@ void BasicPortAllocatorSession::OnPortComplete(Port* port) { ASSERT(data != NULL); // Ignore any late signals. - if (!data->inprogress()) { + if (data->complete() || data->error()) { return; } @@ -702,7 +622,7 @@ void BasicPortAllocatorSession::OnPortError(Port* port) { PortData* data = FindPort(port); ASSERT(data != NULL); // We might have already given up on this port and stopped it. - if (!data->inprogress()) { + if (data->complete() || data->error()) { return; } @@ -1108,11 +1028,13 @@ void AllocationSequence::CreateRelayPorts() { return; } - for (RelayServerConfig& relay : config_->relays) { - if (relay.type == RELAY_GTURN) { - CreateGturnPort(relay); - } else if (relay.type == RELAY_TURN) { - CreateTurnPort(relay); + PortConfiguration::RelayList::const_iterator relay; + for (relay = config_->relays.begin(); + relay != config_->relays.end(); ++relay) { + if (relay->type == RELAY_GTURN) { + CreateGturnPort(*relay); + } else if (relay->type == RELAY_TURN) { + CreateTurnPort(*relay); } else { ASSERT(false); } diff --git a/webrtc/p2p/client/basicportallocator.h b/webrtc/p2p/client/basicportallocator.h index bddb3967e5..8ea66c431e 100644 --- a/webrtc/p2p/client/basicportallocator.h +++ b/webrtc/p2p/client/basicportallocator.h @@ -123,15 +123,7 @@ class BasicPortAllocatorSession : public PortAllocatorSession, bool has_pairable_candidate() const { return has_pairable_candidate_; } bool complete() const { return state_ == STATE_COMPLETE; } bool error() const { return state_ == STATE_ERROR; } - bool pruned() const { return state_ == STATE_PRUNED; } - bool inprogress() const { return state_ == STATE_INPROGRESS; } - // Returns true if this port is ready to be used. - bool ready() const { - return has_pairable_candidate_ && state_ != STATE_ERROR && - state_ != STATE_PRUNED; - } - void set_pruned() { state_ = STATE_PRUNED; } void set_has_pairable_candidate(bool has_pairable_candidate) { if (has_pairable_candidate) { ASSERT(state_ == STATE_INPROGRESS); @@ -150,9 +142,7 @@ class BasicPortAllocatorSession : public PortAllocatorSession, enum State { STATE_INPROGRESS, // Still gathering candidates. STATE_COMPLETE, // All candidates allocated and ready for process. - STATE_ERROR, // Error in gathering candidates. - STATE_PRUNED // Pruned by higher priority ports on the same network - // interface. Only TURN ports may be pruned. + STATE_ERROR // Error in gathering candidates. }; Port* port_ = nullptr; AllocationSequence* sequence_ = nullptr; @@ -188,10 +178,6 @@ class BasicPortAllocatorSession : public PortAllocatorSession, // in order to avoid leaking any information. Candidate SanitizeRelatedAddress(const Candidate& c) 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); - BasicPortAllocator* allocator_; rtc::Thread* network_thread_; std::unique_ptr owned_socket_factory_; @@ -204,8 +190,6 @@ class BasicPortAllocatorSession : public PortAllocatorSession, std::vector sequences_; std::vector ports_; uint32_t candidate_filter_ = CF_ALL; - // Whether to prune low-priority ports, taken from the port allocator. - bool prune_turn_ports_; friend class AllocationSequence; }; diff --git a/webrtc/p2p/client/basicportallocator_unittest.cc b/webrtc/p2p/client/basicportallocator_unittest.cc index 28a522fcd2..6d5d1086a9 100644 --- a/webrtc/p2p/client/basicportallocator_unittest.cc +++ b/webrtc/p2p/client/basicportallocator_unittest.cc @@ -39,15 +39,12 @@ using rtc::SocketAddress; using rtc::Thread; static const SocketAddress kClientAddr("11.11.11.11", 0); -static const SocketAddress kClientAddr2("22.22.22.22", 0); static const SocketAddress kLoopbackAddr("127.0.0.1", 0); static const SocketAddress kPrivateAddr("192.168.1.11", 0); static const SocketAddress kPrivateAddr2("192.168.1.12", 0); static const SocketAddress kClientIPv6Addr("2401:fa00:4:1000:be30:5bff:fee5:c3", 0); -static const SocketAddress kClientIPv6Addr2( - "2401:fa00:4:2000:be30:5bff:fee5:c3", - 0); +static const SocketAddress kClientAddr2("22.22.22.22", 0); static const SocketAddress kNatUdpAddr("77.77.77.77", rtc::NAT_SERVER_UDP_PORT); static const SocketAddress kNatTcpAddr("77.77.77.77", rtc::NAT_SERVER_TCP_PORT); static const SocketAddress kRemoteClientAddr("22.22.22.22", 0); @@ -59,13 +56,7 @@ static const SocketAddress kRelayTcpExtAddr("99.99.99.3", 5003); static const SocketAddress kRelaySslTcpIntAddr("99.99.99.2", 5004); static const SocketAddress kRelaySslTcpExtAddr("99.99.99.3", 5005); static const SocketAddress kTurnUdpIntAddr("99.99.99.4", 3478); -static const SocketAddress kTurnUdpIntIPv6Addr( - "2402:fb00:4:1000:be30:5bff:fee5:c3", - 3479); static const SocketAddress kTurnTcpIntAddr("99.99.99.5", 3478); -static const SocketAddress kTurnTcpIntIPv6Addr( - "2402:fb00:4:2000:be30:5bff:fee5:c3", - 3479); static const SocketAddress kTurnUdpExtAddr("99.99.99.6", 0); // Minimum and maximum port for port range tests. @@ -193,10 +184,12 @@ class BasicPortAllocatorTest : public testing::Test, turn_server.credentials = credentials; if (!udp_turn.IsNil()) { - turn_server.ports.push_back(ProtocolAddress(udp_turn, PROTO_UDP, false)); + turn_server.ports.push_back( + ProtocolAddress(kTurnUdpIntAddr, PROTO_UDP, false)); } if (!tcp_turn.IsNil()) { - turn_server.ports.push_back(ProtocolAddress(tcp_turn, PROTO_TCP, false)); + turn_server.ports.push_back( + ProtocolAddress(kTurnTcpIntAddr, PROTO_TCP, false)); } allocator_->AddTurnServer(turn_server); } @@ -239,8 +232,6 @@ class BasicPortAllocatorTest : public testing::Test, sid, content_name, component, ice_ufrag, ice_pwd); session->SignalPortReady.connect(this, &BasicPortAllocatorTest::OnPortReady); - session->SignalPortPruned.connect(this, - &BasicPortAllocatorTest::OnPortPruned); session->SignalCandidatesReady.connect( this, &BasicPortAllocatorTest::OnCandidatesReady); session->SignalCandidatesAllocationDone.connect( @@ -260,20 +251,6 @@ class BasicPortAllocatorTest : public testing::Test, (pattern.port() != 0 && address.port() == pattern.port())); } - // Returns the number of ports that have matching type, protocol and - // address. - static int CountPorts(const std::vector& ports, - const std::string& type, - ProtocolType protocol, - const SocketAddress& client_addr) { - return std::count_if( - ports.begin(), ports.end(), - [type, protocol, client_addr](PortInterface* port) { - return port->Type() == type && port->GetProtocol() == protocol && - port->Network()->GetBestIP() == client_addr.ipaddr(); - }); - } - // Find a candidate and return it. static bool FindCandidate(const std::vector& candidates, const std::string& type, @@ -415,15 +392,6 @@ class BasicPortAllocatorTest : public testing::Test, EXPECT_NE(ready_ports.end(), std::find(ready_ports.begin(), ready_ports.end(), port)); } - void OnPortPruned(PortAllocatorSession* ses, PortInterface* port) { - LOG(LS_INFO) << "OnPortPruned: " << port->ToString(); - ports_.erase(std::remove(ports_.begin(), ports_.end(), port), ports_.end()); - // Make sure the pruned port is not in ReadyPorts. - auto ready_ports = ses->ReadyPorts(); - EXPECT_EQ(ready_ports.end(), - std::find(ready_ports.begin(), ready_ports.end(), port)); - } - void OnCandidatesReady(PortAllocatorSession* ses, const std::vector& candidates) { for (const Candidate& candidate : candidates) { @@ -1206,145 +1174,6 @@ TEST_F(BasicPortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) { EXPECT_EQ(3U, candidates_.size()); } -// Test that if prune_turn_ports is set, TCP TurnPort will not -// be used if UDP TurnPort is used. -TEST_F(BasicPortAllocatorTest, TestUdpTurnPortDisablesTcpTurnPorts) { - turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); - AddInterface(kClientAddr); - allocator_.reset(new BasicPortAllocator(&network_manager_)); - allocator_->SetConfiguration(allocator_->stun_servers(), - allocator_->turn_servers(), 0, true); - AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); - allocator_->set_step_delay(kMinimumStepDelay); - allocator_->set_flags(allocator().flags() | - PORTALLOCATOR_ENABLE_SHARED_SOCKET | - PORTALLOCATOR_DISABLE_TCP); - - EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP)); - session_->StartGettingPorts(); - EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); - // Only 2 ports (one STUN and one TURN) are actually being used. - EXPECT_EQ(2U, session_->ReadyPorts().size()); - // We have verified that each port, when it is added to |ports_|, it is found - // in |ready_ports|, and when it is pruned, it is not found in |ready_ports|, - // so we only need to verify the content in one of them. - EXPECT_EQ(2U, ports_.size()); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr)); - EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientAddr)); - EXPECT_EQ(0, CountPorts(ports_, "relay", PROTO_TCP, kClientAddr)); - - // We don't remove candidates, so the size of |candidates_| will depend on - // when the TCP TURN port becomes ready. If it is ready after the UDP TURN - // port becomes ready, its candidates will be used there will be 3 candidates. - // Otherwise there will be only 2 candidates. - EXPECT_LE(2U, candidates_.size()); - // There will only be 2 candidates in |ready_candidates| because it only - // includes the candidates in the ready ports. - const std::vector& ready_candidates = session_->ReadyCandidates(); - EXPECT_EQ(2U, ready_candidates.size()); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr); - EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp", - rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0)); -} - -// Tests that if prune_turn_ports is set, IPv4 TurnPort will not -// be used if IPv6 TurnPort is used. -TEST_F(BasicPortAllocatorTest, TestIPv6TurnPortDisablesIPv4TurnPorts) { - turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP); - // Add two IP addresses on the same interface. - AddInterface(kClientAddr, "net1"); - AddInterface(kClientIPv6Addr, "net1"); - allocator_.reset(new BasicPortAllocator(&network_manager_)); - allocator_->SetConfiguration(allocator_->stun_servers(), - allocator_->turn_servers(), 0, true); - AddTurnServers(kTurnUdpIntIPv6Addr, rtc::SocketAddress()); - - allocator_->set_step_delay(kMinimumStepDelay); - allocator_->set_flags(allocator().flags() | - PORTALLOCATOR_ENABLE_SHARED_SOCKET | - PORTALLOCATOR_ENABLE_IPV6 | PORTALLOCATOR_DISABLE_TCP); - - EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP)); - session_->StartGettingPorts(); - EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); - rtc::Thread::Current()->ProcessMessages(1000); - // Three ports (one IPv4 STUN, one IPv6 STUN and one TURN) will be ready. - EXPECT_EQ(3U, session_->ReadyPorts().size()); - EXPECT_EQ(3U, ports_.size()); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr)); - EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr)); - EXPECT_EQ(0, CountPorts(ports_, "relay", PROTO_UDP, kClientAddr)); - - // We don't remove candidates, so there may be more than 3 elemenets in - // |candidates_|, although |ready_candidates| only includes the candidates - // in |ready_ports|. - EXPECT_LE(3U, candidates_.size()); - const std::vector& ready_candidates = session_->ReadyCandidates(); - EXPECT_EQ(3U, ready_candidates.size()); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr); - EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp", - rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0)); -} - -// Tests that if prune_turn_ports is set, each network interface -// will has its own set of TurnPorts based on their priorities. -TEST_F(BasicPortAllocatorTest, TestEachInterfaceHasItsOwnTurnPorts) { - turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); - turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP); - turn_server_.AddInternalSocket(kTurnTcpIntIPv6Addr, PROTO_TCP); - // Add two interfaces both having IPv4 and IPv6 addresses. - AddInterface(kClientAddr, "net1"); - AddInterface(kClientIPv6Addr, "net1"); - AddInterface(kClientAddr2, "net2"); - AddInterface(kClientIPv6Addr2, "net2"); - allocator_.reset(new BasicPortAllocator(&network_manager_)); - allocator_->SetConfiguration(allocator_->stun_servers(), - allocator_->turn_servers(), 0, true); - // Have both UDP/TCP and IPv4/IPv6 TURN ports. - AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); - AddTurnServers(kTurnUdpIntIPv6Addr, kTurnTcpIntIPv6Addr); - - allocator_->set_step_delay(kMinimumStepDelay); - allocator_->set_flags(allocator().flags() | - PORTALLOCATOR_ENABLE_SHARED_SOCKET | - PORTALLOCATOR_ENABLE_IPV6); - EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP)); - session_->StartGettingPorts(); - EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); - // 10 ports (4 STUN and 1 TURN ports on each interface) will be ready to use. - EXPECT_EQ(10U, session_->ReadyPorts().size()); - EXPECT_EQ(10U, ports_.size()); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr2)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr2)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientAddr)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientAddr2)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientIPv6Addr)); - EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientIPv6Addr2)); - EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr)); - EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr2)); - - // We don't remove candidates, so there may be more than 10 candidates - // in |candidates_|. - EXPECT_LE(10U, candidates_.size()); - const std::vector& ready_candidates = session_->ReadyCandidates(); - EXPECT_EQ(10U, ready_candidates.size()); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr2); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientIPv6Addr); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", - kClientIPv6Addr2); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientAddr); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientAddr2); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientIPv6Addr); - EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", - kClientIPv6Addr2); - EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp", - rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0)); -} - // Testing DNS resolve for the TURN server, this will test AllocationSequence // handling the unresolved address signal from TurnPort. TEST_F(BasicPortAllocatorTest, TestSharedSocketWithServerAddressResolve) { @@ -1640,7 +1469,7 @@ TEST_F(BasicPortAllocatorTest, TestTransportInformationUpdated) { AddInterface(kClientAddr); int pool_size = 1; allocator_->SetConfiguration(allocator_->stun_servers(), - allocator_->turn_servers(), pool_size, false); + allocator_->turn_servers(), pool_size); const PortAllocatorSession* peeked_session = allocator_->GetPooledSession(); ASSERT_NE(nullptr, peeked_session); EXPECT_EQ_WAIT(true, peeked_session->CandidatesAllocationDone(), @@ -1676,7 +1505,7 @@ TEST_F(BasicPortAllocatorTest, TestSetCandidateFilterAfterCandidatesGathered) { AddInterface(kClientAddr); int pool_size = 1; allocator_->SetConfiguration(allocator_->stun_servers(), - allocator_->turn_servers(), pool_size, false); + allocator_->turn_servers(), pool_size); const PortAllocatorSession* peeked_session = allocator_->GetPooledSession(); ASSERT_NE(nullptr, peeked_session); EXPECT_EQ_WAIT(true, peeked_session->CandidatesAllocationDone(),