diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index c1df1d7520..d3fad7e9aa 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -2737,8 +2737,8 @@ TEST_P(P2PTransportChannelMultihomedTest, TestFailoverWithManyConnections) { RelayServerConfig turn_server; turn_server.credentials = kRelayCredentials; turn_server.ports.push_back(ProtocolAddress(kTurnTcpIntAddr, PROTO_TCP)); - GetAllocator(0)->AddTurnServerForTesting(turn_server); - GetAllocator(1)->AddTurnServerForTesting(turn_server); + GetAllocator(0)->AddTurnServer(turn_server); + GetAllocator(1)->AddTurnServer(turn_server); // Enable IPv6 SetAllocatorFlags( 0, PORTALLOCATOR_ENABLE_IPV6 | PORTALLOCATOR_ENABLE_IPV6_ON_WIFI); @@ -5238,7 +5238,7 @@ TEST_P(P2PTransportChannelMostLikelyToWorkFirstTest, TestTcpTurn) { RelayServerConfig config; config.credentials = kRelayCredentials; config.ports.push_back(ProtocolAddress(kTurnTcpIntAddr, PROTO_TCP)); - allocator()->AddTurnServerForTesting(config); + allocator()->AddTurnServer(config); P2PTransportChannel& ch = StartTransportChannel(true, 500, &field_trials_); EXPECT_TRUE_WAIT(ch.ports().size() == 3, kDefaultTimeout); diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index 3ca63cbd41..46358439ac 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -171,12 +171,14 @@ struct RTC_EXPORT RelayServerConfig { ~RelayServerConfig(); bool operator==(const RelayServerConfig& o) const { - return ports == o.ports && credentials == o.credentials; + return ports == o.ports && credentials == o.credentials && + priority == o.priority; } bool operator!=(const RelayServerConfig& o) const { return !(*this == o); } PortList ports; RelayCredentials credentials; + int priority = 0; TlsCertPolicy tls_cert_policy = TlsCertPolicy::TLS_CERT_POLICY_SECURE; std::vector tls_alpn_protocols; std::vector tls_elliptic_curves; diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 0672931d1c..d0b2079465 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -81,7 +81,7 @@ class TurnPort : public Port { return absl::WrapUnique( new TurnPort(args.network_thread, args.socket_factory, args.network, socket, args.username, args.password, *args.server_address, - args.config->credentials, args.relative_priority, + args.config->credentials, args.config->priority, args.config->tls_alpn_protocols, args.config->tls_elliptic_curves, args.turn_customizer, args.config->tls_cert_verifier, args.field_trials)); @@ -100,7 +100,7 @@ class TurnPort : public Port { new TurnPort(args.network_thread, args.socket_factory, args.network, min_port, max_port, args.username, args.password, *args.server_address, args.config->credentials, - args.relative_priority, args.config->tls_alpn_protocols, + args.config->priority, args.config->tls_alpn_protocols, args.config->tls_elliptic_curves, args.turn_customizer, args.config->tls_cert_verifier, args.field_trials)); } diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index e36f266f15..5611403bb2 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -298,8 +298,7 @@ PortAllocatorSession* BasicPortAllocator::CreateSessionInternal( return session; } -void BasicPortAllocator::AddTurnServerForTesting( - const RelayServerConfig& turn_server) { +void BasicPortAllocator::AddTurnServer(const RelayServerConfig& turn_server) { CheckRunOnValidThreadAndInitialized(); std::vector new_turn_servers = turn_servers(); new_turn_servers.push_back(turn_server); @@ -1648,17 +1647,12 @@ void AllocationSequence::CreateRelayPorts() { return; } - // Relative priority of candidates from this TURN server in relation - // to the candidates from other servers. Required because ICE priorities - // need to be unique. - int relative_priority = config_->relays.size(); for (RelayServerConfig& relay : config_->relays) { - CreateTurnPort(relay, relative_priority--); + CreateTurnPort(relay); } } -void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, - int relative_priority) { +void AllocationSequence::CreateTurnPort(const RelayServerConfig& config) { PortList::const_iterator relay_port; for (relay_port = config.ports.begin(); relay_port != config.ports.end(); ++relay_port) { @@ -1691,7 +1685,6 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, args.config = &config; args.turn_customizer = session_->allocator()->turn_customizer(); args.field_trials = session_->allocator()->field_trials(); - args.relative_priority = relative_priority; std::unique_ptr port; // Shared socket mode must be enabled only for UDP based ports. Hence diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h index 38c3835ee8..173d789545 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -79,7 +79,7 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator { absl::string_view ice_pwd) override; // Convenience method that adds a TURN server to the configuration. - void AddTurnServerForTesting(const RelayServerConfig& turn_server); + void AddTurnServer(const RelayServerConfig& turn_server); RelayPortFactoryInterface* relay_port_factory() { CheckRunOnValidThreadIfInitialized(); @@ -386,9 +386,11 @@ class AllocationSequence : public sigslot::has_slots<> { void Start(); void Stop(); - private: - void CreateTurnPort(const RelayServerConfig& config, int relative_priority); + protected: + // For testing. + void CreateTurnPort(const RelayServerConfig& config); + private: typedef std::vector ProtocolList; void Process(int epoch); diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc index d1a91afd63..0e32bc7a32 100644 --- a/p2p/client/basic_port_allocator_unittest.cc +++ b/p2p/client/basic_port_allocator_unittest.cc @@ -251,7 +251,7 @@ class BasicPortAllocatorTestBase : public ::testing::Test, void AddTurnServers(const rtc::SocketAddress& udp_turn, const rtc::SocketAddress& tcp_turn) { RelayServerConfig turn_server = CreateTurnServers(udp_turn, tcp_turn); - allocator_->AddTurnServerForTesting(turn_server); + allocator_->AddTurnServer(turn_server); } bool CreateSession(int component) { @@ -1818,7 +1818,7 @@ TEST_F(BasicPortAllocatorTestWithRealClock, turn_server.credentials = credentials; turn_server.ports.push_back( ProtocolAddress(rtc::SocketAddress("localhost", 3478), PROTO_UDP)); - allocator_->AddTurnServerForTesting(turn_server); + allocator_->AddTurnServer(turn_server); allocator_->set_step_delay(kMinimumStepDelay); allocator_->set_flags(allocator().flags() | @@ -2496,29 +2496,6 @@ TEST_F(BasicPortAllocatorTest, TestDoNotUseTurnServerAsStunSever) { EXPECT_EQ(1U, port_config.StunServers().size()); } -// Test that candidates from different servers get assigned a unique local -// preference (the middle 16 bits of the priority) -TEST_F(BasicPortAllocatorTest, AssignsUniqueLocalPreferencetoRelayCandidates) { - allocator_->SetCandidateFilter(CF_RELAY); - allocator_->AddTurnServerForTesting( - CreateTurnServers(kTurnUdpIntAddr, SocketAddress())); - allocator_->AddTurnServerForTesting( - CreateTurnServers(kTurnUdpIntAddr, SocketAddress())); - allocator_->AddTurnServerForTesting( - CreateTurnServers(kTurnUdpIntAddr, SocketAddress())); - - AddInterface(kClientAddr); - ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP)); - session_->StartGettingPorts(); - ASSERT_TRUE_SIMULATED_WAIT(candidate_allocation_done_, - kDefaultAllocationTimeout, fake_clock); - EXPECT_EQ(3u, candidates_.size()); - EXPECT_GT((candidates_[0].priority() >> 8) & 0xFFFF, - (candidates_[1].priority() >> 8) & 0xFFFF); - EXPECT_GT((candidates_[1].priority() >> 8) & 0xFFFF, - (candidates_[2].priority() >> 8) & 0xFFFF); -} - // Test that no more than allocator.max_ipv6_networks() IPv6 networks are used // to gather candidates. TEST_F(BasicPortAllocatorTest, TwoIPv6AreSelectedBecauseOfMaxIpv6Limit) { diff --git a/p2p/client/relay_port_factory_interface.h b/p2p/client/relay_port_factory_interface.h index edfca3697b..4eec5dbf28 100644 --- a/p2p/client/relay_port_factory_interface.h +++ b/p2p/client/relay_port_factory_interface.h @@ -45,10 +45,6 @@ struct CreateRelayPortArgs { std::string password; webrtc::TurnCustomizer* turn_customizer = nullptr; const webrtc::FieldTrialsView* field_trials = nullptr; - // Relative priority of candidates from this TURN server in relation - // to the candidates from other servers. Required because ICE priorities - // need to be unique. - int relative_priority = 0; }; // A factory for creating RelayPort's. diff --git a/pc/ice_server_parsing.cc b/pc/ice_server_parsing.cc index 9322fd12d4..1a30d2def5 100644 --- a/pc/ice_server_parsing.cc +++ b/pc/ice_server_parsing.cc @@ -338,6 +338,13 @@ RTCError ParseIceServersOrError( "ICE server parsing failed: Empty uri."); } } + // Candidates must have unique priorities, so that connectivity checks + // are performed in a well-defined order. + int priority = static_cast(turn_servers->size() - 1); + for (cricket::RelayServerConfig& turn_server : *turn_servers) { + // First in the list gets highest priority. + turn_server.priority = priority--; + } return RTCError::OK(); } diff --git a/pc/ice_server_parsing_unittest.cc b/pc/ice_server_parsing_unittest.cc index 4cb7c47b0b..408c790346 100644 --- a/pc/ice_server_parsing_unittest.cc +++ b/pc/ice_server_parsing_unittest.cc @@ -237,4 +237,22 @@ TEST_F(IceServerParsingTest, ParseMultipleUrls) { EXPECT_EQ(1U, turn_servers_.size()); } +// Ensure that TURN servers are given unique priorities, +// so that their resulting candidates have unique priorities. +TEST_F(IceServerParsingTest, TurnServerPrioritiesUnique) { + PeerConnectionInterface::IceServers servers; + PeerConnectionInterface::IceServer server; + server.urls.push_back("turn:hostname"); + server.urls.push_back("turn:hostname2"); + server.username = "foo"; + server.password = "bar"; + servers.push_back(server); + + EXPECT_TRUE( + webrtc::ParseIceServersOrError(servers, &stun_servers_, &turn_servers_) + .ok()); + EXPECT_EQ(2U, turn_servers_.size()); + EXPECT_NE(turn_servers_[0].priority, turn_servers_[1].priority); +} + } // namespace webrtc