Merge "Record SRIOV PF MAC in the binding profile" into stable/xena

Author: Zuul

nova/compute/manager.py

@@ -10857,6 +10857,8 @@ def _update_migrate_vifs_profile_with_pci(self,
             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,

nova/compute/resource_tracker.py

@@ -328,7 +328,12 @@ def _move_claim(
             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,

nova/network/neutron.py

@@ -667,7 +667,8 @@ def _unbind_ports(self, context, ports,
             # 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 @@ def _update_ports_for_instance(self, context, instance, neutron,
                     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 @@ def delete_port_binding(self, context, port_id, host):
     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 @@ def _get_pci_mapping_for_migration(self, instance, migration):
                   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 @@ def _update_port_binding_for_instance(
                             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

nova/objects/pci_device.py

@@ -148,6 +148,12 @@ def obj_make_compatible(self, primitive, target_version):
                     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 @@ def update_device(self, dev_dict):
                 # 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 @@ def free(self, instance=None):
     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 @@ def get_by_parent_address(cls, context, node_id, parent_addr):
                                                            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]})"

nova/tests/fixtures/libvirt.py

@@ -299,7 +299,7 @@ def __init__(
         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 @@ def __init__(
         :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 @@ def __init__(
         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 @@ def generate_xml(self, skip_capability=False):
             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 @@ def generate_xml(self, skip_capability=False):
     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 @@ def add_device(
         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 @@ def add_device(
             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 @@ def get_all_mdev_capable_devices(self):
         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 @@ def setUp(self):
             '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))

nova/tests/functional/libvirt/base.py

@@ -42,7 +42,7 @@ def setUp(self):
         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 @@ def _start_compute(hostname, host_info):
                 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 = {