diff --git a/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml b/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml new file mode 100644 index 00000000..2f46ffa8 --- /dev/null +++ b/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml @@ -0,0 +1,24 @@ +--- +features: + - | + In this release the OVS plugin was extended to always plug VIFs even when + libvirt could plug the vif. This will enable faster migration leveraging + the multiple port bindings work completed in the Rocky release. +security: + - | + In this release an edgecase where libvirt plugged the VIF instead of os-vif + was addressed. Previously if ``ovs_hybrid_plug`` was set to ``False`` in + the port binding details, os-vif would only ensure the ovs bridge existed + and the plugging would be done by libvirt. As a result during live + migration, there was a short interval where a guest could receive tagged + broadcast, multicast, or flooded traffic to/from another tenant. + This vulnerability is described in `bug 1734320`_. By ensuring that + os-vif always creates the OVS port as part of vif plugging we enable + neutron to isolate the port prior to nova resuming the VM on the + destination node. Note that as Nova cannot rely on Neutron to send + ``network-vif-plugged`` events on completion of wiring up an interface + it cannot wait to receive a notification before proceeding with the + migration. As a result this is a partial mitigation and additional changes + will be required to fully address this bug. + + .. _bug 1734320: https://bugs.launchpad.net/neutron/+bug/1734320 \ No newline at end of file diff --git a/vif_plug_ovs/linux_net.py b/vif_plug_ovs/linux_net.py index 48c395bf..689a4e1b 100644 --- a/vif_plug_ovs/linux_net.py +++ b/vif_plug_ovs/linux_net.py @@ -66,6 +66,7 @@ def interface_in_bridge(bridge, device): {'bridge': bridge, 'device': device}) +@privsep.vif_plug.entrypoint def delete_net_dev(dev): """Delete a network device only if it exists.""" if ip_lib.exists(dev): @@ -139,7 +140,8 @@ def add_bridge_port(bridge, dev): @privsep.vif_plug.entrypoint def set_device_mtu(dev, mtu): """Set the device MTU.""" - ip_lib.set(dev, mtu=mtu, check_exit_code=[0, 2, 254]) + if ip_lib.exists(dev): + ip_lib.set(dev, mtu=mtu, check_exit_code=[0, 2, 254]) @privsep.vif_plug.entrypoint diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 3d7352fe..82d4e7c6 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -197,6 +197,12 @@ class OvsPlugin(plugin.PluginBase): self._get_vif_datapath_type(vif)) self._create_vif_port(vif, vif.id, instance_info) + def _plug_vif_generic(self, vif, instance_info): + """Create a per-VIF OVS port.""" + self.ovsdb.ensure_ovs_bridge(vif.network.bridge, + self._get_vif_datapath_type(vif)) + self._create_vif_port(vif, vif.vif_name, instance_info) + def _plug_vf_passthrough(self, vif, instance_info): self.ovsdb.ensure_ovs_bridge( vif.network.bridge, constants.OVS_DATAPATH_SYSTEM) @@ -218,8 +224,7 @@ class OvsPlugin(plugin.PluginBase): if isinstance(vif, objects.vif.VIFOpenVSwitch): if sys.platform != constants.PLATFORM_WIN32: - self.ovsdb.ensure_ovs_bridge(vif.network.bridge, - self._get_vif_datapath_type(vif)) + self._plug_vif_generic(vif, instance_info) else: self._plug_vif_windows(vif, instance_info) elif isinstance(vif, objects.vif.VIFBridge): @@ -255,6 +260,10 @@ class OvsPlugin(plugin.PluginBase): """Remove port from OVS.""" self.ovsdb.delete_ovs_vif_port(vif.network.bridge, vif.id) + def _unplug_vif_generic(self, vif, instance_info): + """Remove port from OVS.""" + self.ovsdb.delete_ovs_vif_port(vif.network.bridge, vif.vif_name) + def _unplug_vf_passthrough(self, vif, instance_info): """Remove port from OVS.""" pci_slot = vif.dev_address @@ -278,7 +287,9 @@ class OvsPlugin(plugin.PluginBase): profile=vif.port_profile.__class__.__name__) if isinstance(vif, objects.vif.VIFOpenVSwitch): - if sys.platform == constants.PLATFORM_WIN32: + if sys.platform != constants.PLATFORM_WIN32: + self._unplug_vif_generic(vif, instance_info) + else: self._unplug_vif_windows(vif, instance_info) elif isinstance(vif, objects.vif.VIFBridge): if sys.platform != constants.PLATFORM_WIN32: diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index 82fb0f6b..9686be56 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/vif_plug_ovs/tests/unit/test_plugin.py @@ -69,7 +69,7 @@ class PluginTest(testtools.TestCase): id='b679325f-ca89-4ee0-a8be-6db1409b69ea', address='ca:fe:de:ad:be:ef', network=self.network_ovs, - dev_name='tap-xxx-yyy-zzz', + vif_name='tap-xxx-yyy-zzz', bridge_name="qbrvif-xxx-yyy", port_profile=self.profile_ovs_no_datatype) @@ -77,7 +77,7 @@ class PluginTest(testtools.TestCase): id='b679325f-ca89-4ee0-a8be-6db1409b69ea', address='ca:fe:de:ad:be:ef', network=self.network_ovs, - dev_name='tap-xxx-yyy-zzz', + vif_name='tap-xxx-yyy-zzz', port_profile=self.profile_ovs) self.vif_vhostuser = objects.vif.VIFVHostUser( @@ -147,16 +147,22 @@ class PluginTest(testtools.TestCase): interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) @mock.patch.object(ovs, 'sys') - @mock.patch.object(ovsdb_lib.BaseOVS, 'ensure_ovs_bridge') - def test_plug_ovs(self, ensure_ovs_bridge, mock_sys): + @mock.patch.object(ovs.OvsPlugin, '_plug_vif_generic') + def test_plug_ovs(self, plug_vif_generic, mock_sys): mock_sys.platform = 'linux' - plug_bridge_mock = mock.Mock() plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) - plugin._plug_bridge = plug_bridge_mock plugin.plug(self.vif_ovs, self.instance) - dp_type = ovs.OvsPlugin._get_vif_datapath_type(self.vif_ovs) - ensure_ovs_bridge.assert_called_once_with(self.vif_ovs.network.bridge, - dp_type) + plug_vif_generic.assert_called_once_with(self.vif_ovs, + self.instance) + + @mock.patch.object(ovsdb_lib.BaseOVS, 'ensure_ovs_bridge') + @mock.patch.object(ovs.OvsPlugin, "_create_vif_port") + def test_plug_vif_generic(self, create_port, ensure_bridge): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + plugin._plug_vif_generic(self.vif_ovs, self.instance) + ensure_bridge.assert_called_once() + create_port.assert_called_once_with(self.vif_ovs, + self.vif_ovs.vif_name, self.instance) @mock.patch.object(linux_net, 'set_interface_state') @mock.patch.object(ovsdb_lib.BaseOVS, 'ensure_ovs_bridge') @@ -250,12 +256,19 @@ class PluginTest(testtools.TestCase): def test_plug_ovs_bridge_windows(self): self._check_plug_ovs_windows(self.vif_ovs_hybrid) - def test_unplug_ovs(self): - unplug_bridge_mock = mock.Mock() + @mock.patch.object(ovs, 'sys') + @mock.patch.object(ovs.OvsPlugin, '_unplug_vif_generic') + def test_unplug_ovs(self, unplug, mock_sys): + mock_sys.platform = 'linux' plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) - plugin._unplug_bridge = unplug_bridge_mock plugin.unplug(self.vif_ovs, self.instance) - unplug_bridge_mock.assert_not_called() + unplug.assert_called_once_with(self.vif_ovs, self.instance) + + @mock.patch.object(ovs.OvsPlugin, '_unplug_vif_generic') + def test_unplug_vif_generic(self, delete_port): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + plugin._unplug_vif_generic(self.vif_ovs, self.instance) + delete_port.assert_called_once() @mock.patch.object(ovsdb_lib.BaseOVS, 'delete_ovs_vif_port') @mock.patch.object(linux_net, 'delete_bridge')