Skip to content

Commit

Permalink
Merge OVSVethInterfaceDriver into OVSInterfaceDriver
Browse files Browse the repository at this point in the history
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
  • Loading branch information
amotoki committed Oct 11, 2012
1 parent 94e4861 commit 95c4d0e
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 88 deletions.
128 changes: 53 additions & 75 deletions quantum/agent/linux/interface.py
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
38 changes: 25 additions & 13 deletions quantum/tests/unit/test_linux_interface.py
Expand Up @@ -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()

Expand Down Expand Up @@ -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)

Expand All @@ -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',
Expand All @@ -217,20 +229,20 @@ 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),
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()])
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)
Expand All @@ -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')])
Expand Down

0 comments on commit 95c4d0e

Please sign in to comment.