admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Relanding: Move "max IPv6 networks" logic to BasicPortAllocator, and fix sorting.

Browse Source 
Relanding because the broken chromium test has been fixed:
https://chromium-review.googlesource.com/582196

This CL moves the responsibility for restricting the number of IPv6
interfaces used for ICE to BasicPortAllocator. This is the right place
to do it in the first place; it's where all the rest of the filtering
occurs. And NetworkManager shouldn't need to know about ICE limitations;
only the ICE classes should.

Part of the reason I'm doing this is that I want to add a
"max_ipv6_networks" API to RTCConfiguration, so that applications can
override the default easily (see linked bug). But that means that
PeerConnection would need to be able to call "set_max_ipv6_networks" on
the underlying object that does the filtering, and that method isn't
available on the "NetworkManager" base class. So rather than adding
another method to a place it doesn't belong, I'm moving it to the place
it does belong.

In the process, I noticed that "CompareNetworks" is inconsistent with
"SortNetworks"; the former orders interfaces alphabetically, and the
latter reverse-alphabetically. I believe this was unintentional, and
results in undesirable behavior (like "eth1" being preferred over
"eth0"), so I'm fixing it and adding a test.

BUG=webrtc:7703

Review-Url: https://codereview.webrtc.org/2983213002
Cr-Original-Commit-Position: refs/heads/master@{#19112}
Committed: ad9561404c
Review-Url: https://codereview.webrtc.org/2983213002
Cr-Commit-Position: refs/heads/master@{#19159}
This commit is contained in:
deadbeef 2017-07-26 16:09:33 -07:00 committed by Commit Bot
parent 58f1725ff1
commit 3427f538de
7 changed files with 148 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
webrtc/p2p/base/p2ptransportchannel_unittest.cc
View File

@ -2270,10 +2270,9 @@ TEST_F(P2PTransportChannelMultihomedTest, TestBasic) {
TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {
rtc::ScopedFakeClock clock;
AddAddress(0, kPublicAddrs[0]);
// Adding alternate address will make sure |kPublicAddrs| has the higher
// priority than others. This is due to FakeNetwork::AddInterface method.
AddAddress(1, kAlternateAddrs[1]);
AddAddress(1, kPublicAddrs[1]);
// Simulate failing over from Wi-Fi to cell interface.
AddAddress(1, kPublicAddrs[1], "eth0", rtc::ADAPTER_TYPE_WIFI);
AddAddress(1, kAlternateAddrs[1], "wlan0", rtc::ADAPTER_TYPE_CELLULAR);
// Use only local ports for simplicity.
SetAllocatorFlags(0, kOnlyLocalPorts);
@ -2322,10 +2321,9 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {
// The controlling side has two interfaces and one will die.
TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
rtc::ScopedFakeClock clock;
// Adding alternate address will make sure |kPublicAddrs| has the higher
// priority than others. This is due to FakeNetwork::AddInterface method.
AddAddress(0, kAlternateAddrs[0]);
AddAddress(0, kPublicAddrs[0]);
// Simulate failing over from Wi-Fi to cell interface.
AddAddress(0, kPublicAddrs[0], "eth0", rtc::ADAPTER_TYPE_WIFI);
AddAddress(0, kAlternateAddrs[0], "wlan0", rtc::ADAPTER_TYPE_CELLULAR);
AddAddress(1, kPublicAddrs[1]);
// Use only local ports for simplicity.
@ -2469,10 +2467,9 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverWithManyConnections) {
// increase.
TEST_F(P2PTransportChannelMultihomedTest, TestIceRenomination) {
rtc::ScopedFakeClock clock;
// Adding alternate address will make sure |kPublicAddrs| has the higher
// priority than others. This is due to FakeNetwork::AddInterface method.
AddAddress(0, kAlternateAddrs[0]);
AddAddress(0, kPublicAddrs[0]);
// Simulate failing over from Wi-Fi to cell interface.
AddAddress(0, kPublicAddrs[0], "eth0", rtc::ADAPTER_TYPE_WIFI);
AddAddress(0, kAlternateAddrs[0], "wlan0", rtc::ADAPTER_TYPE_CELLULAR);
AddAddress(1, kPublicAddrs[1]);
// Use only local ports for simplicity.
@ -2530,10 +2527,9 @@ TEST_F(P2PTransportChannelMultihomedTest,
TestConnectionSwitchDampeningControlledSide) {
rtc::ScopedFakeClock clock;
AddAddress(0, kPublicAddrs[0]);
// Adding alternate address will make sure |kPublicAddrs| has the higher
// priority than others. This is due to FakeNetwork::AddInterface method.
AddAddress(1, kAlternateAddrs[1]);
AddAddress(1, kPublicAddrs[1]);
// Simulate failing over from Wi-Fi to cell interface.
AddAddress(1, kPublicAddrs[1], "eth0", rtc::ADAPTER_TYPE_WIFI);
AddAddress(1, kAlternateAddrs[1], "wlan0", rtc::ADAPTER_TYPE_CELLULAR);
// Use only local ports for simplicity.
SetAllocatorFlags(0, kOnlyLocalPorts);
@ -2589,10 +2585,9 @@ TEST_F(P2PTransportChannelMultihomedTest,
TEST_F(P2PTransportChannelMultihomedTest,
TestConnectionSwitchDampeningControllingSide) {
rtc::ScopedFakeClock clock;
// Adding alternate address will make sure |kPublicAddrs| has the higher
// priority than others. This is due to FakeNetwork::AddInterface method.
AddAddress(0, kAlternateAddrs[0]);
AddAddress(0, kPublicAddrs[0]);
// Simulate failing over from Wi-Fi to cell interface.
AddAddress(0, kPublicAddrs[0], "eth0", rtc::ADAPTER_TYPE_WIFI);
AddAddress(0, kAlternateAddrs[0], "wlan0", rtc::ADAPTER_TYPE_CELLULAR);
AddAddress(1, kPublicAddrs[1]);
// Use only local ports for simplicity.

34
webrtc/p2p/base/portallocator.h
View File

@ -103,6 +103,12 @@ const uint32_t kDefaultStepDelay = 1000; // 1 sec step delay.
// internal. Less than 20ms is not acceptable. We choose 50ms as our default.
const uint32_t kMinimumStepDelay = 50;
// Turning on IPv6 could make many IPv6 interfaces available for connectivity
// check and delay the call setup time. kDefaultMaxIPv6Networks is the default
// upper limit of IPv6 networks but could be changed by
// set_max_ipv6_networks().
constexpr int kDefaultMaxIPv6Networks = 5;
// CF = CANDIDATE FILTER
enum {
CF_NONE = 0x0,
@ -324,14 +330,14 @@ class PortAllocatorSession : public sigslot::has_slots<> {
// passing it into an object that uses it on a different thread.
class PortAllocator : public sigslot::has_slots<> {
public:
PortAllocator() :
flags_(kDefaultPortAllocatorFlags),
min_port_(0),
max_port_(0),
step_delay_(kDefaultStepDelay),
allow_tcp_listen_(true),
candidate_filter_(CF_ALL) {
}
PortAllocator()
: flags_(kDefaultPortAllocatorFlags),
min_port_(0),
max_port_(0),
max_ipv6_networks_(kDefaultMaxIPv6Networks),
step_delay_(kDefaultStepDelay),
allow_tcp_listen_(true),
candidate_filter_(CF_ALL) {}
virtual ~PortAllocator() {}
@ -429,6 +435,17 @@ class PortAllocator : public sigslot::has_slots<> {
return true;
}
// Can be used to change the default numer of IPv6 network interfaces used
// (5). Can set to INT_MAX to effectively disable the limit.
//
// TODO(deadbeef): Applications shouldn't have to arbitrarily limit the
// number of available IPv6 network interfaces just because they could slow
// ICE down. We should work on making our ICE logic smarter (for example,
// prioritizing pinging connections that are most likely to work) so that
// every network interface can be used without impacting ICE's speed.
void set_max_ipv6_networks(int networks) { max_ipv6_networks_ = networks; }
int max_ipv6_networks() { return max_ipv6_networks_; }
uint32_t step_delay() const { return step_delay_; }
void set_step_delay(uint32_t delay) { step_delay_ = delay; }
@ -472,6 +489,7 @@ class PortAllocator : public sigslot::has_slots<> {
rtc::ProxyInfo proxy_;
int min_port_;
int max_port_;
int max_ipv6_networks_;
uint32_t step_delay_;
bool allow_tcp_listen_;
uint32_t candidate_filter_;

23
webrtc/p2p/client/basicportallocator.cc
View File

@ -588,13 +588,14 @@ std::vector<rtc::Network*> BasicPortAllocatorSession::GetNetworks() {
network_manager->GetAnyAddressNetworks(&networks);
}
}
// Do some more filtering, depending on the network ignore mask and "disable
// costly networks" flag.
networks.erase(std::remove_if(networks.begin(), networks.end(),
[this](rtc::Network* network) {
return allocator_->network_ignore_mask() &
network->type();
}),
networks.end());
if (flags() & PORTALLOCATOR_DISABLE_COSTLY_NETWORKS) {
uint16_t lowest_cost = rtc::kNetworkCostMax;
for (rtc::Network* network : networks) {
@ -607,6 +608,26 @@ std::vector<rtc::Network*> BasicPortAllocatorSession::GetNetworks() {
}),
networks.end());
}
// Lastly, if we have a limit for the number of IPv6 network interfaces (by
// default, it's 5), remove networks to ensure that limit is satisfied.
//
// TODO(deadbeef): Instead of just taking the first N arbitrary IPv6
// networks, we could try to choose a set that's "most likely to work". It's
// hard to define what that means though; it's not just "lowest cost".
// Alternatively, we could just focus on making our ICE pinging logic smarter
// such that this filtering isn't necessary in the first place.
int ipv6_networks = 0;
for (auto it = networks.begin(); it != networks.end();) {
if ((*it)->prefix().family() == AF_INET6) {
if (ipv6_networks >= allocator_->max_ipv6_networks()) {
it = networks.erase(it);
continue;
} else {
++ipv6_networks;
}
}
++it;
}
return networks;
}

54
webrtc/p2p/client/basicportallocator_unittest.cc
View File

@ -55,6 +55,9 @@ static const SocketAddress kClientIPv6Addr("2401:fa00:4:1000:be30:5bff:fee5:c3",
static const SocketAddress kClientIPv6Addr2(
"2401:fa00:4:2000:be30:5bff:fee5:c3",
0);
static const SocketAddress kClientIPv6Addr3(
"2401:fa00:4:3000:be30:5bff:fee5:c3",
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);
@ -799,6 +802,57 @@ TEST_F(BasicPortAllocatorTest, TestGatherLowCostNetworkOnly) {
EXPECT_TRUE(addr_wifi.EqualIPs(candidates_[0].address()));
}
// Test that no more than allocator.max_ipv6_networks() IPv6 networks are used
// to gather candidates.
TEST_F(BasicPortAllocatorTest, MaxIpv6NetworksLimitEnforced) {
// Add three IPv6 network interfaces, but tell the allocator to only use two.
allocator().set_max_ipv6_networks(2);
AddInterface(kClientIPv6Addr, "eth0", rtc::ADAPTER_TYPE_ETHERNET);
AddInterface(kClientIPv6Addr2, "eth1", rtc::ADAPTER_TYPE_ETHERNET);
AddInterface(kClientIPv6Addr3, "eth2", rtc::ADAPTER_TYPE_ETHERNET);
// To simplify the test, only gather UDP host candidates.
allocator().set_flags(PORTALLOCATOR_ENABLE_IPV6 | PORTALLOCATOR_DISABLE_TCP |
PORTALLOCATOR_DISABLE_STUN |
PORTALLOCATOR_DISABLE_RELAY);
EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
kDefaultAllocationTimeout, fake_clock);
EXPECT_EQ(2U, candidates_.size());
// Ensure the expected two interfaces (eth0 and eth1) were used.
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", kClientIPv6Addr);
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", kClientIPv6Addr2);
}
// Ensure that allocator.max_ipv6_networks() doesn't prevent IPv4 networks from
// being used.
TEST_F(BasicPortAllocatorTest, MaxIpv6NetworksLimitDoesNotImpactIpv4Networks) {
// Set the "max IPv6" limit to 1, adding two IPv6 and two IPv4 networks.
allocator().set_max_ipv6_networks(1);
AddInterface(kClientIPv6Addr, "eth0", rtc::ADAPTER_TYPE_ETHERNET);
AddInterface(kClientIPv6Addr2, "eth1", rtc::ADAPTER_TYPE_ETHERNET);
AddInterface(kClientAddr, "eth2", rtc::ADAPTER_TYPE_ETHERNET);
AddInterface(kClientAddr2, "eth3", rtc::ADAPTER_TYPE_ETHERNET);
// To simplify the test, only gather UDP host candidates.
allocator().set_flags(PORTALLOCATOR_ENABLE_IPV6 | PORTALLOCATOR_DISABLE_TCP |
PORTALLOCATOR_DISABLE_STUN |
PORTALLOCATOR_DISABLE_RELAY);
EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
kDefaultAllocationTimeout, fake_clock);
EXPECT_EQ(3U, candidates_.size());
// Ensure that only one IPv6 interface was used, but both IPv4 interfaces
// were used.
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", kClientIPv6Addr);
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", kClientAddr);
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", kClientAddr2);
}
// Test that we could use loopback interface as host candidate.
TEST_F(BasicPortAllocatorTest, TestLoopbackNetworkInterface) {
AddInterface(kLoopbackAddr, "test_loopback", rtc::ADAPTER_TYPE_LOOPBACK);

20
webrtc/rtc_base/network.cc
View File

@ -45,11 +45,6 @@
namespace rtc {
namespace {
// Turning on IPv6 could make many IPv6 interfaces available for connectivity
// check and delay the call setup time. kMaxIPv6Networks is the default upper
// limit of IPv6 networks but could be changed by set_max_ipv6_networks().
const int kMaxIPv6Networks = 5;
const uint32_t kUpdateNetworksMessage = 1;
const uint32_t kSignalNetworksMessage = 2;
@ -93,7 +88,7 @@ bool SortNetworks(const Network* a, const Network* b) {
// TODO(mallinath) - Add VPN and Link speed conditions while sorting.
// Networks are sorted last by key.
return a->key() > b->key();
return a->key() < b->key();
}
std::string AdapterTypeToString(AdapterType type) {
@ -175,7 +170,6 @@ bool NetworkManager::GetDefaultLocalAddress(int family, IPAddress* addr) const {
NetworkManagerBase::NetworkManagerBase()
: enumeration_permission_(NetworkManager::ENUMERATION_ALLOWED),
max_ipv6_networks_(kMaxIPv6Networks),
ipv6_enabled_(true) {
}
@ -213,18 +207,8 @@ void NetworkManagerBase::GetAnyAddressNetworks(NetworkList* networks) {
}
void NetworkManagerBase::GetNetworks(NetworkList* result) const {
int ipv6_networks = 0;
result->clear();
for (Network* network : networks_) {
// Keep the number of IPv6 networks under |max_ipv6_networks_|.
if (network->prefix().family() == AF_INET6) {
if (ipv6_networks >= max_ipv6_networks_) {
continue;
}
++ipv6_networks;
}
result->push_back(network);
}
result->insert(result->begin(), networks_.begin(), networks_.end());
}
void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks,

7
webrtc/rtc_base/network.h
View File

@ -147,13 +147,13 @@ class NetworkManagerBase : public NetworkManager {
void GetNetworks(NetworkList* networks) const override;
void GetAnyAddressNetworks(NetworkList* networks) override;
// Defaults to true.
// TODO(deadbeef): Remove this. Nothing but tests use this; IPv6 is enabled
// by default everywhere else.
bool ipv6_enabled() const { return ipv6_enabled_; }
void set_ipv6_enabled(bool enabled) { ipv6_enabled_ = enabled; }
void set_max_ipv6_networks(int networks) { max_ipv6_networks_ = networks; }
int max_ipv6_networks() { return max_ipv6_networks_; }
EnumerationPermission enumeration_permission() const override;
bool GetDefaultLocalAddress(int family, IPAddress* ipaddr) const override;
@ -187,7 +187,6 @@ class NetworkManagerBase : public NetworkManager {
EnumerationPermission enumeration_permission_;
NetworkList networks_;
int max_ipv6_networks_;
NetworkMap networks_map_;
bool ipv6_enabled_;

66
webrtc/rtc_base/network_unittest.cc
View File

@ -431,45 +431,6 @@ TEST_F(NetworkTest, TestIPv6MergeNetworkList) {
}
}
// Test that no more than manager.max_ipv6_networks() IPv6 networks get
// returned.
TEST_F(NetworkTest, TestIPv6MergeNetworkListTrimExcessive) {
BasicNetworkManager manager;
manager.SignalNetworksChanged.connect(static_cast<NetworkTest*>(this),
&NetworkTest::OnNetworksChanged);
NetworkManager::NetworkList original_list;
// Add twice the allowed number of IPv6 networks.
for (int i = 0; i < 2 * manager.max_ipv6_networks(); i++) {
// Make a network with different prefix length.
IPAddress ip;
EXPECT_TRUE(IPFromString("2401:fa01:4:1000:be30:faa:fee:faa", &ip));
IPAddress prefix = TruncateIP(ip, 64 - i);
Network* ipv6_network =
new Network("test_eth0", "Test Network Adapter 1", prefix, 64 - i);
ipv6_network->AddIP(ip);
original_list.push_back(ipv6_network);
}
// Add one IPv4 network.
Network* ipv4_network = new Network("test_eth0", "Test Network Adapter 1",
IPAddress(0x12345600U), 24);
ipv4_network->AddIP(IPAddress(0x12345600U));
original_list.push_back(ipv4_network);
bool changed = false;
MergeNetworkList(manager, original_list, &changed);
EXPECT_TRUE(changed);
NetworkManager::NetworkList list;
manager.GetNetworks(&list);
// List size should be the max allowed IPv6 networks plus one IPv4 network.
EXPECT_EQ(manager.max_ipv6_networks() + 1, (int)list.size());
// Verify that the IPv4 network is in the list.
EXPECT_NE(list.end(), std::find(list.begin(), list.end(), ipv4_network));
}
// Tests that when two network lists that describe the same set of networks are
// merged, that the changed callback is not called, and that the original
// objects remain in the result list.
@ -703,7 +664,9 @@ TEST_F(NetworkTest, MAYBE_TestIPv6Toggle) {
}
}
TEST_F(NetworkTest, TestNetworkListSorting) {
// Test that when network interfaces are sorted and given preference values,
// IPv6 comes first.
TEST_F(NetworkTest, IPv6NetworksPreferredOverIPv4) {
BasicNetworkManager manager;
Network ipv4_network1("test_eth0", "Test Network Adapter 1",
IPAddress(0x12345600U), 24);
@ -730,6 +693,29 @@ TEST_F(NetworkTest, TestNetworkListSorting) {
EXPECT_TRUE(net1->preference() < net2->preference());
}
// When two interfaces are equivalent in everything but name, they're expected
// to be preference-ordered by name. For example, "eth0" before "eth1".
TEST_F(NetworkTest, NetworksSortedByInterfaceName) {
BasicNetworkManager manager;
Network* eth0 = new Network("test_eth0", "Test Network Adapter 1",
IPAddress(0x65432100U), 24);
eth0->AddIP(IPAddress(0x65432100U));
Network* eth1 = new Network("test_eth1", "Test Network Adapter 2",
IPAddress(0x12345600U), 24);
eth1->AddIP(IPAddress(0x12345600U));
NetworkManager::NetworkList list;
// Add them to the list in the opposite of the expected sorted order, to
// ensure sorting actually occurs.
list.push_back(eth1);
list.push_back(eth0);
bool changed = false;
MergeNetworkList(manager, list, &changed);
ASSERT_TRUE(changed);
// "test_eth0" should be preferred over "test_eth1".
EXPECT_TRUE(eth0->preference() > eth1->preference());
}
TEST_F(NetworkTest, TestNetworkAdapterTypes) {
Network wifi("wlan0", "Wireless Adapter", IPAddress(0x12345600U), 24,
ADAPTER_TYPE_WIFI);