netdev: Remove netdev from global shash when the user is changing interface configura...

When the user changes port type (i.e. changing p0 from type 'internal' to
'gre'), the netdev must first be deleted, then re-created with the new type.
Deleting the netdev requires there exist no more references to the netdev.
However, the xlate cache holds references to netdevs and the cache is only
invalidated by revalidator threads. Thus, if cache is not invalidated prior to
the netdev being re-created, the netdev will not be able to be re-created and
the configuration change will fail.

This patch always removes the netdev from the global netdev shash when the
user changes port type. This ensures that the new netdev can always be created
while handler and revalidator threads can retain references to the old netdev
until they are finished.

Signed-off-by: Ryan Wilson <wryan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index 7ca92e53c64f21719dfc789bed702cfa00569341..90d897a53502fb3f9242d6d80cf07dbc987991c1 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -106,6 +106,7 @@ Remko Tronçon           git@el-tramo.be
 Rich Lane               rlane@bigswitch.com
 Rob Hoes                rob.hoes@citrix.com
 Romain Lenglet          romain.lenglet@berabera.info
+Ryan Wilson             wryan@nicira.com
 Sajjad Lateef           slateef@nicira.com
 Sanjay Sane             ssane@nicira.com
 Saurabh Shah            ssaurabh@nicira.com

diff --git a/lib/netdev.c b/lib/netdev.c
index 5bf2220c4463f7b38a5355d7bb03df0dba3f2bd4..f545a514b4dbd5bfc824796349782f5e29cd5355 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -489,7 +489,9 @@ netdev_unref(struct netdev *dev)
 
         dev->netdev_class->destruct(dev);
 
-        shash_delete(&netdev_shash, dev->node);
+        if (dev->node) {
+            shash_delete(&netdev_shash, dev->node);
+        }
         free(dev->name);
         dev->netdev_class->dealloc(dev);
         ovs_mutex_unlock(&netdev_mutex);
@@ -515,6 +517,28 @@ netdev_close(struct netdev *netdev)
     }
 }
 
+/* Removes 'netdev' from the global shash and unrefs 'netdev'.
+ *
+ * This allows handler and revalidator threads to still retain references
+ * to this netdev while the main thread changes interface configuration.
+ *
+ * This function should only be called by the main thread when closing
+ * netdevs during user configuration changes. Otherwise, netdev_close should be
+ * used to close netdevs. */
+void
+netdev_remove(struct netdev *netdev)
+{
+    if (netdev) {
+        ovs_mutex_lock(&netdev_mutex);
+        if (netdev->node) {
+            shash_delete(&netdev_shash, netdev->node);
+            netdev->node = NULL;
+            netdev_change_seq_changed(netdev);
+        }
+        netdev_unref(netdev);
+    }
+}
+
 /* Parses 'netdev_name_', which is of the form [type@]name into its component
  * pieces.  'name' and 'type' must be freed by the caller. */
 void

diff --git a/lib/netdev.h b/lib/netdev.h
index 7cb3c8031842af9b9eb34759d20eada1858b2a82..9b35972dcbab7d43887f5da6949448f76ea93fdc 100644 (file)
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -141,6 +141,7 @@ bool netdev_is_pmd(const struct netdev *netdev);
 int netdev_open(const char *name, const char *type, struct netdev **netdevp);
 
 struct netdev *netdev_ref(const struct netdev *);
+void netdev_remove(struct netdev *);
 void netdev_close(struct netdev *);
 
 void netdev_parse_name(const char *netdev_name, char **name, char **type);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 1b06ec90f699b10289d38541a42aaff49f4c8b16..123c15ee2df42d1ab48afe6564c866302c09134c 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3452,7 +3452,9 @@ iface_destroy__(struct iface *iface)
         list_remove(&iface->port_elem);
         hmap_remove(&br->iface_by_name, &iface->name_node);
 
-        netdev_close(iface->netdev);
+        /* The user is changing configuration here, so netdev_remove needs to be
+         * used as opposed to netdev_close */
+        netdev_remove(iface->netdev);
 
         free(iface->name);
         free(iface);