[ovs-dev] [netdev 19/27] netdev: Make netdev_get_devices() take a reference to each netdev.

[Ben Pfaff]

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 <b...@nicira.com>
---
 lib/netdev-bsd.c   |   32 ++++++++++----------------------
 lib/netdev-linux.c |    4 ++++
 lib/netdev.c       |    5 +++--
 3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 6ff6b3e..d6a8631 100644
--- 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(dev);
         }
         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 ba0d863..5bbaf63 100644
--- 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 0079dc2..2561538 100644
--- 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);
         }
     }
-- 
1.7.10.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev