netdev: Make netdev_find_dev_by_in4() use netdevs, not device names.

The netdev_nodev_*() functions have always been a bit of a kluge.  It's
better to keep a network device open than to open it every time that it is
needed.

Also updates the only user of netdev_find_dev_by_in4().

diff --git a/lib/netdev.c b/lib/netdev.c
index 701bb53..a36ab97 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -1315,43 +1315,32 @@ netdev_enumerate(struct svec *svec)
     }
 }
 
-/* Attempts to locate a device based on its IPv4 address.  The caller
- * may provide a hint as to the device by setting 'netdev_name' to a
- * likely device name.  This string must be malloc'd, since if it is 
- * not correct then it will be freed.  If there is no hint, then
- * 'netdev_name' must be the NULL pointer.
- *
- * If the device is found, the return value will be true and 'netdev_name' 
- * contains the device's name as a string, which the caller is responsible 
- * for freeing.  If the device is not found, the return value is false. */
-bool
-netdev_find_dev_by_in4(const struct in_addr *in4, char **netdev_name)
+/* Returns a network device that has 'in4' as its IP address, if one exists,
+ * otherwise a null pointer. */
+struct netdev *
+netdev_find_dev_by_in4(const struct in_addr *in4)
 {
-    int i;
-    struct in_addr dev_in4;
+    struct netdev *netdev;
     struct svec dev_list;
+    size_t i;
 
-    /* Check the hint first. */
-    if (*netdev_name && !netdev_nodev_get_in4(*netdev_name, &dev_in4)
-            && (dev_in4.s_addr == in4->s_addr)) {
-        return true;
-    }
-
-    free(*netdev_name);
-    *netdev_name = NULL;
     netdev_enumerate(&dev_list);
-
-    for (i=0; i<dev_list.n; i++) {
-        if (!netdev_nodev_get_in4(dev_list.names[i], &dev_in4)
-                && (dev_in4.s_addr == in4->s_addr)) {
-            *netdev_name = xstrdup(dev_list.names[i]);
-            svec_destroy(&dev_list);
-            return true;
+    for (i = 0; i < dev_list.n; i++) {
+        const char *name = dev_list.names[i];
+        struct in_addr dev_in4;
+
+        if (!netdev_open(name, NETDEV_ETH_TYPE_NONE, &netdev)
+            && !netdev_get_in4(netdev, &dev_in4)
+            && dev_in4.s_addr == in4->s_addr) {
+            goto exit;
         }
+        netdev_close(netdev);
     }
+    netdev = NULL;
 
+exit:
     svec_destroy(&dev_list);
-    return false;
+    return netdev;
 }
 
 /* Obtains the current flags for the network device named 'netdev_name' and

diff --git a/lib/netdev.h b/lib/netdev.h
index fb8c1e5..2ac0f2e 100644 (file)
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -107,7 +107,8 @@ int netdev_set_policing(struct netdev *, uint32_t kbits_rate,
                         uint32_t kbits_burst);
 
 void netdev_enumerate(struct svec *);
-bool netdev_find_dev_by_in4(const struct in_addr *in4, char **netdev_name);
+struct netdev *netdev_find_dev_by_in4(const struct in_addr *);
+bool netdev_nodev_find_dev_by_in4(const struct in_addr *in4, char **netdev_name);
 int netdev_nodev_get_flags(const char *netdev_name, enum netdev_flags *);
 int netdev_nodev_get_in4(const char *netdev_name, struct in_addr *);
 int netdev_nodev_set_etheraddr(const char *name, const uint8_t mac[6]);

diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 9b699ca..a08af07 100644 (file)
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -74,7 +74,7 @@ struct in_band {
     uint32_t last_ip;           /* Last known IP, 0 if never known. */
     uint8_t mac[ETH_ADDR_LEN];  /* Current MAC, 0 if unknown. */
     uint8_t last_mac[ETH_ADDR_LEN]; /* Last known MAC, 0 if never known */
-    char *dev_name;
+    struct netdev *netdev;
     time_t next_refresh;        /* Next time to refresh MAC address. */
 
     /* Keeping track of the local port's MAC address. */
@@ -102,14 +102,21 @@ get_controller_mac(struct in_band *ib)
         /* Look up MAC address. */
         memset(ib->mac, 0, sizeof ib->mac);
         if (ib->ip) {
-            uint32_t local_ip = rconn_get_local_ip(ib->controller);
+            struct in_addr local_in4 = { rconn_get_local_ip(ib->controller) };
             struct in_addr in4;
             int retval;
 
-            in4.s_addr = local_ip;
-            if (netdev_find_dev_by_in4(&in4, &ib->dev_name)) {
-                retval = netdev_nodev_arp_lookup(ib->dev_name, ib->ip,
-                        ib->mac);
+            /* Refresh device with IP address 'in4'. */
+            if (!ib->netdev
+                || netdev_get_in4(ib->netdev, &in4)
+                || in4.s_addr != local_in4.s_addr)
+            {
+                netdev_close(ib->netdev);
+                ib->netdev = netdev_find_dev_by_in4(&local_in4);
+            }
+
+            if (ib->netdev) {
+                retval = netdev_arp_lookup(ib->netdev, ib->ip, ib->mac);
                 if (retval) {
                     VLOG_DBG_RL(&rl, "cannot look up controller MAC address "
                                 "("IP_FMT"): %s",
@@ -117,7 +124,7 @@ get_controller_mac(struct in_band *ib)
                 }
             } else {
                 VLOG_DBG_RL(&rl, "cannot find device with IP address "IP_FMT,
-                    IP_ARGS(&local_ip));
+                    IP_ARGS(&local_in4.s_addr));
             }
         }
         have_mac = !eth_addr_is_zero(ib->mac);
@@ -151,7 +158,7 @@ get_local_mac(struct in_band *ib)
     time_t now = time_now();
     if (now >= ib->next_local_refresh) {
         uint8_t ea[ETH_ADDR_LEN];
-        if (ib->dev_name && (!netdev_nodev_get_etheraddr(ib->dev_name, ea))) {
+        if (ib->netdev && !netdev_get_etheraddr(ib->netdev, ea)) {
             memcpy(ib->local_mac, ea, ETH_ADDR_LEN);
         }
         ib->next_local_refresh = now + 1;
@@ -333,7 +340,7 @@ in_band_create(struct ofproto *ofproto, struct switch_status *ss,
                                              in_band_status_cb, in_band);
     in_band->next_refresh = TIME_MIN;
     in_band->next_local_refresh = TIME_MIN;
-    in_band->dev_name = NULL;
+    in_band->netdev = NULL;
 
     *in_bandp = in_band;
 }
@@ -343,6 +350,7 @@ in_band_destroy(struct in_band *in_band)
 {
     if (in_band) {
         switch_status_unregister(in_band->ss_cat);
+        netdev_close(in_band->netdev);
         /* We don't own the rconn. */
     }
 }