netdev-linux: Ref and unref the netdev_linux_cache_notifier for taps too.
authorBen Pfaff <blp@nicira.com>
Wed, 30 Nov 2011 18:59:12 +0000 (10:59 -0800)
committerBen Pfaff <blp@nicira.com>
Fri, 2 Dec 2011 18:39:07 +0000 (10:39 -0800)
netdev-linux uses netdev_linux_cache_notifier to flush its cache when the
kernel notifies userspace that a particular network device's configuration
or status has changed.  This is as applicable to tap devices as to system
and internal devices, so we should create and destroy the notifier for
tap devices also.

I doubt that in practice it's possible to run ovs-vswitchd without having
a non-tap device open, at least with the kernel datapath, because the
local port for a bridge is not a tap device, so there should be no need to
backport this to older versions.

Reported-by: Gaetano Catalli <gaetano.catalli@gmail.com>
lib/netdev-linux.c

index 44167da4a4a4c4af6d07b65b921273d6ee0876ea..90e88c76d9efcf3c1c8a7a8291963be6fbc84d15 100644 (file)
@@ -534,13 +534,9 @@ netdev_linux_cache_cb(const struct rtnetlink_link_change *change,
     }
 }
 
-/* Creates system and internal devices. */
 static int
-netdev_linux_create(const struct netdev_class *class, const char *name,
-                    struct netdev_dev **netdev_devp)
+cache_notifier_ref(void)
 {
-    struct netdev_dev_linux *netdev_dev;
-
     if (!cache_notifier_refcount) {
         assert(!netdev_linux_cache_notifier);
 
@@ -553,6 +549,33 @@ netdev_linux_create(const struct netdev_class *class, const char *name,
     }
     cache_notifier_refcount++;
 
+    return 0;
+}
+
+static void
+cache_notifier_unref(void)
+{
+    assert(cache_notifier_refcount > 0);
+    if (!--cache_notifier_refcount) {
+        assert(netdev_linux_cache_notifier);
+        rtnetlink_link_notifier_destroy(netdev_linux_cache_notifier);
+        netdev_linux_cache_notifier = NULL;
+    }
+}
+
+/* Creates system and internal devices. */
+static int
+netdev_linux_create(const struct netdev_class *class, const char *name,
+                    struct netdev_dev **netdev_devp)
+{
+    struct netdev_dev_linux *netdev_dev;
+    int error;
+
+    error = cache_notifier_ref();
+    if (error) {
+        return error;
+    }
+
     netdev_dev = xzalloc(sizeof *netdev_dev);
     netdev_dev->change_seq = 1;
     netdev_dev_init(&netdev_dev->netdev_dev, name, class);
@@ -581,12 +604,17 @@ netdev_linux_create_tap(const struct netdev_class *class OVS_UNUSED,
     netdev_dev = xzalloc(sizeof *netdev_dev);
     state = &netdev_dev->state.tap;
 
+    error = cache_notifier_ref();
+    if (error) {
+        goto error;
+    }
+
     /* Open tap device. */
     state->fd = open(tap_dev, O_RDWR);
     if (state->fd < 0) {
         error = errno;
         VLOG_WARN("opening \"%s\" failed: %s", tap_dev, strerror(error));
-        goto error;
+        goto error_unref_notifier;
     }
 
     /* Create tap device. */
@@ -596,19 +624,21 @@ netdev_linux_create_tap(const struct netdev_class *class OVS_UNUSED,
         VLOG_WARN("%s: creating tap device failed: %s", name,
                   strerror(errno));
         error = errno;
-        goto error;
+        goto error_unref_notifier;
     }
 
     /* Make non-blocking. */
     error = set_nonblocking(state->fd);
     if (error) {
-        goto error;
+        goto error_unref_notifier;
     }
 
     netdev_dev_init(&netdev_dev->netdev_dev, name, &netdev_tap_class);
     *netdev_devp = &netdev_dev->netdev_dev;
     return 0;
 
+error_unref_notifier:
+    cache_notifier_unref();
 error:
     free(netdev_dev);
     return error;
@@ -635,21 +665,12 @@ netdev_linux_destroy(struct netdev_dev *netdev_dev_)
         netdev_dev->tc->ops->tc_destroy(netdev_dev->tc);
     }
 
-    if (class == &netdev_linux_class || class == &netdev_internal_class) {
-        cache_notifier_refcount--;
-
-        if (!cache_notifier_refcount) {
-            assert(netdev_linux_cache_notifier);
-            rtnetlink_link_notifier_destroy(netdev_linux_cache_notifier);
-            netdev_linux_cache_notifier = NULL;
-        }
-    } else if (class == &netdev_tap_class) {
+    if (class == &netdev_tap_class) {
         destroy_tap(netdev_dev);
-    } else {
-        NOT_REACHED();
     }
-
     free(netdev_dev);
+
+    cache_notifier_unref();
 }
 
 static int