From 07f2bf8035927fc469cd10171cc3dd6f23f1ccf4 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 17 Jun 2022 19:24:47 +0200 Subject: [PATCH] Reject mixed VF rc and trait config If two VFs from the same PF are configured by two separate [pci]device_spec entries then it is possible to define contradicting resource classes or traits. This patch detects and rejects such configuration. blueprint: pci-device-tracking-in-placement Change-Id: I623ab24940169991a400eba854c9619a11662a91 --- doc/source/admin/pci-passthrough.rst | 5 ++ nova/compute/pci_placement_translator.py | 18 +++++ nova/exception.py | 16 +++++ .../libvirt/test_pci_in_placement.py | 68 +++++++++++++++++++ .../compute/test_pci_placement_translator.py | 43 ++++++++++++ 5 files changed, 150 insertions(+) diff --git a/doc/source/admin/pci-passthrough.rst b/doc/source/admin/pci-passthrough.rst index 2fa3619dae..764922d044 100644 --- a/doc/source/admin/pci-passthrough.rst +++ b/doc/source/admin/pci-passthrough.rst @@ -367,6 +367,11 @@ resource class can be customized via the ``resource_class`` tag in the tag in that configuration that allows specifying a list of placement traits to be added to the resource provider representing the matching PCI devices. +.. note:: + Having different resource class or traits configuration for VFs under the + same parent PF is not supported and the nova-compute service will refuse to + start with such configuration. + .. important:: While nova supported configuring both the PF and its children VFs for PCI passthrough in the past, it only allowed consuming either the parent PF or diff --git a/nova/compute/pci_placement_translator.py b/nova/compute/pci_placement_translator.py index d90c70428b..49d853b5fa 100644 --- a/nova/compute/pci_placement_translator.py +++ b/nova/compute/pci_placement_translator.py @@ -131,7 +131,25 @@ class PciResourceProvider: ) rc = _get_rc_for_dev(dev, dev_spec_tags) + if self.resource_class and rc != self.resource_class: + raise exception.PlacementPciMixedResourceClassException( + new_rc=rc, + new_dev=dev.address, + current_rc=self.resource_class, + current_devs=",".join( + dev.address for dev in self.children_devs) + ) + traits = _get_traits_for_dev(dev_spec_tags) + if self.traits is not None and self.traits != traits: + raise exception.PlacementPciMixedTraitsException( + new_traits=",".join(sorted(traits)), + new_dev=dev.address, + current_traits=",".join(sorted(self.traits)), + current_devs=",".join( + dev.address for dev in self.children_devs), + ) + self.children_devs.append(dev) self.resource_class = rc self.traits = traits diff --git a/nova/exception.py b/nova/exception.py index 5d167bd5d4..bf69b4409d 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -2440,3 +2440,19 @@ class PlacementPciDependentDeviceException(PlacementPciException): "[pci]device_spec is not supported. Either the parent PF or its " "children VFs can be configured." ) + + +class PlacementPciMixedResourceClassException(PlacementPciException): + msg_fmt = _( + "VFs from the same PF cannot be configured with different " + "'resource_class' values in [pci]device_spec. We got %(new_rc)s " + "for %(new_dev)s and %(current_rc)s for %(current_devs)s." + ) + + +class PlacementPciMixedTraitsException(PlacementPciException): + msg_fmt = _( + "VFs from the same PF cannot be configured with different set " + "of 'traits' in [pci]device_spec. We got %(new_traits)s for " + "%(new_dev)s and %(current_traits)s for %(current_devs)s." + ) diff --git a/nova/tests/functional/libvirt/test_pci_in_placement.py b/nova/tests/functional/libvirt/test_pci_in_placement.py index 2655beabbe..e731b123ba 100644 --- a/nova/tests/functional/libvirt/test_pci_in_placement.py +++ b/nova/tests/functional/libvirt/test_pci_in_placement.py @@ -216,3 +216,71 @@ class PlacementPCIReportingTests(test_pci_sriov_servers._PCIServersTestBase): "[pci]device_spec is not supported", str(ex) ) + + def test_sibling_vfs_with_contradicting_resource_classes_rejected(self): + # The fake libvirt will emulate on the host: + # * one type-PF dev in slot 0 with two type-VF under it + pci_info = fakelibvirt.HostPCIDevicesInfo( + num_pci=0, num_pfs=1, num_vfs=2) + # the config matches the two VFs separately and tries to configure + # them with different resource class + device_spec = self._to_device_spec_conf( + [ + { + "address": "0000:81:00.1", + "resource_class": "vf1" + }, + { + "address": "0000:81:00.2", + "resource_class": "vf2" + }, + ] + ) + self.flags(group='pci', device_spec=device_spec) + + ex = self.assertRaises( + exception.PlacementPciMixedResourceClassException, + self.start_compute, + hostname="compute1", + pci_info=pci_info + ) + self.assertIn( + "VFs from the same PF cannot be configured with different " + "'resource_class' values in [pci]device_spec. We got " + "CUSTOM_VF2 for 0000:81:00.2 and CUSTOM_VF1 for 0000:81:00.1.", + str(ex) + ) + + def test_sibling_vfs_with_contradicting_traits_rejected(self): + # The fake libvirt will emulate on the host: + # * one type-PF dev in slot 0 with two type-VF under it + pci_info = fakelibvirt.HostPCIDevicesInfo( + num_pci=0, num_pfs=1, num_vfs=2) + # the config matches the two VFs separately and tries to configure + # them with different trait list + device_spec = self._to_device_spec_conf( + [ + { + "address": "0000:81:00.1", + "traits": "foo", + }, + { + "address": "0000:81:00.2", + "traits": "bar", + }, + ] + ) + self.flags(group='pci', device_spec=device_spec) + + ex = self.assertRaises( + exception.PlacementPciMixedTraitsException, + self.start_compute, + hostname="compute1", + pci_info=pci_info + ) + self.assertIn( + "VFs from the same PF cannot be configured with different set of " + "'traits' in [pci]device_spec. We got CUSTOM_BAR for 0000:81:00.2 " + "and CUSTOM_FOO for 0000:81:00.1.", + str(ex) + ) diff --git a/nova/tests/unit/compute/test_pci_placement_translator.py b/nova/tests/unit/compute/test_pci_placement_translator.py index da06274637..9499e64bb0 100644 --- a/nova/tests/unit/compute/test_pci_placement_translator.py +++ b/nova/tests/unit/compute/test_pci_placement_translator.py @@ -163,3 +163,46 @@ class TestTranslator(test.NoDBTestCase): "children VFs can be configured.", str(ex), ) + + def test_mixed_rc_for_sibling_vfs(self): + pv = ppt.PlacementView("fake-node") + vf1, vf2, vf3, vf4 = [ + pci_device.PciDevice( + address="0000:81:00.%d" % f, + parent_addr="0000:71:00.0", + dev_type=fields.PciDeviceType.SRIOV_VF) + for f in range(0, 4) + ] + + pv.add_dev(vf1, {"resource_class": "a", "traits": "foo,bar,baz"}) + # order is irrelevant + pv.add_dev(vf2, {"resource_class": "a", "traits": "foo,baz,bar"}) + # but missing trait is rejected + ex = self.assertRaises( + exception.PlacementPciMixedTraitsException, + pv.add_dev, + vf3, + {"resource_class": "a", "traits": "foo,bar"}, + ) + self.assertEqual( + "VFs from the same PF cannot be configured with different set of " + "'traits' in [pci]device_spec. We got CUSTOM_BAR,CUSTOM_FOO for " + "0000:81:00.2 and CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_FOO for " + "0000:81:00.0,0000:81:00.1.", + str(ex), + ) + # as well as additional trait + ex = self.assertRaises( + exception.PlacementPciMixedTraitsException, + pv.add_dev, + vf4, + {"resource_class": "a", "traits": "foo,bar,baz,extra"} + ) + self.assertEqual( + "VFs from the same PF cannot be configured with different set of " + "'traits' in [pci]device_spec. We got " + "CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_EXTRA,CUSTOM_FOO for 0000:81:00.3 " + "and CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_FOO for " + "0000:81:00.0,0000:81:00.1.", + str(ex), + )