Use Netlink instead of NetworkManager (via D-bus) to monitor network

net/base/address_tracker_linux.cc

Index: net/base/address_tracker_linux.cc
diff --git a/net/base/address_tracker_linux.cc b/net/base/address_tracker_linux.cc
index af2a88dafc6da6c15a3deab72a5c7bb3ac49e71a..1c8e5e08a3a701e29bc984167e44fe833d01e8f4 100644
--- a/net/base/address_tracker_linux.cc
+++ b/net/base/address_tracker_linux.cc
@@ -4,10 +4,13 @@
 
 #include "net/base/address_tracker_linux.h"
 
+#include <linux/if.h>
+
 #include <errno.h>
 
 #include "base/eintr_wrapper.h"
 #include "base/logging.h"
+#include "base/threading/thread_restrictions.h"
 #include "net/base/network_change_notifier_linux.h"
 
 namespace net {
@@ -70,10 +73,15 @@ void CloseSocket(int fd) {
 
 }  // namespace
 
-AddressTrackerLinux::AddressTrackerLinux(const base::Closure& callback)
-    : callback_(callback),
-      netlink_fd_(-1) {
-  DCHECK(!callback.is_null());
+AddressTrackerLinux::AddressTrackerLinux(const base::Closure& address_callback,
+                                         const base::Closure& link_callback)
+    : address_callback_(address_callback),
+      link_callback_(link_callback),
+      netlink_fd_(-1),
+      is_offline_(false),
+      offline_state_initialized_(true /*manual_reset*/, false) {
+  DCHECK(!address_callback.is_null());
+  DCHECK(!link_callback.is_null());
 }
 
 AddressTrackerLinux::~AddressTrackerLinux() {
@@ -81,6 +89,55 @@ AddressTrackerLinux::~AddressTrackerLinux() {
     CloseSocket(netlink_fd_);
 }
 
+void AddressTrackerLinux::GetInitialSettings() {
+  // Request dump of addresses.
+  struct sockaddr_nl peer = {};
+  peer.nl_family = AF_NETLINK;
+
+  struct {
+    struct nlmsghdr header;
+    struct rtgenmsg msg;
+  } request = {};
+
+  request.header.nlmsg_len = NLMSG_LENGTH(sizeof(request));
+  request.header.nlmsg_type = RTM_GETADDR;
+  request.header.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+  request.header.nlmsg_pid = getpid();
+  request.msg.rtgen_family = AF_UNSPEC;
+
+  int rv = HANDLE_EINTR(sendto(netlink_fd_, &request, request.header.nlmsg_len,
+                               0, reinterpret_cast<struct sockaddr*>(&peer),
+                               sizeof(peer)));
+  if (rv < 0) {
+    PLOG(ERROR) << "Could not send NETLINK request";
+    CloseSocket(netlink_fd_);
+    netlink_fd_ = -1;
+    return;
+  }
+
+  // Consume pending message to populate the AddressMap, but don't notify.
+  // Sending another request without first reading responses results in EBUSY.
+  bool address_changed;
+  bool link_changed;
+  ReadMessages(&address_changed, &link_changed);
+
+  // Request dump of link state
+  request.header.nlmsg_type = RTM_GETLINK;
+
+  rv = HANDLE_EINTR(sendto(netlink_fd_, &request, request.header.nlmsg_len, 0,
+                           reinterpret_cast<struct sockaddr*>(&peer),
+                           sizeof(peer)));
+  if (rv < 0) {
+    PLOG(ERROR) << "Could not send NETLINK request";
+    CloseSocket(netlink_fd_);
+    netlink_fd_ = -1;
+    return;
+  }
+
+  // Consume pending message to populate links_online_, but don't notify.
+  ReadMessages(&address_changed, &link_changed);
+}
+
 void AddressTrackerLinux::Init() {
   int sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
   if (sock < 0) {
@@ -93,134 +150,152 @@ void AddressTrackerLinux::Init() {
   addr.nl_family = AF_NETLINK;
   addr.nl_pid = getpid();
   // TODO(szym): Track RTMGRP_LINK as well for ifi_type, http://crbug.com/113993
-  addr.nl_groups = RTMGRP_IPV4_IFADDR | RTMGRP_IPV6_IFADDR | RTMGRP_NOTIFY;
+  addr.nl_groups = RTMGRP_IPV4_IFADDR | RTMGRP_IPV6_IFADDR | RTMGRP_NOTIFY |
+      RTMGRP_LINK;
   int rv = bind(sock, reinterpret_cast<struct sockaddr*>(&addr), sizeof(addr));
   if (rv < 0) {
     PLOG(ERROR) << "Could not bind NETLINK socket";
     CloseSocket(sock);
     return;
   }
+  netlink_fd_ = sock;
 
-  // Watch for asynchronous messages.
-  if (SetNonBlocking(sock)) {
-    PLOG(ERROR) << "Could not make NETLINK socket non-blocking";
-    CloseSocket(sock);
-    return;
-  }
+  GetInitialSettings();
+  offline_state_initialized_.Signal();
 
   rv = MessageLoopForIO::current()->WatchFileDescriptor(
-      sock, true, MessageLoopForIO::WATCH_READ, &watcher_, this);
+      netlink_fd_, true, MessageLoopForIO::WATCH_READ, &watcher_, this);
   if (rv < 0) {
     PLOG(ERROR) << "Could not watch NETLINK socket";
-    CloseSocket(sock);
-    return;
-  }
-
-  // Request dump of addresses.
-  struct sockaddr_nl peer = {};
-  peer.nl_family = AF_NETLINK;
-
-  struct {
-    struct nlmsghdr header;
-    struct rtgenmsg msg;
-  } request = {};
-
-  request.header.nlmsg_len = NLMSG_LENGTH(sizeof(request.msg));
-  request.header.nlmsg_type = RTM_GETADDR;
-  request.header.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
-  request.header.nlmsg_pid = getpid();
-  request.msg.rtgen_family = AF_UNSPEC;
-
-  rv = HANDLE_EINTR(sendto(sock, &request, request.header.nlmsg_len, 0,
-                           reinterpret_cast<struct sockaddr*>(&peer),
-                           sizeof(peer)));
-  if (rv < 0) {
-    PLOG(ERROR) << "Could not send NETLINK request";
-    CloseSocket(sock);
+    CloseSocket(netlink_fd_);
+    netlink_fd_ = -1;

[szym, 2012/11/11 16:21:55]

Since you need to do this at least once, I suggest you set netlink_fd_ =
socket(...) up above and change CloseSocket(int) to CloseSocket() which closes
|netlink_fd_| and sets it to -1.

[pauljensen, 2012/11/12 16:45:21]

Done.

     return;
   }
-
-  netlink_fd_ = sock;
-
-  // Consume any pending messages to populate the AddressMap, but don't notify.
-  ReadMessages();
 }
 
 AddressTrackerLinux::AddressMap AddressTrackerLinux::GetAddressMap() const {
-  base::AutoLock lock(lock_);
-  return map_;
+  base::AutoLock lock(address_map_lock_);
+  return address_map_;
+}
+
+NetworkChangeNotifier::ConnectionType
+AddressTrackerLinux::GetCurrentConnectionType() {
+  // http://crbug.com/125097

[szym, 2012/11/11 16:21:55]

Is it still needed? Can't we return offline until we know we are online?

I'm guessing the concern is around unnecessary aborts of HostResolver::Request
and flushing of SocketPools during initialization. My understanding of netlink
(seen in the current AddressTrackerLinux) is that if you sendto(kernel) the
response will be immediately available for reading.

If that's not the case, then we should create an explicit mechanism to suppress
the initial signal from reaching NCN Observers, or a way for the observers to
distinguish this signal from actual change signals. AFAIK, AddressSorterPosix is
the only observer which must be told when AddressMap is complete.

Also, if that's not the case, then I think blocking on initialization makes
http://crbug.com/157162 sad, because getifaddrs probably takes as much as
blocking for kernel's response over netlink, although Android has it's own
implementation of NetworkChangeNotifier.

[pauljensen, 2012/11/12 16:45:21]

I'm hesitant to return offline when uninitialized; I feel like this might result
in some funny Chrome startup behavior like the new tab icons going from grey to
colored or if something else goes wrong ERR_INTERNET_DISCONNECTED being blamed
or maybe sync being delayed.  We really need a PENDING result but that would be
a big change.  I could maybe see returning CONNECTION_UNKNOWN, but then do we
send out a network change event when we know its really something more specific
like CONNECTION_WIFI?  I think on linux the delay is going to be very short so
this shouldn't be such a big issue.  This waiting code is moved from
NetworkManagerApi.
This seems to be the case but I could find no documentation of this, so I
switched to using a blocking socket to prevent a race.
I prevent initial signaling by calling ReadMessages() in GetInitialSettings().
Ya, on Linux this should be quite fast versus Android, and Android doesn't use
this code.

+  base::ThreadRestrictions::ScopedAllowWait allow_wait;
+  offline_state_initialized_.Wait();

[szym, 2012/11/11 16:21:55]

I'm concerned that you Wait after locking. I think you must hold the lock before
waiting on the variable.

[pauljensen, 2012/11/12 16:45:21]

I'm confused; your two sentences contradict each other.  I think we need to wait
then lock to avoid deadlocking.

+  base::AutoLock lock(is_offline_lock_);
+  // TODO(droger): Return something more detailed than CONNECTION_UNKNOWN.

[szym, 2012/11/11 16:21:55]

Could you find the bug for droger and link it here?

[pauljensen, 2012/11/12 16:45:21]

I don't think there is one.  I filed one.

+  return is_offline_ ? NetworkChangeNotifier::CONNECTION_NONE :
+                       NetworkChangeNotifier::CONNECTION_UNKNOWN;
 }
 
-bool AddressTrackerLinux::ReadMessages() {
+void AddressTrackerLinux::ReadMessages(bool* address_changed,
+                                       bool* link_changed) {
+  *address_changed = false;
+  *link_changed = false;
   char buffer[4096];
-  bool changed = false;
+  bool first_loop = true;
   for (;;) {
-    int rv = HANDLE_EINTR(recv(netlink_fd_, buffer, sizeof(buffer), 0));
+    int rv = HANDLE_EINTR(recv(netlink_fd_,
+                               buffer,
+                               sizeof(buffer),
+                               // Block the first time through loop.

[szym, 2012/11/11 16:21:55]

If we are ok with returning offline initially, you won't need to call
GetInitialSettings and ever block here.

+                               first_loop ? 0 : MSG_DONTWAIT));
+    first_loop = false;
     if (rv == 0) {
       LOG(ERROR) << "Unexpected shutdown of NETLINK socket.";
-      return false;
+      return;
     }
     if (rv < 0) {
       if ((errno == EAGAIN) || (errno == EWOULDBLOCK))
         break;
       PLOG(ERROR) << "Failed to recv from netlink socket";
-      return false;
+      return;
     }
-    changed |= HandleMessage(buffer, rv);
+    HandleMessage(buffer, rv, address_changed, link_changed);
   };
-  return changed;
+  if (*link_changed) {
+    base::AutoLock lock(is_offline_lock_);
+    is_offline_ = online_links_.empty();
+  }
 }
 
-bool AddressTrackerLinux::HandleMessage(const char* buffer, size_t length) {
+void AddressTrackerLinux::HandleMessage(const char* buffer,
+                                        size_t length,
+                                        bool* address_changed,
+                                        bool* link_changed) {
   DCHECK(buffer);
-  bool changed = false;
   for (const struct nlmsghdr* header =
           reinterpret_cast<const struct nlmsghdr*>(buffer);
        NLMSG_OK(header, length);
        header = NLMSG_NEXT(header, length)) {
     switch (header->nlmsg_type) {
       case NLMSG_DONE:
-        return changed;
-      case NLMSG_ERROR:
-        LOG(ERROR) << "Unexpected netlink error.";
-        return changed;
+        return;
+      case NLMSG_ERROR: {
+        const struct nlmsgerr* msg =
+            reinterpret_cast<struct nlmsgerr*>(NLMSG_DATA(header));
+        LOG(ERROR) << "Unexpected netlink error " << msg->error << ".";
+      } return;
       case RTM_NEWADDR: {
         IPAddressNumber address;
         if (GetAddress(header, &address)) {
-          base::AutoLock lock(lock_);
+          base::AutoLock lock(address_map_lock_);
           const struct ifaddrmsg* msg =
               reinterpret_cast<struct ifaddrmsg*>(NLMSG_DATA(header));
           // Only indicate change if the address is new or ifaddrmsg info has
           // changed.
-          AddressMap::iterator it = map_.find(address);
-          if (it == map_.end()) {
-            map_.insert(it, std::make_pair(address, *msg));
-            changed = true;
+          AddressMap::iterator it = address_map_.find(address);
+          if (it == address_map_.end()) {
+            address_map_.insert(it, std::make_pair(address, *msg));
+            *address_changed = true;
           } else if (memcmp(&it->second, msg, sizeof(*msg))) {
             it->second = *msg;
-            changed = true;
+            *address_changed = true;
           }
         }
       } break;
       case RTM_DELADDR: {
         IPAddressNumber address;
         if (GetAddress(header, &address)) {
-          base::AutoLock lock(lock_);
-          if (map_.erase(address))
-            changed = true;
+          base::AutoLock lock(address_map_lock_);
+          if (address_map_.erase(address))
+            *address_changed = true;
+        }
+      } break;
+      case RTM_NEWLINK: {
+        const struct ifinfomsg* msg =
+            reinterpret_cast<struct ifinfomsg*>(NLMSG_DATA(header));
+        if (!(msg->ifi_flags & IFF_LOOPBACK) && (msg->ifi_flags & IFF_UP) &&
+            (msg->ifi_flags & IFF_LOWER_UP) && (msg->ifi_flags & IFF_RUNNING)) {
+          if (online_links_.insert(msg->ifi_index).second)
+            *link_changed = true;
+        } else {
+          if (online_links_.erase(msg->ifi_index))
+            *link_changed = true;
         }
       } break;
+      case RTM_DELLINK: {
+        const struct ifinfomsg* msg =
+            reinterpret_cast<struct ifinfomsg*>(NLMSG_DATA(header));
+        if (online_links_.erase(msg->ifi_index))
+          *link_changed = true;
+      } break;
       default:
         break;
     }
   }
-  return changed;
 }
 
 void AddressTrackerLinux::OnFileCanReadWithoutBlocking(int fd) {
   DCHECK_EQ(netlink_fd_, fd);
-  if (ReadMessages())
-    callback_.Run();
+  bool address_changed;
+  bool link_changed;
+  ReadMessages(&address_changed, &link_changed);
+  if (address_changed)
+    address_callback_.Run();
+  if (link_changed)
+    link_callback_.Run();
 }
 
 void AddressTrackerLinux::OnFileCanWriteWithoutBlocking(int /* fd */) {}