summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBalazs Gibizer <balazs.gibizer@est.tech>2022-02-15 14:38:41 +0100
committerSean Mooney <work@seanmooney.info>2022-12-01 18:03:51 +0000
commita340630c5c78e5f5856004d0a551dfa93df7f28d (patch)
tree30f991dd24b2479ff75b7fc4f7ed8fef505fbb21
parente7c2e55c6685ccda23ad5c020db5b204f82d296e (diff)
downloadnova-a340630c5c78e5f5856004d0a551dfa93df7f28d.tar.gz
Record SRIOV PF MAC in the binding profile
Today Nova updates the mac_address of a direct-physical port to reflect the MAC address of the physical device the port is bound to. But this can only be done before the port is bound. However during migration Nova does not update the MAC when the port is bound to a different physical device on the destination host. This patch extends the libvirt virt driver to provide the MAC address of the PF in the pci_info returned to the resource tracker. This information will be then persisted in the extra_info field of the PciDevice object. Then the port update logic during migration, resize, live migration, evacuation and unshelve is also extended to record the MAC of physical device in the port binding profile according to the device on the destination host. The related neutron change Ib0638f5db69cb92daf6932890cb89e83cf84f295 uses this info from the binding profile to update the mac_address field of the port when the binding is activated. Closes-Bug: #1942329 Conflicts: nova/network/neutron.py nova/objects/pci_device.py nova/tests/fixtures/libvirt.py nova/tests/unit/network/test_neutron.py nova/tests/unit/virt/libvirt/test_host.py nova/virt/fake.py nova/virt/libvirt/host.py Most of the conlficts are due to the lack of the vpd feature and I83a128a260acdd8bf78fede566af6881b8b82a9c the addtional unit tests in test_neutron and test_compute_mgr are form the vpd feature but have been modified to drop assertions related to the vpd feature while keeping the assertions related to SRIOV PF MAC adress binding. Change-Id: Iad5e70b43a65c076134e1874cb8e75d1ba214fde (cherry picked from commit cd03bbc1c33e33872594cf002f0e7011ab8ea047) (cherry picked from commit 813377077bd0173bdf128823e46b5df7c0a575b9)
-rw-r--r--nova/compute/manager.py2
-rw-r--r--nova/compute/resource_tracker.py7
-rw-r--r--nova/network/neutron.py47
-rw-r--r--nova/objects/pci_device.py19
-rw-r--r--nova/tests/fixtures/libvirt.py39
-rw-r--r--nova/tests/functional/libvirt/base.py24
-rw-r--r--nova/tests/functional/libvirt/test_pci_sriov_servers.py413
-rw-r--r--nova/tests/unit/compute/test_compute.py6
-rw-r--r--nova/tests/unit/compute/test_compute_mgr.py109
-rw-r--r--nova/tests/unit/network/test_neutron.py284
-rw-r--r--nova/tests/unit/virt/libvirt/test_driver.py5
-rw-r--r--nova/tests/unit/virt/libvirt/test_host.py24
-rw-r--r--nova/virt/fake.py39
-rw-r--r--nova/virt/libvirt/host.py15
-rw-r--r--releasenotes/notes/bug-1942329-22b08fa4b322881d.yaml9
15 files changed, 907 insertions, 135 deletions
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index a838e316c8..a4b1d34c97 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -10820,6 +10820,8 @@ class ComputeManager(manager.Manager):
profile['pci_slot'] = pci_dev.address
profile['pci_vendor_info'] = ':'.join([pci_dev.vendor_id,
pci_dev.product_id])
+ 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/compute/resource_tracker.py b/nova/compute/resource_tracker.py
index 1c492bcb27..ed683e312e 100644
--- a/nova/compute/resource_tracker.py
+++ b/nova/compute/resource_tracker.py
@@ -328,7 +328,12 @@ class ResourceTracker(object):
migration_id=migration.id,
old_numa_topology=instance.numa_topology,
new_numa_topology=claim.claimed_numa_topology,
- old_pci_devices=instance.pci_devices,
+ # NOTE(gibi): the _update_usage_from_migration call below appends
+ # the newly claimed pci devices to the instance.pci_devices list
+ # to keep the migration context independent we need to make a copy
+ # that list here. We need a deep copy as we need to duplicate
+ # the instance.pci_devices.objects list
+ old_pci_devices=copy.deepcopy(instance.pci_devices),
new_pci_devices=claimed_pci_devices,
old_pci_requests=instance.pci_requests,
new_pci_requests=new_pci_requests,
diff --git a/nova/network/neutron.py b/nova/network/neutron.py
index ba814d40b2..8ea3f7e6bb 100644
--- a/nova/network/neutron.py
+++ b/nova/network/neutron.py
@@ -667,7 +667,8 @@ class API:
# for the physical device but don't want to overwrite the other
# information in the binding profile.
for profile_key in ('pci_vendor_info', 'pci_slot',
- constants.ALLOCATION, 'arq_uuid'):
+ constants.ALLOCATION, 'arq_uuid',
+ 'device_mac_address'):
if profile_key in port_profile:
del port_profile[profile_key]
port_req_body['port'][constants.BINDING_PROFILE] = port_profile
@@ -1286,6 +1287,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)
@@ -1506,11 +1511,27 @@ class API:
def _get_pci_device_profile(self, pci_dev):
dev_spec = self.pci_whitelist.get_devspec(pci_dev)
if dev_spec:
- return {'pci_vendor_info': "%s:%s" %
- (pci_dev.vendor_id, pci_dev.product_id),
- 'pci_slot': pci_dev.address,
- 'physical_network':
- dev_spec.get_tags().get('physical_network')}
+ dev_profile = {
+ 'pci_vendor_info': "%s:%s"
+ % (pci_dev.vendor_id, pci_dev.product_id),
+ 'pci_slot': pci_dev.address,
+ 'physical_network': dev_spec.get_tags().get(
+ 'physical_network'
+ ),
+ }
+ 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,
address=pci_dev.address)
@@ -3512,11 +3533,10 @@ class API:
migration.get('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.
@@ -3604,15 +3624,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 0be94897e3..e42b1bb00e 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})
@@ -511,6 +520,13 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
def is_available(self):
return self.status == fields.PciDeviceStatus.AVAILABLE
+ @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):
@@ -550,3 +566,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 ac14ecf049..031ddcb53f 100644
--- a/nova/tests/fixtures/libvirt.py
+++ b/nova/tests/fixtures/libvirt.py
@@ -299,7 +299,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,
+ prod_name=None, driver_name=None, mac_address=None,
):
"""Populate pci devices
@@ -321,6 +321,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
@@ -339,6 +341,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.generate_xml()
@@ -352,7 +355,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'
@@ -460,6 +465,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
@@ -572,7 +581,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,
+ prod_name=None, driver_name=None, mac_address=None,
):
pci_dev_name = _get_libvirt_nodedev_name(bus, slot, function)
@@ -593,7 +602,9 @@ class HostPCIDevicesInfo(object):
vend_name=vend_name,
prod_id=prod_id,
prod_name=prod_name,
- driver_name=driver_name)
+ driver_name=driver_name,
+ mac_address=mac_address,
+ )
self.devices[pci_dev_name] = dev
return dev
@@ -612,6 +623,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 = """
@@ -2141,6 +2159,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()
@@ -2162,6 +2189,10 @@ class LibvirtFixture(fixtures.Fixture):
'nova.pci.utils.get_ifname_by_pci_address',
return_value='fake_pf_interface_name'))
+ self.useFixture(fixtures.MockPatch(
+ 'nova.pci.utils.get_mac_by_pci_address',
+ 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(
'nova.pci.utils.get_vf_num_by_pci_address', return_value=1))
diff --git a/nova/tests/functional/libvirt/base.py b/nova/tests/functional/libvirt/base.py
index c8db975d3a..24148c3a6f 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',
@@ -361,6 +367,22 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture):
'binding:vnic_type': 'direct',
}
+ 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 2aa95a3016..51fc8e8726 100644
--- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py
+++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py
@@ -72,7 +72,47 @@ class _PCIServersTestBase(base.ServersTestBase):
self.assertEqual(free, len([d for d in devices if d.is_available()]))
-class SRIOVServersTest(_PCIServersTestBase):
+class _PCIServersWithMigrationTestBase(_PCIServersTestBase):
+
+ def setUp(self):
+ super().setUp()
+
+ self.useFixture(fixtures.MonkeyPatch(
+ 'nova.tests.fixtures.libvirt.Domain.migrateToURI3',
+ self._migrate_stub))
+
+ def _migrate_stub(self, domain, destination, params, flags):
+ """Stub out migrateToURI3."""
+
+ src_hostname = domain._connection.hostname
+ dst_hostname = urlparse.urlparse(destination).netloc
+
+ # In a real live migration, libvirt and QEMU on the source and
+ # destination talk it out, resulting in the instance starting to exist
+ # on the destination. Fakelibvirt cannot do that, so we have to
+ # manually create the "incoming" instance on the destination
+ # fakelibvirt.
+ dst = self.computes[dst_hostname]
+ dst.driver._host.get_connection().createXML(
+ params['destination_xml'],
+ 'fake-createXML-doesnt-care-about-flags')
+
+ src = self.computes[src_hostname]
+ conn = src.driver._host.get_connection()
+
+ # because migrateToURI3 is spawned in a background thread, this method
+ # does not block the upper nova layers. Because we don't want nova to
+ # think the live migration has finished until this method is done, the
+ # last thing we do is make fakelibvirt's Domain.jobStats() return
+ # VIR_DOMAIN_JOB_COMPLETED.
+ server = etree.fromstring(
+ params['destination_xml']
+ ).find('./uuid').text
+ dom = conn.lookupByUUIDString(server)
+ dom.complete_job()
+
+
+class SRIOVServersTest(_PCIServersWithMigrationTestBase):
# TODO(stephenfin): We're using this because we want to be able to force
# the host during scheduling. We should instead look at overriding policy
@@ -120,40 +160,6 @@ class SRIOVServersTest(_PCIServersTestBase):
# fixture already stubbed.
self.neutron = self.useFixture(base.LibvirtNeutronFixture(self))
- self.useFixture(fixtures.MonkeyPatch(
- 'nova.tests.fixtures.libvirt.Domain.migrateToURI3',
- self._migrate_stub))
-
- def _migrate_stub(self, domain, destination, params, flags):
- """Stub out migrateToURI3."""
-
- src_hostname = domain._connection.hostname
- dst_hostname = urlparse.urlparse(destination).netloc
-
- # In a real live migration, libvirt and QEMU on the source and
- # destination talk it out, resulting in the instance starting to exist
- # on the destination. Fakelibvirt cannot do that, so we have to
- # manually create the "incoming" instance on the destination
- # fakelibvirt.
- dst = self.computes[dst_hostname]
- dst.driver._host.get_connection().createXML(
- params['destination_xml'],
- 'fake-createXML-doesnt-care-about-flags')
-
- src = self.computes[src_hostname]
- conn = src.driver._host.get_connection()
-
- # because migrateToURI3 is spawned in a background thread, this method
- # does not block the upper nova layers. Because we don't want nova to
- # think the live migration has finished until this method is done, the
- # last thing we do is make fakelibvirt's Domain.jobStats() return
- # VIR_DOMAIN_JOB_COMPLETED.
- server = etree.fromstring(
- params['destination_xml']
- ).find('./uuid').text
- dom = conn.lookupByUUIDString(server)
- dom.complete_job()
-
def _disable_sriov_in_pf(self, pci_info):
# Check for PF and change the capability from virt_functions
# Delete all the VFs
@@ -357,31 +363,66 @@ class SRIOVServersTest(_PCIServersTestBase):
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(
@@ -389,6 +430,7 @@ class SRIOVServersTest(_PCIServersTestBase):
# 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'])
@@ -404,11 +446,24 @@ class SRIOVServersTest(_PCIServersTestBase):
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()
@@ -425,14 +480,28 @@ class SRIOVServersTest(_PCIServersTestBase):
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))
@@ -460,6 +529,169 @@ class SRIOVServersTest(_PCIServersTestBase):
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
@@ -477,17 +709,44 @@ class SRIOVServersTest(_PCIServersTestBase):
"""
# 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'}
@@ -495,7 +754,8 @@ class SRIOVServersTest(_PCIServersTestBase):
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',
)
@@ -503,8 +763,8 @@ class SRIOVServersTest(_PCIServersTestBase):
# 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
@@ -533,13 +793,26 @@ class SRIOVServersTest(_PCIServersTestBase):
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
@@ -564,6 +837,18 @@ class SRIOVServersTest(_PCIServersTestBase):
},
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 f99ce1b6e2..0d4c50bdb0 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 cb35268fd1..a90497a0c5 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -8977,10 +8977,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(
@@ -10951,40 +10953,95 @@ 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',
+ 'device_mac_address': 'b4:96:91:34:f4:36'
+ },
+ ),
+ ]
+ )
+
+ 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 75a2e5ad78..488deabfba 100644
--- a/nova/tests/unit/network/test_neutron.py
+++ b/nova/tests/unit/network/test_neutron.py
@@ -39,6 +39,7 @@ from nova.network import constants
from nova.network import model
from nova.network import neutron as neutronapi
from nova import objects
+from nova.objects import fields as obj_fields
from nova.objects import network_request as net_req_obj
from nova.objects import virtual_interface as obj_vif
from nova.pci import manager as pci_manager
@@ -4498,13 +4499,16 @@ class TestAPI(TestAPIBase):
product_id='0047',
address='0000:0a:00.1',
compute_node_id=1,
- request_id='1234567890')])
+ request_id='1234567890',
+ dev_type='type-VF'
+ )])
instance.migration_context.new_pci_devices = objects.PciDeviceList(
objects=[objects.PciDevice(vendor_id='1377',
product_id='0047',
address='0000:0b:00.1',
compute_node_id=2,
- request_id='1234567890')])
+ request_id='1234567890',
+ dev_type='type-VF')])
instance.pci_devices = instance.migration_context.old_pci_devices
# Validate that non-direct port aren't updated (fake-port-2).
@@ -4775,6 +4779,174 @@ class TestAPI(TestAPIBase):
'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),
+ )
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test_update_port_bindings_for_instance_with_resource_req(
self, get_client_mock):
@@ -7091,23 +7263,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()
@@ -7116,7 +7286,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)
@@ -7443,9 +7613,11 @@ class TestAPIPortbinding(TestAPIBase):
pci_dev = {'vendor_id': '1377',
'product_id': '0047',
'address': '0000:0a:00.1',
+ 'dev_type': 'type-VF'
}
- PciDevice = collections.namedtuple('PciDevice',
- ['vendor_id', 'product_id', 'address'])
+ PciDevice = collections.namedtuple(
+ 'PciDevice', ['vendor_id', 'product_id', 'address', 'dev_type']
+ )
mydev = PciDevice(**pci_dev)
profile = {'pci_vendor_info': '1377:0047',
'pci_slot': '0000:0a:00.1',
@@ -7515,9 +7687,11 @@ class TestAPIPortbinding(TestAPIBase):
pci_dev = {'vendor_id': '1377',
'product_id': '0047',
'address': '0000:0a:00.1',
+ 'dev_type': 'type-VF'
}
- PciDevice = collections.namedtuple('PciDevice',
- ['vendor_id', 'product_id', 'address'])
+ PciDevice = collections.namedtuple(
+ 'PciDevice', ['vendor_id', 'product_id', 'address', 'dev_type']
+ )
mydev = PciDevice(**pci_dev)
profile = {'pci_vendor_info': '1377:0047',
'pci_slot': '0000:0a:00.1',
@@ -7538,6 +7712,89 @@ class TestAPIPortbinding(TestAPIBase):
@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
+ if vf_a == '0000:0a:00.1' else None)))
+ @mock.patch.object(
+ pci_utils, 'get_mac_by_pci_address',
+ new=mock.MagicMock(side_effect=(lambda vf_a: {
+ '0000:0a:00.0': '52:54:00:1e:59:c6'}.get(vf_a)))
+ )
+ @mock.patch.object(
+ pci_utils, 'get_mac_by_pci_address',
+ new=mock.MagicMock(
+ side_effect=exception.PciDeviceNotFoundById(id='0000:0a:00.1'))
+ )
+ @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec')
+ def test__get_pci_device_profile_pf(self, mock_get_pci_device_devspec):
+ devspec = mock.Mock()
+ devspec.get_tags.return_value = {'physical_network': 'physnet1'}
+ mock_get_pci_device_devspec.return_value = devspec
+
+ 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',
+ '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')
def test_populate_neutron_extension_values_binding_sriov_fail(
self, mock_get_instance_pci_devs, mock_get_pci_device_devspec):
host_id = 'my_host_id'
@@ -7588,6 +7845,7 @@ class TestAPIPortbinding(TestAPIBase):
pci_dev = {'vendor_id': '1377',
'product_id': '0047',
'address': '0000:0a:00.1',
+ 'dev_type': 'type-VF'
}
whitelist = pci_whitelist.Whitelist(CONF.pci.passthrough_whitelist)
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index b251ded389..7908b16d01 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -17427,7 +17427,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 57036ff827..7629de3aa3 100644
--- a/nova/tests/unit/virt/libvirt/test_host.py
+++ b/nova/tests/unit/virt/libvirt/test_host.py
@@ -1152,9 +1152,9 @@ Active: 8381604 kB
dev for dev in node_devs.values() if dev.name() not 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',
@@ -1162,8 +1162,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(
@@ -1218,9 +1220,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',
@@ -1228,13 +1230,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',
@@ -1242,8 +1246,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)
def test_list_pci_devices(self):
with mock.patch.object(self.host, "_list_devices") as mock_listDevices:
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index 008aa94486..fde712231e 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -887,6 +887,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'
@@ -951,6 +981,15 @@ 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'))
+
def get_available_resource(self, nodename):
host_status = super(
FakeDriverWithPciResources, self).get_available_resource(nodename)
diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py
index 73628b0d0b..604e25c49c 100644
--- a/nova/virt/libvirt/host.py
+++ b/nova/virt/libvirt/host.py
@@ -1229,6 +1229,20 @@ class Host(object):
cfgdev.parse_str(xmlstr)
return cfgdev.pci_capability.features
+ 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,
@@ -1340,6 +1354,7 @@ class Host(object):
device.update(
_get_device_type(cfgdev, address, dev, net_devs, vdpa_devs))
device.update(_get_device_capabilities(device, dev, 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 0000000000..496508ca13
--- /dev/null
+++ b/releasenotes/notes/bug-1942329-22b08fa4b322881d.yaml
@@ -0,0 +1,9 @@
+---
+fixes:
+ - |
+ As a fix for `bug 1942329 <https://bugs.launchpad.net/neutron/+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.