From f42fb1241bb70b03b0715412b99257339e6bdc8d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 20 May 2021 11:38:04 +0100 Subject: [PATCH] Add 'hw:vif_multiqueue_enabled' flavor extra spec This mirrors the 'hw_vif_multiqueue_enabled' image metadata property. Providing a way to set this via flavor extra specs allows admins to enable this by default and easily enable it for existing instances without the need to rebuild (a destructive operation). Note that, in theory at least, the image import workflow provided by glance should allows admins to enable this by default, but the legacy image create workflow does not allow this and admins cannot really control which API end users use when uploading their own images. Also note that we could provide this behavior using a host-level configuration option. This would be similar to what we do for other attributes such as machine type ('hw_machine_type' image meta prop or '[libvirt] hw_machine_type' config option) or pointer model ('hw_pointer_model' image meta prop or '[compute] pointer_model' config option) and would be well suited to things that we don't expect to change, such as enabling multiqueue (it's a sensible default). However, we would need to start storing this information in system_metadata, like we do for machine type (since Wallaby) to prevent things changing over live migration. We have also started avoiding host-level config options for things like this since one must ensure that the value configured are consistent across deployments to behavior that varies depending on the host the guest is initially created on. Change-Id: I405d0324abe32b31a434105cf2c104876fe9c127 Signed-off-by: Stephen Finucane --- nova/api/validation/extra_specs/hw.py | 14 +++++ nova/compute/api.py | 1 + nova/tests/unit/virt/libvirt/test_vif.py | 58 +++++++++++++------ nova/tests/unit/virt/test_hardware.py | 44 ++++++++++++++ nova/virt/hardware.py | 49 ++++++++++++++++ nova/virt/libvirt/vif.py | 26 +++------ ...iqueue-configuration-41e2cbc4ca024682.yaml | 8 +++ 7 files changed, 164 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/flavor-based-multiqueue-configuration-41e2cbc4ca024682.yaml diff --git a/nova/api/validation/extra_specs/hw.py b/nova/api/validation/extra_specs/hw.py index b28eb7fb0720..4aaccf639a43 100644 --- a/nova/api/validation/extra_specs/hw.py +++ b/nova/api/validation/extra_specs/hw.py @@ -368,6 +368,20 @@ feature_flag_validators = [ 'description': 'Whether to enable the boot menu', }, ), + base.ExtraSpecValidator( + name='hw:vif_multiqueue_enabled', + description=( + 'Whether to enable the virtio-net multiqueue feature. ' + 'When set, the driver sets the number of queues equal to the ' + 'number of guest vCPUs. This makes the network performance scale ' + 'across a number of vCPUs. This requires guest support and is ' + 'only supported by the libvirt driver.' + ), + value={ + 'type': bool, + 'description': 'Whether to enable multiqueue', + }, + ), base.ExtraSpecValidator( name='hw:mem_encryption', description=( diff --git a/nova/compute/api.py b/nova/compute/api.py index ae8817e02d7b..28368d910fde 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -852,6 +852,7 @@ class API: hardware.get_number_of_serial_ports(flavor, image_meta) hardware.get_realtime_cpu_constraint(flavor, image_meta) hardware.get_cpu_topology_constraints(flavor, image_meta) + hardware.get_vif_multiqueue_constraint(flavor, image_meta) if validate_numa: hardware.numa_get_constraints(flavor, image_meta) if validate_pci: diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 689b13007ee1..43504efeb539 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -702,17 +702,20 @@ class LibvirtVifTestCase(test.NoDBTestCase): image_meta = objects.ImageMeta.from_dict( {'properties': {'hw_vif_model': 'virtio', 'hw_vif_multiqueue_enabled': 'true'}}) - flavor = objects.Flavor(name='m1.small', - memory_mb=128, - vcpus=4, - root_gb=0, - ephemeral_gb=0, - swap=0, - deleted_at=None, - deleted=0, - created_at=None, flavorid=1, - is_public=True, vcpu_weight=None, - id=2, disabled=False, rxtx_factor=1.0) + flavor = objects.Flavor( + id=2, + name='m1.small', + memory_mb=128, + vcpus=4, + root_gb=0, + ephemeral_gb=0, + swap=0, + deleted_at=None, + deleted=0, + created_at=None, flavorid=1, + is_public=True, vcpu_weight=None, + disabled=False, + extra_specs={}) conf = d.get_base_config(None, 'ca:fe:de:ad:be:ef', image_meta, flavor, 'kvm', 'normal') self.assertEqual(4, conf.vhost_queues) @@ -728,7 +731,8 @@ class LibvirtVifTestCase(test.NoDBTestCase): self.flags(tx_queue_size=1024, group='libvirt') v = vif.LibvirtGenericVIFDriver() conf = v.get_base_config( - None, 'ca:fe:de:ad:be:ef', {}, objects.Flavor(), 'kvm', vnic_type) + None, 'ca:fe:de:ad:be:ef', {}, + objects.Flavor(vcpus=2), 'kvm', vnic_type) return v, conf def test_virtio_vhost_queue_sizes(self): @@ -874,8 +878,22 @@ class LibvirtVifTestCase(test.NoDBTestCase): d = vif.LibvirtGenericVIFDriver() image_meta = {'properties': {'os_name': 'fedora22'}} image_meta = objects.ImageMeta.from_dict(image_meta) + flavor = objects.Flavor( + id=2, + name='m1.small', + memory_mb=128, + vcpus=4, + root_gb=0, + ephemeral_gb=0, + swap=0, + deleted_at=None, + deleted=0, + created_at=None, flavorid=1, + is_public=True, vcpu_weight=None, + disabled=False, + extra_specs={}) d.get_base_config(None, 'ca:fe:de:ad:be:ef', image_meta, - None, 'kvm', 'normal') + flavor, 'kvm', 'normal') mock_set.assert_called_once_with(mock.ANY, 'ca:fe:de:ad:be:ef', 'virtio', None, None, None) @@ -1089,9 +1107,9 @@ class LibvirtVifTestCase(test.NoDBTestCase): mq_ins = objects.Instance( id=1, uuid='f0000000-0000-0000-0000-000000000001', image_ref=uuids.image_ref, flavor=self.flavor_2vcpu, - project_id=723, system_metadata={ + system_metadata={ 'image_hw_vif_multiqueue_enabled': 'True' - } + }, ) d2.plug(mq_ins, self.vif_tap) mock_create_tap_dev.assert_called_once_with( @@ -1129,9 +1147,10 @@ class LibvirtVifTestCase(test.NoDBTestCase): ins = objects.Instance( id=1, uuid='f0000000-0000-0000-0000-000000000001', image_ref=uuids.image_ref, flavor=self.flavor_2vcpu, - project_id=723, system_metadata={ + project_id=723, + system_metadata={ 'image_hw_vif_multiqueue_enabled': 'True' - } + }, ) d1.plug(ins, self.vif_tap) mock_create_tap_dev.assert_called_once_with( @@ -1147,10 +1166,11 @@ class LibvirtVifTestCase(test.NoDBTestCase): ins = objects.Instance( id=1, uuid='f0000000-0000-0000-0000-000000000001', image_ref=uuids.image_ref, flavor=self.flavor_2vcpu, - project_id=723, system_metadata={ + project_id=723, + system_metadata={ 'image_hw_vif_multiqueue_enabled': 'True', 'image_hw_vif_model': 'e1000', - } + }, ) d1.plug(ins, self.vif_tap) mock_create_tap_dev.assert_called_once_with( diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py index 8d999d9ec208..6f552255d4c3 100644 --- a/nova/tests/unit/virt/test_hardware.py +++ b/nova/tests/unit/virt/test_hardware.py @@ -5452,6 +5452,50 @@ class PCINUMAAffinityPolicyTest(test.NoDBTestCase): image_meta.properties.hw_pci_numa_affinity_policy = "fake" +@ddt.ddt +class VIFMultiqueueEnabledTest(test.NoDBTestCase): + + @ddt.unpack + @ddt.data( + # pass: no configuration + (None, None, False), + # pass: flavor-only configuration + ('yes', None, True), + # pass: image-only configuration + (None, False, False), + # pass: identical image and flavor configuration + ('yes', True, True), + # fail: mismatched image and flavor configuration + ('no', True, exception.FlavorImageConflict), + ) + def test_get_vif_multiqueue_constraint( + self, flavor_policy, image_policy, expected, + ): + extra_specs = {} + + if flavor_policy: + extra_specs['hw:vif_multiqueue_enabled'] = flavor_policy + + image_meta_props = {} + + if image_policy: + image_meta_props['hw_vif_multiqueue_enabled'] = image_policy + + flavor = objects.Flavor( + name='foo', vcpus=2, memory_mb=1024, extra_specs=extra_specs) + image_meta = objects.ImageMeta.from_dict( + {'name': 'bar', 'properties': image_meta_props}) + + if isinstance(expected, type) and issubclass(expected, Exception): + self.assertRaises( + expected, hw.get_vif_multiqueue_constraint, flavor, image_meta, + ) + else: + self.assertEqual( + expected, hw.get_vif_multiqueue_constraint(flavor, image_meta), + ) + + @ddt.ddt class VTPMConfigTest(test.NoDBTestCase): diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index 87b483bc8f0b..994be564180c 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -1784,6 +1784,55 @@ def get_pci_numa_policy_constraint( return policy +def get_vif_multiqueue_constraint( + flavor: 'objects.Flavor', + image_meta: 'objects.ImageMeta', +) -> bool: + """Validate and return the requested VIF multiqueue configuration. + + :param flavor: ``nova.objects.Flavor`` instance + :param image_meta: ``nova.objects.ImageMeta`` instance + :raises: nova.exception.FlavorImageConflict if a value is specified in both + the flavor and the image, but the values do not match + :raises: nova.exception.Invalid if a value or combination of values is + invalid + :returns: True if the multiqueue must be enabled, else False. + """ + if flavor.vcpus < 2: + return False + + flavor_value_str, image_value = _get_flavor_image_meta( + 'vif_multiqueue_enabled', flavor, image_meta) + + flavor_value = None + if flavor_value_str is not None: + flavor_value = strutils.bool_from_string(flavor_value_str) + + if ( + image_value is not None and + flavor_value is not None and + image_value != flavor_value + ): + msg = _( + "Flavor %(flavor_name)s has %(prefix)s:%(key)s extra spec " + "explicitly set to %(flavor_val)s, conflicting with image " + "%(image_name)s which has %(prefix)s_%(key)s explicitly set to " + "%(image_val)s." + ) + raise exception.FlavorImageConflict( + msg % { + 'prefix': 'hw', + 'key': 'vif_multiqueue_enabled', + 'flavor_name': flavor.name, + 'flavor_val': flavor_value, + 'image_name': image_meta.name, + 'image_val': image_value, + } + ) + + return flavor_value or image_value or False + + def get_vtpm_constraint( flavor: 'objects.Flavor', image_meta: 'objects.ImageMeta', diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index d9ec024aa1cb..85c83572e193 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -39,6 +39,7 @@ from nova.pci import utils as pci_utils import nova.privsep.linux_net from nova import profiler from nova import utils +from nova.virt import hardware from nova.virt.libvirt import config as vconfig from nova.virt.libvirt import designer from nova.virt.libvirt import host as libvirt_host @@ -256,9 +257,12 @@ class LibvirtGenericVIFDriver(object): """A methods to set the number of virtio queues, if it has been requested in extra specs. """ + if not isinstance(image_meta, objects.ImageMeta): + image_meta = objects.ImageMeta.from_dict(image_meta) + driver = None vhost_queues = None - if self._requests_multiqueue(image_meta): + if hardware.get_vif_multiqueue_constraint(flavor, image_meta): driver = 'vhost' max_tap_queues = self._get_max_tap_queues() if max_tap_queues: @@ -269,19 +273,6 @@ class LibvirtGenericVIFDriver(object): return (driver, vhost_queues) - def _requests_multiqueue(self, image_meta): - """Check if multiqueue property is set in the image metadata.""" - - if not isinstance(image_meta, objects.ImageMeta): - image_meta = objects.ImageMeta.from_dict(image_meta) - - img_props = image_meta.properties - - if img_props.get('hw_vif_multiqueue_enabled'): - return True - - return False - def _get_max_tap_queues(self): # Note(sean-k-mooney): some linux distros have backported # changes for newer kernels which make the kernel version @@ -692,9 +683,10 @@ class LibvirtGenericVIFDriver(object): vif_model = self.get_vif_model(image_meta=image_meta) # TODO(ganso): explore whether multiqueue works for other vif models # that go through this code path. - multiqueue = (instance.get_flavor().vcpus > 1 and - self._requests_multiqueue(image_meta) and - vif_model == network_model.VIF_MODEL_VIRTIO) + multiqueue = False + if vif_model == network_model.VIF_MODEL_VIRTIO: + multiqueue = hardware.get_vif_multiqueue_constraint( + instance.flavor, image_meta) nova.privsep.linux_net.create_tap_dev(dev, mac, multiqueue=multiqueue) network = vif.get('network') mtu = network.get_meta('mtu') if network else None diff --git a/releasenotes/notes/flavor-based-multiqueue-configuration-41e2cbc4ca024682.yaml b/releasenotes/notes/flavor-based-multiqueue-configuration-41e2cbc4ca024682.yaml new file mode 100644 index 000000000000..a73096104ad8 --- /dev/null +++ b/releasenotes/notes/flavor-based-multiqueue-configuration-41e2cbc4ca024682.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The ``hw:vif_multiqueue_enabled`` flavor extra spec has been added. This + is a boolean option that, when set, can be used to enable or disable + multiqueue for virtio-net VIFs. It complements the equivalent image + metadata property, ``hw_vif_multiqueue_enabled``. If both values are set, + they must be identical or an error will be raised.