Add a new interface driver OVSVethInterfaceDriver.

Fixes bug 1048681

This commit fixes a bug that quantum agent using a namespace does not
work with NEC plugin. NEC plugin uses an Open vSwitch as an OpenFlow switch,
but an OVS port created by ovs-vsctl add-port becomes down when the port is
moved to some network namespace. Usual OpenFlow controllers respect the
OpenFlow port status. Since DHCP server (dnsmasq) is connected to the down
port, DHCP server disappears from the virtual network.

This behavior can be avoided if we use a similar approach used in
BridgeInterfaceDriver, i.e., we first create a veth pair, add one veth device
to OVS and move the other veth device to a network namespace. To accomplish it
this patch introduces a new interface driver OVSVethInterfaceDriver.

In addition, this patch adds a 'prefix' paramter to unplug(). When deleting
a veth pair, a veth device added to OVS bridge is not removed automatically.
Thus we need a mean to know the veth device name from a device_name to be
removed. It is the reason to add 'prefix' parameter to unplug().

Change-Id: I3f4eae371a27a3171a6a8c9e38ddc9354ed1b6b0

diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py
index 6a302b888f484ba16e40e646488d07a0a778bcfe..9714ec2e5e0c1dbb0072cc708fdc835a4fac5a53 100644 (file)
--- a/quantum/agent/l3_agent.py
+++ b/quantum/agent/l3_agent.py
@@ -151,11 +151,13 @@ class L3NATAgent(object):
         for d in ns_ip.get_devices(exclude_loopback=True):
             if d.name.startswith(INTERNAL_DEV_PREFIX):
                 # device is on default bridge
-                self.driver.unplug(d.name, namespace=namespace)
+                self.driver.unplug(d.name, namespace=namespace,
+                                   prefix=INTERNAL_DEV_PREFIX)
             elif d.name.startswith(EXTERNAL_DEV_PREFIX):
                 self.driver.unplug(d.name,
                                    bridge=self.conf.external_network_bridge,
-                                   namespace=namespace)
+                                   namespace=namespace,
+                                   prefix=EXTERNAL_DEV_PREFIX)
         #(TODO) Address the failure for the deletion of the namespace
 
     def _create_router_namespace(self, ri):
@@ -386,7 +388,8 @@ class L3NATAgent(object):
                                 namespace=ri.ns_name()):
             self.driver.unplug(interface_name,
                                bridge=self.conf.external_network_bridge,
-                               namespace=ri.ns_name())
+                               namespace=ri.ns_name(),
+                               prefix=EXTERNAL_DEV_PREFIX)
 
         ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address']
         for c, r in self.external_gateway_filter_rules():
@@ -444,7 +447,8 @@ class L3NATAgent(object):
         if ip_lib.device_exists(interface_name,
                                 root_helper=self.conf.root_helper,
                                 namespace=ri.ns_name()):
-            self.driver.unplug(interface_name, namespace=ri.ns_name())
+            self.driver.unplug(interface_name, namespace=ri.ns_name(),
+                               prefix=INTERNAL_DEV_PREFIX)
 
         if ex_gw_port:
             ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address']

diff --git a/quantum/agent/linux/interface.py b/quantum/agent/linux/interface.py
index 36017c05ebd6d2d2c63b0f05642a1dd62003c7cd..d6f16c2a118f62b95f1a60c062e5012803689ffa 100644 (file)
--- a/quantum/agent/linux/interface.py
+++ b/quantum/agent/linux/interface.py
@@ -92,7 +92,7 @@ class LinuxInterfaceDriver(object):
         """Plug in the interface."""
 
     @abc.abstractmethod
-    def unplug(self, device_name, bridge=None, namespace=None):
+    def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
 
 
@@ -101,13 +101,27 @@ class NullDriver(LinuxInterfaceDriver):
              bridge=None, namespace=None, prefix=None):
         pass
 
-    def unplug(self, device_name, bridge=None, namespace=None):
+    def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         pass
 
 
 class OVSInterfaceDriver(LinuxInterfaceDriver):
     """Driver for creating an internal interface on an OVS bridge."""
 
+    def _ovs_add_port(self, bridge, device_name, port_id, mac_address,
+                      internal=True):
+        cmd = ['ovs-vsctl', '--', '--may-exist',
+               'add-port', bridge, device_name]
+        if internal:
+            cmd += ['--', 'set', 'Interface', device_name, 'type=internal']
+        cmd += ['--', 'set', 'Interface', device_name,
+                'external-ids:iface-id=%s' % port_id,
+                '--', 'set', 'Interface', device_name,
+                'external-ids:iface-status=active',
+                '--', 'set', 'Interface', device_name,
+                'external-ids:attached-mac=%s' % mac_address]
+        utils.execute(cmd, self.conf.root_helper)
+
     def plug(self, network_id, port_id, device_name, mac_address,
              bridge=None, namespace=None, prefix=None):
         """Plug in the interface."""
@@ -120,19 +134,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
                                     self.conf.root_helper,
                                     namespace=namespace):
 
-            utils.execute(['ovs-vsctl',
-                           '--', '--may-exist', 'add-port', bridge,
-                           device_name,
-                           '--', 'set', 'Interface', device_name,
-                           'type=internal',
-                           '--', 'set', 'Interface', device_name,
-                           'external-ids:iface-id=%s' % port_id,
-                           '--', 'set', 'Interface', device_name,
-                           'external-ids:iface-status=active',
-                           '--', 'set', 'Interface', device_name,
-                           'external-ids:attached-mac=%s' %
-                           mac_address],
-                          self.conf.root_helper)
+            self._ovs_add_port(bridge, device_name, port_id, mac_address)
 
         ip = ip_lib.IPWrapper(self.conf.root_helper)
         device = ip.device(device_name)
@@ -145,7 +147,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
             namespace_obj.add_device_to_namespace(device)
         device.link.set_up()
 
-    def unplug(self, device_name, bridge=None, namespace=None):
+    def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
         if not bridge:
             bridge = self.conf.ovs_integration_bridge
@@ -186,7 +188,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver):
         else:
             LOG.warn(_("Device %s already exists") % device_name)
 
-    def unplug(self, device_name, bridge=None, namespace=None):
+    def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         """Unplug the interface."""
         device = ip_lib.IPDevice(device_name, self.conf.root_helper, namespace)
         try:
@@ -224,6 +226,69 @@ 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)
@@ -266,9 +331,9 @@ class MetaInterfaceDriver(LinuxInterfaceDriver):
         return driver.plug(network_id, port_id, device_name, mac_address,
                            bridge=bridge, namespace=namespace, prefix=prefix)
 
-    def unplug(self, device_name, bridge=None, namespace=None):
+    def unplug(self, device_name, bridge=None, namespace=None, prefix=None):
         driver = self._get_driver_by_device_name(device_name, namespace=None)
-        return driver.unplug(device_name, bridge, namespace)
+        return driver.unplug(device_name, bridge, namespace, prefix)
 
     def _load_driver(self, driver_provider):
         LOG.debug("Driver location:%s", driver_provider)

diff --git a/quantum/tests/unit/test_linux_interface.py b/quantum/tests/unit/test_linux_interface.py
index 8f9b9ba1e6b010ff49b2195a5792d81ae8d34a2a..1e70b4be68d82c831d4df0da384ff79780b5ad23 100644 (file)
--- a/quantum/tests/unit/test_linux_interface.py
+++ b/quantum/tests/unit/test_linux_interface.py
@@ -171,6 +171,83 @@ class TestOVSInterfaceDriver(TestBase):
                                      mock.call().delete_port('tap0')])
 
 
+class TestOVSVethInterfaceDriver(TestOVSInterfaceDriver):
+
+    def test_get_device_name(self):
+        br = interface.OVSVethInterfaceDriver(self.conf)
+        device_name = br.get_device_name(FakePort())
+        self.assertEqual('ns-abcdef01-12', device_name)
+
+    def test_plug_with_prefix(self):
+        self._test_plug(devname='qr-0', prefix='qr-')
+
+    def _test_plug(self, devname=None, bridge=None, namespace=None,
+                   prefix=None, mtu=None):
+
+        if not devname:
+            devname = 'ns-0'
+        if not bridge:
+            bridge = 'br-int'
+
+        def device_exists(dev, root_helper=None, namespace=None):
+            return dev == bridge
+
+        ovs = interface.OVSVethInterfaceDriver(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)]
+
+        vsctl_cmd = ['ovs-vsctl', '--', '--may-exist', 'add-port',
+                     bridge, 'tap0', '--', 'set', 'Interface', 'tap0',
+                     'external-ids:iface-id=port-1234', '--', 'set',
+                     'Interface', 'tap0',
+                     'external-ids:iface-status=active', '--', 'set',
+                     'Interface', 'tap0',
+                     'external-ids:attached-mac=aa:bb:cc:dd:ee:ff']
+        with mock.patch.object(utils, 'execute') as execute:
+            ovs.plug('01234567-1234-1234-99',
+                     'port-1234',
+                     devname,
+                     'aa:bb:cc:dd:ee:ff',
+                     bridge=bridge,
+                     namespace=namespace,
+                     prefix=prefix)
+            execute.assert_called_once_with(vsctl_cmd, 'sudo')
+
+        ns_veth.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)])
+        if namespace:
+            expected.extend(
+                [mock.call().ensure_namespace(namespace),
+                 mock.call().ensure_namespace().add_device_to_namespace(
+                     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()])
+
+    def test_plug_mtu(self):
+        self.conf.set_override('network_device_mtu', 9000)
+        self._test_plug(mtu=9000)
+
+    def test_unplug(self, bridge=None):
+        if not bridge:
+            bridge = 'br-int'
+        with mock.patch('quantum.agent.linux.ovs_lib.OVSBridge') as ovs_br:
+            ovs = interface.OVSVethInterfaceDriver(self.conf)
+            ovs.unplug('ns-0', bridge=bridge)
+            ovs_br.assert_has_calls([mock.call(bridge, 'sudo'),
+                                     mock.call().delete_port('tap0')])
+        self.ip_dev.assert_has_calls([mock.call('ns-0', 'sudo', None),
+                                      mock.call().link.delete()])
+
+
 class TestBridgeInterfaceDriver(TestBase):
     def test_get_device_name(self):
         br = interface.BridgeInterfaceDriver(self.conf)