From a69845f3732843ee1451b2e4ebf547d9801e898d Mon Sep 17 00:00:00 2001 From: Rodrigo Barbieri Date: Thu, 27 Aug 2020 17:20:19 -0300 Subject: [PATCH] Allow tap interface with multiqueue When vif_type="tap" (such as when using calico), attempting to create an instance using an image that has the property hw_vif_multiqueue_enabled=True fails, because the interface is always being created without multiqueue flags. This change checks if the property is defined and passes the multiqueue parameter to create the tap interface accordingly. In case the multiqueue parameter is passed but the vif_model is not virtio (or unspecified), the old behavior is maintained. Conflicts: nova/virt/libvirt/vif.py NOTE: The conflict is due to not having patch Iefa6009874395cafe3121e44a48d882c38230d16 in stable/ussuri. Change-Id: I0307c43dcd0cace1620d2ac75925651d4ee2e96c Closes-bug: #1893263 (cherry picked from commit 84cfc8e9ab1396ec17abcfc9646c7d40f1d966ae) --- nova/tests/unit/virt/libvirt/test_vif.py | 67 ++++++++++++++++++- nova/virt/libvirt/vif.py | 57 +++++++++++----- .../notes/bug-1893263-769acadc4b6141d0.yaml | 5 ++ 3 files changed, 111 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 25aca122cc7..dcb8dba9908 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -1091,10 +1091,71 @@ def test_tap_ethernet_vif_driver(self): @mock.patch('nova.privsep.linux_net.device_exists', return_value=True) @mock.patch('nova.privsep.linux_net.set_device_mtu') @mock.patch('nova.privsep.linux_net.create_tap_dev') - def test_plug_tap(self, mock_create_tap_dev, mock_set_mtu, + def test_plug_tap_kvm_virtio(self, mock_create_tap_dev, mock_set_mtu, mock_device_exists): - d = vif.LibvirtGenericVIFDriver() - d.plug(self.instance, self.vif_tap) + + d1 = vif.LibvirtGenericVIFDriver() + ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + project_id=723, system_metadata={} + ) + d1.plug(ins, self.vif_tap) + mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', None, + multiqueue=False) + + mock_create_tap_dev.reset_mock() + + d2 = vif.LibvirtGenericVIFDriver() + mq_ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + project_id=723, system_metadata={ + 'image_hw_vif_multiqueue_enabled': 'True' + } + ) + d2.plug(mq_ins, self.vif_tap) + mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', None, + multiqueue=True) + + @mock.patch('nova.privsep.linux_net.device_exists', return_value=True) + @mock.patch('nova.privsep.linux_net.set_device_mtu') + @mock.patch('nova.privsep.linux_net.create_tap_dev') + def test_plug_tap_mq_ignored_virt_type( + self, mock_create_tap_dev, mock_set_mtu, mock_device_exists): + + self.flags(use_virtio_for_bridges=True, + virt_type='xen', + group='libvirt') + + d1 = vif.LibvirtGenericVIFDriver() + ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + 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('tap-xxx-yyy-zzz', + None, + multiqueue=False) + + @mock.patch('nova.privsep.linux_net.device_exists', return_value=True) + @mock.patch('nova.privsep.linux_net.set_device_mtu') + @mock.patch('nova.privsep.linux_net.create_tap_dev') + def test_plug_tap_mq_ignored_vif_model( + self, mock_create_tap_dev, mock_set_mtu, mock_device_exists): + + d1 = vif.LibvirtGenericVIFDriver() + ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + 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('tap-xxx-yyy-zzz', + None, + multiqueue=False) def test_unplug_tap(self): d = vif.LibvirtGenericVIFDriver() diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index 2d01da249dd..cf8aff35a0a 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -166,6 +166,24 @@ def get_vif_devname(self, vif): return vif['devname'] return ("nic" + vif['id'])[:network_model.NIC_NAME_LEN] + def get_vif_model(self, image_meta=None, vif_model=None): + + model = vif_model + + # If the user has specified a 'vif_model' against the + # image then honour that model + if image_meta: + model = osinfo.HardwareProperties(image_meta).network_model + + # If the virt type is KVM/QEMU/VZ(Parallels), then use virtio according + # to the global config parameter + if (model is None and CONF.libvirt.virt_type in + ('kvm', 'qemu', 'parallels') and + CONF.libvirt.use_virtio_for_bridges): + model = network_model.VIF_MODEL_VIRTIO + + return model + def get_base_config(self, instance, mac, image_meta, inst_type, virt_type, vnic_type, host): # TODO(sahid): We should rewrite it. This method handles too @@ -186,16 +204,9 @@ def get_base_config(self, instance, mac, image_meta, conf, mac, model, driver, vhost_queues, rx_queue_size) return conf - # If the user has specified a 'vif_model' against the - # image then honour that model - if image_meta: - model = osinfo.HardwareProperties(image_meta).network_model - - # If the virt type is KVM/QEMU/VZ(Parallels), then use virtio according - # to the global config parameter - if (model is None and virt_type in ('kvm', 'qemu', 'parallels') and - CONF.libvirt.use_virtio_for_bridges): - model = network_model.VIF_MODEL_VIRTIO + # if model has already been defined, + # image_meta contents will override it + model = self.get_vif_model(image_meta=image_meta, vif_model=model) if not is_vif_model_valid_for_virt(virt_type, model): raise exception.UnsupportedHardware(model=model, virt=virt_type) @@ -252,10 +263,7 @@ def _get_virtio_mq_settings(self, image_meta, flavor): """ driver = None vhost_queues = None - 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'): + if self._requests_multiqueue(image_meta): driver = 'vhost' max_tap_queues = self._get_max_tap_queues() if max_tap_queues: @@ -266,6 +274,19 @@ def _get_virtio_mq_settings(self, image_meta, flavor): 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 @@ -697,7 +718,13 @@ def plug_tap(self, instance, vif): """Plug a VIF_TYPE_TAP virtual interface.""" dev = self.get_vif_devname(vif) mac = vif['details'].get(network_model.VIF_DETAILS_TAP_MAC_ADDRESS) - nova.privsep.linux_net.create_tap_dev(dev, mac) + image_meta = instance.image_meta + 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 = (self._requests_multiqueue(image_meta) and + vif_model == network_model.VIF_MODEL_VIRTIO) + nova.privsep.linux_net.create_tap_dev(dev, mac, multiqueue=multiqueue) network = vif.get('network') mtu = network.get_meta('mtu') if network else None nova.privsep.linux_net.set_device_mtu(dev, mtu) diff --git a/releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml b/releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml new file mode 100644 index 00000000000..3874489513b --- /dev/null +++ b/releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Addressed an issue that prevented instances using multiqueue feature from + being created successfully when their vif_type is TAP.