From aa5a7aaf47b6cf97a6f35934f68570ecfc16bd4e Mon Sep 17 00:00:00 2001 From: Akihiro MOTOKI Date: Mon, 10 Sep 2012 22:21:35 +0900 Subject: [PATCH] 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 --- quantum/agent/l3_agent.py | 12 ++- quantum/agent/linux/interface.py | 103 +++++++++++++++++---- quantum/tests/unit/test_linux_interface.py | 77 +++++++++++++++ 3 files changed, 169 insertions(+), 23 deletions(-) diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py index 6a302b888..9714ec2e5 100644 --- 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 36017c05e..d6f16c2a1 100644 --- 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 8f9b9ba1e..1e70b4be6 100644 --- 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) -- 2.45.2