Merge OVSVethInterfaceDriver into OVSInterfaceDriver

Fixes bug 1049385

Due to some issues using OVS internal interfaces across namespaces
with OpenFlow controllers (bug 1048681), a patch introduced the
OVSVethInterfaceDriver in addition to the base OVSInterfaceDriver.
However, OVSVethInterfaceDriver is just a variation of OVSInterfaceDriver
and the difference is how to create an interface (OVS internal vs veth).
This patch merge OVSVethInterfaceDriver into OVSInterfaceDriver
by introducing a new flag 'ovs_use_veth' (which defaults to False).

Change-Id: Ie8b01e6776bf703f72a9e2a471b24e126f6e2322

diff --git a/quantum/agent/linux/interface.py b/quantum/agent/linux/interface.py
index d6f16c2a118f62b95f1a60c062e5012803689ffa..4330629e498ca9ae3557a650048b48214be58aad 100644 (file)
--- a/quantum/agent/linux/interface.py
+++ b/quantum/agent/linux/interface.py
@@ -34,6 +34,9 @@ OPTS = [
     cfg.StrOpt('ovs_integration_bridge',
                default='br-int',
                help='Name of Open vSwitch bridge to use'),
+    cfg.BoolOpt('ovs_use_veth',
+                default=False,
+                help='Uses veth for an interface or not'),
     cfg.StrOpt('network_device_mtu',
                help='MTU setting for device.'),
     cfg.StrOpt('ryu_api_host',
@@ -108,6 +111,18 @@ class NullDriver(LinuxInterfaceDriver):
 class OVSInterfaceDriver(LinuxInterfaceDriver):
     """Driver for creating an internal interface on an OVS bridge."""
 
+    DEV_NAME_PREFIX = 'tap'
+
+    def __init__(self, conf):
+        super(OVSInterfaceDriver, self).__init__(conf)
+        if self.conf.ovs_use_veth:
+            self.DEV_NAME_PREFIX = 'ns-'
+
+    def _get_tap_name(self, dev_name, prefix=None):
+        if self.conf.ovs_use_veth:
+            dev_name = dev_name.replace(prefix or self.DEV_NAME_PREFIX, 'tap')
+        return dev_name
+
     def _ovs_add_port(self, bridge, device_name, port_id, mac_address,
                       internal=True):
         cmd = ['ovs-vsctl', '--', '--may-exist',
@@ -134,27 +149,53 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
                                     self.conf.root_helper,
                                     namespace=namespace):
 
-            self._ovs_add_port(bridge, device_name, port_id, mac_address)
+            ip = ip_lib.IPWrapper(self.conf.root_helper)
+            tap_name = self._get_tap_name(device_name, prefix)
 
-        ip = ip_lib.IPWrapper(self.conf.root_helper)
-        device = ip.device(device_name)
-        device.link.set_address(mac_address)
-        if self.conf.network_device_mtu:
-            device.link.set_mtu(self.conf.network_device_mtu)
+            if self.conf.ovs_use_veth:
+                root_dev, ns_dev = ip.add_veth(tap_name, device_name)
 
-        if namespace:
-            namespace_obj = ip.ensure_namespace(namespace)
-            namespace_obj.add_device_to_namespace(device)
-        device.link.set_up()
+            internal = not self.conf.ovs_use_veth
+            self._ovs_add_port(bridge, tap_name, port_id, mac_address,
+                               internal=internal)
+
+            ns_dev = ip.device(device_name)
+            ns_dev.link.set_address(mac_address)
+
+            if self.conf.network_device_mtu:
+                ns_dev.link.set_mtu(self.conf.network_device_mtu)
+                if self.conf.ovs_use_veth:
+                    root_dev.link.set_mtu(self.conf.network_device_mtu)
+
+            if namespace:
+                namespace_obj = ip.ensure_namespace(namespace)
+                namespace_obj.add_device_to_namespace(ns_dev)
+
+            ns_dev.link.set_up()
+            if self.conf.ovs_use_veth:
+                root_dev.link.set_up()
+        else:
+            LOG.warn(_("Device %s already exists") % device_name)
 
     def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
         if not bridge:
             bridge = self.conf.ovs_integration_bridge
 
+        tap_name = self._get_tap_name(device_name, prefix)
         self.check_bridge_exists(bridge)
-        bridge = ovs_lib.OVSBridge(bridge, self.conf.root_helper)
-        bridge.delete_port(device_name)
+        ovs = ovs_lib.OVSBridge(bridge, self.conf.root_helper)
+
+        try:
+            ovs.delete_port(tap_name)
+            if self.conf.ovs_use_veth:
+                device = ip_lib.IPDevice(device_name, self.conf.root_helper,
+                                         namespace)
+                device.link.delete()
+                LOG.debug(_("Unplugged interface '%s'") % device_name)
+        except RuntimeError:
+            LOG.error(_("Failed unplugging interface '%s'") %
+                      device_name)
 
 
 class BridgeInterfaceDriver(LinuxInterfaceDriver):
@@ -226,69 +267,6 @@ class RyuInterfaceDriver(OVSInterfaceDriver):
         self.ryu_client.create_port(network_id, datapath_id, port_no)
 
 
-class OVSVethInterfaceDriver(OVSInterfaceDriver):
-    """Driver for creating an OVS interface using veth."""
-
-    DEV_NAME_PREFIX = 'ns-'
-
-    def _get_tap_name(self, device_name, prefix=None):
-        if not prefix:
-            prefix = self.DEV_NAME_PREFIX
-        return device_name.replace(prefix, 'tap')
-
-    def plug(self, network_id, port_id, device_name, mac_address,
-             bridge=None, namespace=None, prefix=None):
-        """Plugin the interface."""
-        if not bridge:
-            bridge = self.conf.ovs_integration_bridge
-
-        self.check_bridge_exists(bridge)
-
-        if not ip_lib.device_exists(device_name,
-                                    self.conf.root_helper,
-                                    namespace=namespace):
-            ip = ip_lib.IPWrapper(self.conf.root_helper)
-
-            tap_name = self._get_tap_name(device_name, prefix)
-            root_veth, ns_veth = ip.add_veth(tap_name, device_name)
-
-            self._ovs_add_port(bridge, tap_name, port_id, mac_address,
-                               internal=False)
-
-            ns_veth.link.set_address(mac_address)
-            if self.conf.network_device_mtu:
-                ns_veth.link.set_mtu(self.conf.network_device_mtu)
-                root_veth.link.set_mtu(self.conf.network_device_mtu)
-
-            if namespace:
-                namespace_obj = ip.ensure_namespace(namespace)
-                namespace_obj.add_device_to_namespace(ns_veth)
-
-            root_veth.link.set_up()
-            ns_veth.link.set_up()
-        else:
-            LOG.warn(_("Device %s already exists") % device_name)
-
-    def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
-        """Unplug the interface."""
-        if not bridge:
-            bridge = self.conf.ovs_integration_bridge
-
-        tap_name = self._get_tap_name(device_name, prefix)
-        self.check_bridge_exists(bridge)
-        ovs = ovs_lib.OVSBridge(bridge, self.conf.root_helper)
-
-        try:
-            ovs.delete_port(tap_name)
-            device = ip_lib.IPDevice(device_name, self.conf.root_helper,
-                                     namespace)
-            device.link.delete()
-            LOG.debug(_("Unplugged interface '%s'") % device_name)
-        except RuntimeError:
-            LOG.error(_("Failed unplugging interface '%s'") %
-                      device_name)
-
-
 class MetaInterfaceDriver(LinuxInterfaceDriver):
     def __init__(self, conf):
         super(MetaInterfaceDriver, self).__init__(conf)

diff --git a/quantum/tests/unit/test_linux_interface.py b/quantum/tests/unit/test_linux_interface.py
index 1e70b4be68d82c831d4df0da384ff79780b5ad23..f449a7307592a7e721ea9527acc1718b9ab7ab0d 100644 (file)
--- a/quantum/tests/unit/test_linux_interface.py
+++ b/quantum/tests/unit/test_linux_interface.py
@@ -106,6 +106,11 @@ class TestABCDriver(TestBase):
 
 class TestOVSInterfaceDriver(TestBase):
 
+    def test_get_device_name(self):
+        br = interface.OVSInterfaceDriver(self.conf)
+        device_name = br.get_device_name(FakePort())
+        self.assertEqual('tapabcdef01-12', device_name)
+
     def test_plug_no_ns(self):
         self._test_plug()
 
@@ -171,10 +176,14 @@ class TestOVSInterfaceDriver(TestBase):
                                      mock.call().delete_port('tap0')])
 
 
-class TestOVSVethInterfaceDriver(TestOVSInterfaceDriver):
+class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
+
+    def setUp(self):
+        super(TestOVSInterfaceDriverWithVeth, self).setUp()
+        self.conf.set_override('ovs_use_veth', True)
 
     def test_get_device_name(self):
-        br = interface.OVSVethInterfaceDriver(self.conf)
+        br = interface.OVSInterfaceDriver(self.conf)
         device_name = br.get_device_name(FakePort())
         self.assertEqual('ns-abcdef01-12', device_name)
 
@@ -192,13 +201,16 @@ class TestOVSVethInterfaceDriver(TestOVSInterfaceDriver):
         def device_exists(dev, root_helper=None, namespace=None):
             return dev == bridge
 
-        ovs = interface.OVSVethInterfaceDriver(self.conf)
+        ovs = interface.OVSInterfaceDriver(self.conf)
         self.device_exists.side_effect = device_exists
 
-        root_veth = mock.Mock()
-        ns_veth = mock.Mock()
-        self.ip().add_veth = mock.Mock(return_value=(root_veth, ns_veth))
-        expected = [mock.call('sudo'), mock.call().add_veth('tap0', devname)]
+        root_dev = mock.Mock()
+        _ns_dev = mock.Mock()
+        ns_dev = mock.Mock()
+        self.ip().add_veth = mock.Mock(return_value=(root_dev, _ns_dev))
+        self.ip().device = mock.Mock(return_value=(ns_dev))
+        expected = [mock.call('sudo'), mock.call().add_veth('tap0', devname),
+                    mock.call().device(devname)]
 
         vsctl_cmd = ['ovs-vsctl', '--', '--may-exist', 'add-port',
                      bridge, 'tap0', '--', 'set', 'Interface', 'tap0',
@@ -217,11 +229,11 @@ class TestOVSVethInterfaceDriver(TestOVSInterfaceDriver):
                      prefix=prefix)
             execute.assert_called_once_with(vsctl_cmd, 'sudo')
 
-        ns_veth.assert_has_calls(
+        ns_dev.assert_has_calls(
             [mock.call.link.set_address('aa:bb:cc:dd:ee:ff')])
         if mtu:
-            ns_veth.assert_has_calls([mock.call.link.set_mtu(mtu)])
-            root_veth.assert_has_calls([mock.call.link.set_mtu(mtu)])
+            ns_dev.assert_has_calls([mock.call.link.set_mtu(mtu)])
+            root_dev.assert_has_calls([mock.call.link.set_mtu(mtu)])
         if namespace:
             expected.extend(
                 [mock.call().ensure_namespace(namespace),
@@ -229,8 +241,8 @@ class TestOVSVethInterfaceDriver(TestOVSInterfaceDriver):
                      mock.ANY)])
 
         self.ip.assert_has_calls(expected)
-        root_veth.assert_has_calls([mock.call.link.set_up()])
-        ns_veth.assert_has_calls([mock.call.link.set_up()])
+        root_dev.assert_has_calls([mock.call.link.set_up()])
+        ns_dev.assert_has_calls([mock.call.link.set_up()])
 
     def test_plug_mtu(self):
         self.conf.set_override('network_device_mtu', 9000)
@@ -240,7 +252,7 @@ class TestOVSVethInterfaceDriver(TestOVSInterfaceDriver):
         if not bridge:
             bridge = 'br-int'
         with mock.patch('quantum.agent.linux.ovs_lib.OVSBridge') as ovs_br:
-            ovs = interface.OVSVethInterfaceDriver(self.conf)
+            ovs = interface.OVSInterfaceDriver(self.conf)
             ovs.unplug('ns-0', bridge=bridge)
             ovs_br.assert_has_calls([mock.call(bridge, 'sudo'),
                                      mock.call().delete_port('tap0')])