Skip to content

Commit df7df19

Browse files
sam-viWebRTC LUCI CQ
authored andcommitted
Clean up IPv6 fixes field trial artifacts.
The fixes have been default enabled, so clean up dead code. Bug: webrtc:14334 Change-Id: I4967d5ad451ac333c54294fc14bea6c7ba1445e3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301180 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Reviewed-by: Jonas Oreland <jonaso@webrtc.org> Commit-Queue: Sameer Vijaykar <samvi@google.com> Cr-Commit-Position: refs/heads/main@{#39949}
1 parent 5227584 commit df7df19

File tree

9 files changed

+39
-113
lines changed

9 files changed

+39
-113
lines changed

p2p/base/port_unittest.cc

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -517,12 +517,10 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
517517
return &networks_.back();
518518
}
519519

520-
rtc::Network* MakeNetworkMultipleAddrs(
521-
const SocketAddress& global_addr,
522-
const SocketAddress& link_local_addr,
523-
const webrtc::FieldTrialsView* field_trials) {
520+
rtc::Network* MakeNetworkMultipleAddrs(const SocketAddress& global_addr,
521+
const SocketAddress& link_local_addr) {
524522
networks_.emplace_back("unittest", "unittest", global_addr.ipaddr(), 32,
525-
rtc::ADAPTER_TYPE_UNKNOWN, field_trials);
523+
rtc::ADAPTER_TYPE_UNKNOWN);
526524
networks_.back().AddIP(link_local_addr.ipaddr());
527525
networks_.back().AddIP(global_addr.ipaddr());
528526
networks_.back().AddIP(link_local_addr.ipaddr());
@@ -548,8 +546,8 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
548546
PacketSocketFactory* socket_factory) {
549547
auto port = UDPPort::Create(
550548
&main_, socket_factory,
551-
MakeNetworkMultipleAddrs(global_addr, link_local_addr, &field_trials_),
552-
0, 0, username_, password_, true, absl::nullopt, &field_trials_);
549+
MakeNetworkMultipleAddrs(global_addr, link_local_addr), 0, 0, username_,
550+
password_, true, absl::nullopt, &field_trials_);
553551
port->SetIceTiebreaker(kTiebreakerDefault);
554552
return port;
555553
}

p2p/base/stun_port.cc

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,6 @@
2929

3030
namespace cricket {
3131

32-
namespace {
33-
34-
bool ResolveStunHostnameForFamily(const webrtc::FieldTrialsView& field_trials) {
35-
// TODO(bugs.webrtc.org/14334) cleanup
36-
return true;
37-
}
38-
39-
} // namespace
40-
4132
// TODO(?): Move these to a common place (used in relayport too)
4233
const int RETRY_TIMEOUT = 50 * 1000; // 50 seconds
4334

@@ -152,11 +143,7 @@ void UDPPort::AddressResolver::Resolve(
152143
done_(it->first, it->second->result().GetError());
153144
}
154145
};
155-
if (ResolveStunHostnameForFamily(field_trials)) {
156-
resolver_ptr->Start(address, family, std::move(callback));
157-
} else {
158-
resolver_ptr->Start(address, std::move(callback));
159-
}
146+
resolver_ptr->Start(address, family, std::move(callback));
160147
}
161148

162149
bool UDPPort::AddressResolver::GetResolvedAddress(

p2p/base/turn_port.cc

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,6 @@
3434

3535
namespace cricket {
3636

37-
namespace {
38-
39-
bool ResolveTurnHostnameForFamily(const webrtc::FieldTrialsView& field_trials) {
40-
// TODO(bugs.webrtc.org/14334) cleanup
41-
return true;
42-
}
43-
44-
} // namespace
45-
4637
using ::webrtc::SafeTask;
4738
using ::webrtc::TaskQueueBase;
4839
using ::webrtc::TimeDelta;
@@ -850,12 +841,7 @@ void TurnPort::ResolveTurnAddress(const rtc::SocketAddress& address) {
850841
server_address_.address = resolved_address;
851842
PrepareAddress();
852843
};
853-
// TODO(bugs.webrtc.org/14733): remove duplicate resolution with STUN port.
854-
if (ResolveTurnHostnameForFamily(field_trials())) {
855-
resolver_->Start(address, Network()->family(), std::move(callback));
856-
} else {
857-
resolver_->Start(address, std::move(callback));
858-
}
844+
resolver_->Start(address, Network()->family(), std::move(callback));
859845
}
860846

861847
void TurnPort::OnSendStunPacket(const void* data,

p2p/client/basic_port_allocator.cc

Lines changed: 12 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,6 @@ std::string NetworksToString(const std::vector<const rtc::Network*>& networks) {
154154
return ost.Release();
155155
}
156156

157-
bool IsDiversifyIpv6InterfacesEnabled(
158-
const webrtc::FieldTrialsView* field_trials) {
159-
// TODO(bugs.webrtc.org/14334) cleanup
160-
return true;
161-
}
162-
163157
} // namespace
164158

165159
const uint32_t DISABLE_ALL_PHASES =
@@ -804,41 +798,20 @@ std::vector<const rtc::Network*> BasicPortAllocatorSession::GetNetworks() {
804798
}
805799

806800
// Lastly, if we have a limit for the number of IPv6 network interfaces (by
807-
// default, it's 5), remove networks to ensure that limit is satisfied.
808-
//
809-
// TODO(deadbeef): Instead of just taking the first N arbitrary IPv6
810-
// networks, we could try to choose a set that's "most likely to work". It's
811-
// hard to define what that means though; it's not just "lowest cost".
812-
// Alternatively, we could just focus on making our ICE pinging logic smarter
813-
// such that this filtering isn't necessary in the first place.
814-
const webrtc::FieldTrialsView* field_trials = allocator_->field_trials();
815-
if (IsDiversifyIpv6InterfacesEnabled(field_trials)) {
816-
std::vector<const rtc::Network*> ipv6_networks;
817-
for (auto it = networks.begin(); it != networks.end();) {
818-
if ((*it)->prefix().family() == AF_INET6) {
819-
ipv6_networks.push_back(*it);
820-
it = networks.erase(it);
821-
continue;
822-
}
823-
++it;
824-
}
825-
ipv6_networks =
826-
SelectIPv6Networks(ipv6_networks, allocator_->max_ipv6_networks());
827-
networks.insert(networks.end(), ipv6_networks.begin(), ipv6_networks.end());
828-
} else {
829-
int ipv6_networks = 0;
830-
for (auto it = networks.begin(); it != networks.end();) {
831-
if ((*it)->prefix().family() == AF_INET6) {
832-
if (ipv6_networks >= allocator_->max_ipv6_networks()) {
833-
it = networks.erase(it);
834-
continue;
835-
} else {
836-
++ipv6_networks;
837-
}
838-
}
839-
++it;
801+
// default, it's 5), pick IPv6 networks from different interfaces in a
802+
// priority order and stick to the limit.
803+
std::vector<const rtc::Network*> ipv6_networks;
804+
for (auto it = networks.begin(); it != networks.end();) {
805+
if ((*it)->prefix().family() == AF_INET6) {
806+
ipv6_networks.push_back(*it);
807+
it = networks.erase(it);
808+
continue;
840809
}
810+
++it;
841811
}
812+
ipv6_networks =
813+
SelectIPv6Networks(ipv6_networks, allocator_->max_ipv6_networks());
814+
networks.insert(networks.end(), ipv6_networks.begin(), ipv6_networks.end());
842815
return networks;
843816
}
844817

p2p/client/basic_port_allocator_unittest.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,8 +2649,7 @@ TEST_F(BasicPortAllocatorTest, IPv6EtherAndWifiHaveHigherPriorityThanOthers) {
26492649
EXPECT_TRUE(HasNetwork(networks, ethe1));
26502650
}
26512651

2652-
TEST_F(BasicPortAllocatorTest,
2653-
Select2DifferentIntefacesIfDiversifyIpv6InterfacesEnabled) {
2652+
TEST_F(BasicPortAllocatorTest, Select2DifferentIntefaces) {
26542653
allocator().set_max_ipv6_networks(2);
26552654
AddInterface(kClientIPv6Addr, "ethe1", rtc::ADAPTER_TYPE_ETHERNET);
26562655
AddInterface(kClientIPv6Addr2, "ethe2", rtc::ADAPTER_TYPE_ETHERNET);
@@ -2675,8 +2674,7 @@ TEST_F(BasicPortAllocatorTest,
26752674
EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientIPv6Addr3));
26762675
}
26772676

2678-
TEST_F(BasicPortAllocatorTest,
2679-
Select3DifferentIntefacesIfDiversifyIpv6InterfacesEnabled) {
2677+
TEST_F(BasicPortAllocatorTest, Select3DifferentIntefaces) {
26802678
allocator().set_max_ipv6_networks(3);
26812679
AddInterface(kClientIPv6Addr, "ethe1", rtc::ADAPTER_TYPE_ETHERNET);
26822680
AddInterface(kClientIPv6Addr2, "ethe2", rtc::ADAPTER_TYPE_ETHERNET);
@@ -2702,8 +2700,7 @@ TEST_F(BasicPortAllocatorTest,
27022700
EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientIPv6Addr5));
27032701
}
27042702

2705-
TEST_F(BasicPortAllocatorTest,
2706-
Select4DifferentIntefacesIfDiversifyIpv6InterfacesEnabled) {
2703+
TEST_F(BasicPortAllocatorTest, Select4DifferentIntefaces) {
27072704
allocator().set_max_ipv6_networks(4);
27082705
AddInterface(kClientIPv6Addr, "ethe1", rtc::ADAPTER_TYPE_ETHERNET);
27092706
AddInterface(kClientIPv6Addr2, "ethe2", rtc::ADAPTER_TYPE_ETHERNET);

rtc_base/fake_network.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class FakeNetworkManager : public NetworkManagerBase {
110110
IPAddress prefix = TruncateIP(it->socket_address.ipaddr(), prefix_length);
111111
auto net = std::make_unique<Network>(
112112
it->socket_address.hostname(), it->socket_address.hostname(), prefix,
113-
prefix_length, it->adapter_type, /*field_trials=*/nullptr);
113+
prefix_length, it->adapter_type);
114114
if (it->underlying_vpn_adapter_type.has_value()) {
115115
net->set_underlying_type_for_vpn(*it->underlying_vpn_adapter_type);
116116
}

rtc_base/network.cc

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,6 @@ bool ShouldAdapterChangeTriggerNetworkChange(rtc::AdapterType old_type,
186186
return true;
187187
}
188188

189-
bool PreferGlobalIPv6Address(const webrtc::FieldTrialsView* field_trials) {
190-
// TODO(bugs.webrtc.org/14334) cleanup
191-
return true;
192-
}
193-
194189
#if defined(WEBRTC_WIN)
195190
bool IpAddressAttributesEnabled(const webrtc::FieldTrialsView* field_trials) {
196191
// Field trial key reserved in bugs.webrtc.org/14334
@@ -334,7 +329,7 @@ std::unique_ptr<Network> NetworkManagerBase::CreateNetwork(
334329
int prefix_length,
335330
AdapterType type) const {
336331
return std::make_unique<Network>(name, description, prefix, prefix_length,
337-
type, field_trials_.get());
332+
type);
338333
}
339334

340335
std::vector<const Network*> NetworkManagerBase::GetAnyAddressNetworks() {
@@ -1119,10 +1114,8 @@ Network::Network(absl::string_view name,
11191114
absl::string_view desc,
11201115
const IPAddress& prefix,
11211116
int prefix_length,
1122-
AdapterType type,
1123-
const webrtc::FieldTrialsView* field_trials)
1124-
: field_trials_(field_trials),
1125-
name_(name),
1117+
AdapterType type)
1118+
: name_(name),
11261119
description_(desc),
11271120
prefix_(prefix),
11281121
prefix_length_(prefix_length),
@@ -1166,15 +1159,13 @@ IPAddress Network::GetBestIP() const {
11661159
}
11671160

11681161
InterfaceAddress selected_ip, link_local_ip, ula_ip;
1169-
const bool prefer_global_ipv6_to_link_local =
1170-
PreferGlobalIPv6Address(field_trials_);
11711162

11721163
for (const InterfaceAddress& ip : ips_) {
11731164
// Ignore any address which has been deprecated already.
11741165
if (ip.ipv6_flags() & IPV6_ADDRESS_FLAG_DEPRECATED)
11751166
continue;
11761167

1177-
if (prefer_global_ipv6_to_link_local && IPIsLinkLocal(ip)) {
1168+
if (IPIsLinkLocal(ip)) {
11781169
link_local_ip = ip;
11791170
continue;
11801171
}
@@ -1193,7 +1184,7 @@ IPAddress Network::GetBestIP() const {
11931184
}
11941185

11951186
if (IPIsUnspec(selected_ip)) {
1196-
if (prefer_global_ipv6_to_link_local && !IPIsUnspec(link_local_ip)) {
1187+
if (!IPIsUnspec(link_local_ip)) {
11971188
// No proper global IPv6 address found, use link local address instead.
11981189
selected_ip = link_local_ip;
11991190
} else if (!IPIsUnspec(ula_ip)) {

rtc_base/network.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -382,21 +382,18 @@ class RTC_EXPORT Network {
382382
Network(absl::string_view name,
383383
absl::string_view description,
384384
const IPAddress& prefix,
385-
int prefix_length,
386-
const webrtc::FieldTrialsView* field_trials = nullptr)
385+
int prefix_length)
387386
: Network(name,
388387
description,
389388
prefix,
390389
prefix_length,
391-
rtc::ADAPTER_TYPE_UNKNOWN,
392-
field_trials) {}
390+
rtc::ADAPTER_TYPE_UNKNOWN) {}
393391

394392
Network(absl::string_view name,
395393
absl::string_view description,
396394
const IPAddress& prefix,
397395
int prefix_length,
398-
AdapterType type,
399-
const webrtc::FieldTrialsView* field_trials = nullptr);
396+
AdapterType type);
400397

401398
Network(const Network&);
402399
~Network();
@@ -579,7 +576,6 @@ class RTC_EXPORT Network {
579576
std::string ToString() const;
580577

581578
private:
582-
const webrtc::FieldTrialsView* field_trials_ = nullptr;
583579
const DefaultLocalAddressProvider* default_local_address_provider_ = nullptr;
584580
const MdnsResponderProvider* mdns_responder_provider_ = nullptr;
585581
std::string name_;

rtc_base/network_unittest.cc

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,9 @@ TEST_F(NetworkTest, TestNetworkConstruct) {
340340

341341
TEST_F(NetworkTest, TestIsIgnoredNetworkIgnoresIPsStartingWith0) {
342342
Network ipv4_network1("test_eth0", "Test Network Adapter 1",
343-
IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET,
344-
&field_trials_);
343+
IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET);
345344
Network ipv4_network2("test_eth1", "Test Network Adapter 2",
346-
IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET,
347-
&field_trials_);
345+
IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET);
348346
PhysicalSocketServer socket_server;
349347
BasicNetworkManager network_manager(&socket_server);
350348
network_manager.StartUpdating();
@@ -824,19 +822,19 @@ TEST_F(NetworkTest, NetworksSortedByInterfaceName) {
824822

825823
TEST_F(NetworkTest, TestNetworkAdapterTypes) {
826824
Network wifi("wlan0", "Wireless Adapter", IPAddress(0x12345600U), 24,
827-
ADAPTER_TYPE_WIFI, &field_trials_);
825+
ADAPTER_TYPE_WIFI);
828826
EXPECT_EQ(ADAPTER_TYPE_WIFI, wifi.type());
829827
Network ethernet("eth0", "Ethernet", IPAddress(0x12345600U), 24,
830-
ADAPTER_TYPE_ETHERNET, &field_trials_);
828+
ADAPTER_TYPE_ETHERNET);
831829
EXPECT_EQ(ADAPTER_TYPE_ETHERNET, ethernet.type());
832830
Network cellular("test_cell", "Cellular Adapter", IPAddress(0x12345600U), 24,
833-
ADAPTER_TYPE_CELLULAR, &field_trials_);
831+
ADAPTER_TYPE_CELLULAR);
834832
EXPECT_EQ(ADAPTER_TYPE_CELLULAR, cellular.type());
835833
Network vpn("bridge_test", "VPN Adapter", IPAddress(0x12345600U), 24,
836-
ADAPTER_TYPE_VPN, &field_trials_);
834+
ADAPTER_TYPE_VPN);
837835
EXPECT_EQ(ADAPTER_TYPE_VPN, vpn.type());
838836
Network unknown("test", "Test Adapter", IPAddress(0x12345600U), 24,
839-
ADAPTER_TYPE_UNKNOWN, &field_trials_);
837+
ADAPTER_TYPE_UNKNOWN);
840838
EXPECT_EQ(ADAPTER_TYPE_UNKNOWN, unknown.type());
841839
}
842840

@@ -1167,7 +1165,7 @@ TEST_F(NetworkTest, TestGetBestIPWithPreferGlobalIPv6ToLinkLocalEnabled) {
11671165

11681166
// Create a network with this prefix.
11691167
Network ipv6_network("test_eth0", "Test NetworkAdapter", TruncateIP(ip, 64),
1170-
64, ADAPTER_TYPE_UNKNOWN, &field_trials_);
1168+
64, ADAPTER_TYPE_UNKNOWN);
11711169

11721170
// When there is no address added, it should return an unspecified
11731171
// address.

0 commit comments

Comments
 (0)