mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-13 05:40:42 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

ICE: adjust priority of non-relay candidates

Browse source 
Introduces a field trial
  WebRTC-IncreaseIceCandidatePriorityHostSrflx
that adjusts the priority of non-relay candidates such that the STUN priority attribute calculated as
  (prflx-type-preference << 24) | (priority & 0x00FFFFFF)
as described in
  https://www.rfc-editor.org/rfc/rfc5245#section-7.1.2.1
will satisfy the condition that the STUN priority of server-reflexive candidates will always be higher than the STUN priority of relay candidates.

Previously this was not the case because the TURN relay preference was added to the local_preference for relay candidates, making it higher than the local_preference of srflx candidates gathered from the same interface.
This led to cases where the resulting candidate pair priority of a srflx-relay pair was higher than the candidate pair priority of a srflx-srflx pair, i.e. using a TURN server in cases that work using a direct P2P connection.

Whether the field trial is active can be observed by checking that
  priority-of-srflx-candidate&0x00FFFFFF
is greater than
  priority-of-relay-candidate&0x00FFFFFF

BUG=webrtc:13195,webrtc:5813,webrtc:15020

Change-Id: Ib91708fbe7310b6454f93158a45c9d77da009091
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/292700
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#40311}
This commit is contained in:
Philipp Hancke 2023-06-16 13:58:45 +02:00 committed by WebRTC LUCI CQ
parent 52f902abc6
commit 17ec0569c6
5 changed files with 170 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
api/candidate.cc
View file

@ -88,7 +88,8 @@ std::string Candidate::ToStringInternal(bool sensitive) const {
uint32_t Candidate::GetPriority(uint32_t type_preference,
int network_adapter_preference,
int relay_preference) const {
int relay_preference,
bool adjust_local_preference) const {
// RFC 5245 - 4.1.2.1.
// priority = (2^24)*(type preference) +
// (2^8)*(local preference) +
@ -106,11 +107,25 @@ uint32_t Candidate::GetPriority(uint32_t type_preference,
// local preference = (NIC Type << 8 | Addr_Pref) + relay preference.
// The relay preference is based on the number of TURN servers, the
// first TURN server gets the highest preference.
int addr_pref = IPAddressPrecedence(address_.ipaddr());
int local_preference =
((network_adapter_preference << 8) | addr_pref) + relay_preference;
// Ensure that the added relay preference will not result in a relay candidate
// whose STUN priority attribute has a higher priority than a server-reflexive
// candidate.
// The STUN priority attribute is calculated as
// (peer-reflexive type preference) << 24 | (priority & 0x00FFFFFF)
// as described in
// https://www.rfc-editor.org/rfc/rfc5245#section-7.1.2.1
// To satisfy that condition, add kMaxTurnServers to the local preference.
// This can not overflow the field width since the highest "NIC pref"
// assigned is kHighestNetworkPreference = 127
RTC_DCHECK_LT(local_preference + kMaxTurnServers, 0x10000);
if (adjust_local_preference && relay_protocol_.empty()) {
local_preference += kMaxTurnServers;
}
return (type_preference << 24) | (local_preference << 8) | (256 - component_);
}

3
api/candidate.h
View file

@ -173,7 +173,8 @@ class RTC_EXPORT Candidate {
uint32_t GetPriority(uint32_t type_preference,
int network_adapter_preference,
int relay_preference) const;
int relay_preference,
bool adjust_local_preference) const;
bool operator==(const Candidate& o) const;
bool operator!=(const Candidate& o) const;

6
p2p/base/port.cc
View file

@ -270,9 +270,11 @@ void Port::AddAddress(const rtc::SocketAddress& address,
ComputeFoundation(type, protocol, relay_protocol, base_address);
Candidate c(component_, protocol, address, 0U, username_fragment(), password_,
type, generation_, foundation, network_->id(), network_cost_);
c.set_priority(
c.GetPriority(type_preference, network_->preference(), relay_preference));
c.set_relay_protocol(relay_protocol);
c.set_priority(
c.GetPriority(type_preference, network_->preference(), relay_preference,
field_trials_->IsEnabled(
"WebRTC-IncreaseIceCandidatePriorityHostSrflx")));
c.set_tcptype(tcptype);
c.set_network_name(network_->name());
c.set_network_type(network_->type());

103
p2p/base/port_unittest.cc
View file

@ -144,7 +144,8 @@ class TestPort : public Port {
uint16_t min_port,
uint16_t max_port,
absl::string_view username_fragment,
absl::string_view password)
absl::string_view password,
const webrtc::FieldTrialsView* field_trials = nullptr)
: Port(thread,
type,
factory,
@ -152,7 +153,8 @@ class TestPort : public Port {
min_port,
max_port,
username_fragment,
password) {}
password,
field_trials) {}
~TestPort() {}
// Expose GetStunMessage so that we can test it.
@ -802,12 +804,14 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
STUN_ATTR_USERNAME, std::string(username)));
return msg;
}
std::unique_ptr<TestPort> CreateTestPort(const rtc::SocketAddress& addr,
absl::string_view username,
absl::string_view password) {
auto port =
std::make_unique<TestPort>(&main_, "test", &socket_factory_,
MakeNetwork(addr), 0, 0, username, password);
std::unique_ptr<TestPort> CreateTestPort(
const rtc::SocketAddress& addr,
absl::string_view username,
absl::string_view password,
const webrtc::FieldTrialsView* field_trials = nullptr) {
auto port = std::make_unique<TestPort>(&main_, "test", &socket_factory_,
MakeNetwork(addr), 0, 0, username,
password, field_trials);
port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict);
return port;
}
@ -2626,6 +2630,44 @@ TEST_F(PortTest, TestComputeCandidatePriority) {
ASSERT_EQ(expected_priority_6bone, port->Candidates()[8].priority());
}
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));
port->AddCandidateAddress(SocketAddress("2001:db8::1234", 1234));
port->AddCandidateAddress(SocketAddress("fc12:3456::1234", 1234));
port->AddCandidateAddress(SocketAddress("::ffff:192.168.1.4", 1234));
port->AddCandidateAddress(SocketAddress("::192.168.1.4", 1234));
port->AddCandidateAddress(SocketAddress("2002::1234:5678", 1234));
port->AddCandidateAddress(SocketAddress("2001::1234:5678", 1234));
port->AddCandidateAddress(SocketAddress("fecf::1234:5678", 1234));
port->AddCandidateAddress(SocketAddress("3ffe::1234:5678", 1234));
// These should all be:
// (90 << 24) | (([rfc3484 pref value] << 8) + kMaxTurnServers) | (256 - 177)
uint32_t expected_priority_v4 = 1509957199U + (kMaxTurnServers << 8);
uint32_t expected_priority_v6 = 1509959759U + (kMaxTurnServers << 8);
uint32_t expected_priority_ula = 1509962319U + (kMaxTurnServers << 8);
uint32_t expected_priority_v4mapped = expected_priority_v4;
uint32_t expected_priority_v4compat = 1509949775U + (kMaxTurnServers << 8);
uint32_t expected_priority_6to4 = 1509954639U + (kMaxTurnServers << 8);
uint32_t expected_priority_teredo = 1509952079U + (kMaxTurnServers << 8);
uint32_t expected_priority_sitelocal = 1509949775U + (kMaxTurnServers << 8);
uint32_t expected_priority_6bone = 1509949775U + (kMaxTurnServers << 8);
ASSERT_EQ(expected_priority_v4, port->Candidates()[0].priority());
ASSERT_EQ(expected_priority_v6, port->Candidates()[1].priority());
ASSERT_EQ(expected_priority_ula, port->Candidates()[2].priority());
ASSERT_EQ(expected_priority_v4mapped, port->Candidates()[3].priority());
ASSERT_EQ(expected_priority_v4compat, port->Candidates()[4].priority());
ASSERT_EQ(expected_priority_6to4, port->Candidates()[5].priority());
ASSERT_EQ(expected_priority_teredo, port->Candidates()[6].priority());
ASSERT_EQ(expected_priority_sitelocal, port->Candidates()[7].priority());
ASSERT_EQ(expected_priority_6bone, port->Candidates()[8].priority());
}
// In the case of shared socket, one port may be shared by local and stun.
// Test that candidates with different types will have different foundation.
TEST_F(PortTest, TestFoundation) {
@ -2788,6 +2830,51 @@ TEST_F(PortTest, TestConnectionPriority) {
#endif
}
// Test the Connection priority is calculated correctly.
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));
rport->set_component(23);
rport->AddCandidateAddress(SocketAddress("10.1.1.100", 1234));
EXPECT_EQ(0x7E001E85U + (kMaxTurnServers << 8),
lport->Candidates()[0].priority());
EXPECT_EQ(0x2001EE9U + (kMaxTurnServers << 8),
rport->Candidates()[0].priority());
// RFC 5245
// pair priority = 2^32*MIN(G,D) + 2*MAX(G,D) + (G>D?1:0)
lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
Connection* lconn =
lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
#if defined(WEBRTC_WIN)
EXPECT_EQ(0x2003EE9FC007D0BU, lconn->priority());
#else
EXPECT_EQ(0x2003EE9FC007D0BLLU, lconn->priority());
#endif
lport->SetIceRole(cricket::ICEROLE_CONTROLLED);
rport->SetIceRole(cricket::ICEROLE_CONTROLLING);
Connection* rconn =
rport->CreateConnection(lport->Candidates()[0], Port::ORIGIN_MESSAGE);
RTC_LOG(LS_ERROR) << "RCONN " << rconn->priority();
#if defined(WEBRTC_WIN)
EXPECT_EQ(0x2003EE9FC007D0AU, rconn->priority());
#else
EXPECT_EQ(0x2003EE9FC007D0ALLU, rconn->priority());
#endif
}
// Note that UpdateState takes into account the estimated RTT, and the
// correctness of using `kMaxExpectedSimulatedRtt` as an upper bound of RTT in
// the following tests depends on the link rate and the delay distriubtion

52
p2p/base/stun_port_unittest.cc
View file

@ -334,6 +334,31 @@ TEST_F(StunPortWithMockDnsResolverTest, TestPrepareAddressHostname) {
EXPECT_EQ(kStunCandidatePriority, port()->Candidates()[0].priority());
}
TEST_F(StunPortWithMockDnsResolverTest,
TestPrepareAddressHostnameWithPriorityAdjustment) {
webrtc::test::ScopedKeyValueConfig field_trials(
"WebRTC-IncreaseIceCandidatePriorityHostSrflx/Enabled/");
SetDnsResolverExpectations(
[](webrtc::MockAsyncDnsResolver* resolver,
webrtc::MockAsyncDnsResolverResult* resolver_result) {
EXPECT_CALL(*resolver, Start(kValidHostnameAddr, /*family=*/AF_INET, _))
.WillOnce(InvokeArgument<2>());
EXPECT_CALL(*resolver, result)
.WillRepeatedly(ReturnPointee(resolver_result));
EXPECT_CALL(*resolver_result, GetError).WillOnce(Return(0));
EXPECT_CALL(*resolver_result, GetResolvedAddress(AF_INET, _))
.WillOnce(DoAll(SetArgPointee<1>(SocketAddress("127.0.0.1", 5000)),
Return(true)));
});
CreateStunPort(kValidHostnameAddr);
PrepareAddress();
EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock);
ASSERT_EQ(1U, port()->Candidates().size());
EXPECT_TRUE(kLocalAddr.EqualIPs(port()->Candidates()[0].address()));
EXPECT_EQ(kStunCandidatePriority + (cricket::kMaxTurnServers << 8),
port()->Candidates()[0].priority());
}
// Test that we handle hostname lookup failures properly.
TEST_F(StunPortTestWithRealClock, TestPrepareAddressHostnameFail) {
CreateStunPort(kBadHostnameAddr);
@ -684,4 +709,31 @@ TEST_F(StunIPv6PortTestWithMockDnsResolver, TestPrepareAddressHostname) {
EXPECT_EQ(kIPv6StunCandidatePriority, port()->Candidates()[0].priority());
}
// Same as before but with a field trial that changes the priority.
TEST_F(StunIPv6PortTestWithMockDnsResolver,
TestPrepareAddressHostnameWithPriorityAdjustment) {
webrtc::test::ScopedKeyValueConfig field_trials(
"WebRTC-IncreaseIceCandidatePriorityHostSrflx/Enabled/");
SetDnsResolverExpectations(
[](webrtc::MockAsyncDnsResolver* resolver,
webrtc::MockAsyncDnsResolverResult* resolver_result) {
EXPECT_CALL(*resolver,
Start(kValidHostnameAddr, /*family=*/AF_INET6, _))
.WillOnce(InvokeArgument<2>());
EXPECT_CALL(*resolver, result)
.WillRepeatedly(ReturnPointee(resolver_result));
EXPECT_CALL(*resolver_result, GetError).WillOnce(Return(0));
EXPECT_CALL(*resolver_result, GetResolvedAddress(AF_INET6, _))
.WillOnce(DoAll(SetArgPointee<1>(SocketAddress("::1", 5000)),
Return(true)));
});
CreateStunPort(kValidHostnameAddr, &field_trials);
PrepareAddress();
EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock);
ASSERT_EQ(1U, port()->Candidates().size());
EXPECT_TRUE(kIPv6LocalAddr.EqualIPs(port()->Candidates()[0].address()));
EXPECT_EQ(kIPv6StunCandidatePriority + (cricket::kMaxTurnServers << 8),
port()->Candidates()[0].priority());
}
} // namespace