Skip to content

Commit

Permalink
Revert of Revert of Revert of Add functions to ignore network interfa…
Browse files Browse the repository at this point in the history 
…ces 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}
  • Loading branch information
@zhenyao
zhenyao authored and Commit bot committed May 6, 2015
1 parent c2f94a8 commit f3e4f27
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 229 deletions.
3 changes: 1 addition & 2 deletions net/android/network_change_notifier_android.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>()) {}
base::Bind(NotifyNetworkChangeNotifierObservers)) {}

~DnsConfigServiceThread() override {
NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
Expand Down
32 changes: 5 additions & 27 deletions net/base/address_tracker_linux.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>& 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),
Expand Down Expand Up @@ -238,15 +235,6 @@ base::hash_set<int> 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
Expand Down Expand Up @@ -310,12 +298,10 @@ void AddressTrackerLinux::HandleMessage(char* buffer,
case RTM_NEWADDR: {
IPAddressNumber address;
bool really_deprecated;
struct ifaddrmsg* msg =
reinterpret_cast<struct ifaddrmsg*>(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<struct ifaddrmsg*>(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
Expand All @@ -339,10 +325,6 @@ void AddressTrackerLinux::HandleMessage(char* buffer,
} break;
case RTM_DELADDR: {
IPAddressNumber address;
const struct ifaddrmsg* msg =
reinterpret_cast<struct ifaddrmsg*>(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))
Expand All @@ -352,8 +334,6 @@ void AddressTrackerLinux::HandleMessage(char* buffer,
case RTM_NEWLINK: {
const struct ifinfomsg* msg =
reinterpret_cast<struct ifinfomsg*>(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_);
Expand All @@ -374,8 +354,6 @@ void AddressTrackerLinux::HandleMessage(char* buffer,
case RTM_DELLINK: {
const struct ifinfomsg* msg =
reinterpret_cast<struct ifinfomsg*>(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;
Expand Down
15 changes: 1 addition & 14 deletions net/base/address_tracker_linux.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>& ignored_interfaces);
const base::Closure& tunnel_callback);
~AddressTrackerLinux() override;

// In tracking mode, it starts watching the system configuration for
Expand Down Expand Up @@ -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();

Expand All @@ -157,9 +147,6 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux :
mutable base::Lock online_links_lock_;
base::hash_set<int> online_links_;

// Set of interface names that should be ignored.
const base::hash_set<std::string> ignored_interfaces_;

base::Lock connection_type_lock_;
bool connection_type_initialized_;
base::ConditionVariable connection_type_initialized_cv_;
Expand Down
Loading

0 comments on commit f3e4f27

Please sign in to comment.