From: Joe Stringer Date: Sun, 4 May 2014 22:14:18 +0000 (+1200) Subject: tunnel: Fix bug where misconfiguration persists. X-Git-Tag: v2.3~155 X-Git-Url: http://git.cascardo.eti.br/?a=commitdiff_plain;h=780f7a2ed0e37f3b4f8c5a313b719e2dce48e617;p=cascardo%2Fovs.git tunnel: Fix bug where misconfiguration persists. Previously, misconfiguring a tunnel port to use the exact same settings would cause the corresponding netdev to never be destroyed. When attempting to re-use the port as a different type, this would fail and result in a discrepancy between reported port type and actual netdev in use. 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. Even after deleting and re-adding the port, the message would reappear. This patch fixes the bug by dereferencing the netdev in the failure case of tnl_port_add__(), and ensures that the tnl_port structure is freed in that case as well. Bug #1198386. Signed-off-by: Joe Stringer Acked-by: Ryan Wilson Acked-by: Alex Wang --- diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c index d2e9584f1..2b5aa503e 100644 --- a/ofproto/tunnel.c +++ b/ofproto/tunnel.c @@ -159,8 +159,9 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev, "port '%s' (%s)", tnl_port_get_name(tnl_port), tnl_port_get_name(existing_port), ds_cstr(&ds)); ds_destroy(&ds); - free(tnl_port); } + netdev_close(tnl_port->netdev); + free(tnl_port); return false; } diff --git a/tests/interface-reconfigure.at b/tests/interface-reconfigure.at index 26a77eb9c..63c62af74 100644 --- a/tests/interface-reconfigure.at +++ b/tests/interface-reconfigure.at @@ -1028,3 +1028,20 @@ action_down: bring down physical devices - [u'eth0', u'eth1'] ]]) 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