From f3e4f27cb754552c1e28e5a0038b5c6f90e8765b Mon Sep 17 00:00:00 2001 From: zmo Date: Wed, 6 May 2015 09:55:52 -0700 Subject: [PATCH] Revert of Revert of Revert of Add functions to ignore network interfaces to NetworkChangeNotifier. (patchset #2 id:190001 of https://codereview.chromium.org/1128533003/) Reason for revert: See crbug.com/485135 If it's unrelated, I'll reland this. Original issue's description: > (Reland) Enable ignoring network interfaces in NetworkChangeNotifierLinux. > > NetworkChangeNotifier should not report network changes from interfaces > that are not used to connect to the internet (ie. wifi APs). Adjust constructor > for NetworkChangeNotifierLinux to accept a list of interfaces that are known to > not be used to connect to internet. > > R=pauljensen@chromium.org,akuegel@chromium.org,jochen@chromium.org > BUG=469863 > > Committed: https://crrev.com/09c74490237aeefe5f41c1cad07a7d1e2b384590 > Cr-Commit-Position: refs/heads/master@{#328378} TBR=pauljensen@chromium.org,akuegel@chromium.org,jochen@chromium.org,derekjchow@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=469863 Review URL: https://codereview.chromium.org/1126303002 Cr-Commit-Position: refs/heads/master@{#328550} --- .../network_change_notifier_android.cc | 3 +- net/base/address_tracker_linux.cc | 32 +-- net/base/address_tracker_linux.h | 15 +- net/base/address_tracker_linux_unittest.cc | 199 +++++------------- net/base/network_change_notifier.cc | 2 +- net/base/network_change_notifier_linux.cc | 17 +- net/base/network_change_notifier_linux.h | 10 +- net/tools/net_watcher/net_watcher.cc | 28 --- 8 files changed, 77 insertions(+), 229 deletions(-) diff --git a/net/android/network_change_notifier_android.cc b/net/android/network_change_notifier_android.cc index 982923507d8a1b..3224c632652411 100644 --- a/net/android/network_change_notifier_android.cc +++ b/net/android/network_change_notifier_android.cc @@ -78,8 +78,7 @@ class NetworkChangeNotifierAndroid::DnsConfigServiceThread address_tracker_(base::Bind(base::DoNothing), base::Bind(base::DoNothing), // We're only interested in tunnel interface changes. - base::Bind(NotifyNetworkChangeNotifierObservers), - base::hash_set()) {} + base::Bind(NotifyNetworkChangeNotifierObservers)) {} ~DnsConfigServiceThread() override { NetworkChangeNotifier::RemoveNetworkChangeObserver(this); diff --git a/net/base/address_tracker_linux.cc b/net/base/address_tracker_linux.cc index 45cd1ae36310e1..731bf4f97785eb 100644 --- a/net/base/address_tracker_linux.cc +++ b/net/base/address_tracker_linux.cc @@ -107,17 +107,14 @@ AddressTrackerLinux::AddressTrackerLinux() tracking_(false) { } -AddressTrackerLinux::AddressTrackerLinux( - const base::Closure& address_callback, - const base::Closure& link_callback, - const base::Closure& tunnel_callback, - const base::hash_set& ignored_interfaces) +AddressTrackerLinux::AddressTrackerLinux(const base::Closure& address_callback, + const base::Closure& link_callback, + const base::Closure& tunnel_callback) : get_interface_name_(GetInterfaceName), address_callback_(address_callback), link_callback_(link_callback), tunnel_callback_(tunnel_callback), netlink_fd_(-1), - ignored_interfaces_(ignored_interfaces), connection_type_initialized_(false), connection_type_initialized_cv_(&connection_type_lock_), current_connection_type_(NetworkChangeNotifier::CONNECTION_NONE), @@ -238,15 +235,6 @@ base::hash_set AddressTrackerLinux::GetOnlineLinks() const { return online_links_; } -bool AddressTrackerLinux::IsInterfaceIgnored(int interface_index) const { - if (ignored_interfaces_.empty()) - return false; - - char buf[IFNAMSIZ] = {0}; - const char* interface_name = get_interface_name_(interface_index, buf); - return ignored_interfaces_.find(interface_name) != ignored_interfaces_.end(); -} - NetworkChangeNotifier::ConnectionType AddressTrackerLinux::GetCurrentConnectionType() { // http://crbug.com/125097 @@ -310,12 +298,10 @@ void AddressTrackerLinux::HandleMessage(char* buffer, case RTM_NEWADDR: { IPAddressNumber address; bool really_deprecated; - struct ifaddrmsg* msg = - reinterpret_cast(NLMSG_DATA(header)); - if (IsInterfaceIgnored(msg->ifa_index)) - break; if (GetAddress(header, &address, &really_deprecated)) { AddressTrackerAutoLock lock(*this, address_map_lock_); + struct ifaddrmsg* msg = + reinterpret_cast(NLMSG_DATA(header)); // Routers may frequently (every few seconds) output the IPv6 ULA // prefix which can cause the linux kernel to frequently output two // back-to-back messages, one without the deprecated flag and one with @@ -339,10 +325,6 @@ void AddressTrackerLinux::HandleMessage(char* buffer, } break; case RTM_DELADDR: { IPAddressNumber address; - const struct ifaddrmsg* msg = - reinterpret_cast(NLMSG_DATA(header)); - if (IsInterfaceIgnored(msg->ifa_index)) - break; if (GetAddress(header, &address, NULL)) { AddressTrackerAutoLock lock(*this, address_map_lock_); if (address_map_.erase(address)) @@ -352,8 +334,6 @@ void AddressTrackerLinux::HandleMessage(char* buffer, case RTM_NEWLINK: { const struct ifinfomsg* msg = reinterpret_cast(NLMSG_DATA(header)); - if (IsInterfaceIgnored(msg->ifi_index)) - break; if (!(msg->ifi_flags & IFF_LOOPBACK) && (msg->ifi_flags & IFF_UP) && (msg->ifi_flags & IFF_LOWER_UP) && (msg->ifi_flags & IFF_RUNNING)) { AddressTrackerAutoLock lock(*this, online_links_lock_); @@ -374,8 +354,6 @@ void AddressTrackerLinux::HandleMessage(char* buffer, case RTM_DELLINK: { const struct ifinfomsg* msg = reinterpret_cast(NLMSG_DATA(header)); - if (IsInterfaceIgnored(msg->ifi_index)) - break; AddressTrackerAutoLock lock(*this, online_links_lock_); if (online_links_.erase(msg->ifi_index)) { *link_changed = true; diff --git a/net/base/address_tracker_linux.h b/net/base/address_tracker_linux.h index 172179923ff202..29cb8c0437d20c 100644 --- a/net/base/address_tracker_linux.h +++ b/net/base/address_tracker_linux.h @@ -44,16 +44,9 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux : // the AddressMap changes, |link_callback| when the list of online // links changes, and |tunnel_callback| when the list of online // tunnels changes. - // |ignored_interfaces| is the list of interfaces to ignore. Changes to an - // ignored interface will not cause any callback to be run. An ignored - // interface will not have entries in GetAddressMap() and GetOnlineLinks(). - // NOTE: Only ignore interfaces not used to connect to the internet. Adding - // interfaces used to connect to the internet can cause critical network - // changed signals to be lost allowing incorrect stale state to persist. AddressTrackerLinux(const base::Closure& address_callback, const base::Closure& link_callback, - const base::Closure& tunnel_callback, - const base::hash_set& ignored_interfaces); + const base::Closure& tunnel_callback); ~AddressTrackerLinux() override; // In tracking mode, it starts watching the system configuration for @@ -132,9 +125,6 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux : // Does |interface_index| refer to a tunnel interface? bool IsTunnelInterface(int interface_index) const; - // Is interface with index |interface_index| in list of ignored interfaces? - bool IsInterfaceIgnored(int interface_index) const; - // Updates current_connection_type_ based on the network list. void UpdateCurrentConnectionType(); @@ -157,9 +147,6 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux : mutable base::Lock online_links_lock_; base::hash_set online_links_; - // Set of interface names that should be ignored. - const base::hash_set ignored_interfaces_; - base::Lock connection_type_lock_; bool connection_type_initialized_; base::ConditionVariable connection_type_initialized_cv_; diff --git a/net/base/address_tracker_linux_unittest.cc b/net/base/address_tracker_linux_unittest.cc index 2e853ae4cc03c4..1eba58fbe6af35 100644 --- a/net/base/address_tracker_linux_unittest.cc +++ b/net/base/address_tracker_linux_unittest.cc @@ -20,23 +20,12 @@ namespace net { namespace internal { namespace { -const int kTestInterfaceEth = 1; const int kTestInterfaceTun = 123; -const int kTestInterfaceAp = 456; - -const char kIgnoredInterfaceName[] = "uap0"; char* TestGetInterfaceName(int interface_index, char* buf) { - if (interface_index == kTestInterfaceEth) { - snprintf(buf, IFNAMSIZ, "%s", "eth0"); - } else if (interface_index == kTestInterfaceTun) { - snprintf(buf, IFNAMSIZ, "%s", "tun0"); - } else if (interface_index == kTestInterfaceAp) { - snprintf(buf, IFNAMSIZ, "%s", kIgnoredInterfaceName); - } else { - snprintf(buf, IFNAMSIZ, "%s", ""); - } - return buf; + if (interface_index == kTestInterfaceTun) + return strncpy(buf, "tun0", IFNAMSIZ); + return strncpy(buf, "eth0", IFNAMSIZ); } } // namespace @@ -49,9 +38,9 @@ class AddressTrackerLinuxTest : public testing::Test { void InitializeAddressTracker(bool tracking) { if (tracking) { - tracker_.reset(new AddressTrackerLinux( - base::Bind(&base::DoNothing), base::Bind(&base::DoNothing), - base::Bind(&base::DoNothing), ignored_interfaces_)); + tracker_.reset(new AddressTrackerLinux(base::Bind(&base::DoNothing), + base::Bind(&base::DoNothing), + base::Bind(&base::DoNothing))); } else { tracker_.reset(new AddressTrackerLinux()); } @@ -100,11 +89,6 @@ class AddressTrackerLinuxTest : public testing::Test { return tracker_->GetOnlineLinks(); } - void IgnoreInterface(const std::string& interface_name) { - ignored_interfaces_.insert(interface_name); - } - - base::hash_set ignored_interfaces_; scoped_ptr tracker_; AddressTrackerLinux::GetInterfaceNameFunction original_get_interface_name_; }; @@ -166,7 +150,6 @@ class NetlinkMessage { void MakeAddrMessageWithCacheInfo(uint16 type, uint8 flags, uint8 family, - int index, const IPAddressNumber& address, const IPAddressNumber& local, uint32 preferred_lifetime, @@ -175,7 +158,6 @@ void MakeAddrMessageWithCacheInfo(uint16 type, struct ifaddrmsg msg = {}; msg.ifa_family = family; msg.ifa_flags = flags; - msg.ifa_index = index; nlmsg.AddPayload(&msg, sizeof(msg)); if (address.size()) nlmsg.AddAttribute(IFA_ADDRESS, &address[0], address.size()); @@ -191,11 +173,10 @@ void MakeAddrMessageWithCacheInfo(uint16 type, void MakeAddrMessage(uint16 type, uint8 flags, uint8 family, - int index, const IPAddressNumber& address, const IPAddressNumber& local, Buffer* output) { - MakeAddrMessageWithCacheInfo(type, flags, family, index, address, local, + MakeAddrMessageWithCacheInfo(type, flags, family, address, local, INFINITY_LIFE_TIME, output); } @@ -225,8 +206,8 @@ TEST_F(AddressTrackerLinuxTest, NewAddress) { const IPAddressNumber kAddr3(kAddress3, kAddress3 + arraysize(kAddress3)); Buffer buffer; - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); AddressTrackerLinux::AddressMap map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -234,8 +215,8 @@ TEST_F(AddressTrackerLinuxTest, NewAddress) { EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags); buffer.clear(); - MakeAddrMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kTestInterfaceEth, - kAddr1, kAddr2, &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kAddr1, kAddr2, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(2u, map.size()); @@ -244,8 +225,7 @@ TEST_F(AddressTrackerLinuxTest, NewAddress) { EXPECT_EQ(IFA_F_HOMEADDRESS, map[kAddr2].ifa_flags); buffer.clear(); - MakeAddrMessage(RTM_NEWADDR, 0, AF_INET6, kTestInterfaceEth, kEmpty, kAddr3, - &buffer); + MakeAddrMessage(RTM_NEWADDR, 0, AF_INET6, kEmpty, kAddr3, &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(3u, map.size()); @@ -259,8 +239,8 @@ TEST_F(AddressTrackerLinuxTest, NewAddressChange) { const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0)); Buffer buffer; - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); AddressTrackerLinux::AddressMap map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -268,8 +248,8 @@ TEST_F(AddressTrackerLinuxTest, NewAddressChange) { EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags); buffer.clear(); - MakeAddrMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kAddr0, kEmpty, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -278,10 +258,10 @@ TEST_F(AddressTrackerLinuxTest, NewAddressChange) { // Both messages in one buffer. buffer.clear(); - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); - MakeAddrMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, + &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kAddr0, kEmpty, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -294,8 +274,8 @@ TEST_F(AddressTrackerLinuxTest, NewAddressDuplicate) { const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0)); Buffer buffer; - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceEth, - kAddr0, kAddr0, &buffer); + MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kAddr0, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); AddressTrackerLinux::AddressMap map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -317,17 +297,14 @@ TEST_F(AddressTrackerLinuxTest, DeleteAddress) { const IPAddressNumber kAddr2(kAddress2, kAddress2 + arraysize(kAddress2)); Buffer buffer; - MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kTestInterfaceEth, kAddr0, kEmpty, - &buffer); - MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kTestInterfaceEth, kAddr1, kAddr2, - &buffer); + MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kAddr0, kEmpty, &buffer); + MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kAddr1, kAddr2, &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); AddressTrackerLinux::AddressMap map = GetAddressMap(); EXPECT_EQ(2u, map.size()); buffer.clear(); - MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kTestInterfaceEth, kEmpty, kAddr0, - &buffer); + MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kEmpty, kAddr0, &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -335,16 +312,14 @@ TEST_F(AddressTrackerLinuxTest, DeleteAddress) { EXPECT_EQ(1u, map.count(kAddr2)); buffer.clear(); - MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kTestInterfaceEth, kAddr2, kAddr1, - &buffer); + MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kAddr2, kAddr1, &buffer); // kAddr1 does not exist in the map. EXPECT_FALSE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); buffer.clear(); - MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kTestInterfaceEth, kAddr2, kEmpty, - &buffer); + MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kAddr2, kEmpty, &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(0u, map.size()); @@ -357,8 +332,7 @@ TEST_F(AddressTrackerLinuxTest, DeprecatedLifetime) { const IPAddressNumber kAddr3(kAddress3, kAddress3 + arraysize(kAddress3)); Buffer buffer; - MakeAddrMessage(RTM_NEWADDR, 0, AF_INET6, kTestInterfaceEth, kEmpty, kAddr3, - &buffer); + MakeAddrMessage(RTM_NEWADDR, 0, AF_INET6, kEmpty, kAddr3, &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); AddressTrackerLinux::AddressMap map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -367,8 +341,8 @@ TEST_F(AddressTrackerLinuxTest, DeprecatedLifetime) { // Verify 0 preferred lifetime implies deprecated. buffer.clear(); - MakeAddrMessageWithCacheInfo(RTM_NEWADDR, 0, AF_INET6, kTestInterfaceEth, - kEmpty, kAddr3, 0, &buffer); + MakeAddrMessageWithCacheInfo(RTM_NEWADDR, 0, AF_INET6, kEmpty, kAddr3, 0, + &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -376,8 +350,8 @@ TEST_F(AddressTrackerLinuxTest, DeprecatedLifetime) { // Verify properly flagged message doesn't imply change. buffer.clear(); - MakeAddrMessageWithCacheInfo(RTM_NEWADDR, IFA_F_DEPRECATED, AF_INET6, - kTestInterfaceEth, kEmpty, kAddr3, 0, &buffer); + MakeAddrMessageWithCacheInfo(RTM_NEWADDR, IFA_F_DEPRECATED, AF_INET6, kEmpty, + kAddr3, 0, &buffer); EXPECT_FALSE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -385,8 +359,8 @@ TEST_F(AddressTrackerLinuxTest, DeprecatedLifetime) { // Verify implied deprecated doesn't imply change. buffer.clear(); - MakeAddrMessageWithCacheInfo(RTM_NEWADDR, 0, AF_INET6, kTestInterfaceEth, - kEmpty, kAddr3, 0, &buffer); + MakeAddrMessageWithCacheInfo(RTM_NEWADDR, 0, AF_INET6, kEmpty, kAddr3, 0, + &buffer); EXPECT_FALSE(HandleAddressMessage(buffer)); map = GetAddressMap(); EXPECT_EQ(1u, map.size()); @@ -402,14 +376,11 @@ TEST_F(AddressTrackerLinuxTest, IgnoredMessage) { Buffer buffer; // Ignored family. - MakeAddrMessage(RTM_NEWADDR, 0, AF_UNSPEC, kTestInterfaceEth, kAddr3, kAddr0, - &buffer); + MakeAddrMessage(RTM_NEWADDR, 0, AF_UNSPEC, kAddr3, kAddr0, &buffer); // No address. - MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kTestInterfaceEth, kEmpty, kEmpty, - &buffer); + MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kEmpty, kEmpty, &buffer); // Ignored type. - MakeAddrMessage(RTM_DELROUTE, 0, AF_INET6, kTestInterfaceEth, kAddr3, kEmpty, - &buffer); + MakeAddrMessage(RTM_DELROUTE, 0, AF_INET6, kAddr3, kEmpty, &buffer); EXPECT_FALSE(HandleAddressMessage(buffer)); EXPECT_TRUE(GetAddressMap().empty()); @@ -436,41 +407,37 @@ TEST_F(AddressTrackerLinuxTest, AddInterface) { // Ignores loopback. MakeLinkMessage(RTM_NEWLINK, IFF_LOOPBACK | IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + 0, &buffer); EXPECT_FALSE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); // Ignores not IFF_LOWER_UP. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_RUNNING, kTestInterfaceEth, - &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_RUNNING, 0, &buffer); EXPECT_FALSE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); // Ignores deletion. - MakeLinkMessage(RTM_DELLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + MakeLinkMessage(RTM_DELLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_FALSE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); // Verify success. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); - EXPECT_EQ(1u, GetOnlineLinks().count(kTestInterfaceEth)); + EXPECT_EQ(1u, GetOnlineLinks().count(0)); EXPECT_EQ(1u, GetOnlineLinks().size()); // Ignores redundant enables. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_FALSE(HandleLinkMessage(buffer)); - EXPECT_EQ(1u, GetOnlineLinks().count(kTestInterfaceEth)); + EXPECT_EQ(1u, GetOnlineLinks().count(0)); EXPECT_EQ(1u, GetOnlineLinks().size()); // Verify adding another online device (e.g. VPN) is considered a change. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 2, &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 1, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); - EXPECT_EQ(1u, GetOnlineLinks().count(kTestInterfaceEth)); - EXPECT_EQ(1u, GetOnlineLinks().count(2)); + EXPECT_EQ(1u, GetOnlineLinks().count(0)); + EXPECT_EQ(1u, GetOnlineLinks().count(1)); EXPECT_EQ(2u, GetOnlineLinks().size()); } @@ -480,82 +447,32 @@ TEST_F(AddressTrackerLinuxTest, RemoveInterface) { Buffer buffer; // Should disappear when not IFF_LOWER_UP. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); EXPECT_FALSE(GetOnlineLinks().empty()); - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_RUNNING, kTestInterfaceEth, - &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_RUNNING, 0, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); // Ignores redundant disables. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_RUNNING, kTestInterfaceEth, - &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_RUNNING, 0, &buffer); EXPECT_FALSE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); // Ignores deleting down interfaces. - MakeLinkMessage(RTM_DELLINK, IFF_UP | IFF_RUNNING, kTestInterfaceEth, - &buffer); + MakeLinkMessage(RTM_DELLINK, IFF_UP | IFF_RUNNING, 0, &buffer); EXPECT_FALSE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); // Should disappear when deleted. - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); EXPECT_FALSE(GetOnlineLinks().empty()); - MakeLinkMessage(RTM_DELLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); + MakeLinkMessage(RTM_DELLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); EXPECT_TRUE(GetOnlineLinks().empty()); } -TEST_F(AddressTrackerLinuxTest, IgnoreInterface) { - IgnoreInterface(kIgnoredInterfaceName); - InitializeAddressTracker(true); - - Buffer buffer; - const IPAddressNumber kEmpty; - const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0)); - - // Verify online links and address map has been not been updated - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceAp, - kAddr0, kEmpty, &buffer); - EXPECT_FALSE(HandleAddressMessage(buffer)); - AddressTrackerLinux::AddressMap map = GetAddressMap(); - EXPECT_EQ(0u, map.size()); - EXPECT_EQ(0u, map.count(kAddr0)); - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceAp, &buffer); - EXPECT_FALSE(HandleLinkMessage(buffer)); - EXPECT_EQ(0u, GetOnlineLinks().count(kTestInterfaceAp)); - EXPECT_EQ(0u, GetOnlineLinks().size()); -} - -TEST_F(AddressTrackerLinuxTest, IgnoreInterface_NonIgnoredInterface) { - IgnoreInterface(kIgnoredInterfaceName); - InitializeAddressTracker(true); - - Buffer buffer; - const IPAddressNumber kEmpty; - const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0)); - - // Verify eth0 is not ignored when only uap0 is ignored - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); - EXPECT_TRUE(HandleAddressMessage(buffer)); - AddressTrackerLinux::AddressMap map = GetAddressMap(); - EXPECT_EQ(1u, map.size()); - EXPECT_EQ(1u, map.count(kAddr0)); - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, - kTestInterfaceEth, &buffer); - EXPECT_TRUE(HandleLinkMessage(buffer)); - EXPECT_EQ(1u, GetOnlineLinks().count(kTestInterfaceEth)); - EXPECT_EQ(1u, GetOnlineLinks().size()); -} - TEST_F(AddressTrackerLinuxTest, TunnelInterface) { InitializeAddressTracker(true); @@ -564,7 +481,7 @@ TEST_F(AddressTrackerLinuxTest, TunnelInterface) { // Ignores without "tun" prefixed name. MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING | IFF_POINTOPOINT, - kTestInterfaceEth, &buffer); + 0, &buffer); EXPECT_FALSE(HandleTunnelMessage(buffer)); // Verify success. @@ -616,17 +533,17 @@ TEST_F(AddressTrackerLinuxTest, NonTrackingMode) { const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0)); Buffer buffer; - MakeAddrMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kTestInterfaceEth, - kAddr0, kEmpty, &buffer); + MakeAddrMessage( + RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, &buffer); EXPECT_TRUE(HandleAddressMessage(buffer)); AddressTrackerLinux::AddressMap map = GetAddressMap(); EXPECT_EQ(1u, map.size()); EXPECT_EQ(1u, map.count(kAddr0)); EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags); - MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 1, &buffer); + MakeLinkMessage(RTM_NEWLINK, IFF_UP | IFF_LOWER_UP | IFF_RUNNING, 0, &buffer); EXPECT_TRUE(HandleLinkMessage(buffer)); - EXPECT_EQ(1u, GetOnlineLinks().count(1)); + EXPECT_EQ(1u, GetOnlineLinks().count(0)); EXPECT_EQ(1u, GetOnlineLinks().size()); } diff --git a/net/base/network_change_notifier.cc b/net/base/network_change_notifier.cc index 7586953f9dc937..52f348ef51c769 100644 --- a/net/base/network_change_notifier.cc +++ b/net/base/network_change_notifier.cc @@ -519,7 +519,7 @@ NetworkChangeNotifier* NetworkChangeNotifier::Create() { #endif return NULL; #elif defined(OS_LINUX) - return new NetworkChangeNotifierLinux(base::hash_set()); + return NetworkChangeNotifierLinux::Create(); #elif defined(OS_MACOSX) return new NetworkChangeNotifierMac(); #else diff --git a/net/base/network_change_notifier_linux.cc b/net/base/network_change_notifier_linux.cc index 7e471726d9aaaa..82daf7fffa0792 100644 --- a/net/base/network_change_notifier_linux.cc +++ b/net/base/network_change_notifier_linux.cc @@ -14,7 +14,7 @@ namespace net { class NetworkChangeNotifierLinux::Thread : public base::Thread { public: - explicit Thread(const base::hash_set& ignored_interfaces); + Thread(); ~Thread() override; // Plumbing for NetworkChangeNotifier::GetCurrentConnectionType. @@ -43,16 +43,14 @@ class NetworkChangeNotifierLinux::Thread : public base::Thread { DISALLOW_COPY_AND_ASSIGN(Thread); }; -NetworkChangeNotifierLinux::Thread::Thread( - const base::hash_set& ignored_interfaces) +NetworkChangeNotifierLinux::Thread::Thread() : base::Thread("NetworkChangeNotifier"), address_tracker_( base::Bind(&NetworkChangeNotifierLinux::Thread::OnIPAddressChanged, base::Unretained(this)), base::Bind(&NetworkChangeNotifierLinux::Thread::OnLinkChanged, base::Unretained(this)), - base::Bind(base::DoNothing), - ignored_interfaces), + base::Bind(base::DoNothing)), last_type_(NetworkChangeNotifier::CONNECTION_NONE) { } @@ -85,10 +83,13 @@ void NetworkChangeNotifierLinux::Thread::OnLinkChanged() { } } -NetworkChangeNotifierLinux::NetworkChangeNotifierLinux( - const base::hash_set& ignored_interfaces) +NetworkChangeNotifierLinux* NetworkChangeNotifierLinux::Create() { + return new NetworkChangeNotifierLinux(); +} + +NetworkChangeNotifierLinux::NetworkChangeNotifierLinux() : NetworkChangeNotifier(NetworkChangeCalculatorParamsLinux()), - notifier_thread_(new Thread(ignored_interfaces)) { + notifier_thread_(new Thread()) { // We create this notifier thread because the notification implementation // needs a MessageLoopForIO, and there's no guarantee that // MessageLoop::current() meets that criterion. diff --git a/net/base/network_change_notifier_linux.h b/net/base/network_change_notifier_linux.h index 0da7d0c695c174..5a0f6dff308afe 100644 --- a/net/base/network_change_notifier_linux.h +++ b/net/base/network_change_notifier_linux.h @@ -16,18 +16,12 @@ namespace net { class NET_EXPORT_PRIVATE NetworkChangeNotifierLinux : public NetworkChangeNotifier { public: - // Creates NetworkChangeNotifierLinux with a list of ignored interfaces. - // |ignored_interfaces| is the list of interfaces to ignore. An ignored - // interface will not trigger IP address or connection type notifications. - // NOTE: Only ignore interfaces not used to connect to the internet. Adding - // interfaces used to connect to the internet can cause critical network - // changed signals to be lost allowing incorrect stale state to persist. - explicit NetworkChangeNotifierLinux( - const base::hash_set& ignored_interfaces); + static NetworkChangeNotifierLinux* Create(); private: class Thread; + NetworkChangeNotifierLinux(); ~NetworkChangeNotifierLinux() override; static NetworkChangeCalculatorParams NetworkChangeCalculatorParamsLinux(); diff --git a/net/tools/net_watcher/net_watcher.cc b/net/tools/net_watcher/net_watcher.cc index 8d1d056c5afef3..1643bc4a18f80f 100644 --- a/net/tools/net_watcher/net_watcher.cc +++ b/net/tools/net_watcher/net_watcher.cc @@ -14,7 +14,6 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" -#include "base/strings/string_split.h" #include "base/values.h" #include "build/build_config.h" #include "net/base/network_change_notifier.h" @@ -26,22 +25,12 @@ #include #endif -#if defined(OS_LINUX) && !defined(OS_CHROMEOS) -#include "net/base/network_change_notifier_linux.h" -#endif - #if defined(OS_MACOSX) #include "base/mac/scoped_nsautorelease_pool.h" #endif namespace { -#if defined(OS_LINUX) && !defined(OS_CHROMEOS) -// Flag to specifies which network interfaces to ignore. Interfaces should -// follow as a comma seperated list. -const char kIgnoreNetifFlag[] = "ignore-netif"; -#endif - // Conversions from various network-related types to string. const char* ConnectionTypeToString( @@ -164,25 +153,8 @@ int main(int argc, char* argv[]) { NetWatcher net_watcher; -#if defined(OS_LINUX) && !defined(OS_CHROMEOS) - base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); - std::string ignored_netifs_str = - command_line->GetSwitchValueASCII(kIgnoreNetifFlag); - base::hash_set ignored_interfaces; - if (!ignored_netifs_str.empty()) { - std::vector ignored_netifs; - base::SplitString(ignored_netifs_str, ',', &ignored_netifs); - for (const std::string& ignored_netif : ignored_netifs) { - LOG(INFO) << "Ignoring: " << ignored_netif; - ignored_interfaces.insert(ignored_netif); - } - } - scoped_ptr network_change_notifier( - new net::NetworkChangeNotifierLinux(ignored_interfaces)); -#else scoped_ptr network_change_notifier( net::NetworkChangeNotifier::Create()); -#endif // Use the network loop as the file loop also. scoped_ptr proxy_config_service(