netdev: Make netdev_get_devices() take a reference to each netdev.

This API change is necessary for thread safety, to be added in an upcoming
commit.  Otherwise, the client would not be able to actually use any of
the returned netdevs because they could already have been destroyed.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 6ff6b3e95db97f98e7a0620d93f5bf737ea74ada..5c23d38100969926b83e4b40cd4a2ed06ff46f1c 100644 (file)
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -255,6 +255,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change *change,
                 dev->cache_valid = 0;
                 netdev_bsd_changed(dev);
             }
+            netdev_close(base_dev);
         }
     } else {
         /*
@@ -271,6 +272,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change *change,
             dev = netdev_bsd_cast(netdev);
             dev->cache_valid = 0;
             netdev_bsd_changed(dev);
+            netdev_close(netdev);
         }
         shash_destroy(&device_shash);
     }
@@ -1200,9 +1202,10 @@ netdev_bsd_get_in6(const struct netdev *netdev_, struct in6_addr *in6)
 }
 
 #if defined(__NetBSD__)
-static struct netdev *
-find_netdev_by_kernel_name(const char *kernel_name)
+static char *
+netdev_bsd_kernel_name_to_ovs_name(const char *kernel_name)
 {
+    char *ovs_name = NULL;
     struct shash device_shash;
     struct shash_node *node;
 
@@ -1213,24 +1216,14 @@ find_netdev_by_kernel_name(const char *kernel_name)
         struct netdev_bsd * const dev = netdev_bsd_cast(netdev);
 
         if (!strcmp(dev->kernel_name, kernel_name)) {
-            shash_destroy(&device_shash);
-            return &dev->up;
+            free(ovs_name);
+            ovs_name = xstrdup(netdev_get_name(&dev->up));
         }
+        netdev_close(netdev);
     }
     shash_destroy(&device_shash);
-    return NULL;
-}
-
-static const char *
-netdev_bsd_convert_kernel_name_to_ovs_name(const char *kernel_name)
-{
-    const struct netdev * const netdev =
-      find_netdev_by_kernel_name(kernel_name);
 
-    if (netdev == NULL) {
-        return NULL;
-    }
-    return netdev_get_name(netdev);
+    return ovs_name ? ovs_name : xstrdup(kernel_name);
 }
 #endif
 
@@ -1315,12 +1308,7 @@ netdev_bsd_get_next_hop(const struct in_addr *host OVS_UNUSED,
                 char *kernel_name;
 
                 kernel_name = xmemdup0(sdl->sdl_data, sdl->sdl_nlen);
-                name = netdev_bsd_convert_kernel_name_to_ovs_name(kernel_name);
-                if (name == NULL) {
-                    ifname = xstrdup(kernel_name);
-                } else {
-                    ifname = xstrdup(name);
-                }
+                ifname = netdev_bsd_kernel_name_to_ovs_name(kernel_name);
                 free(kernel_name);
             }
             RT_ADVANCE(cp, sa);

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index ba0d8633c3cc47dd61cd1e88f88c8ba3a6230d9b..5bbaf632f26d38f4172660a2c57fd7ad50ae215a 100644 (file)
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -555,6 +555,7 @@ netdev_linux_cache_cb(const struct rtnetlink_link_change *change,
 
             get_flags(&dev->up, &flags);
             netdev_linux_changed(dev, flags, 0);
+            netdev_close(netdev);
         }
         shash_destroy(&device_shash);
     }
@@ -1180,6 +1181,7 @@ netdev_linux_miimon_run(void)
         bool miimon;
 
         if (dev->miimon_interval <= 0 || !timer_expired(&dev->miimon_timer)) {
+            netdev_close(netdev);
             continue;
         }
 
@@ -1190,6 +1192,7 @@ netdev_linux_miimon_run(void)
         }
 
         timer_set_duration(&dev->miimon_timer, dev->miimon_interval);
+        netdev_close(netdev);
     }
 
     shash_destroy(&device_shash);
@@ -1210,6 +1213,7 @@ netdev_linux_miimon_wait(void)
         if (dev->miimon_interval > 0) {
             timer_wait(&dev->miimon_timer);
         }
+        netdev_close(netdev);
     }
     shash_destroy(&device_shash);
 }

diff --git a/lib/netdev.c b/lib/netdev.c
index 0079dc209ad9ab4cbf7b489e841fe1bf1d94529f..25615380f4febd7cbc5e35ac6f60e0549d12e91e 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -1395,8 +1395,8 @@ netdev_from_name(const char *name)
 
 /* Fills 'device_list' with devices that match 'netdev_class'.
  *
- * The caller is responsible for initializing and destroying 'device_list'
- * but the contained netdevs must not be freed. */
+ * The caller is responsible for initializing and destroying 'device_list' and
+ * must close each device on the list. */
 void
 netdev_get_devices(const struct netdev_class *netdev_class,
                    struct shash *device_list)
@@ -1406,6 +1406,7 @@ netdev_get_devices(const struct netdev_class *netdev_class,
         struct netdev *dev = node->data;
 
         if (dev->netdev_class == netdev_class) {
+            dev->ref_cnt++;
             shash_add(device_list, node->name, node->data);
         }
     }