Merge "Record SRIOV PF MAC in the binding profile"

Author: Zuul

nova/compute/manager.py

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

nova/network/neutron.py

@@ -684,7 +684,8 @@ def _unbind_ports(self, context, ports,
             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 @@ 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)
 
@@ -1598,6 +1603,18 @@ def _get_pci_device_profile(self, pci_dev):
             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 @@ def _get_pci_mapping_for_migration(self, instance, migration):
         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 @@ 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})
@@ -572,6 +581,13 @@ def sriov_cap(self):
         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 @@ 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

@@ -309,7 +309,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, vpd_fields=None
+        prod_name=None, driver_name=None, vpd_fields=None, mac_address=None,
     ):
         """Populate pci devices
 
@@ -331,6 +331,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
@@ -349,6 +351,7 @@ def __init__(
         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 @@ 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'
@@ -497,6 +502,10 @@ def format_vpd_cap(self):
     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 @@ 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, 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 @@ def add_device(
             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 @@ 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 = """
@@ -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 @@ def setUp(self):
 
         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(

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',
@@ -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 = {