Update binding:profile for SR-IOV ports

The libvirt driver relies on the network information for configuring the
SR-IOV interfaces (binding:vnic_type). The network information is specified
when creating a Neutron port (binding:profile - which contain the PCI address
of the device).

During a migration, the Neutron port information needs to be updated based on
the allocated PCI device on the migrated node.

Co-Authored-By: Moshe Levi <moshele@mellanox.com>

Closes-Bug: #1512880

Change-Id: I7423907ef33648669decc561fc3461415b877ba6

nova/exception.py

@@ -822,6 +822,10 @@ class PortBindingFailed(Invalid):
                 "logs for more information.")
 
 
+class PortUpdateFailed(Invalid):
+    msg_fmt = _("Port update failed for port %(port_id)s: %(reason)s")
+
+
 class FixedIpExists(NovaException):
     msg_fmt = _("Fixed IP %(address)s already exists.")
 

nova/network/neutronv2/api.py

@@ -176,6 +176,18 @@ def _filter_hypervisor_macs(instance, ports, hypervisor_macs):
     return available_macs
 
 
+def get_pci_device_profile(pci_dev):
+    dev_spec = pci_whitelist.get_pci_device_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')}
+    raise exception.PciDeviceNotFound(node_id=pci_dev.compute_node_id,
+                                      address=pci_dev.address)
+
+
 class API(base_api.NetworkAPI):
     """API for interacting with the neutron 2.x API."""
 
@@ -888,13 +900,7 @@ def _populate_neutron_binding_profile(instance, pci_request_id,
         if pci_request_id:
             pci_dev = pci_manager.get_instance_pci_devs(
                 instance, pci_request_id).pop()
-            devspec = pci_whitelist.get_pci_device_devspec(pci_dev)
-            profile = {'pci_vendor_info': "%s:%s" % (pci_dev.vendor_id,
-                                                     pci_dev.product_id),
-                       'pci_slot': pci_dev.address,
-                       'physical_network':
-                           devspec.get_tags().get('physical_network')
-                      }
+            profile = get_pci_device_profile(pci_dev)
             port_req_body['port']['binding:profile'] = profile
 
     @staticmethod
@@ -2151,25 +2157,84 @@ def cleanup_instance_network_on_host(self, context, instance, host):
         """Cleanup network for specified instance on host."""
         pass
 
+    def _get_pci_mapping_for_migration(self, context, instance):
+        """Get the mapping between the old PCI devices and the new PCI
+        devices that have been allocated during this migration.  The
+        correlation is based on PCI request ID which is unique per PCI
+        devices for SR-IOV ports.
+
+        :param context:  The request context.
+        :param instance: Get PCI mapping for this instance.
+        :Returns: dictionary of mapping {'<old pci address>': <New PciDevice>}
+        """
+        migration_context = instance.migration_context
+        if not migration_context:
+            return {}
+
+        old_pci_devices = migration_context.old_pci_devices
+        new_pci_devices = migration_context.new_pci_devices
+        if old_pci_devices and new_pci_devices:
+            LOG.debug("Determining PCI devices mapping using migration"
+                      "context: old_pci_devices: %(old)s, "
+                      "new_pci_devices: %(new)s" %
+                      {'old': [dev for dev in old_pci_devices],
+                      'new': [dev for dev in new_pci_devices]})
+            return {old.address: new
+                    for old in old_pci_devices
+                        for new in new_pci_devices
+                            if old.request_id == new.request_id}
+        return {}
+
     def _update_port_binding_for_instance(self, context, instance, host):
         if not self._has_port_binding_extension(context, refresh_cache=True):
             return
         neutron = get_client(context, admin=True)
         search_opts = {'device_id': instance.uuid,
                        'tenant_id': instance.project_id}
         data = neutron.list_ports(**search_opts)
+        pci_mapping = self._get_pci_mapping_for_migration(context, instance)
+        port_updates = []
         ports = data['ports']
         for p in ports:
+            updates = {}
+
             # If the host hasn't changed, like in the case of resizing to the
             # same host, there is nothing to do.
             if p.get('binding:host_id') != host:
+                updates['binding:host_id'] = host
+
+            # Update port with newly allocated PCI devices.  Even if the
+            # resize is happening on the same host, a new PCI device can be
+            # allocated.
+            vnic_type = p.get('binding:vnic_type')
+            if vnic_type in network_model.VNIC_TYPES_SRIOV:
+                binding_profile = p.get('binding:profile', {})
+                pci_slot = binding_profile.get('pci_slot')
+                new_dev = pci_mapping.get(pci_slot)
+                if new_dev:
+                    updates['binding:profile'] = \
+                        get_pci_device_profile(new_dev)
+                else:
+                    raise exception.PortUpdateFailed(port_id=p['id'],
+                        reason=_("Unable to correlate PCI slot %s") %
+                                 pci_slot)
+
+            port_updates.append((p['id'], updates))
+
+        # Avoid rolling back updates if we catch an error above.
+        # TODO(lbeliveau): Batch up the port updates in one neutron call.
+        for port_id, updates in port_updates:
+            if updates:
+                LOG.info(_LI("Updating port %(port)s with "
+                             "attributes %(attributes)s"),
+                         {"port": p['id'], "attributes": updates},
+                         instance=instance)
                 try:
-                    neutron.update_port(p['id'],
-                                        {'port': {'binding:host_id': host}})
+                    neutron.update_port(port_id, {'port': updates})
                 except Exception:
                     with excutils.save_and_reraise_exception():
                         LOG.exception(_LE("Unable to update host of port %s"),
-                                      p['id'], instance=instance)
+                                      port_id, instance=instance)
 
     def update_instance_vnic_index(self, context, instance, vif, index):
         """Update instance vnic index.

nova/tests/unit/network/test_neutronv2.py

@@ -3557,6 +3557,133 @@ def test_update_port_bindings_for_instance_same_host(self,
         update_port_mock.assert_called_once_with(
             'fake-port-2', {'port': {'binding:host_id': instance.host}})
 
+    @mock.patch.object(pci_whitelist, 'get_pci_device_devspec')
+    @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
+    def test_update_port_bindings_for_instance_with_pci(self,
+                                            get_client_mock,
+                                            get_pci_device_devspec_mock):
+        self.api._has_port_binding_extension = mock.Mock(return_value=True)
+
+        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='1377',
+                                       product_id='0047',
+                                       address='0000:0a:00.1',
+                                       compute_node_id=1,
+                                       request_id='1234567890')])
+        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')])
+        instance.pci_devices = instance.migration_context.old_pci_devices
+
+        # Validate that non-direct port aren't updated (fake-port-2).
+        fake_ports = {'ports': [
+                        {'id': 'fake-port-1',
+                         'binding:vnic_type': 'direct',
+                         'binding:host_id': 'fake-host-old',
+                         'binding:profile':
+                            {'pci_slot': '0000:0a:00.1',
+                             'physical_network': 'old_phys_net',
+                             'pci_vendor_info': 'old_pci_vendor_info'}},
+                        {'id': 'fake-port-2',
+                         'binding:host_id': instance.host}]}
+        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.
+        update_port_mock.assert_called_once_with(
+            'fake-port-1',
+                {'port':
+                    {'binding:host_id': 'fake-host',
+                     'binding:profile': {'pci_slot': '0000:0b:00.1',
+                                         'physical_network': 'physnet1',
+                                         'pci_vendor_info': '1377:0047'}}})
+
+    @mock.patch.object(pci_whitelist, 'get_pci_device_devspec')
+    @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
+    def test_update_port_bindings_for_instance_with_pci_fail(self,
+                                            get_client_mock,
+                                            get_pci_device_devspec_mock):
+        self.api._has_port_binding_extension = mock.Mock(return_value=True)
+
+        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='1377',
+                                       product_id='0047',
+                                       address='0000:0c:00.1',
+                                       compute_node_id=1,
+                                       request_id='1234567890')])
+        instance.migration_context.new_pci_devices = objects.PciDeviceList(
+            objects=[objects.PciDevice(vendor_id='1377',
+                                       product_id='0047',
+                                       address='0000:0d:00.1',
+                                       compute_node_id=2,
+                                       request_id='1234567890')])
+        instance.pci_devices = instance.migration_context.old_pci_devices
+
+        fake_ports = {'ports': [
+                        {'id': 'fake-port-1',
+                         'binding:vnic_type': 'direct',
+                         'binding:host_id': 'fake-host-old',
+                         'binding:profile':
+                            {'pci_slot': '0000:0a:00.1',
+                             'physical_network': 'old_phys_net',
+                             'pci_vendor_info': 'old_pci_vendor_info'}}]}
+        list_ports_mock = mock.Mock(return_value=fake_ports)
+        get_client_mock.return_value.list_ports = list_ports_mock
+
+        # Assert exception is raised if the mapping is wrong.
+        self.assertRaises(exception.PortUpdateFailed,
+                          self.api._update_port_binding_for_instance,
+                          self.context,
+                          instance,
+                          instance.host)
+
+    def test_get_pci_mapping_for_migration(self):
+        instance = fake_instance.fake_instance_obj(self.context)
+        instance.migration_context = objects.MigrationContext()
+        old_pci_devices = objects.PciDeviceList(
+            objects=[objects.PciDevice(vendor_id='1377',
+                                       product_id='0047',
+                                       address='0000:0a:00.1',
+                                       compute_node_id=1,
+                                       request_id='1234567890')])
+
+        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')])
+
+        instance.migration_context.old_pci_devices = old_pci_devices
+        instance.migration_context.new_pci_devices = new_pci_devices
+        instance.pci_devices = instance.migration_context.old_pci_devices
+
+        pci_mapping = self.api._get_pci_mapping_for_migration(
+            self.context, instance)
+        self.assertEqual(
+            {old_pci_devices[0].address: new_pci_devices[0]}, pci_mapping)
+
     @mock.patch('nova.network.neutronv2.api.compute_utils')
     def test_get_preexisting_port_ids(self, mocked_comp_utils):
         mocked_comp_utils.get_nw_info_for_instance.return_value = [model.VIF(
@@ -4091,6 +4218,28 @@ def test_populate_neutron_extension_values_binding_sriov(self,
 
         self.assertEqual(profile, port_req_body['port']['binding:profile'])
 
+    @mock.patch.object(pci_whitelist, 'get_pci_device_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):
+        api = neutronapi.API()
+        host_id = 'my_host_id'
+        instance = {'host': host_id}
+        port_req_body = {'port': {}}
+        pci_req_id = 'my_req_id'
+        pci_objs = [objects.PciDevice(vendor_id='1377',
+                                      product_id='0047',
+                                      address='0000:0a:00.1',
+                                      compute_node_id=1,
+                                      request_id='1234567890')]
+
+        mock_get_instance_pci_devs.return_value = pci_objs
+        mock_get_pci_device_devspec.return_value = None
+
+        self.assertRaises(
+            exception.PciDeviceNotFound, api._populate_neutron_binding_profile,
+            instance, pci_req_id, port_req_body)
+
     def _populate_pci_mac_address_fakes(self):
         instance = fake_instance.fake_instance_obj(self.context)
         pci_dev = {'vendor_id': '1377',
@@ -4214,8 +4363,9 @@ def _test_update_port_true_exception(self, expected_bind_host,
                           *args)
 
     def test_migrate_instance_finish_binding_false(self):
+        instance = fake_instance.fake_instance_obj(self.context)
         self._test_update_port_binding_false('migrate_instance_finish',
-                                             self.context, None,
+                                             self.context, instance,
                                              {'dest_compute': 'fake'})
 
     def test_migrate_instance_finish_binding_true(self):
@@ -4239,8 +4389,9 @@ def test_migrate_instance_finish_binding_true_exception(self):
                                               migration)
 
     def test_setup_instance_network_on_host_false(self):
+        instance = fake_instance.fake_instance_obj(self.context)
         self._test_update_port_binding_false(
-            'setup_instance_network_on_host', self.context, None,
+            'setup_instance_network_on_host', self.context, instance,
             'fake_host')
 
     def test_setup_instance_network_on_host_true(self):