From 95c4d0e82f92fdf6d2a4e93453df3b831193f54a Mon Sep 17 00:00:00 2001 From: Akihiro MOTOKI Date: Fri, 28 Sep 2012 22:05:50 +0900 Subject: [PATCH] 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 --- quantum/agent/linux/interface.py | 128 +++++++++------------ quantum/tests/unit/test_linux_interface.py | 38 +++--- 2 files changed, 78 insertions(+), 88 deletions(-) diff --git a/quantum/agent/linux/interface.py b/quantum/agent/linux/interface.py index d6f16c2a118..4330629e498 100644 --- a/quantum/agent/linux/interface.py +++ b/quantum/agent/linux/interface.py @@ -34,6 +34,9 @@ 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 @@ def unplug(self, device_name, bridge=None, namespace=None, prefix=None): 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 @@ def plug(self, network_id, port_id, device_name, mac_address, 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 @@ def plug(self, network_id, port_id, device_name, mac_address, 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 1e70b4be68d..f449a730759 100644 --- a/quantum/tests/unit/test_linux_interface.py +++ b/quantum/tests/unit/test_linux_interface.py @@ -106,6 +106,11 @@ def test_l3_init(self): 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 @@ def test_unplug(self, bridge=None): 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 @@ def _test_plug(self, devname=None, bridge=None, namespace=None, 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 @@ def device_exists(dev, root_helper=None, namespace=None): 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 @@ def device_exists(dev, root_helper=None, namespace=None): 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 @@ 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 = interface.OVSInterfaceDriver(self.conf) ovs.unplug('ns-0', bridge=bridge) ovs_br.assert_has_calls([mock.call(bridge, 'sudo'), mock.call().delete_port('tap0')])