diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 6fdcceb0..786d0d7d 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -34,6 +34,10 @@ from vif_plug_ovs.ovsdb import ovsdb_lib LOG = logging.getLogger(__name__) +def is_trunk_bridge(bridge_name): + return bridge_name.startswith(constants.TRUNK_BR_PREFIX) + + class OvsPlugin(plugin.PluginBase): """An OVS plugin that can setup VIFs in many ways @@ -204,7 +208,7 @@ class OvsPlugin(plugin.PluginBase): bridge = kwargs.pop('bridge', vif.network.bridge) # See bug #2069543. if (self._isolate_vif(vif_name, bridge) and - not self._is_trunk_bridge(bridge)): + not is_trunk_bridge(bridge)): kwargs['tag'] = constants.DEAD_VLAN kwargs['vlan_mode'] = 'trunk' kwargs['trunks'] = constants.DEAD_VLAN @@ -388,11 +392,8 @@ class OvsPlugin(plugin.PluginBase): vif=vif, err="This vif type is not supported by this plugin") - def _is_trunk_bridge(self, bridge_name): - return bridge_name.startswith(constants.TRUNK_BR_PREFIX) - def _delete_bridge_if_trunk(self, vif): - if self._is_trunk_bridge(vif.network.bridge): + if is_trunk_bridge(vif.network.bridge): self.ovsdb.delete_ovs_bridge(vif.network.bridge) def _unplug_vhostuser(self, vif, instance_info): diff --git a/vif_plug_ovs/ovsdb/ovsdb_lib.py b/vif_plug_ovs/ovsdb/ovsdb_lib.py index b3a15420..12904022 100644 --- a/vif_plug_ovs/ovsdb/ovsdb_lib.py +++ b/vif_plug_ovs/ovsdb/ovsdb_lib.py @@ -16,6 +16,7 @@ from oslo_log import log as logging from vif_plug_ovs import constants from vif_plug_ovs import linux_net +from vif_plug_ovs import ovs from vif_plug_ovs.ovsdb import api as ovsdb_api @@ -167,6 +168,21 @@ class BaseOVS(object): 'iface-status': 'active', 'attached-mac': mac, 'vm-uuid': instance_id} + + # Note(lajoskatona): Neutron fills external_ids for trunk, see: + # https://opendev.org/openstack/neutron/src/commit/ + # 1bc4b526e9c743423069ab4cf6ef3883d5e48217/neutron/services/trunk/ + # drivers/openvswitch/agent/ovsdb_handler.py#L418 + # The following keys are added there: bridge_name, trunk_id and + # subport_ids. These values are used during the cleanup after the + # deletion of the trunk. It can happen that Neutron can't fill these + # fields. + # In os-vif when the plug happens we can use the same transaction to + # add bridge_name to external_ids in case of it is a trunk. + # By this Neutron can do the cleanup of trunk related interfaces. + if ovs.is_trunk_bridge(bridge): + external_ids['bridge_name'] = bridge + col_values = [('external_ids', external_ids)] if set_ids else [] if interface_type: col_values.append(('type', interface_type)) diff --git a/vif_plug_ovs/tests/functional/test_plugin.py b/vif_plug_ovs/tests/functional/test_plugin.py index e0b2d1e6..67890067 100644 --- a/vif_plug_ovs/tests/functional/test_plugin.py +++ b/vif_plug_ovs/tests/functional/test_plugin.py @@ -13,6 +13,7 @@ import testscenarios import time from unittest import mock +import uuid from oslo_concurrency import processutils from oslo_config import cfg @@ -218,3 +219,64 @@ class TestOVSPlugin(testscenarios.WithScenarios, self.plugin.plug(vif, self.instance) self.addCleanup(self._del_bridge, 'tbr-ef98b384') self._check_parameter('Port', vif.vif_name, 'tag', []) + + def test_plug_trunk_bridge_fills_bridge_name(self): + mac = 'ca:fe:de:ad:be:ef' + iface_id = str(uuid.uuid4()) + vif_name = 'port-%s' % iface_id[:8] + trunk_id = str(uuid.uuid4()) + bridge_name = 'tbr-%s' % trunk_id[:8] + + network = objects.network.Network( + id=trunk_id, + bridge=bridge_name, + subnets=self.subnets, + vlan=99) + vif = objects.vif.VIFOpenVSwitch( + id=iface_id, + address=mac, + network=network, + port_profile=self.profile_ovs_system, + vif_name=vif_name) + self.plugin.plug(vif, self.instance) + self.addCleanup(self._del_bridge, bridge_name) + expected_external_ids = { + 'attached-mac': mac, + 'bridge_name': bridge_name, + 'iface-id': self.profile_ovs.interface_id, + 'iface-status': 'active', + 'vm-uuid': self.instance.uuid, + } + + self._check_parameter('Interface', vif.vif_name, + 'external_ids', expected_external_ids) + + def test_plug_non_trunk_leave_bridge_name_empty(self): + mac = 'ca:fe:de:ad:be:ef' + iface_id = str(uuid.uuid4()) + vif_name = 'port-%s' % iface_id[:8] + bridge_name = 'br-something' + + network = objects.network.Network( + id=str(uuid.uuid4()), + bridge=bridge_name, + subnets=self.subnets, + vlan=99) + vif = objects.vif.VIFOpenVSwitch( + id=iface_id, + address=mac, + network=network, + port_profile=self.profile_ovs_system, + vif_name=vif_name) + self.plugin.plug(vif, self.instance) + self.addCleanup(self._del_bridge, bridge_name) + # bridge_name is filled only in case of trunk plug + expected_external_ids = { + 'attached-mac': mac, + 'iface-id': self.profile_ovs.interface_id, + 'iface-status': 'active', + 'vm-uuid': self.instance.uuid, + } + + self._check_parameter('Interface', vif.vif_name, + 'external_ids', expected_external_ids) diff --git a/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py b/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py index 498c2b1f..c1830087 100644 --- a/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py +++ b/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py @@ -89,19 +89,22 @@ class BaseOVSTest(testtools.TestCase): ) mock_set_mtu_request.assert_not_called() - def test_create_ovs_vif_port(self): + def _test_create_ovs_vif_port(self, bridge_name='bridge', + check_br_name=False): iface_id = 'iface_id' mac = 'ca:fe:ca:fe:ca:fe' instance_id = uuidutils.generate_uuid() interface_type = constants.OVS_VHOSTUSER_INTERFACE_TYPE vhost_server_path = '/fake/path' device = 'device' - bridge = 'bridge' + bridge = bridge_name mtu = 1500 external_ids = {'iface-id': iface_id, 'iface-status': 'active', 'attached-mac': mac, 'vm-uuid': instance_id} + if check_br_name: + external_ids['bridge_name'] = bridge_name values = [('external_ids', external_ids), ('type', interface_type), ('options', {'vhost-server-path': vhost_server_path}) @@ -128,6 +131,13 @@ class BaseOVSTest(testtools.TestCase): ] ) + def test_create_ovs_vif_port(self): + self._test_create_ovs_vif_port() + + def test_create_ovs_vif_port_for_trunk(self): + self._test_create_ovs_vif_port(bridge_name='tbr-12345', + check_br_name=True) + def test_create_ovs_vif_port_type_dpdk(self): iface_id = 'iface_id' mac = 'ca:fe:ca:fe:ca:fe'