MAC address of direct-physical port is not updated during migration

[Balazs Gibizer]

Description
===========
Nova update the MAC of the direct-physical port based on the MAC of the PF selected during the initial boot of the VM. But Nova does not update the MAC when the VM is migrated to another compute and therefore using another PF.

Steps to reproduce
==================
Needs a multi node devstack with available SRIOV PFs.

stack@master0:~$ openstack hypervisor list
+----+---------------------+-----------------+-----------+-------+
| ID | Hypervisor Hostname | Hypervisor Type | Host IP | State |
+----+---------------------+-----------------+-----------+-------+
| 1 | master0 | QEMU | 10.1.0.21 | up |
| 2 | node0 | QEMU | 10.1.0.22 | up |
+----+---------------------+-----------------+-----------+-------+

stack@master0:~$ mysql -D nova_cell1 -e "select status, address, parent_addr, dev_type, compute_node_id, product_id, instance_uuid from pci_devices;"
+-----------+--------------+--------------+----------+-----------------+------------+---------------+
| status | address | parent_addr | dev_type | compute_node_id | product_id | instance_uuid |
+-----------+--------------+--------------+----------+-----------------+------------+---------------+
| available | 0000:81:00.0 | NULL | type-PF | 1 | 154d | NULL |
| available | 0000:81:00.1 | NULL | type-PF | 1 | 154d | NULL |
| available | 0000:81:10.0 | 0000:81:00.0 | type-VF | 1 | 10ed | NULL |
| available | 0000:81:10.2 | 0000:81:00.0 | type-VF | 1 | 10ed | NULL |
| available | 0000:81:10.4 | 0000:81:00.0 | type-VF | 1 | 10ed | NULL |
| available | 0000:81:10.6 | 0000:81:00.0 | type-VF | 1 | 10ed | NULL |
| available | 0000:81:00.0 | NULL | type-PF | 2 | 154d | NULL |
| available | 0000:81:00.1 | NULL | type-PF | 2 | 154d | NULL |
| available | 0000:81:10.0 | 0000:81:00.0 | type-VF | 2 | 10ed | NULL |
| available | 0000:81:10.2 | 0000:81:00.0 | type-VF | 2 | 10ed | NULL |
| available | 0000:81:10.4 | 0000:81:00.0 | type-VF | 2 | 10ed | NULL |
| available | 0000:81:10.6 | 0000:81:00.0 | type-VF | 2 | 10ed | NULL |
+-----------+--------------+--------------+----------+-----------------+------------+---------------+

These are the PF MAC's

stack@master0:~$ ip a | grep b4:96:91:34
    link/ether b4:96:91:34:f4:34 brd ff:ff:ff:ff:ff:ff
    link/ether b4:96:91:34:f4:36 brd ff:ff:ff:ff:ff:ff

stack@node0:~/nova$ ip a | grep b4:96:91:34
    link/ether b4:96:91:34:ed:d4 brd ff:ff:ff:ff:ff:ff
    link/ether b4:96:91:34:ed:d6 brd ff:ff:ff:ff:ff:ff

1) create a port with vnic_type=directy-physical

stack@master0:~$ openstack port show port-direct-physical
+-------------------------+---------------------------------------------------------------------------+
| Field | Value |
+-------------------------+---------------------------------------------------------------------------+
| admin_state_up | UP |
| allowed_address_pairs | |
| binding_host_id | |
| binding_profile | physical_network='physnet0' |
| binding_vif_details | |
| binding_vif_type | unbound |
| binding_vnic_type | direct-physical |
| created_at | 2021-08-19T12:02:06Z |
| data_plane_status | None |
| description | |
| device_id | |
| device_owner | |
| device_profile | None |
| dns_assignment | None |
| dns_domain | None |
| dns_name | None |
| extra_dhcp_opts | |
| fixed_ips | ip_address='10.0.4.227', subnet_id='a3960124-77c3-4127-8a91-9ae99ed552ea' |
| id | 864ee6f2-2e72-47b6-9917-6b8e931afc3e |
| ip_allocation | None |
| mac_address | fa:16:3e:71:fe:26 |
| name | port-direct-physical |
| network_id | 96d669e1-0c1e-474e-aad6-57cd9e026980 |
| numa_affinity_policy | None |
| port_security_enabled | True |
| project_id | 493cf4fe555d4ea0b50bdd46a8edf5c2 |
| propagate_uplink_status | None |
| qos_network_policy_id | None |
| qos_policy_id | None |
| resource_request | None |
| revision_number | 10 |
| security_group_ids | 16150c45-2217-4e73-8e4d-ca946cbc21cc |
| status | DOWN |
| tags | |
| trunk_details | None |
| updated_at | 2021-09-01T13:12:42Z |
+-------------------------+---------------------------------------------------------------------------+

2) boot a VM with that port

stack@master0:~$ openstack --os-compute-api-version 2.80 server create --image cirros-0.5.2-x86_64-disk --flavor c1 --nic port-id=port-direct-physical vm1 --wait

+-------------------------------------+-----------------------------------------------------------------------------------------------------------------------+
| Field | Value |
+-------------------------------------+-----------------------------------------------------------------------------------------------------------------------+
| OS-DCF:diskConfig | MANUAL |
| OS-EXT-AZ:availability_zone | nova |
| OS-EXT-SRV-ATTR:host | node0 |
| OS-EXT-SRV-ATTR:hostname | vm1 |
| OS-EXT-SRV-ATTR:hypervisor_hostname | node0 |
| OS-EXT-SRV-ATTR:instance_name | instance-000000ff |
| OS-EXT-SRV-ATTR:kernel_id | |
| OS-EXT-SRV-ATTR:launch_index | 0 |
| OS-EXT-SRV-ATTR:ramdisk_id | |
| OS-EXT-SRV-ATTR:reservation_id | r-ou2kcw3c |
| OS-EXT-SRV-ATTR:root_device_name | /dev/vda |
| OS-EXT-SRV-ATTR:user_data | None |
| OS-EXT-STS:power_state | Running |
| OS-EXT-STS:task_state | None |
| OS-EXT-STS:vm_state | active |
| OS-SRV-USG:launched_at | 2021-09-01T13:23:26.000000 |
| OS-SRV-USG:terminated_at | None |
| accessIPv4 | |
| accessIPv6 | |
| addresses | net0=10.0.4.227 |
| adminPass | B5Zje4m3vbq6 |
| config_drive | |
| created | 2021-09-01T13:23:23Z |
| description | None |
| flavor | disk='1', ephemeral='0', extra_specs.hw_rng:allowed='True', original_name='cirros256', ram='256', swap='0', vcpus='1' |
| hostId | 512757d0f0cb65d56c6cabcc54b317bbc1c73dbbe8eb603d9452bc59 |
| host_status | UP |
| id | 4707526d-6980-4af7-9476-d85e3f6aec4f |
| image | cirros-0.5.2-x86_64-disk (1193639c-3c6a-4d3c-9c66-ce1ab741a9d3) |
| key_name | None |
| locked | False |
| locked_reason | None |
| name | vm1 |
| progress | 0 |
| project_id | 493cf4fe555d4ea0b50bdd46a8edf5c2 |
| properties | |
| security_groups | name='default' |
| server_groups | [] |
| status | ACTIVE |
| tags | |
| trusted_image_certificates | None |
| updated | 2021-09-01T13:23:26Z |
| user_id | e08f3a84c1484d6ebdde1faf24fd0373 |
| volumes_attached | |
+-------------------------------------+-----------------------------------------------------------------------------------------------------------------------+

3) Observer the the VM is booted on node0 and check that the MAC of the port is updated to the mac of one of the PF on node0

stack@master0:~$ openstack port show port-direct-physical | grep mac
| mac_address | b4:96:91:34:ed:d6 |

4) Cold migrate the VM
stack@master0:~$ openstack server migrate vm1
stack@master0:~$ openstack server migration confirm vm1

5) Observe that the VM is now running on master0 but MAC of the port is not changed

stack@master0:~$ openstack server show vm1 | grep hypervisor
| OS-EXT-SRV-ATTR:hypervisor_hostname | master0 |

stack@master0:~$ openstack port show port-direct-physical | grep mac
| mac_address | b4:96:91:34:ed:d6 |

Expected behavior
=================

The MAC of the neutron port is updated according the the MAC of the PF on master0

Environment
===========
multinode devstack on recent master (nova on 033af94179)

[Balazs Gibizer (balazs-gibizer)]

Nova updates the MAC during initial boot via https://opendev.org/openstack/nova/src/commit/d20c1926553a6dcc50597a4ad14fcf93ad0b4b78/nova/network/neutron.py#L1173 but that code path is not called during cold migration

[Balazs Gibizer (balazs-gibizer)]

Sean had a lot of good info about this bug [1]. Bottom line:
* all the move operations are affected (resize, cold, live migration, evacuation, unshelve)
* as the port keeps the MAC of the source host PF after the migration the source host PF cannot be used for another VM as that result in duplicate MAC error from neutron.
* as the port keeps the MAC of the source host PF after the migration the VM on the dest host will not get IP from the neutron DHCP server as the DHCP server still see the old MAC in neutron.

[1] https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2021-09-01.log.html#t2021-09-01T13:33:48

[sean mooney (sean-k-mooney)]

triaging this as medium as this has never worked.

move operations were never implemented when direct-physical support was added.
the spec that enabled PF passthough did not enable or consider any move operations
https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/sriov-pf-passthrough-neutron-port.html

we have a downstream bug for this that has been open for some time
https://bugzilla.redhat.com/show_bug.cgi?id=174584

i also have a numer of similar bugs downstream for other move operations.

i mention this breifly during the xena ptg line 420
https://etherpad.opendev.org/p/r.0a1509fc788d92391f50397f2ee4af9f

[Balazs Gibizer (balazs-gibizer)]

I tried to fix this by updating the mac_address field for the port when the port is being bound to the target host of a move operation. But Neutron rejects the update as the mac_address cannot be changed on an already bound port and this port is currently bound to the source host of the migration. Nova does an atomic re-bind during migration, by changing the binding:host_id of the port from the source host to the destination. So there is no time window to update the MAC.

The only way forward I see that neutron needs to allow MAC update on a bound port in the same request where the binding:host_id is changed.

Currently neutron reject the update with:
Feb 14 17:13:23 node0 nova-compute[3131239]: ERROR oslo_messaging.rpc.server neutronclient.common.exceptions.Conflict: Unable to complete operation on port 168a0263-5ae5-44f3-9be9-32fcf6d30f05, port is already bound, port type: hostdev_physical, old_mac b4:96:91:34:f4:36, new_mac b4:96:91:34:ed:d6.

[Balazs Gibizer (balazs-gibizer)]

We have a slight complication. There are two cases:

1) During cold migration, resize, evacuation nova directly updates the binding:host_id to rebind the port to the destination host. In this case it is OK to allow that the request changing the host_id also changes the MAC.

2) During live migration nova first creates an inactive port binding for the port on the dest, then later activates that binding. In this case there is no single request changing the port mac and the active binding:host_id. It seems that in this case the MAC address is more related to the binding objects and the port.mac_address should reflect the MAC address in the active port binding. Nova could add the MAC to the inactive binding before activating it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/829247

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/nova/+/829248

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron-lib/+/831935

[Balazs Gibizer (balazs-gibizer)]

There were multiple discussion around the neutron solution:

0) A patch was proposed that only changes the MAC in the port_dict returned on the REST API.

1) A concern was raised that if we only modify the port dict the neutron internal users of the port OVO will not see the change of the MAC when the port is bound. So we investigated a solution that would do the MAC overwrite in the OVO layer. It turned out that GET /ports/<port_id> codepath is not using the port OVO layer, it reads the DB directly. So simply moving the overwrite to the OVO layer is not a viable solution.

1.1) We discussed with Rodolfo over IRC[1] that it is probably safe to ignore the OVO path as there we assume that there is no neutron internal code path that uses the port OVO and also depends on the MAC address of a direct-physical the port.

2) There is a suggestion to wrap the whole change in an API extension to signal the API behavior change. But there is an open question if such a change can be backportable or not.

3) There was an alternative suggested to overwrite the port.mac_address value in the DB. This would remove the need to consider both the OVO and the port_dict code path separately. This still allows to wrap the change in an API extension.

For me the #3) is the preferred alternative right now as it would lead to more consistent data in system.

[1] https://meetings.opendev.org/irclogs/%23openstack-neutron/%23openstack-neutron.2022-03-07.log.html#t2022-03-07T09:48:06

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-lib/+/831935
Committed: https://opendev.org/openstack/neutron-lib/commit/b5f0a2642794a3855097bb1a5221076849ab675f
Submitter: "Zuul (22348)"
Branch: master

commit b5f0a2642794a3855097bb1a5221076849ab675f
Author: Balazs Gibizer <email address hidden>
Date: Fri Mar 4 13:50:19 2022 +0100

    Add port-mac-address-override shim extension

    The neutron patch Ib0638f5db69cb92daf6932890cb89e83cf84f295 introduces a
    way to provide a MAC address for a direct-physical port at binding time.
    So this patch adds an API extension and document the new behavior in the API ref.

    Related-Bug: #1942329
    Change-Id: I54b4c85ffc4856fba7ad5e9e29f77f74815e1275

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/829247
Committed: https://opendev.org/openstack/neutron/commit/4e78aaa694683f812d091a794bd140a7d363dd9b
Submitter: "Zuul (22348)"
Branch: master

commit 4e78aaa694683f812d091a794bd140a7d363dd9b
Author: Balazs Gibizer <email address hidden>
Date: Tue Feb 15 13:56:34 2022 +0100

    Update port MAC from binding profile for PFs

    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
    is not able to update the MAC when the port is bound to a different
    physical device on the destination host.

    This patch extends port binding logic for direct-physical ports to allow
    providing the MAC address of the physical device via the binding profile.
    If it is provided then Neutron overwrites the value of the mac_address
    field of the port with the value from the active binding profile.

    Also when the port is being unbound or the MAC address is removed from
    the active binding porfile then neutron resets the mac_address field of
    port to a generated MAC to avoid duplicated MAC issues when another port
    is being bound to the same physical device.

    The shim API extension for this change is being proposed in
    I54b4c85ffc4856fba7ad5e9e29f77f74815e1275 in neutron-lib.

    Depends-On: https://review.opendev.org/c/openstack/neutron-lib/+/831935

    Closes-Bug: #1942329

    Change-Id: Ib0638f5db69cb92daf6932890cb89e83cf84f295

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/844680

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/nova/+/829248
Committed: https://opendev.org/openstack/nova/commit/cd03bbc1c33e33872594cf002f0e7011ab8ea047
Submitter: "Zuul (22348)"
Branch: master

commit cd03bbc1c33e33872594cf002f0e7011ab8ea047
Author: Balazs Gibizer <email address hidden>
Date: Tue Feb 15 14:38:41 2022 +0100

    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

    Change-Id: Iad5e70b43a65c076134e1874cb8e75d1ba214fde

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/844680
Committed: https://opendev.org/openstack/neutron/commit/326c0076eff2c311fa461ac57018646466290856
Submitter: "Zuul (22348)"
Branch: master

commit 326c0076eff2c311fa461ac57018646466290856
Author: sunxifa <email address hidden>
Date: Sat Jun 4 15:19:52 2022 +0800

    Update port-mac-address-override shim extension

    The port-mac-address-override shim extension proposed in
    I54b4c85ffc4856fba7ad5e9e29f77f74815e1275 in neutron-lib has merged
    and the neutron-lib has being released. So this patch updates the
    API extension and replaces the import with the new neutron_lib api
    definitions.

    Depends-On: https://review.opendev.org/c/openstack/neutron-lib/+/831935

    Change-Id: Ic332769af532003a9a5b2d2cee38b6210b5aac91
    Related-Bug: #1942329

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/nova/+/855023

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 21.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 26.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/nova/+/864933

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/865336

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/nova/+/855023
Committed: https://opendev.org/openstack/nova/commit/813377077bd0173bdf128823e46b5df7c0a575b9
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 813377077bd0173bdf128823e46b5df7c0a575b9
Author: Balazs Gibizer <email address hidden>
Date: Tue Feb 15 14:38:41 2022 +0100

    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/objects/pci_device.py
        nova/virt/libvirt/host.py

    Change-Id: Iad5e70b43a65c076134e1874cb8e75d1ba214fde
    (cherry picked from commit cd03bbc1c33e33872594cf002f0e7011ab8ea047)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/nova/+/864933
Committed: https://opendev.org/openstack/nova/commit/a340630c5c78e5f5856004d0a551dfa93df7f28d
Submitter: "Zuul (22348)"
Branch: stable/xena

commit a340630c5c78e5f5856004d0a551dfa93df7f28d
Author: Balazs Gibizer <email address hidden>
Date: Tue Feb 15 14:38:41 2022 +0100

    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)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 24.2.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 25.1.0 release.

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/865336
Reason: stable/wallaby branch of openstack/nova is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/wallaby if you want to further work on this patch.