From 21b076e7dfb8a28eb520769f0b257e761f863597 Mon Sep 17 00:00:00 2001 From: Miguel Lavalle Date: Sun, 20 Feb 2022 18:21:37 -0600 Subject: [PATCH] Fix race with DPDK and vhostuserclient mode When a VM is rebooted and it has a port in a Neutron trunk with DPDK and vhostuserclient mode, Nova will delete the OVS port and then recreate it when the VM reboots. This quick transition can create a race condition whereby Neutron deletes the trunk's bridge between the interface removal and addition by os-vif, so the latter operation fails because the bridge doesn't exist anymore. To fix this, ensuring the bridge existance and the vif addition becomes an atomic operation from the point of view of the OVSDB transaction. This change is associated to [1] on the Neutron side. [1] https://review.opendev.org/c/openstack/neutron/+/829139 Partial-Bug: #1869244 Change-Id: Id7ece4ebc9239d9776c43b8d7f9e82b0319a08c6 --- vif_plug_ovs/ovs.py | 5 ++-- vif_plug_ovs/ovsdb/ovsdb_lib.py | 6 ++++- vif_plug_ovs/tests/unit/test_plugin.py | 36 ++++++++++---------------- 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 69e5f3d1..3d459635 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -196,12 +196,11 @@ class OvsPlugin(plugin.PluginBase): return profile.datapath_type def _plug_vhostuser(self, vif, instance_info): - self.ovsdb.ensure_ovs_bridge( - vif.network.bridge, self._get_vif_datapath_type( - vif, datapath=constants.OVS_DATAPATH_NETDEV)) vif_name = OvsPlugin.gen_port_name( constants.OVS_VHOSTUSER_PREFIX, vif.id) args = {} + args['datapath_type'] = self._get_vif_datapath_type(vif, + datapath=constants.OVS_DATAPATH_NETDEV) if vif.mode == "client": args['interface_type'] = \ constants.OVS_VHOSTUSER_INTERFACE_TYPE diff --git a/vif_plug_ovs/ovsdb/ovsdb_lib.py b/vif_plug_ovs/ovsdb/ovsdb_lib.py index ad11ba5d..98cba802 100644 --- a/vif_plug_ovs/ovsdb/ovsdb_lib.py +++ b/vif_plug_ovs/ovsdb/ovsdb_lib.py @@ -138,7 +138,7 @@ class BaseOVS(object): def create_ovs_vif_port( self, bridge, dev, iface_id, mac, instance_id, mtu=None, interface_type=None, vhost_server_path=None, - tag=None, pf_pci=None, vf_num=None, set_ids=True + tag=None, pf_pci=None, vf_num=None, set_ids=True, datapath_type=None ): """Create OVS port @@ -154,6 +154,7 @@ class BaseOVS(object): :param pf_pci: PCI address of PF for dpdk representor port. :param vf_num: VF number of PF for dpdk representor port. :param set_ids: set external ids on port (bool). + :param datapath_type: datapath type for port's bridge .. note:: create DPDK representor port by setting all three values: `interface_type`, `pf_pci` and `vf_num`. if interface type is @@ -177,6 +178,9 @@ class BaseOVS(object): col_values.append(('options', {'dpdk-devargs': devargs_string})) with self.ovsdb.transaction() as txn: + if datapath_type: + txn.add(self.ovsdb.add_br(bridge, may_exist=True, + datapath_type=datapath_type)) txn.add(self.ovsdb.add_port(bridge, dev)) if tag: txn.add(self.ovsdb.db_set('Port', dev, ('tag', tag))) diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index d7ec01d6..a0576f7e 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/vif_plug_ovs/tests/unit/test_plugin.py @@ -362,47 +362,37 @@ class PluginTest(testtools.TestCase): def test_unplug_ovs_bridge_windows(self): self._check_unplug_ovs_windows(self.vif_ovs_hybrid) - @mock.patch.object(ovsdb_lib.BaseOVS, 'ensure_ovs_bridge') @mock.patch.object(ovs.OvsPlugin, '_create_vif_port') - def test_plug_ovs_vhostuser(self, _create_vif_port, ensure_ovs_bridge): + def test_plug_ovs_vhostuser(self, _create_vif_port): dp_type = ovs.OvsPlugin._get_vif_datapath_type(self.vif_vhostuser) - calls = { - - '_create_vif_port': [mock.call( - self.vif_vhostuser, 'vhub679325f-ca', - self.instance, - interface_type='dpdkvhostuser')], - 'ensure_ovs_bridge': [mock.call('br0', dp_type)] - } + calls = [mock.call( + self.vif_vhostuser, 'vhub679325f-ca', + self.instance, + interface_type='dpdkvhostuser', + datapath_type=dp_type)] plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) plugin.plug(self.vif_vhostuser, self.instance) - _create_vif_port.assert_has_calls(calls['_create_vif_port']) - ensure_ovs_bridge.assert_has_calls(calls['ensure_ovs_bridge']) + _create_vif_port.assert_has_calls(calls) - @mock.patch.object(ovsdb_lib.BaseOVS, 'ensure_ovs_bridge') @mock.patch.object(ovsdb_lib.BaseOVS, 'create_ovs_vif_port') - def test_plug_ovs_vhostuser_client(self, create_ovs_vif_port, - ensure_ovs_bridge): + def test_plug_ovs_vhostuser_client(self, create_ovs_vif_port): dp_type = ovs.OvsPlugin._get_vif_datapath_type( self.vif_vhostuser_client) - calls = { - 'create_ovs_vif_port': [ + calls = [ mock.call( 'br0', 'vhub679325f-ca', 'e65867e0-9340-4a7f-a256-09af6eb7a3aa', 'ca:fe:de:ad:be:ef', 'f0000000-0000-0000-0000-000000000001', mtu=1500, interface_type='dpdkvhostuserclient', - vhost_server_path='/var/run/openvswitch/vhub679325f-ca' - )], - 'ensure_ovs_bridge': [mock.call('br0', dp_type)] - } + vhost_server_path='/var/run/openvswitch/vhub679325f-ca', + datapath_type=dp_type + )] plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) plugin.plug(self.vif_vhostuser_client, self.instance) - create_ovs_vif_port.assert_has_calls(calls['create_ovs_vif_port']) - ensure_ovs_bridge.assert_has_calls(calls['ensure_ovs_bridge']) + create_ovs_vif_port.assert_has_calls(calls) @mock.patch.object(ovsdb_lib.BaseOVS, 'delete_ovs_vif_port') def test_unplug_ovs_vhostuser(self, delete_ovs_vif_port):