diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 99eb48e92fa7..2e4cfceac03a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -10929,6 +10929,9 @@ class ComputeManager(manager.Manager): if profile.get('vf_num'): profile['vf_num'] = pci_dev.sriov_cap['vf_num'] + if pci_dev.mac_address: + profile['device_mac_address'] = pci_dev.mac_address + mig_vif.profile = profile LOG.debug("Updating migrate VIF profile for port %(port_id)s:" "%(profile)s", {'port_id': port_id, diff --git a/nova/network/neutron.py b/nova/network/neutron.py index e12dabfe9467..a707fc37c5f2 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -684,7 +684,8 @@ class API: for profile_key in ('pci_vendor_info', 'pci_slot', constants.ALLOCATION, 'arq_uuid', 'physical_network', 'card_serial_number', - 'vf_num', 'pf_mac_address'): + 'vf_num', 'pf_mac_address', + 'device_mac_address'): if profile_key in port_profile: del port_profile[profile_key] port_req_body['port'][constants.BINDING_PROFILE] = port_profile @@ -1307,6 +1308,10 @@ class API: network=network, neutron=neutron, bind_host_id=bind_host_id, port_arq=port_arq) + # NOTE(gibi): Remove this once we are sure that the fix for + # bug 1942329 is always present in the deployed neutron. The + # _populate_neutron_extension_values() call above already + # populated this MAC to the binding profile instead. self._populate_pci_mac_address(instance, request.pci_request_id, port_req_body) @@ -1598,6 +1603,18 @@ class API: if pci_dev.dev_type == obj_fields.PciDeviceType.SRIOV_VF: dev_profile.update( self._get_vf_pci_device_profile(pci_dev)) + + if pci_dev.dev_type == obj_fields.PciDeviceType.SRIOV_PF: + # In general the MAC address information flows fom the neutron + # port to the device in the backend. Except for direct-physical + # ports. In that case the MAC address flows from the physical + # device, the PF, to the neutron port. So when such a port is + # being bound to a host the port's MAC address needs to be + # updated. Nova needs to put the new MAC into the binding + # profile. + if pci_dev.mac_address: + dev_profile['device_mac_address'] = pci_dev.mac_address + return dev_profile raise exception.PciDeviceNotFound(node_id=pci_dev.compute_node_id, @@ -3639,11 +3656,10 @@ class API: revert = migration and migration.status == 'reverted' return instance.migration_context.get_pci_mapping_for_migration(revert) - def _get_port_pci_dev(self, context, instance, port): + def _get_port_pci_dev(self, instance, port): """Find the PCI device corresponding to the port. Assumes the port is an SRIOV one. - :param context: The request context. :param instance: The instance to which the port is attached. :param port: The Neutron port, as obtained from the Neutron API JSON form. @@ -3731,15 +3747,14 @@ class API: raise exception.PortUpdateFailed(port_id=p['id'], reason=_("Unable to correlate PCI slot %s") % pci_slot) - # NOTE(artom) If migration is None, this is an unshevle, and we - # need to figure out the pci_slot from the InstancePCIRequest - # and PciDevice objects. + # NOTE(artom) If migration is None, this is an unshelve, and we + # need to figure out the pci related binding information from + # the InstancePCIRequest and PciDevice objects. else: - pci_dev = self._get_port_pci_dev(context, instance, p) + pci_dev = self._get_port_pci_dev(instance, p) if pci_dev: binding_profile.update( - self._get_pci_device_profile(pci_dev) - ) + self._get_pci_device_profile(pci_dev)) updates[constants.BINDING_PROFILE] = binding_profile # NOTE(gibi): during live migration the conductor already sets the diff --git a/nova/objects/pci_device.py b/nova/objects/pci_device.py index 1c5be87e3c43..554d68feca2b 100644 --- a/nova/objects/pci_device.py +++ b/nova/objects/pci_device.py @@ -148,6 +148,12 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): reason='dev_type=%s not supported in version %s' % ( dev_type, target_version)) + def __repr__(self): + return ( + f'PciDevice(address={self.address}, ' + f'compute_node_id={self.compute_node_id})' + ) + def update_device(self, dev_dict): """Sync the content from device dictionary to device object. @@ -175,6 +181,9 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): # NOTE(ralonsoh): list of parameters currently added to # "extra_info" dict: # - "capabilities": dict of (strings/list of strings) + # - "parent_ifname": the netdev name of the parent (PF) + # device of a VF + # - "mac_address": the MAC address of the PF extra_info = self.extra_info data = v if isinstance(v, str) else jsonutils.dumps(v) extra_info.update({k: data}) @@ -572,6 +581,13 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject): caps = jsonutils.loads(caps_json) return caps.get('sriov', {}) + @property + def mac_address(self): + """The MAC address of the PF physical device or None if the device is + not a PF or if the MAC is not available. + """ + return self.extra_info.get('mac_address') + @base.NovaObjectRegistry.register class PciDeviceList(base.ObjectListBase, base.NovaObject): @@ -611,3 +627,6 @@ class PciDeviceList(base.ObjectListBase, base.NovaObject): parent_addr) return base.obj_make_list(context, cls(context), objects.PciDevice, db_dev_list) + + def __repr__(self): + return f"PciDeviceList(objects={[repr(obj) for obj in self.objects]})" diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 891e9572005d..f6d5d656a2e8 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -309,7 +309,7 @@ class FakePCIDevice(object): self, dev_type, bus, slot, function, iommu_group, numa_node, *, vf_ratio=None, multiple_gpu_types=False, generic_types=False, parent=None, vend_id=None, vend_name=None, prod_id=None, - prod_name=None, driver_name=None, vpd_fields=None + prod_name=None, driver_name=None, vpd_fields=None, mac_address=None, ): """Populate pci devices @@ -331,6 +331,8 @@ class FakePCIDevice(object): :param prod_id: (str) The product ID. :param prod_name: (str) The product name. :param driver_name: (str) The driver name. + :param mac_address: (str) The MAC of the device. + Used in case of SRIOV PFs """ self.dev_type = dev_type @@ -349,6 +351,7 @@ class FakePCIDevice(object): self.prod_id = prod_id self.prod_name = prod_name self.driver_name = driver_name + self.mac_address = mac_address self.vpd_fields = vpd_fields @@ -364,7 +367,9 @@ class FakePCIDevice(object): assert not self.vf_ratio, 'vf_ratio does not apply for PCI devices' if self.dev_type in ('PF', 'VF'): - assert self.vf_ratio, 'require vf_ratio for PFs and VFs' + assert ( + self.vf_ratio is not None + ), 'require vf_ratio for PFs and VFs' if self.dev_type == 'VF': assert self.parent, 'require parent for VFs' @@ -497,6 +502,10 @@ class FakePCIDevice(object): def XMLDesc(self, flags): return self.pci_device + @property + def address(self): + return "0000:%02x:%02x.%1x" % (self.bus, self.slot, self.function) + # TODO(stephenfin): Remove all of these HostFooDevicesInfo objects in favour of # a unified devices object @@ -609,7 +618,7 @@ class HostPCIDevicesInfo(object): self, dev_type, bus, slot, function, iommu_group, numa_node, vf_ratio=None, multiple_gpu_types=False, generic_types=False, parent=None, vend_id=None, vend_name=None, prod_id=None, - prod_name=None, driver_name=None, vpd_fields=None, + prod_name=None, driver_name=None, vpd_fields=None, mac_address=None, ): pci_dev_name = _get_libvirt_nodedev_name(bus, slot, function) @@ -632,6 +641,7 @@ class HostPCIDevicesInfo(object): prod_name=prod_name, driver_name=driver_name, vpd_fields=vpd_fields, + mac_address=mac_address, ) self.devices[pci_dev_name] = dev return dev @@ -651,6 +661,13 @@ class HostPCIDevicesInfo(object): return [dev for dev in self.devices if self.devices[dev].is_capable_of_mdevs] + def get_pci_address_mac_mapping(self): + return { + device.address: device.mac_address + for dev_addr, device in self.devices.items() + if device.mac_address + } + class FakeMdevDevice(object): template = """ @@ -2182,6 +2199,15 @@ class LibvirtFixture(fixtures.Fixture): def __init__(self, stub_os_vif=True): self.stub_os_vif = stub_os_vif + self.pci_address_to_mac_map = collections.defaultdict( + lambda: '52:54:00:1e:59:c6') + + def update_sriov_mac_address_mapping(self, pci_address_to_mac_map): + self.pci_address_to_mac_map.update(pci_address_to_mac_map) + + def fake_get_mac_by_pci_address(self, pci_addr, pf_interface=False): + res = self.pci_address_to_mac_map[pci_addr] + return res def setUp(self): super().setUp() @@ -2205,7 +2231,7 @@ class LibvirtFixture(fixtures.Fixture): self.useFixture(fixtures.MockPatch( 'nova.pci.utils.get_mac_by_pci_address', - return_value='52:54:00:1e:59:c6')) + new=self.fake_get_mac_by_pci_address)) # libvirt calls out to sysfs to get the vfs ID during macvtap plug self.useFixture(fixtures.MockPatch( diff --git a/nova/tests/functional/libvirt/base.py b/nova/tests/functional/libvirt/base.py index 3d8aec8106bc..c325c0b04073 100644 --- a/nova/tests/functional/libvirt/base.py +++ b/nova/tests/functional/libvirt/base.py @@ -42,7 +42,7 @@ class ServersTestBase(integrated_helpers._IntegratedTestBase): super(ServersTestBase, self).setUp() self.useFixture(nova_fixtures.LibvirtImageBackendFixture()) - self.useFixture(nova_fixtures.LibvirtFixture()) + self.libvirt = self.useFixture(nova_fixtures.LibvirtFixture()) self.useFixture(nova_fixtures.OSBrickFixture()) self.useFixture(fixtures.MockPatch( @@ -134,6 +134,12 @@ class ServersTestBase(integrated_helpers._IntegratedTestBase): host_info, pci_info, mdev_info, vdpa_info, libvirt_version, qemu_version, hostname, ) + # If the compute is configured with PCI devices then we need to + # make sure that the stubs around sysfs has the MAC address + # information for the PCI PF devices + if pci_info: + self.libvirt.update_sriov_mac_address_mapping( + pci_info.get_pci_address_mac_mapping()) # This is fun. Firstly we need to do a global'ish mock so we can # actually start the service. with mock.patch('nova.virt.libvirt.host.Host.get_connection', @@ -392,6 +398,22 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture): 'binding:vnic_type': 'remote-managed', } + network_4_port_pf = { + 'id': 'c6f51315-9202-416f-9e2f-eb78b3ac36d9', + 'network_id': network_4['id'], + 'status': 'ACTIVE', + 'mac_address': 'b5:bc:2e:e7:51:01', + 'fixed_ips': [ + { + 'ip_address': '192.168.4.8', + 'subnet_id': subnet_4['id'] + } + ], + 'binding:vif_details': {'vlan': 42}, + 'binding:vif_type': 'hostdev_physical', + 'binding:vnic_type': 'direct-physical', + } + def __init__(self, test): super(LibvirtNeutronFixture, self).__init__(test) self._networks = { diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py index dd5a4249faa4..e045bc64d9be 100644 --- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py +++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py @@ -364,31 +364,66 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): expect_fail=False): # The purpose here is to force an observable PCI slot update when # moving from source to dest. This is accomplished by having a single - # PCI device on the source, 2 PCI devices on the test, and relying on - # the fact that our fake HostPCIDevicesInfo creates predictable PCI - # addresses. The PCI device on source and the first PCI device on dest - # will have identical PCI addresses. By sticking a "placeholder" - # instance on that first PCI device on the dest, the incoming instance - # from source will be forced to consume the second dest PCI device, - # with a different PCI address. + # PCI VF device on the source, 2 PCI VF devices on the dest, and + # relying on the fact that our fake HostPCIDevicesInfo creates + # predictable PCI addresses. The PCI VF device on source and the first + # PCI VF device on dest will have identical PCI addresses. By sticking + # a "placeholder" instance on that first PCI VF device on the dest, the + # incoming instance from source will be forced to consume the second + # dest PCI VF device, with a different PCI address. + # We want to test server operations with SRIOV VFs and SRIOV PFs so + # the config of the compute hosts also have one extra PCI PF devices + # without any VF children. But the two compute has different PCI PF + # addresses and MAC so that the test can observe the slot update as + # well as the MAC updated during migration and after revert. + source_pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=1) + # add an extra PF without VF to be used by direct-physical ports + source_pci_info.add_device( + dev_type='PF', + bus=0x82, # the HostPCIDevicesInfo use the 0x81 by default + slot=0x0, + function=0, + iommu_group=42, + numa_node=0, + vf_ratio=0, + mac_address='b4:96:91:34:f4:aa', + ) self.start_compute( hostname='source', - pci_info=fakelibvirt.HostPCIDevicesInfo( - num_pfs=1, num_vfs=1)) + pci_info=source_pci_info) + + dest_pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=2) + # add an extra PF without VF to be used by direct-physical ports + dest_pci_info.add_device( + dev_type='PF', + bus=0x82, # the HostPCIDevicesInfo use the 0x81 by default + slot=0x6, # make it different from the source host + function=0, + iommu_group=42, + numa_node=0, + vf_ratio=0, + mac_address='b4:96:91:34:f4:bb', + ) self.start_compute( hostname='dest', - pci_info=fakelibvirt.HostPCIDevicesInfo( - num_pfs=1, num_vfs=2)) + pci_info=dest_pci_info) source_port = self.neutron.create_port( {'port': self.neutron.network_4_port_1}) + source_pf_port = self.neutron.create_port( + {'port': self.neutron.network_4_port_pf}) dest_port1 = self.neutron.create_port( {'port': self.neutron.network_4_port_2}) dest_port2 = self.neutron.create_port( {'port': self.neutron.network_4_port_3}) source_server = self._create_server( - networks=[{'port': source_port['port']['id']}], host='source') + networks=[ + {'port': source_port['port']['id']}, + {'port': source_pf_port['port']['id']} + ], + host='source', + ) dest_server1 = self._create_server( networks=[{'port': dest_port1['port']['id']}], host='dest') dest_server2 = self._create_server( @@ -396,6 +431,7 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): # Refresh the ports. source_port = self.neutron.show_port(source_port['port']['id']) + source_pf_port = self.neutron.show_port(source_pf_port['port']['id']) dest_port1 = self.neutron.show_port(dest_port1['port']['id']) dest_port2 = self.neutron.show_port(dest_port2['port']['id']) @@ -411,11 +447,24 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): same_slot_port = dest_port2 self._delete_server(dest_server1) - # Before moving, explictly assert that the servers on source and dest + # Before moving, explicitly assert that the servers on source and dest # have the same pci_slot in their port's binding profile self.assertEqual(source_port['port']['binding:profile']['pci_slot'], same_slot_port['port']['binding:profile']['pci_slot']) + # Assert that the direct-physical port got the pci_slot information + # according to the source host PF PCI device. + self.assertEqual( + '0000:82:00.0', # which is in sync with the source host pci_info + source_pf_port['port']['binding:profile']['pci_slot'] + ) + # Assert that the direct-physical port is updated with the MAC address + # of the PF device from the source host + self.assertEqual( + 'b4:96:91:34:f4:aa', + source_pf_port['port']['binding:profile']['device_mac_address'] + ) + # Before moving, assert that the servers on source and dest have the # same PCI source address in their XML for their SRIOV nic. source_conn = self.computes['source'].driver._host.get_connection() @@ -432,14 +481,28 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): move_operation(source_server) # Refresh the ports again, keeping in mind the source_port is now bound - # on the dest after unshelving. + # on the dest after the move. source_port = self.neutron.show_port(source_port['port']['id']) same_slot_port = self.neutron.show_port(same_slot_port['port']['id']) + source_pf_port = self.neutron.show_port(source_pf_port['port']['id']) self.assertNotEqual( source_port['port']['binding:profile']['pci_slot'], same_slot_port['port']['binding:profile']['pci_slot']) + # Assert that the direct-physical port got the pci_slot information + # according to the dest host PF PCI device. + self.assertEqual( + '0000:82:06.0', # which is in sync with the dest host pci_info + source_pf_port['port']['binding:profile']['pci_slot'] + ) + # Assert that the direct-physical port is updated with the MAC address + # of the PF device from the dest host + self.assertEqual( + 'b4:96:91:34:f4:bb', + source_pf_port['port']['binding:profile']['device_mac_address'] + ) + conn = self.computes['dest'].driver._host.get_connection() vms = [vm._def for vm in conn._vms.values()] self.assertEqual(2, len(vms)) @@ -467,6 +530,169 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): self._confirm_resize(source_server) self._test_move_operation_with_neutron(move_operation) + def test_cold_migrate_and_rever_server_with_neutron(self): + # The purpose here is to force an observable PCI slot update when + # moving from source to dest and the from dest to source after the + # revert. This is accomplished by having a single + # PCI VF device on the source, 2 PCI VF devices on the dest, and + # relying on the fact that our fake HostPCIDevicesInfo creates + # predictable PCI addresses. The PCI VF device on source and the first + # PCI VF device on dest will have identical PCI addresses. By sticking + # a "placeholder" instance on that first PCI VF device on the dest, the + # incoming instance from source will be forced to consume the second + # dest PCI VF device, with a different PCI address. + # We want to test server operations with SRIOV VFs and SRIOV PFs so + # the config of the compute hosts also have one extra PCI PF devices + # without any VF children. But the two compute has different PCI PF + # addresses and MAC so that the test can observe the slot update as + # well as the MAC updated during migration and after revert. + source_pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=1) + # add an extra PF without VF to be used by direct-physical ports + source_pci_info.add_device( + dev_type='PF', + bus=0x82, # the HostPCIDevicesInfo use the 0x81 by default + slot=0x0, + function=0, + iommu_group=42, + numa_node=0, + vf_ratio=0, + mac_address='b4:96:91:34:f4:aa', + ) + self.start_compute( + hostname='source', + pci_info=source_pci_info) + dest_pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=2) + # add an extra PF without VF to be used by direct-physical ports + dest_pci_info.add_device( + dev_type='PF', + bus=0x82, # the HostPCIDevicesInfo use the 0x81 by default + slot=0x6, # make it different from the source host + function=0, + iommu_group=42, + numa_node=0, + vf_ratio=0, + mac_address='b4:96:91:34:f4:bb', + ) + self.start_compute( + hostname='dest', + pci_info=dest_pci_info) + source_port = self.neutron.create_port( + {'port': self.neutron.network_4_port_1}) + source_pf_port = self.neutron.create_port( + {'port': self.neutron.network_4_port_pf}) + dest_port1 = self.neutron.create_port( + {'port': self.neutron.network_4_port_2}) + dest_port2 = self.neutron.create_port( + {'port': self.neutron.network_4_port_3}) + source_server = self._create_server( + networks=[ + {'port': source_port['port']['id']}, + {'port': source_pf_port['port']['id']} + ], + host='source', + ) + dest_server1 = self._create_server( + networks=[{'port': dest_port1['port']['id']}], host='dest') + dest_server2 = self._create_server( + networks=[{'port': dest_port2['port']['id']}], host='dest') + # Refresh the ports. + source_port = self.neutron.show_port(source_port['port']['id']) + source_pf_port = self.neutron.show_port(source_pf_port['port']['id']) + dest_port1 = self.neutron.show_port(dest_port1['port']['id']) + dest_port2 = self.neutron.show_port(dest_port2['port']['id']) + # Find the server on the dest compute that's using the same pci_slot as + # the server on the source compute, and delete the other one to make + # room for the incoming server from the source. + source_pci_slot = source_port['port']['binding:profile']['pci_slot'] + dest_pci_slot1 = dest_port1['port']['binding:profile']['pci_slot'] + if dest_pci_slot1 == source_pci_slot: + same_slot_port = dest_port1 + self._delete_server(dest_server2) + else: + same_slot_port = dest_port2 + self._delete_server(dest_server1) + # Before moving, explicitly assert that the servers on source and dest + # have the same pci_slot in their port's binding profile + self.assertEqual(source_port['port']['binding:profile']['pci_slot'], + same_slot_port['port']['binding:profile']['pci_slot']) + # Assert that the direct-physical port got the pci_slot information + # according to the source host PF PCI device. + self.assertEqual( + '0000:82:00.0', # which is in sync with the source host pci_info + source_pf_port['port']['binding:profile']['pci_slot'] + ) + # Assert that the direct-physical port is updated with the MAC address + # of the PF device from the source host + self.assertEqual( + 'b4:96:91:34:f4:aa', + source_pf_port['port']['binding:profile']['device_mac_address'] + ) + # Before moving, assert that the servers on source and dest have the + # same PCI source address in their XML for their SRIOV nic. + source_conn = self.computes['source'].driver._host.get_connection() + dest_conn = self.computes['source'].driver._host.get_connection() + source_vms = [vm._def for vm in source_conn._vms.values()] + dest_vms = [vm._def for vm in dest_conn._vms.values()] + self.assertEqual(1, len(source_vms)) + self.assertEqual(1, len(dest_vms)) + self.assertEqual(1, len(source_vms[0]['devices']['nics'])) + self.assertEqual(1, len(dest_vms[0]['devices']['nics'])) + self.assertEqual(source_vms[0]['devices']['nics'][0]['source'], + dest_vms[0]['devices']['nics'][0]['source']) + + # TODO(stephenfin): The mock of 'migrate_disk_and_power_off' should + # probably be less...dumb + with mock.patch('nova.virt.libvirt.driver.LibvirtDriver' + '.migrate_disk_and_power_off', return_value='{}'): + self._migrate_server(source_server) + + # Refresh the ports again, keeping in mind the ports are now bound + # on the dest after migrating. + source_port = self.neutron.show_port(source_port['port']['id']) + same_slot_port = self.neutron.show_port(same_slot_port['port']['id']) + source_pf_port = self.neutron.show_port(source_pf_port['port']['id']) + self.assertNotEqual( + source_port['port']['binding:profile']['pci_slot'], + same_slot_port['port']['binding:profile']['pci_slot']) + # Assert that the direct-physical port got the pci_slot information + # according to the dest host PF PCI device. + self.assertEqual( + '0000:82:06.0', # which is in sync with the dest host pci_info + source_pf_port['port']['binding:profile']['pci_slot'] + ) + # Assert that the direct-physical port is updated with the MAC address + # of the PF device from the dest host + self.assertEqual( + 'b4:96:91:34:f4:bb', + source_pf_port['port']['binding:profile']['device_mac_address'] + ) + conn = self.computes['dest'].driver._host.get_connection() + vms = [vm._def for vm in conn._vms.values()] + self.assertEqual(2, len(vms)) + for vm in vms: + self.assertEqual(1, len(vm['devices']['nics'])) + self.assertNotEqual(vms[0]['devices']['nics'][0]['source'], + vms[1]['devices']['nics'][0]['source']) + + self._revert_resize(source_server) + + # Refresh the ports again, keeping in mind the ports are now bound + # on the source as the migration is reverted + source_pf_port = self.neutron.show_port(source_pf_port['port']['id']) + + # Assert that the direct-physical port got the pci_slot information + # according to the source host PF PCI device. + self.assertEqual( + '0000:82:00.0', # which is in sync with the source host pci_info + source_pf_port['port']['binding:profile']['pci_slot'] + ) + # Assert that the direct-physical port is updated with the MAC address + # of the PF device from the source host + self.assertEqual( + 'b4:96:91:34:f4:aa', + source_pf_port['port']['binding:profile']['device_mac_address'] + ) + def test_evacuate_server_with_neutron(self): def move_operation(source_server): # Down the source compute to enable the evacuation @@ -484,17 +710,44 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): """ # start two compute services with differing PCI device inventory - self.start_compute( - hostname='test_compute0', - pci_info=fakelibvirt.HostPCIDevicesInfo( - num_pfs=2, num_vfs=8, numa_node=0)) - self.start_compute( - hostname='test_compute1', - pci_info=fakelibvirt.HostPCIDevicesInfo( - num_pfs=1, num_vfs=2, numa_node=1)) + source_pci_info = fakelibvirt.HostPCIDevicesInfo( + num_pfs=2, num_vfs=8, numa_node=0) + # add an extra PF without VF to be used by direct-physical ports + source_pci_info.add_device( + dev_type='PF', + bus=0x82, # the HostPCIDevicesInfo use the 0x81 by default + slot=0x0, + function=0, + iommu_group=42, + numa_node=0, + vf_ratio=0, + mac_address='b4:96:91:34:f4:aa', + ) + self.start_compute(hostname='test_compute0', pci_info=source_pci_info) - # create the port - self.neutron.create_port({'port': self.neutron.network_4_port_1}) + dest_pci_info = fakelibvirt.HostPCIDevicesInfo( + num_pfs=1, num_vfs=2, numa_node=1) + # add an extra PF without VF to be used by direct-physical ports + dest_pci_info.add_device( + dev_type='PF', + bus=0x82, # the HostPCIDevicesInfo use the 0x81 by default + slot=0x6, # make it different from the source host + function=0, + iommu_group=42, + # numa node needs to be aligned with the other pci devices in this + # host as the instance needs to fit into a single host numa node + numa_node=1, + vf_ratio=0, + mac_address='b4:96:91:34:f4:bb', + ) + + self.start_compute(hostname='test_compute1', pci_info=dest_pci_info) + + # create the ports + port = self.neutron.create_port( + {'port': self.neutron.network_4_port_1})['port'] + pf_port = self.neutron.create_port( + {'port': self.neutron.network_4_port_pf})['port'] # create a server using the VF via neutron extra_spec = {'hw:cpu_policy': 'dedicated'} @@ -502,7 +755,8 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): server = self._create_server( flavor_id=flavor_id, networks=[ - {'port': base.LibvirtNeutronFixture.network_4_port_1['id']}, + {'port': port['id']}, + {'port': pf_port['id']}, ], host='test_compute0', ) @@ -510,8 +764,8 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): # our source host should have marked two PCI devices as used, the VF # and the parent PF, while the future destination is currently unused self.assertEqual('test_compute0', server['OS-EXT-SRV-ATTR:host']) - self.assertPCIDeviceCounts('test_compute0', total=10, free=8) - self.assertPCIDeviceCounts('test_compute1', total=3, free=3) + self.assertPCIDeviceCounts('test_compute0', total=11, free=8) + self.assertPCIDeviceCounts('test_compute1', total=4, free=4) # the instance should be on host NUMA node 0, since that's where our # PCI devices are @@ -540,13 +794,26 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): port['binding:profile'], ) + # ensure the binding details sent to "neutron" are correct + pf_port = self.neutron.show_port(pf_port['id'],)['port'] + self.assertIn('binding:profile', pf_port) + self.assertEqual( + { + 'pci_vendor_info': '8086:1528', + 'pci_slot': '0000:82:00.0', + 'physical_network': 'physnet4', + 'device_mac_address': 'b4:96:91:34:f4:aa', + }, + pf_port['binding:profile'], + ) + # now live migrate that server self._live_migrate(server, 'completed') # we should now have transitioned our usage to the destination, freeing # up the source in the process - self.assertPCIDeviceCounts('test_compute0', total=10, free=10) - self.assertPCIDeviceCounts('test_compute1', total=3, free=1) + self.assertPCIDeviceCounts('test_compute0', total=11, free=11) + self.assertPCIDeviceCounts('test_compute1', total=4, free=1) # the instance should now be on host NUMA node 1, since that's where # our PCI devices are for this second host @@ -571,6 +838,18 @@ class SRIOVServersTest(_PCIServersWithMigrationTestBase): }, port['binding:profile'], ) + # ensure the binding details sent to "neutron" are correct + pf_port = self.neutron.show_port(pf_port['id'],)['port'] + self.assertIn('binding:profile', pf_port) + self.assertEqual( + { + 'pci_vendor_info': '8086:1528', + 'pci_slot': '0000:82:06.0', + 'physical_network': 'physnet4', + 'device_mac_address': 'b4:96:91:34:f4:bb', + }, + pf_port['binding:profile'], + ) def test_get_server_diagnostics_server_with_VF(self): """Ensure server disagnostics include info on VF-type PCI devices.""" diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 5a25cf4c97a5..5be699ee8168 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -5714,13 +5714,15 @@ class ComputeTestCase(BaseTestCase, objects=[objects.PciDevice(vendor_id='1377', product_id='0047', address='0000:0a:00.1', - request_id=uuids.req1)]) + request_id=uuids.req1, + compute_node_id=1)]) new_pci_devices = objects.PciDeviceList( objects=[objects.PciDevice(vendor_id='1377', product_id='0047', address='0000:0b:00.1', - request_id=uuids.req2)]) + request_id=uuids.req2, + compute_node_id=2)]) if expected_pci_addr == old_pci_devices[0].address: expected_pci_device = old_pci_devices[0] diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 67332b82da64..00e7ad224f80 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -8988,10 +8988,12 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase, self._mock_rt() old_devs = objects.PciDeviceList( objects=[objects.PciDevice( + compute_node_id=1, address='0000:04:00.2', request_id=uuids.pcidev1)]) new_devs = objects.PciDeviceList( objects=[objects.PciDevice( + compute_node_id=2, address='0000:05:00.3', request_id=uuids.pcidev1)]) self.instance.migration_context = objects.MigrationContext( @@ -10962,40 +10964,94 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase, _test() def test__update_migrate_vifs_profile_with_pci(self): - # Define two migrate vifs with only one pci that is required - # to be updated. Make sure method under test updated the correct one + # Define three migrate vifs with two pci devs that are required + # to be updated, one VF and on PF. + # Make sure method under test updated the correct devs with the correct + # values. nw_vifs = network_model.NetworkInfo( - [network_model.VIF( - id=uuids.port0, - vnic_type='direct', - type=network_model.VIF_TYPE_HW_VEB, - profile={'pci_slot': '0000:04:00.3', - 'pci_vendor_info': '15b3:1018', - 'physical_network': 'default'}), - network_model.VIF( - id=uuids.port1, - vnic_type='normal', - type=network_model.VIF_TYPE_OVS, - profile={'some': 'attribute'})]) - pci_dev = objects.PciDevice(request_id=uuids.pci_req, - address='0000:05:00.4', - vendor_id='15b3', - product_id='1018') - port_id_to_pci_dev = {uuids.port0: pci_dev} - mig_vifs = migrate_data_obj.VIFMigrateData.\ - create_skeleton_migrate_vifs(nw_vifs) - self.compute._update_migrate_vifs_profile_with_pci(mig_vifs, - port_id_to_pci_dev) + [ + network_model.VIF( + id=uuids.port0, + vnic_type='direct', + type=network_model.VIF_TYPE_HW_VEB, + profile={ + 'pci_slot': '0000:04:00.3', + 'pci_vendor_info': '15b3:1018', + 'physical_network': 'default', + }, + ), + network_model.VIF( + id=uuids.port1, + vnic_type='normal', + type=network_model.VIF_TYPE_OVS, + profile={'some': 'attribute'}, + ), + network_model.VIF( + id=uuids.port2, + vnic_type='direct-physical', + type=network_model.VIF_TYPE_HOSTDEV, + profile={ + 'pci_slot': '0000:01:00', + 'pci_vendor_info': '8086:154d', + 'physical_network': 'physnet2', + }, + ), + ] + ) + + pci_vf_dev = objects.PciDevice( + request_id=uuids.pci_req, + address='0000:05:00.4', + parent_addr='0000:05:00', + vendor_id='15b3', + product_id='1018', + compute_node_id=13, + dev_type=fields.PciDeviceType.SRIOV_VF, + ) + pci_pf_dev = objects.PciDevice( + request_id=uuids.pci_req2, + address='0000:01:00', + parent_addr='0000:02:00', + vendor_id='8086', + product_id='154d', + compute_node_id=13, + dev_type=fields.PciDeviceType.SRIOV_PF, + extra_info={'mac_address': 'b4:96:91:34:f4:36'}, + ) + port_id_to_pci_dev = { + uuids.port0: pci_vf_dev, + uuids.port2: pci_pf_dev, + } + mig_vifs = ( + migrate_data_obj.VIFMigrateData.create_skeleton_migrate_vifs( + nw_vifs) + ) + + self.compute._update_migrate_vifs_profile_with_pci( + mig_vifs, port_id_to_pci_dev) + # Make sure method under test updated the correct one. - changed_mig_vif = mig_vifs[0] + changed_vf_mig_vif = mig_vifs[0] unchanged_mig_vif = mig_vifs[1] + changed_pf_mig_vif = mig_vifs[2] # Migrate vifs profile was updated with pci_dev.address # for port ID uuids.port0. - self.assertEqual(changed_mig_vif.profile['pci_slot'], - pci_dev.address) + self.assertEqual(changed_vf_mig_vif.profile['pci_slot'], + pci_vf_dev.address) + # MAC is not added as this is a VF + self.assertNotIn('device_mac_address', changed_vf_mig_vif.profile) # Migrate vifs profile was unchanged for port ID uuids.port1. # i.e 'profile' attribute does not exist. self.assertNotIn('profile', unchanged_mig_vif) + # Migrate vifs profile was updated with pci_dev.address + # for port ID uuids.port2. + self.assertEqual(changed_pf_mig_vif.profile['pci_slot'], + pci_pf_dev.address) + # MAC is updated as this is a PF + self.assertEqual( + 'b4:96:91:34:f4:36', + changed_pf_mig_vif.profile['device_mac_address'] + ) def test_get_updated_nw_info_with_pci_mapping(self): old_dev = objects.PciDevice(address='0000:04:00.2') diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 099d818dc67d..ad987dad90f7 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -4805,6 +4805,174 @@ class TestAPI(TestAPIBase): constants.BINDING_HOST_ID], 'new-host') + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) + @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_bindings_for_instance_with_sriov_pf( + self, get_client_mock, get_pci_device_devspec_mock + ): + devspec = mock.Mock() + devspec.get_tags.return_value = {'physical_network': 'physnet1'} + get_pci_device_devspec_mock.return_value = devspec + + instance = fake_instance.fake_instance_obj(self.context) + instance.migration_context = objects.MigrationContext() + instance.migration_context.old_pci_devices = objects.PciDeviceList( + objects=[ + objects.PciDevice( + vendor_id='8086', + product_id='154d', + address='0000:0a:01', + compute_node_id=1, + request_id=uuids.pci_req, + dev_type=obj_fields.PciDeviceType.SRIOV_PF, + extra_info={'mac_address': 'b4:96:91:34:f4:36'}, + ) + ] + ) + instance.pci_devices = instance.migration_context.old_pci_devices + instance.migration_context.new_pci_devices = objects.PciDeviceList( + objects=[ + objects.PciDevice( + vendor_id='8086', + product_id='154d', + address='0000:0a:02', + compute_node_id=2, + request_id=uuids.pci_req, + dev_type=obj_fields.PciDeviceType.SRIOV_PF, + extra_info={'mac_address': 'b4:96:91:34:f4:dd'}, + ) + ] + ) + instance.pci_devices = instance.migration_context.new_pci_devices + + fake_ports = { + 'ports': [ + { + 'id': uuids.port, + 'binding:vnic_type': 'direct-physical', + constants.BINDING_HOST_ID: 'fake-host-old', + constants.BINDING_PROFILE: { + 'pci_slot': '0000:0a:01', + 'physical_network': 'old_phys_net', + 'pci_vendor_info': 'old_pci_vendor_info', + }, + }, + ] + } + + migration = objects.Migration( + status='confirmed', migration_type='migration') + list_ports_mock = mock.Mock(return_value=fake_ports) + get_client_mock.return_value.list_ports = list_ports_mock + + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + + self.api._update_port_binding_for_instance( + self.context, instance, instance.host, migration) + + # Assert that update_port is called with the binding:profile + # corresponding to the PCI device specified including MAC address. + update_port_mock.assert_called_once_with( + uuids.port, + { + 'port': { + constants.BINDING_HOST_ID: 'fake-host', + 'device_owner': 'compute:%s' % instance.availability_zone, + constants.BINDING_PROFILE: { + 'pci_slot': '0000:0a:02', + 'physical_network': 'physnet1', + 'pci_vendor_info': '8086:154d', + 'device_mac_address': 'b4:96:91:34:f4:dd', + }, + } + }, + ) + + @mock.patch( + 'nova.network.neutron.API.has_extended_resource_request_extension', + new=mock.Mock(return_value=False), + ) + @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_bindings_for_instance_with_sriov_pf_no_migration( + self, get_client_mock, get_pci_device_devspec_mock + ): + devspec = mock.Mock() + devspec.get_tags.return_value = {'physical_network': 'physnet1'} + get_pci_device_devspec_mock.return_value = devspec + + instance = fake_instance.fake_instance_obj(self.context) + instance.pci_requests = objects.InstancePCIRequests( + instance_uuid=instance.uuid, + requests=[ + objects.InstancePCIRequest( + requester_id=uuids.port, + request_id=uuids.pci_req, + ) + ], + ) + instance.pci_devices = objects.PciDeviceList( + objects=[ + objects.PciDevice( + vendor_id='8086', + product_id='154d', + address='0000:0a:02', + compute_node_id=2, + request_id=uuids.pci_req, + dev_type=obj_fields.PciDeviceType.SRIOV_PF, + extra_info={'mac_address': 'b4:96:91:34:f4:36'}, + ) + ] + ) + + fake_ports = { + 'ports': [ + { + 'id': uuids.port, + 'binding:vnic_type': 'direct-physical', + constants.BINDING_HOST_ID: 'fake-host-old', + constants.BINDING_PROFILE: { + 'pci_slot': '0000:0a:01', + 'physical_network': 'old_phys_net', + 'pci_vendor_info': 'old_pci_vendor_info', + 'device_mac_address': 'b4:96:91:34:f4:dd' + }, + }, + ] + } + + list_ports_mock = mock.Mock(return_value=fake_ports) + get_client_mock.return_value.list_ports = list_ports_mock + + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + + self.api._update_port_binding_for_instance( + self.context, instance, instance.host) + + # Assert that update_port is called with the binding:profile + # corresponding to the PCI device specified including MAC address. + update_port_mock.assert_called_once_with( + uuids.port, + { + 'port': { + constants.BINDING_HOST_ID: 'fake-host', + 'device_owner': 'compute:%s' % instance.availability_zone, + constants.BINDING_PROFILE: { + 'pci_slot': '0000:0a:02', + 'physical_network': 'physnet1', + 'pci_vendor_info': '8086:154d', + 'device_mac_address': 'b4:96:91:34:f4:36', + }, + } + }, + ) + @mock.patch( 'nova.network.neutron.API.has_extended_resource_request_extension', new=mock.Mock(return_value=False), @@ -7190,23 +7358,21 @@ class TestAPI(TestAPIBase): request_id=uuids.pci_request_id) bad_request = objects.InstancePCIRequest( requester_id=uuids.wrong_port_id) - device = objects.PciDevice(request_id=uuids.pci_request_id, - address='fake-pci-address') + device = objects.PciDevice(request_id=uuids.pci_request_id) bad_device = objects.PciDevice(request_id=uuids.wrong_request_id) # Test the happy path instance = objects.Instance( pci_requests=objects.InstancePCIRequests(requests=[request]), pci_devices=objects.PciDeviceList(objects=[device])) self.assertEqual( - 'fake-pci-address', - self.api._get_port_pci_dev( - self.context, instance, fake_port).address) + device, + self.api._get_port_pci_dev(instance, fake_port)) # Test not finding the request instance = objects.Instance( pci_requests=objects.InstancePCIRequests( requests=[objects.InstancePCIRequest(bad_request)])) self.assertIsNone( - self.api._get_port_pci_dev(self.context, instance, fake_port)) + self.api._get_port_pci_dev(instance, fake_port)) mock_debug.assert_called_with('No PCI request found for port %s', uuids.fake_port_id, instance=instance) mock_debug.reset_mock() @@ -7215,7 +7381,7 @@ class TestAPI(TestAPIBase): pci_requests=objects.InstancePCIRequests(requests=[request]), pci_devices=objects.PciDeviceList(objects=[bad_device])) self.assertIsNone( - self.api._get_port_pci_dev(self.context, instance, fake_port)) + self.api._get_port_pci_dev(instance, fake_port)) mock_debug.assert_called_with('No PCI device found for request %s', uuids.pci_request_id, instance=instance) @@ -7740,6 +7906,45 @@ class TestAPIPortbinding(TestAPIBase): port_req_body['port'][ constants.BINDING_PROFILE]) + @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') + @mock.patch.object(pci_manager, 'get_instance_pci_devs') + def test_populate_neutron_extension_values_binding_sriov_pf( + self, mock_get_instance_pci_devs, mock_get_devspec + ): + host_id = 'my_host_id' + instance = {'host': host_id} + port_req_body = {'port': {}} + + pci_dev = objects.PciDevice( + request_id=uuids.pci_req, + address='0000:01:00', + parent_addr='0000:02:00', + vendor_id='8086', + product_id='154d', + dev_type=obj_fields.PciDeviceType.SRIOV_PF, + extra_info={'mac_address': 'b4:96:91:34:f4:36'} + ) + + expected_profile = { + 'pci_vendor_info': '8086:154d', + 'pci_slot': '0000:01:00', + 'physical_network': 'physnet1', + 'device_mac_address': 'b4:96:91:34:f4:36', + } + + mock_get_instance_pci_devs.return_value = [pci_dev] + devspec = mock.Mock() + devspec.get_tags.return_value = {'physical_network': 'physnet1'} + mock_get_devspec.return_value = devspec + + self.api._populate_neutron_binding_profile( + instance, uuids.pci_req, port_req_body, None) + + self.assertEqual( + expected_profile, + port_req_body['port'][constants.BINDING_PROFILE] + ) + @mock.patch.object( pci_utils, 'get_vf_num_by_pci_address', new=mock.MagicMock(side_effect=(lambda vf_a: 1 @@ -7811,21 +8016,29 @@ class TestAPIPortbinding(TestAPIBase): devspec.get_tags.return_value = {'physical_network': 'physnet1'} mock_get_pci_device_devspec.return_value = devspec - pci_dev = {'vendor_id': 'a2d6', - 'product_id': '15b3', - 'address': '0000:0a:00.0', - 'card_serial_number': 'MT2113X00000', - 'dev_type': obj_fields.PciDeviceType.SRIOV_PF, - } - PciDevice = collections.namedtuple('PciDevice', - ['vendor_id', 'product_id', 'address', - 'card_serial_number', 'dev_type']) - mydev = PciDevice(**pci_dev) + pci_dev = objects.PciDevice( + request_id=uuids.pci_req, + address='0000:0a:00.0', + parent_addr='0000:02:00', + vendor_id='a2d6', + product_id='15b3', + dev_type=obj_fields.PciDeviceType.SRIOV_PF, + extra_info={ + 'capabilities': jsonutils.dumps( + {'card_serial_number': 'MT2113X00000'}), + 'mac_address': 'b4:96:91:34:f4:36', + }, - self.assertEqual({'pci_slot': '0000:0a:00.0', - 'pci_vendor_info': 'a2d6:15b3', - 'physical_network': 'physnet1'}, - self.api._get_pci_device_profile(mydev)) + ) + self.assertEqual( + { + 'pci_slot': '0000:0a:00.0', + 'pci_vendor_info': 'a2d6:15b3', + 'physical_network': 'physnet1', + 'device_mac_address': 'b4:96:91:34:f4:36', + }, + self.api._get_pci_device_profile(pci_dev), + ) @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') @mock.patch.object(pci_manager, 'get_instance_pci_devs') diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index df9bc71fc40a..7233c00a72cc 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -17669,7 +17669,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, "vendor_id": '8086', "dev_type": fields.PciDeviceType.SRIOV_PF, "phys_function": None, - "numa_node": None}, + "numa_node": None, + # value defined in the LibvirtFixture + "mac_address": "52:54:00:1e:59:c6", + }, { "dev_id": "pci_0000_04_10_7", "domain": 0, diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py index a21abac47169..5bd45cd83a0c 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -1161,9 +1161,9 @@ Active: 8381604 kB dev for dev in node_devs.values() if dev.name() in devs] name = "pci_0000_04_00_3" - actual_vf = self.host._get_pcidev_info( + actual_pf = self.host._get_pcidev_info( name, node_devs[name], net_devs, [], []) - expect_vf = { + expect_pf = { "dev_id": "pci_0000_04_00_3", "address": "0000:04:00.3", "product_id": '1521', @@ -1171,8 +1171,10 @@ Active: 8381604 kB "vendor_id": '8086', "label": 'label_8086_1521', "dev_type": obj_fields.PciDeviceType.SRIOV_PF, + # value defined in the LibvirtFixture + "mac_address": "52:54:00:1e:59:c6", } - self.assertEqual(expect_vf, actual_vf) + self.assertEqual(expect_pf, actual_pf) name = "pci_0000_04_10_7" actual_vf = self.host._get_pcidev_info( @@ -1231,9 +1233,9 @@ Active: 8381604 kB self.assertEqual(expect_vf, actual_vf) name = "pci_0000_03_00_0" - actual_vf = self.host._get_pcidev_info( + actual_pf = self.host._get_pcidev_info( name, node_devs[name], net_devs, [], []) - expect_vf = { + expect_pf = { "dev_id": "pci_0000_03_00_0", "address": "0000:03:00.0", "product_id": '1013', @@ -1241,13 +1243,15 @@ Active: 8381604 kB "vendor_id": '15b3', "label": 'label_15b3_1013', "dev_type": obj_fields.PciDeviceType.SRIOV_PF, + # value defined in the LibvirtFixture + "mac_address": "52:54:00:1e:59:c6", } - self.assertEqual(expect_vf, actual_vf) + self.assertEqual(expect_pf, actual_pf) name = "pci_0000_03_00_1" - actual_vf = self.host._get_pcidev_info( + actual_pf = self.host._get_pcidev_info( name, node_devs[name], net_devs, [], []) - expect_vf = { + expect_pf = { "dev_id": "pci_0000_03_00_1", "address": "0000:03:00.1", "product_id": '1013', @@ -1255,8 +1259,10 @@ Active: 8381604 kB "vendor_id": '15b3', "label": 'label_15b3_1013', "dev_type": obj_fields.PciDeviceType.SRIOV_PF, + # value defined in the LibvirtFixture + "mac_address": "52:54:00:1e:59:c6", } - self.assertEqual(expect_vf, actual_vf) + self.assertEqual(expect_pf, actual_pf) # Parent PF with a VPD cap. name = "pci_0000_82_00_0" @@ -1273,6 +1279,8 @@ Active: 8381604 kB "capabilities": { # Should be obtained from the parent PF in this case. "vpd": {"card_serial_number": "MT2113X00000"}}, + # value defined in the LibvirtFixture + "mac_address": "52:54:00:1e:59:c6", } self.assertEqual(expect_pf, actual_pf) diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 5aab8ce30078..02fc1f07bcb6 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -891,6 +891,36 @@ class FakeLiveMigrateDriverWithNestedCustomResources( class FakeDriverWithPciResources(SmallFakeDriver): + """NOTE: this driver provides symmetric compute nodes. Each compute will + have the same resources with the same addresses. It is dangerous as using + this driver can hide issues when in an asymmetric environment nova fails to + update entities according to the host specific addresses (e.g. pci_slot of + the neutron port bindings). + + The current non virt driver specific functional test environment has many + shortcomings making it really hard to simulate host specific virt drivers. + + 1) The virt driver is instantiated by the service logic from the name of + the driver class. This makes passing input to the driver instance from the + test at init time pretty impossible. This could be solved with some + fixtures around nova.virt.driver.load_compute_driver() + + 2) The compute service access the hypervisor not only via the virt + interface but also reads the sysfs of the host. So simply providing a fake + virt driver instance is not enough to isolate simulated compute services + that are running on the same host. Also these low level sysfs reads are not + having host specific information in the call params. So simply mocking the + low level call does not give a way to provide host specific return values. + + 3) CONF is global, and it is read dynamically by the driver. So + providing host specific CONF to driver instances without race conditions + between the drivers are extremely hard especially if periodic tasks are + enabled. + + The libvirt based functional test env under nova.tests.functional.libvirt + has better support to create asymmetric environments. So please consider + using that if possible instead. + """ PCI_ADDR_PF1 = '0000:01:00.0' PCI_ADDR_PF1_VF1 = '0000:01:00.1' @@ -955,6 +985,11 @@ class FakeDriverWithPciResources(SmallFakeDriver): ], group='pci') + # These mocks should be removed after bug + # https://bugs.launchpad.net/nova/+bug/1961587 has been fixed and + # every SRIOV device related information is transferred through the + # virt driver and the PciDevice object instead of queried with + # sysfs calls by the network.neutron.API code. self.useFixture(fixtures.MockPatch( 'nova.pci.utils.get_mac_by_pci_address', return_value='52:54:00:1e:59:c6')) diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 98fd13ce6ef2..3f5cc085377a 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -1268,6 +1268,20 @@ class Host(object): return None return vpd_cap.card_serial_number + def _get_pf_details(self, device: dict, pci_address: str) -> dict: + if device.get('dev_type') != fields.PciDeviceType.SRIOV_PF: + return {} + + try: + return { + 'mac_address': pci_utils.get_mac_by_pci_address(pci_address) + } + except exception.PciDeviceNotFoundById: + LOG.debug( + 'Cannot get MAC address of the PF %s. It is probably attached ' + 'to a guest already', pci_address) + return {} + def _get_pcidev_info( self, devname: str, @@ -1470,6 +1484,7 @@ class Host(object): _get_device_type(cfgdev, address, dev, net_devs, vdpa_devs)) device.update(_get_device_capabilities(device, dev, pci_devs, net_devs)) + device.update(self._get_pf_details(device, address)) return device def get_vdpa_nodedev_by_address( diff --git a/releasenotes/notes/bug-1942329-22b08fa4b322881d.yaml b/releasenotes/notes/bug-1942329-22b08fa4b322881d.yaml new file mode 100644 index 000000000000..496508ca13ae --- /dev/null +++ b/releasenotes/notes/bug-1942329-22b08fa4b322881d.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + As a fix for `bug 1942329 `_ + nova now updates the MAC address of the ``direct-physical`` ports during + mova operations to reflect the MAC address of the physical device on the + destination host. Those servers that were created before this fix need to be + moved or the port needs to be detached and the re-attached to synchronize the + MAC address.