Previously, it was possible to open a netdevice as one type, then
proceed to open it as a different type without first closing it. This
would result in the original device and netdev class being used rather
than the new one.

An example configuration that would previously give unexpected behaviour:

ovs-vsctl add-port br0 p0 -- set int p0 type=gre options:remote_ip=1.2.3.4
ovs-vsctl add-port br0 p1 -- set int p1 type=internal
ovs-vsctl set int p1 type=gre options:remote_ip=1.2.3.4
ovs-vsctl set int p1 type=internal

The final command would report in the ovs-vswitchd logs that it is
attempting to configure the port with the same gre settings as p0,
despite the command specifying the type as internal.

This patch catches this case in netdev_open(), removes the old device
from the netdev shash and ensures that one with the new type is created.

Bug #1198386.

Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
 lib/netdev.c                   |   11 ++++++++++-
 tests/interface-reconfigure.at |   30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/lib/netdev.c b/lib/netdev.c
index e0b84bd..30935ba 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -342,6 +342,13 @@ netdev_open(const char *name, const char *type, struct 
netdev **netdevp)
     ovs_mutex_lock(&netdev_class_mutex);
     ovs_mutex_lock(&netdev_mutex);
     netdev = shash_find_data(&netdev_shash, name);
+    if (netdev && strcmp(netdev_get_type(netdev), type)) {
+        VLOG_DBG("Re-opening netdev %s as type %s (was previously %s)",
+                 name, type, netdev_get_type(netdev));
+        shash_delete(&netdev_shash, netdev->node);
+        netdev->node = NULL;
+        netdev = NULL;
+    }
     if (!netdev) {
         struct netdev_registered_class *rc;
 
@@ -489,7 +496,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);
diff --git a/tests/interface-reconfigure.at b/tests/interface-reconfigure.at
index 26a77eb..93ca9bd 100644
--- a/tests/interface-reconfigure.at
+++ b/tests/interface-reconfigure.at
@@ -1028,3 +1028,33 @@ action_down: bring down physical devices - [u'eth0', 
u'eth1']
 ]])
 
 AT_CLEANUP
+
+dnl This test configures two tunnels, then reconfigures one to check that the
+dnl configuration is applied correctly.
+AT_SETUP([Re-configure port with different types])
+AT_KEYWORDS([interface-reconfigure])
+OVS_VSWITCHD_START(
+  [add-port br0 p0 -- set int p0 type=gre options:remote_ip=127.0.0.1 -- \
+   add-port br0 p1 -- set int p1 type=internal])
+
+AT_CHECK([ovs-vsctl set int p1 type=gre options:remote_ip=127.0.0.1])
+AT_CHECK([ovs-vsctl set int p1 type=internal])
+
+AT_CLEANUP
+
+dnl This test configures two tunnels, then deletes the second and re-uses its
+dnl name for different types of ports. This was introduced to detect errors
+dnl where port configuration persists even when the port is deleted and
+dnl readded.
+AT_SETUP([Re-create port with different types])
+AT_KEYWORDS([interface-reconfigure])
+OVS_VSWITCHD_START(
+  [add-port br0 p0 -- set int p0 type=gre options:remote_ip=127.0.0.1 -- \
+   add-port br0 p1 -- set int p1 type=dummy -- \
+   add-port br0 p2 -- set int p2 type=dummy])
+
+AT_CHECK([ovs-vsctl set int p1 type=gre options:remote_ip=127.0.0.1])
+AT_CHECK([ovs-vsctl del-port p1])
+AT_CHECK([ovs-vsctl add-port br0 p1 -- set int p1 type=dummy])
+
+AT_CLEANUP
-- 
1.7.10.4

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

Reply via email to