diff options
author | honghaiz <honghaiz@webrtc.org> | 2015-12-21 13:08:46 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-21 21:08:54 +0000 |
commit | db8cf50c5994a1ca37f85d7ea17cadb96c1fb93a (patch) | |
tree | 8212ca49a3c31fc7cd803d1da5498663177332e3 /webrtc | |
parent | 1227e8b3451b1a2f2a765bf6101cb0862f625568 (diff) | |
download | webrtc-db8cf50c5994a1ca37f85d7ea17cadb96c1fb93a.tar.gz |
Fix two problems in network.cc:
1. It signals network changed events whenever there are more than one IP address in a network.
2. It does not signal network changed events if a network disconnects and connects again.
Also changed DumpNetworks for better debugging.
BUG=webrtc:5096
Review URL: https://codereview.webrtc.org/1421433003
Cr-Commit-Position: refs/heads/master@{#11107}
Diffstat (limited to 'webrtc')
-rw-r--r-- | webrtc/base/network.cc | 60 | ||||
-rw-r--r-- | webrtc/base/network.h | 15 | ||||
-rw-r--r-- | webrtc/base/network_unittest.cc | 111 |
3 files changed, 152 insertions, 34 deletions
diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc index 678541d0b1..2dcfa17623 100644 --- a/webrtc/base/network.cc +++ b/webrtc/base/network.cc @@ -242,20 +242,12 @@ void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks, void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks, bool* changed, NetworkManager::Stats* stats) { + *changed = false; // AddressList in this map will track IP addresses for all Networks // with the same key. std::map<std::string, AddressList> consolidated_address_list; NetworkList list(new_networks); - - // Result of Network merge. Element in this list should have unique key. - NetworkList merged_list; std::sort(list.begin(), list.end(), CompareNetworks); - - *changed = false; - - if (networks_.size() != list.size()) - *changed = true; - // First, build a set of network-keys to the ipaddresses. for (Network* network : list) { bool might_add_to_merged_list = false; @@ -287,6 +279,8 @@ void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks, } // Next, look for existing network objects to re-use. + // Result of Network merge. Element in this list should have unique key. + NetworkList merged_list; for (const auto& kv : consolidated_address_list) { const std::string& key = kv.first; Network* net = kv.second.net; @@ -301,17 +295,36 @@ void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks, *changed = true; } else { // This network exists in the map already. Reset its IP addresses. - *changed = existing->second->SetIPs(kv.second.ips, *changed); - merged_list.push_back(existing->second); - if (existing->second != net) { + Network* existing_net = existing->second; + *changed = existing_net->SetIPs(kv.second.ips, *changed); + merged_list.push_back(existing_net); + // If the existing network was not active, networks have changed. + if (!existing_net->active()) { + *changed = true; + } + ASSERT(net->active()); + if (existing_net != net) { delete net; } } } - networks_ = merged_list; + // It may still happen that the merged list is a subset of |networks_|. + // To detect this change, we compare their sizes. + if (merged_list.size() != networks_.size()) { + *changed = true; + } - // If the network lists changes, we resort it. + // If the network list changes, we re-assign |networks_| to the merged list + // and re-sort it. if (*changed) { + networks_ = merged_list; + // Reset the active states of all networks. + for (const auto& kv : networks_map_) { + kv.second->set_active(false); + } + for (Network* network : networks_) { + network->set_active(true); + } std::sort(networks_.begin(), networks_.end(), SortNetworks); // Now network interfaces are sorted, we should set the preference value // for each of the interfaces we are planning to use. @@ -450,6 +463,7 @@ void BasicNetworkManager::ConvertIfAddrs(struct ifaddrs* interfaces, network->AddIP(ip); network->set_ignored(IsIgnoredNetwork(*network)); if (include_ignored || !network->ignored()) { + current_networks[key] = network.get(); networks->push_back(network.release()); } } else { @@ -604,6 +618,7 @@ bool BasicNetworkManager::CreateNetworks(bool include_ignored, bool ignored = IsIgnoredNetwork(*network); network->set_ignored(ignored); if (include_ignored || !network->ignored()) { + current_networks[key] = network.get(); networks->push_back(network.release()); } } else { @@ -801,21 +816,14 @@ void BasicNetworkManager::UpdateNetworksContinually() { thread_->PostDelayed(kNetworksUpdateIntervalMs, this, kUpdateNetworksMessage); } -void BasicNetworkManager::DumpNetworks(bool include_ignored) { +void BasicNetworkManager::DumpNetworks() { NetworkList list; - CreateNetworks(include_ignored, &list); + GetNetworks(&list); LOG(LS_INFO) << "NetworkManager detected " << list.size() << " networks:"; for (const Network* network : list) { - if (!network->ignored() || include_ignored) { - LOG(LS_INFO) << network->ToString() << ": " - << network->description() - << ((network->ignored()) ? ", Ignored" : ""); - } - } - // Release the network list created previously. - // Do this in a seperated for loop for better readability. - for (Network* network : list) { - delete network; + LOG(LS_INFO) << network->ToString() << ": " << network->description() + << ", active ? " << network->active() + << ((network->ignored()) ? ", Ignored" : ""); } } diff --git a/webrtc/base/network.h b/webrtc/base/network.h index 8980b5d57d..d2e013e0cf 100644 --- a/webrtc/base/network.h +++ b/webrtc/base/network.h @@ -111,11 +111,10 @@ class NetworkManager : public DefaultLocalAddressProvider { // TODO(guoweis): remove this body when chromium implements this. virtual void GetAnyAddressNetworks(NetworkList* networks) {} + // Dumps the current list of networks in the network manager. + virtual void DumpNetworks() {} bool GetDefaultLocalAddress(int family, IPAddress* ipaddr) const override; - // Dumps a list of networks available to LS_INFO. - virtual void DumpNetworks(bool include_ignored) {} - struct Stats { int ipv4_network_count; int ipv6_network_count; @@ -195,8 +194,7 @@ class BasicNetworkManager : public NetworkManagerBase, void StartUpdating() override; void StopUpdating() override; - // Logs the available networks. - void DumpNetworks(bool include_ignored) override; + void DumpNetworks() override; // MessageHandler interface. void OnMessage(Message* msg) override; @@ -359,6 +357,12 @@ class Network { int preference() const { return preference_; } void set_preference(int preference) { preference_ = preference; } + // When we enumerate networks and find a previously-seen network is missing, + // we do not remove it (because it may be used elsewhere). Instead, we mark + // it inactive, so that we can detect network changes properly. + bool active() const { return active_; } + void set_active(bool active) { active_ = active; } + // Debugging description of this network std::string ToString() const; @@ -374,6 +378,7 @@ class Network { bool ignored_; AdapterType type_; int preference_; + bool active_ = true; friend class NetworkManager; }; diff --git a/webrtc/base/network_unittest.cc b/webrtc/base/network_unittest.cc index cfba62b306..5ae7589d18 100644 --- a/webrtc/base/network_unittest.cc +++ b/webrtc/base/network_unittest.cc @@ -91,6 +91,46 @@ class NetworkTest : public testing::Test, public sigslot::has_slots<> { NetworkManager::NetworkList* networks) { network_manager.ConvertIfAddrs(interfaces, include_ignored, networks); } + + struct sockaddr_in6* CreateIpv6Addr(const std::string& ip_string, + uint32_t scope_id) { + struct sockaddr_in6* ipv6_addr = new struct sockaddr_in6; + memset(ipv6_addr, 0, sizeof(struct sockaddr_in6)); + ipv6_addr->sin6_family = AF_INET6; + ipv6_addr->sin6_scope_id = scope_id; + IPAddress ip; + IPFromString(ip_string, &ip); + ipv6_addr->sin6_addr = ip.ipv6_address(); + return ipv6_addr; + } + + // Pointers created here need to be released via ReleaseIfAddrs. + struct ifaddrs* AddIpv6Address(struct ifaddrs* list, + char* if_name, + const std::string& ipv6_address, + const std::string& ipv6_netmask, + uint32_t scope_id) { + struct ifaddrs* if_addr = new struct ifaddrs; + memset(if_addr, 0, sizeof(struct ifaddrs)); + if_addr->ifa_name = if_name; + if_addr->ifa_addr = reinterpret_cast<struct sockaddr*>( + CreateIpv6Addr(ipv6_address, scope_id)); + if_addr->ifa_netmask = + reinterpret_cast<struct sockaddr*>(CreateIpv6Addr(ipv6_netmask, 0)); + if_addr->ifa_next = list; + return if_addr; + } + + void ReleaseIfAddrs(struct ifaddrs* list) { + struct ifaddrs* if_addr = list; + while (if_addr != nullptr) { + struct ifaddrs* next_addr = if_addr->ifa_next; + delete if_addr->ifa_addr; + delete if_addr->ifa_netmask; + delete if_addr; + if_addr = next_addr; + } + } #endif // defined(WEBRTC_POSIX) protected: @@ -590,10 +630,13 @@ TEST_F(NetworkTest, TestMultiplePublicNetworksOnOneInterfaceMerge) { } } -// Test that DumpNetworks works. -TEST_F(NetworkTest, TestDumpNetworks) { +// Test that DumpNetworks does not crash. +TEST_F(NetworkTest, TestCreateAndDumpNetworks) { BasicNetworkManager manager; - manager.DumpNetworks(true); + NetworkManager::NetworkList list = GetNetworks(manager, true); + bool changed; + MergeNetworkList(manager, list, &changed); + manager.DumpNetworks(); } // Test that we can toggle IPv6 on and off. @@ -700,6 +743,25 @@ TEST_F(NetworkTest, TestConvertIfAddrsNoAddress) { CallConvertIfAddrs(manager, &list, true, &result); EXPECT_TRUE(result.empty()); } + +// Verify that if there are two addresses on one interface, only one network +// is generated. +TEST_F(NetworkTest, TestConvertIfAddrsMultiAddressesOnOneInterface) { + char if_name[20] = "rmnet0"; + ifaddrs* list = nullptr; + list = AddIpv6Address(list, if_name, "1000:2000:3000:4000:0:0:0:1", + "FFFF:FFFF:FFFF:FFFF::", 0); + list = AddIpv6Address(list, if_name, "1000:2000:3000:4000:0:0:0:2", + "FFFF:FFFF:FFFF:FFFF::", 0); + NetworkManager::NetworkList result; + BasicNetworkManager manager; + CallConvertIfAddrs(manager, list, true, &result); + EXPECT_EQ(1U, result.size()); + bool changed; + // This ensures we release the objects created in CallConvertIfAddrs. + MergeNetworkList(manager, result, &changed); + ReleaseIfAddrs(list); +} #endif // defined(WEBRTC_POSIX) #if defined(WEBRTC_LINUX) && !defined(WEBRTC_ANDROID) @@ -783,6 +845,49 @@ TEST_F(NetworkTest, TestMergeNetworkList) { EXPECT_EQ(list2[0]->GetIPs()[1], ip2); } +// Test that MergeNetworkList successfully detects the change if +// a network becomes inactive and then active again. +TEST_F(NetworkTest, TestMergeNetworkListWithInactiveNetworks) { + BasicNetworkManager manager; + Network network1("test_wifi", "Test Network Adapter 1", + IPAddress(0x12345600U), 24); + Network network2("test_eth0", "Test Network Adapter 2", + IPAddress(0x00010000U), 16); + network1.AddIP(IPAddress(0x12345678)); + network2.AddIP(IPAddress(0x00010004)); + NetworkManager::NetworkList list; + Network* net1 = new Network(network1); + list.push_back(net1); + bool changed; + MergeNetworkList(manager, list, &changed); + EXPECT_TRUE(changed); + list.clear(); + manager.GetNetworks(&list); + ASSERT_EQ(1U, list.size()); + EXPECT_EQ(net1, list[0]); + + list.clear(); + Network* net2 = new Network(network2); + list.push_back(net2); + MergeNetworkList(manager, list, &changed); + EXPECT_TRUE(changed); + list.clear(); + manager.GetNetworks(&list); + ASSERT_EQ(1U, list.size()); + EXPECT_EQ(net2, list[0]); + + // Now network1 is inactive. Try to merge it again. + list.clear(); + list.push_back(new Network(network1)); + MergeNetworkList(manager, list, &changed); + EXPECT_TRUE(changed); + list.clear(); + manager.GetNetworks(&list); + ASSERT_EQ(1U, list.size()); + EXPECT_TRUE(list[0]->active()); + EXPECT_EQ(net1, list[0]); +} + // Test that the filtering logic follows the defined ruleset in network.h. TEST_F(NetworkTest, TestIPv6Selection) { InterfaceAddress ip; |