openstack/os-vif
Code Issues Proposed changes

OVS Trunk: Add bridge_name to external_ids

Browse Source 
In case of trunk wiring os-vif is responsible for
creating and deleting the trunk bridge (see [1] and [2]).
It can happen that Neutron can't set the metadata on the
bridge by adding i.e. bridge_name field the external_ids.
In such case, when the trunk is deleted before all these
operations were finished in Neutron, the clean-up of patches
linking br-int and the trunk bridge will never happen.

This patch adds the bridge_name field to external_ids in case
it is a trunk_bridge to avoid such situation.

Code documentation patch on Neutron side: [3].

[1]: https://review.opendev.org/c/openstack/os-vif/+/841499
[2]: https://review.opendev.org/c/openstack/neutron/+/837780
[3]: https://review.opendev.org/c/openstack/neutron/+/949569

Related-Bug: #2087944
Change-Id: I6951c1bfb5a85b1f422a5f9a99b6ade17f897c6f
This commit is contained in:
elajkat
2025-05-14 13:17:31 +02:00
committed by Lajos Katona
parent cc3b13cde9
commit 6b078f4998
4 changed files with 96 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
vif_plug_ovs/ovs.py
View File

@@ -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):

16
vif_plug_ovs/ovsdb/ovsdb_lib.py
View File

@@ -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))

62
vif_plug_ovs/tests/functional/test_plugin.py
View File

@@ -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)

14
vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py
View File

@@ -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'