diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index 09f0e34e51..d45d1e4998 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h @@ -107,16 +107,15 @@ class FakePortAllocatorSession : public PortAllocatorSession { (rtc::HasIPv6Enabled() && (flags() & PORTALLOCATOR_ENABLE_IPV6)) ? ipv6_network_ : ipv4_network_; - port_.reset( - TestUDPPort::Create({.network_thread = network_thread_, - .socket_factory = factory_, - .network = &network, - .ice_username_fragment = username(), - .ice_password = password(), - .field_trials = field_trials_, - .ice_tiebreaker = allocator_->ice_tiebreaker()}, - 0, 0, false)); + port_.reset(TestUDPPort::Create({.network_thread = network_thread_, + .socket_factory = factory_, + .network = &network, + .ice_username_fragment = username(), + .ice_password = password(), + .field_trials = field_trials_}, + 0, 0, false)); RTC_DCHECK(port_); + port_->SetIceTiebreaker(allocator_->ice_tiebreaker()); port_->SubscribePortDestroyed( [this](PortInterface* port) { OnPortDestroyed(port); }); AddPort(port_.get()); diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 1c44a2f0eb..dcea5b0e47 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -914,6 +914,7 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession* session, // if one is pending. port->SetIceRole(ice_role_); + port->SetIceTiebreaker(allocator_->ice_tiebreaker()); ports_.push_back(port); port->SignalUnknownAddress.connect(this, &P2PTransportChannel::OnUnknownAddress); diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 45b43b3ac6..d9bacf3a5a 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -118,7 +118,7 @@ Port::Port(const PortParametersRef& args, timeout_delay_(kPortTimeoutDelay), enable_port_packets_(false), ice_role_(ICEROLE_UNKNOWN), - ice_tiebreaker_(args.ice_tiebreaker), + tiebreaker_(0), shared_socket_(shared_socket), network_cost_(args.network->GetCost(*field_trials_)), weak_factory_(this) { @@ -160,8 +160,12 @@ void Port::SetIceRole(IceRole role) { ice_role_ = role; } +void Port::SetIceTiebreaker(uint64_t tiebreaker) { + tiebreaker_ = tiebreaker; +} + uint64_t Port::IceTiebreaker() const { - return ice_tiebreaker_; + return tiebreaker_; } bool Port::SharedSocket() const { @@ -220,7 +224,8 @@ void Port::AddAddress(const rtc::SocketAddress& address, type, generation_, "", network_->id(), network_cost_); // Set the relay protocol before computing the foundation field. c.set_relay_protocol(relay_protocol); - c.ComputeFoundation(base_address, ice_tiebreaker_); + // TODO(bugs.webrtc.org/14605): ensure IceTiebreaker() is set. + c.ComputeFoundation(base_address, tiebreaker_); c.set_priority( c.GetPriority(type_preference, network_->preference(), relay_preference, @@ -645,7 +650,7 @@ bool Port::MaybeIceRoleConflict(const rtc::SocketAddress& addr, switch (ice_role_) { case ICEROLE_CONTROLLING: if (ICEROLE_CONTROLLING == remote_ice_role) { - if (remote_tiebreaker >= ice_tiebreaker_) { + if (remote_tiebreaker >= tiebreaker_) { SignalRoleConflict(this); } else { // Send Role Conflict (487) error response. @@ -657,7 +662,7 @@ bool Port::MaybeIceRoleConflict(const rtc::SocketAddress& addr, break; case ICEROLE_CONTROLLED: if (ICEROLE_CONTROLLED == remote_ice_role) { - if (remote_tiebreaker < ice_tiebreaker_) { + if (remote_tiebreaker < tiebreaker_) { SignalRoleConflict(this); } else { // Send Role Conflict (487) error response. diff --git a/p2p/base/port.h b/p2p/base/port.h index b9eb343dba..2df1d35b8e 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -180,7 +180,6 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { absl::string_view ice_username_fragment; absl::string_view ice_password; const webrtc::FieldTrialsView* field_trials; - uint64_t ice_tiebreaker; }; protected: @@ -207,6 +206,7 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { IceRole GetIceRole() const override; void SetIceRole(IceRole role) override; + void SetIceTiebreaker(uint64_t tiebreaker) override; uint64_t IceTiebreaker() const override; bool SharedSocket() const override; @@ -490,7 +490,7 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { int timeout_delay_; bool enable_port_packets_; IceRole ice_role_; - const uint64_t ice_tiebreaker_; + uint64_t tiebreaker_; bool shared_socket_; // A virtual cost perceived by the user, usually based on the network type diff --git a/p2p/base/port_interface.h b/p2p/base/port_interface.h index 6847727b10..34f835d138 100644 --- a/p2p/base/port_interface.h +++ b/p2p/base/port_interface.h @@ -60,6 +60,7 @@ class PortInterface { virtual void SetIceRole(IceRole role) = 0; virtual IceRole GetIceRole() const = 0; + virtual void SetIceTiebreaker(uint64_t tiebreaker) = 0; virtual uint64_t IceTiebreaker() const = 0; virtual bool SharedSocket() const = 0; diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index 35c79542df..be3b365427 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -105,7 +105,10 @@ const RelayCredentials kRelayCredentials("test", "test"); const uint32_t kDefaultPrflxPriority = ICE_TYPE_PREFERENCE_PRFLX << 24 | 30 << 8 | (256 - ICE_CANDIDATE_COMPONENT_DEFAULT); -constexpr uint64_t kTiebreakerDefault = 44444; + +constexpr int kTiebreaker1 = 11111; +constexpr int kTiebreaker2 = 22222; +constexpr int kTiebreakerDefault = 44444; const char* data = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; @@ -532,57 +535,61 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { } std::unique_ptr CreateUdpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - return UDPPort::Create({.network_thread = &main_, - .socket_factory = socket_factory, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault}, - 0, 0, true, absl::nullopt); + auto port = UDPPort::Create({.network_thread = &main_, + .socket_factory = socket_factory, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_}, + 0, 0, true, absl::nullopt); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } std::unique_ptr CreateUdpPortMultipleAddrs( const SocketAddress& global_addr, const SocketAddress& link_local_addr, PacketSocketFactory* socket_factory) { - return UDPPort::Create( + auto port = UDPPort::Create( {.network_thread = &main_, .socket_factory = socket_factory, .network = MakeNetworkMultipleAddrs(global_addr, link_local_addr), .ice_username_fragment = username_, .ice_password = password_, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault}, + .field_trials = &field_trials_}, 0, 0, true, absl::nullopt); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } std::unique_ptr CreateTcpPort(const SocketAddress& addr) { return CreateTcpPort(addr, &socket_factory_); } std::unique_ptr CreateTcpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - return TCPPort::Create({.network_thread = &main_, - .socket_factory = socket_factory, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault}, - 0, 0, true); + auto port = TCPPort::Create({.network_thread = &main_, + .socket_factory = socket_factory, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_}, + 0, 0, true); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } std::unique_ptr CreateStunPort( const SocketAddress& addr, rtc::PacketSocketFactory* socket_factory) { ServerAddresses stun_servers; stun_servers.insert(kStunAddr); - return StunPort::Create({.network_thread = &main_, - .socket_factory = socket_factory, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault}, - 0, 0, stun_servers, absl::nullopt); + auto port = StunPort::Create({.network_thread = &main_, + .socket_factory = socket_factory, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_}, + 0, 0, stun_servers, absl::nullopt); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } std::unique_ptr CreateRelayPort(const SocketAddress& addr, ProtocolType int_proto, @@ -616,9 +623,10 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { args.server_address = &server_address; args.config = &config; args.field_trials = &field_trials_; - args.ice_tiebreaker = kTiebreakerDefault; - return TurnPort::Create(args, 0, 0); + auto port = TurnPort::Create(args, 0, 0); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } std::unique_ptr CreateNatServer(const SocketAddress& addr, @@ -824,8 +832,7 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { .network = MakeNetwork(addr), .ice_username_fragment = username, .ice_password = password, - .field_trials = field_trials, - .ice_tiebreaker = kTiebreakerDefault}; + .field_trials = field_trials}; auto port = std::make_unique(args, 0, 0); port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict); return port; @@ -833,9 +840,11 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateTestPort(const rtc::SocketAddress& addr, absl::string_view username, absl::string_view password, - cricket::IceRole role) { + cricket::IceRole role, + int tiebreaker) { auto port = CreateTestPort(addr, username, password); port->SetIceRole(role); + port->SetIceTiebreaker(tiebreaker); return port; } // Overload to create a test port given an rtc::Network directly. @@ -847,8 +856,7 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { .network = network, .ice_username_fragment = username, .ice_password = password, - .field_trials = nullptr, - .ice_tiebreaker = kTiebreakerDefault}; + .field_trials = nullptr}; auto port = std::make_unique(args, 0, 0); port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict); return port; @@ -1435,8 +1443,10 @@ TEST_F(PortTest, TestConnectionDeadWithDeadConnectionTimeout) { TEST_F(PortTest, TestConnectionDeadOutstandingPing) { auto port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); + port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); port2->SetIceRole(cricket::ICEROLE_CONTROLLED); + port2->SetIceTiebreaker(kTiebreaker2); TestChannel ch1(std::move(port1)); TestChannel ch2(std::move(port2)); @@ -1476,8 +1486,10 @@ TEST_F(PortTest, TestConnectionDeadOutstandingPing) { TEST_F(PortTest, TestLocalToLocalStandard) { auto port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); + port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); port2->SetIceRole(cricket::ICEROLE_CONTROLLED); + port2->SetIceTiebreaker(kTiebreaker2); // Same parameters as TestLocalToLocal above. TestConnectivity("udp", std::move(port1), "udp", std::move(port2), true, true, true, true); @@ -1490,6 +1502,7 @@ TEST_F(PortTest, TestLocalToLocalStandard) { TEST_F(PortTest, TestLoopbackCall) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); lport->PrepareAddress(); ASSERT_FALSE(lport->Candidates().empty()); Connection* conn = @@ -1526,7 +1539,7 @@ TEST_F(PortTest, TestLoopbackCall) { // To make sure we receive error response, adding tiebreaker less than // what's present in request. modified_req->AddAttribute(std::make_unique( - STUN_ATTR_ICE_CONTROLLING, lport->IceTiebreaker() - 1)); + STUN_ATTR_ICE_CONTROLLING, kTiebreaker1 - 1)); modified_req->AddMessageIntegrity("lpass"); modified_req->AddFingerprint(); @@ -1549,8 +1562,10 @@ TEST_F(PortTest, TestLoopbackCall) { TEST_F(PortTest, TestIceRoleConflict) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); rport->SetIceRole(cricket::ICEROLE_CONTROLLING); + rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -1653,7 +1668,9 @@ TEST_F(PortTest, TestDisableInterfaceOfTcpPort) { rsocket->Bind(kLocalAddr2); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -1792,6 +1809,7 @@ TEST_F(PortTest, TestUdpMultipleAddressesV6CrossTypePorts) { factory.set_next_udp_socket(socket); ports[i] = CreateUdpPortMultipleAddrs(addresses[i], kLinkLocalIPv6Addr, &factory); + ports[i]->SetIceTiebreaker(kTiebreakerDefault); socket->set_state(AsyncPacketSocket::STATE_BINDING); socket->SignalAddressReady(socket, addresses[i]); ports[i]->PrepareAddress(); @@ -1849,7 +1867,9 @@ TEST_F(PortTest, TestSendStunMessage) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); // Send a fake ping from lport to rport. lport->PrepareAddress(); @@ -2010,7 +2030,9 @@ TEST_F(PortTest, TestNomination) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2064,7 +2086,9 @@ TEST_F(PortTest, TestRoundTripTime) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2101,7 +2125,9 @@ TEST_F(PortTest, TestUseCandidateAttribute) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); // Send a fake ping from lport to rport. lport->PrepareAddress(); @@ -2128,7 +2154,9 @@ TEST_F(PortTest, TestNetworkCostChange) { auto lport = CreateTestPort(test_network, "lfrag", "lpass"); auto rport = CreateTestPort(test_network, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2183,7 +2211,9 @@ TEST_F(PortTest, TestNetworkInfoAttribute) { auto lport = CreateTestPort(test_network, "lfrag", "lpass"); auto rport = CreateTestPort(test_network, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); uint16_t lnetwork_id = 9; test_network->set_id(lnetwork_id); @@ -2484,9 +2514,9 @@ TEST_F(PortTest, TestHandleStunResponseWithUnknownComprehensionRequiredAttribute) { // Generic setup. auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + cricket::ICEROLE_CONTROLLING, kTiebreakerDefault); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreakerDefault); lport->PrepareAddress(); rport->PrepareAddress(); ASSERT_FALSE(lport->Candidates().empty()); @@ -2523,9 +2553,9 @@ TEST_F(PortTest, TestHandleStunIndicationWithUnknownComprehensionRequiredAttribute) { // Generic set up. auto lport = CreateTestPort(kLocalAddr2, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + cricket::ICEROLE_CONTROLLING, kTiebreakerDefault); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreakerDefault); lport->PrepareAddress(); rport->PrepareAddress(); ASSERT_FALSE(lport->Candidates().empty()); @@ -2549,7 +2579,7 @@ TEST_F(PortTest, // indications are allowed only to the connection which is in read mode. TEST_F(PortTest, TestHandleStunBindingIndication) { auto lport = CreateTestPort(kLocalAddr2, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + cricket::ICEROLE_CONTROLLING, kTiebreaker1); // Verifying encoding and decoding STUN indication message. std::unique_ptr in_msg, out_msg; @@ -2570,6 +2600,7 @@ TEST_F(PortTest, TestHandleStunBindingIndication) { // last_ping_received. auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2604,6 +2635,7 @@ TEST_F(PortTest, TestHandleStunBindingIndication) { TEST_F(PortTest, TestComputeCandidatePriority) { auto port = CreateTestPort(kLocalAddr1, "name", "pass"); + port->SetIceTiebreaker(kTiebreakerDefault); port->set_type_preference(90); port->set_component(177); port->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -2641,6 +2673,7 @@ TEST_F(PortTest, TestComputeCandidatePriorityWithPriorityAdjustment) { webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IncreaseIceCandidatePriorityHostSrflx/Enabled/"); auto port = CreateTestPort(kLocalAddr1, "name", "pass", &field_trials); + port->SetIceTiebreaker(kTiebreakerDefault); port->set_type_preference(90); port->set_component(177); port->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -2678,6 +2711,7 @@ TEST_F(PortTest, TestComputeCandidatePriorityWithPriorityAdjustment) { // Test that candidates with different types will have different foundation. TEST_F(PortTest, TestFoundation) { auto testport = CreateTestPort(kLocalAddr1, "name", "pass"); + testport->SetIceTiebreaker(kTiebreakerDefault); testport->AddCandidateAddress(kLocalAddr1, kLocalAddr1, IceCandidateType::kHost, cricket::ICE_TYPE_PREFERENCE_HOST, false); @@ -2800,9 +2834,11 @@ TEST_F(PortTest, TestCandidatePriority) { // Test the Connection priority is calculated correctly. TEST_F(PortTest, TestConnectionPriority) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); + lport->SetIceTiebreaker(kTiebreakerDefault); lport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_HOST); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); + rport->SetIceTiebreaker(kTiebreakerDefault); rport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_RELAY_UDP); lport->set_component(123); lport->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -2840,9 +2876,11 @@ TEST_F(PortTest, TestConnectionPriorityWithPriorityAdjustment) { webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IncreaseIceCandidatePriorityHostSrflx/Enabled/"); auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass", &field_trials); + lport->SetIceTiebreaker(kTiebreakerDefault); lport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_HOST); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass", &field_trials); + rport->SetIceTiebreaker(kTiebreakerDefault); rport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_RELAY_UDP); lport->set_component(123); lport->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -3047,12 +3085,13 @@ TEST_F(PortTest, TestTimeoutForNeverWritable) { // In this test `ch1` behaves like FULL mode client and we have created // port which responds to the ping message just like LITE client. TEST_F(PortTest, TestIceLiteConnectivity) { - auto ice_full_port = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + auto ice_full_port = + CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING, kTiebreaker1); auto* ice_full_port_ptr = ice_full_port.get(); - auto ice_lite_port = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + auto ice_lite_port = CreateTestPort( + kLocalAddr2, "rfrag", "rpass", cricket::ICEROLE_CONTROLLED, kTiebreaker2); // Setup TestChannel. This behaves like FULL mode client. TestChannel ch1(std::move(ice_full_port)); ch1.SetIceMode(ICEMODE_FULL); @@ -3158,11 +3197,12 @@ TEST_P(GoogPingTest, TestGoogPingAnnounceEnable) { << trials.announce_goog_ping << " enable:" << trials.enable_goog_ping; - auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + auto port1_unique = + CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING, kTiebreaker1); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreaker2); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3248,11 +3288,12 @@ TEST_F(PortTest, TestGoogPingUnsupportedVersionInStunBinding) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + auto port1_unique = + CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING, kTiebreaker1); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreaker2); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3318,11 +3359,12 @@ TEST_F(PortTest, TestGoogPingUnsupportedVersionInStunBindingResponse) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + auto port1_unique = + CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING, kTiebreaker1); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreaker2); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3418,11 +3460,12 @@ TEST_F(PortTest, TestChangeInAttributeMakesGoogPingFallsbackToStunBinding) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + auto port1_unique = + CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING, kTiebreaker1); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreaker2); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3502,11 +3545,12 @@ TEST_F(PortTest, TestErrorResponseMakesGoogPingFallBackToStunBinding) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING); + auto port1_unique = + CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING, kTiebreaker1); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED); + cricket::ICEROLE_CONTROLLED, kTiebreaker2); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3597,11 +3641,13 @@ TEST_F(PortTest, TestPortTimeoutIfNotKeptAlive) { ConnectToSignalDestroyed(port1.get()); port1->set_timeout_delay(timeout_delay); // milliseconds port1->SetIceRole(cricket::ICEROLE_CONTROLLING); + port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2.get()); port2->set_timeout_delay(timeout_delay); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); + port2->SetIceTiebreaker(kTiebreaker2); // Set up channels and ensure both ports will be deleted. TestChannel ch1(std::move(port1)); @@ -3624,12 +3670,14 @@ TEST_F(PortTest, TestPortTimeoutAfterNewConnectionCreatedAndDestroyed) { ConnectToSignalDestroyed(port1.get()); port1->set_timeout_delay(timeout_delay); // milliseconds port1->SetIceRole(cricket::ICEROLE_CONTROLLING); + port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2.get()); port2->set_timeout_delay(timeout_delay); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); + port2->SetIceTiebreaker(kTiebreaker2); // Set up channels and ensure both ports will be deleted. TestChannel ch1(std::move(port1)); @@ -3663,11 +3711,13 @@ TEST_F(PortTest, TestPortNotTimeoutUntilPruned) { ConnectToSignalDestroyed(port1.get()); port1->set_timeout_delay(timeout_delay); // milliseconds port1->SetIceRole(cricket::ICEROLE_CONTROLLING); + port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2.get()); port2->set_timeout_delay(timeout_delay); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); + port2->SetIceTiebreaker(kTiebreaker2); // The connection must not be destroyed before a connection is attempted. EXPECT_EQ(0, ports_destroyed()); @@ -3716,6 +3766,7 @@ TEST_F(PortTest, TestSupportsProtocol) { // on both the port itself and its candidates. TEST_F(PortTest, TestSetIceParameters) { auto port = CreateTestPort(kLocalAddr1, "ufrag1", "password1"); + port->SetIceTiebreaker(kTiebreakerDefault); port->PrepareAddress(); EXPECT_EQ(1UL, port->Candidates().size()); port->SetIceParameters(1, "ufrag2", "password2"); @@ -3730,6 +3781,7 @@ TEST_F(PortTest, TestSetIceParameters) { TEST_F(PortTest, TestAddConnectionWithSameAddress) { auto port = CreateTestPort(kLocalAddr1, "ufrag1", "password1"); + port->SetIceTiebreaker(kTiebreakerDefault); port->PrepareAddress(); EXPECT_EQ(1u, port->Candidates().size()); rtc::SocketAddress address("1.1.1.1", 5000); @@ -3765,7 +3817,9 @@ class ConnectionTest : public PortTest { lport_ = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); rport_ = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport_->SetIceRole(cricket::ICEROLE_CONTROLLING); + lport_->SetIceTiebreaker(kTiebreaker1); rport_->SetIceRole(cricket::ICEROLE_CONTROLLED); + rport_->SetIceTiebreaker(kTiebreaker2); lport_->PrepareAddress(); rport_->PrepareAddress(); diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc index 1277d58e38..423d16b11b 100644 --- a/p2p/base/stun_port_unittest.cc +++ b/p2p/base/stun_port_unittest.cc @@ -142,9 +142,9 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { .network = &network_, .ice_username_fragment = rtc::CreateRandomString(16), .ice_password = rtc::CreateRandomString(22), - .field_trials = field_trials, - .ice_tiebreaker = kTiebreakerDefault}, + .field_trials = field_trials}, 0, 0, stun_servers, absl::nullopt); + stun_port_->SetIceTiebreaker(kTiebreakerDefault); stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_); // If `stun_keepalive_lifetime_` is negative, let the stun port // choose its lifetime from the network type. @@ -179,10 +179,10 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { .network = &network_, .ice_username_fragment = rtc::CreateRandomString(16), .ice_password = rtc::CreateRandomString(22), - .field_trials = field_trials, - .ice_tiebreaker = kTiebreakerDefault}, + .field_trials = field_trials}, socket_.get(), false, absl::nullopt); ASSERT_TRUE(stun_port_ != NULL); + stun_port_->SetIceTiebreaker(kTiebreakerDefault); ServerAddresses stun_servers; stun_servers.insert(server_addr); stun_port_->set_server_addresses(stun_servers); diff --git a/p2p/base/tcp_port_unittest.cc b/p2p/base/tcp_port_unittest.cc index fa3f7f329b..be246a8576 100644 --- a/p2p/base/tcp_port_unittest.cc +++ b/p2p/base/tcp_port_unittest.cc @@ -83,28 +83,29 @@ class TCPPortTest : public ::testing::Test, public sigslot::has_slots<> { } std::unique_ptr CreateTCPPort(const SocketAddress& addr) { - return TCPPort::Create( - { - .network_thread = &main_, - .socket_factory = &socket_factory_, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault, - }, - 0, 0, true); + auto port = std::unique_ptr( + TCPPort::Create({.network_thread = &main_, + .socket_factory = &socket_factory_, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_}, + 0, 0, true)); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } std::unique_ptr CreateTCPPort(const rtc::Network* network) { - return TCPPort::Create({.network_thread = &main_, - .socket_factory = &socket_factory_, - .network = network, - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault}, - 0, 0, true); + auto port = std::unique_ptr( + TCPPort::Create({.network_thread = &main_, + .socket_factory = &socket_factory_, + .network = network, + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_}, + 0, 0, true)); + port->SetIceTiebreaker(kTiebreakerDefault); + return port; } protected: diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 299a8980db..f8783d5daf 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -80,20 +80,17 @@ class TurnPort : public Port { return nullptr; } // Using `new` to access a non-public constructor. - return absl::WrapUnique(new TurnPort( - { - .network_thread = args.network_thread, - .socket_factory = args.socket_factory, - .network = args.network, - .ice_username_fragment = args.username, - .ice_password = args.password, - .field_trials = args.field_trials, - .ice_tiebreaker = args.ice_tiebreaker, - }, - socket, *args.server_address, args.config->credentials, - args.relative_priority, args.config->tls_alpn_protocols, - args.config->tls_elliptic_curves, args.turn_customizer, - args.config->tls_cert_verifier)); + return absl::WrapUnique( + new TurnPort({.network_thread = args.network_thread, + .socket_factory = args.socket_factory, + .network = args.network, + .ice_username_fragment = args.username, + .ice_password = args.password, + .field_trials = args.field_trials}, + socket, *args.server_address, args.config->credentials, + args.relative_priority, args.config->tls_alpn_protocols, + args.config->tls_elliptic_curves, args.turn_customizer, + args.config->tls_cert_verifier)); } // Create a TURN port that will use a new socket, bound to `network` and diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 1a62cf3751..66a154048b 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -295,7 +295,6 @@ class TurnPortTest : public ::testing::Test, args.config = &config; args.turn_customizer = turn_customizer_.get(); args.field_trials = &field_trials_; - args.ice_tiebreaker = kTiebreakerDefault; turn_port_ = TurnPort::Create(args, 0, 0); if (!turn_port_) { @@ -303,6 +302,7 @@ class TurnPortTest : public ::testing::Test, } // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); + turn_port_->SetIceTiebreaker(kTiebreakerDefault); ConnectSignals(); if (server_address.proto == cricket::PROTO_TLS) { @@ -343,10 +343,10 @@ class TurnPortTest : public ::testing::Test, args.config = &config; args.turn_customizer = turn_customizer_.get(); args.field_trials = &field_trials_; - args.ice_tiebreaker = kTiebreakerDefault; turn_port_ = TurnPort::Create(args, socket_.get()); // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); + turn_port_->SetIceTiebreaker(kTiebreakerDefault); ConnectSignals(); } @@ -371,11 +371,11 @@ class TurnPortTest : public ::testing::Test, .network = MakeNetwork(address), .ice_username_fragment = kIceUfrag2, .ice_password = kIcePwd2, - .field_trials = &field_trials_, - .ice_tiebreaker = kTiebreakerDefault}, + .field_trials = &field_trials_}, 0, 0, false, absl::nullopt); // UDP port will be controlled. udp_port_->SetIceRole(ICEROLE_CONTROLLED); + udp_port_->SetIceTiebreaker(kTiebreakerDefault); udp_port_->SignalPortComplete.connect(this, &TurnPortTest::OnUdpPortComplete); } diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index ae136fe35f..f1525b0589 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -1475,8 +1475,7 @@ void AllocationSequence::CreateUDPPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials(), - .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, + .field_trials = session_->allocator()->field_trials()}, udp_socket_.get(), emit_local_candidate_for_anyaddress, session_->allocator()->stun_candidate_keepalive_interval()); } else { @@ -1486,14 +1485,14 @@ void AllocationSequence::CreateUDPPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials(), - .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, + .field_trials = session_->allocator()->field_trials()}, session_->allocator()->min_port(), session_->allocator()->max_port(), emit_local_candidate_for_anyaddress, session_->allocator()->stun_candidate_keepalive_interval()); } if (port) { + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); // If shared socket is enabled, STUN candidate will be allocated by the // UDPPort. if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) { @@ -1528,12 +1527,12 @@ void AllocationSequence::CreateTCPPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials(), - .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, + .field_trials = session_->allocator()->field_trials()}, session_->allocator()->min_port(), session_->allocator()->max_port(), session_->allocator()->allow_tcp_listen()); if (port) { + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); // Since TCPPort is not created using shared socket, `port` will not be // added to the dequeue. @@ -1562,12 +1561,12 @@ void AllocationSequence::CreateStunPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials(), - .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, + .field_trials = session_->allocator()->field_trials()}, session_->allocator()->min_port(), session_->allocator()->max_port(), config_->StunServers(), session_->allocator()->stun_candidate_keepalive_interval()); if (port) { + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); // Since StunPort is not created using shared socket, `port` will not be // added to the dequeue. @@ -1635,7 +1634,6 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, args.turn_customizer = session_->allocator()->turn_customizer(); args.field_trials = session_->allocator()->field_trials(); args.relative_priority = relative_priority; - args.ice_tiebreaker = session_->allocator()->ice_tiebreaker(); std::unique_ptr port; // Shared socket mode must be enabled only for UDP based ports. Hence @@ -1671,6 +1669,7 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, } } RTC_DCHECK(port != NULL); + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); } } diff --git a/p2p/client/relay_port_factory_interface.h b/p2p/client/relay_port_factory_interface.h index bdc8c3d448..edfca3697b 100644 --- a/p2p/client/relay_port_factory_interface.h +++ b/p2p/client/relay_port_factory_interface.h @@ -49,7 +49,6 @@ struct CreateRelayPortArgs { // to the candidates from other servers. Required because ICE priorities // need to be unique. int relative_priority = 0; - uint64_t ice_tiebreaker = 0; }; // A factory for creating RelayPort's.