openstack/neutron
Code Issues Proposed changes

Update port MAC from binding profile for PFs

Browse Source 
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
This commit is contained in:
Balazs Gibizer 2022-02-15 13:56:34 +01:00 committed by Balazs Gibizer
parent fc3d3cc824
commit 4e78aaa694
6 changed files with 291 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
neutron/extensions/_port_mac_address_override.py Normal file
View File

@ -0,0 +1,33 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
# TODO(gibi): remove this when the neutron-lib change
# https://review.opendev.org/c/openstack/neutron-lib/+/831935 merged and
# released
NAME = 'Neutron Port MAC address override'
ALIAS = 'port-mac-override'
DESCRIPTION = (
"Allow overriding the MAC address of a direct-physical Port via the "
"active binding profile")
UPDATED_TIMESTAMP = "2022-03-18T10:00:00-00:00"
RESOURCE_ATTRIBUTE_MAP = {}
IS_SHIM_EXTENSION = True
IS_STANDARD_ATTR_EXTENSION = False
SUB_RESOURCE_ATTRIBUTE_MAP = {}
ACTION_MAP = {}
REQUIRED_EXTENSIONS = []
OPTIONAL_EXTENSIONS = []
ACTION_STATUS = {}

27
neutron/extensions/port_mac_address_override.py Normal file
View File

@ -0,0 +1,27 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# TODO(gibi): replace the import with
# from neutron_lib.api.definitions import port_mac_address_override
# once https://review.opendev.org/c/openstack/neutron-lib/+/831935 merged and
# neutron-lib is released
from neutron.extensions import (
_port_mac_address_override as port_mac_address_override
)
from neutron_lib.api import extensions as api_extensions
class Port_mac_address_override(api_extensions.APIExtensionDescriptor):
"""Extension to support port MAC address override"""
api_definition = port_mac_address_override

39
neutron/plugins/ml2/plugin.py
View File

@ -119,6 +119,13 @@ from neutron.db import securitygroups_rpc_base as sg_db_rpc
from neutron.db import segments_db
from neutron.db import subnet_service_type_mixin
from neutron.db import vlantransparent_db
# TODO(gibi): replace the import with
# from neutron_lib.api.definitions import port_mac_address_override
# once https://review.opendev.org/c/openstack/neutron-lib/+/831935 merged and
# neutron-lib is released
from neutron.extensions import (
_port_mac_address_override as port_mac_address_override
)
from neutron.extensions import dhcpagentscheduler as dhcp_ext
from neutron.extensions import filter_validation
from neutron.extensions import security_groups_shared_filtering_lib
@ -231,6 +238,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
pnap_def.ALIAS,
pdp_def.ALIAS,
quota_check_limit.ALIAS,
port_mac_address_override.ALIAS,
]
# List of agent types for which all binding_failed ports should try to be
@ -751,6 +759,37 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
# updated binding state.
self._update_port_dict_binding(port, cur_context_binding)
# 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 puts the new MAC into the binding profile when
# requesting the binding either by creating it or by activating
# currently inactive one.
if (
cur_context_binding.vnic_type ==
portbindings.VNIC_DIRECT_PHYSICAL
):
profile = jsonutils.loads(cur_context_binding.profile)
new_mac = profile.get('device_mac_address')
# When the port is being unbound or the client removes the
# MAC from the binding profile then the MAC of the port
# needs to be reset. It is necessary to avoid the
# duplicated MAC address issue when an unbound port is
# still using the MAC of a physical device and another
# port is being bound to that physical device.
if not new_mac:
new_mac = self._generate_macs()[0]
old_mac = port_db.mac_address
port_db.mac_address = new_mac
LOG.debug(
'The MAC address of the Port %s is updated from %s to '
'%s due to committing a new port binding.',
port_id, old_mac, new_mac
)
# Update the port status if requested by the bound driver.
if (bind_context._binding_levels and
bind_context._new_port_status):

2
neutron/tests/unit/plugins/ml2/test_plugin.py
View File

@ -1952,7 +1952,7 @@ class TestMl2PluginOnly(Ml2PluginV2TestCase):
plugin = directory.get_plugin()
port = {'device_id': '123', 'device_owner': 'compute:nova'}
new_attrs = ({'device_id': '', 'device_owner': ''} if unbinding else
{'name': 'new'})
{'name': 'new'})
binding = mock.Mock()
binding.vnic_type = (
portbindings.VNIC_DIRECT_PHYSICAL if direct_physical else

177
neutron/tests/unit/plugins/ml2/test_port_binding.py
View File

@ -15,6 +15,7 @@
from unittest import mock
from neutron_lib.api.definitions import port as port_def
from neutron_lib.api.definitions import portbindings
from neutron_lib.api.definitions import portbindings_extended as pbe_ext
from neutron_lib import constants as const
@ -662,3 +663,179 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
response.status_int)
self.assertTrue(exceptions.PortBindingError.__name__ in
response.text)
def _create_unbound_port(self):
with self.port() as port:
return port['port']
def _get_port_mac(self, port_id):
port = self._show('ports', port_id)['port']
return port[port_def.PORT_MAC_ADDRESS]
def _update_port_with_binding(
self, port_id, host, vnic_type, binding_profile_mac
):
device_owner = f'{const.DEVICE_OWNER_COMPUTE_PREFIX}nova'
update_body = {
'port': {
'device_owner': device_owner,
'device_id': 'some-nova-instance',
'binding:vnic_type': vnic_type,
'binding:host_id': host,
'binding:profile': {},
}
}
if binding_profile_mac:
update_body['port']['binding:profile'] = {
'device_mac_address': binding_profile_mac
}
with mock.patch.object(
mechanism_test.TestMechanismDriver, '_check_port_context'
):
req = self.new_update_request('ports', update_body, port_id)
self.assertEqual(200, req.get_response(self.api).status_int)
def test_bind_non_pf_port_with_mac_port_not_updated(self):
new_mac = 'b4:96:91:34:f4:aa'
port = self._create_unbound_port()
# Neutron generates a mac for each port created
generated_mac = port['mac_address']
# provide a new MAC in the binding for a normal port
self._update_port_with_binding(
port['id'],
self.host,
vnic_type=portbindings.VNIC_NORMAL,
binding_profile_mac=new_mac
)
# Neutron ignores the MAC in the binding for non PF ports so the
# generated MAC remains
self.assertEqual(generated_mac, self._get_port_mac(port['id']))
def test_bind_pf_port_with_mac_port_updated(self):
host1 = self.host
host_1_pf_mac = 'b4:96:91:34:f4:aa'
# this hostname is also accepted by the TestMechanismDriver
host2 = 'host-ovs-filter-active'
host_2_pf_mac = 'b4:96:91:34:f4:bb'
port = self._create_unbound_port()
# neutron generates a MAC for each port created
generated_mac = port[port_def.PORT_MAC_ADDRESS]
# Now lets bind the port as Nova to host1
# Nova, when binds a PF port, provides the current MAC in the
# binding profile
self._update_port_with_binding(
port['id'],
host1,
vnic_type=portbindings.VNIC_DIRECT_PHYSICAL,
binding_profile_mac=host_1_pf_mac
)
current_mac = self._get_port_mac(port['id'])
self.assertNotEqual(generated_mac, current_mac)
# we expect that Neutron updates the port's MAC based on the
# binding
self.assertEqual(host_1_pf_mac, current_mac)
# Now create a new, inactive binding on a different host with a
# different MAC
profile = {'device_mac_address': host_2_pf_mac}
kwargs = {
pbe_ext.PROFILE: profile,
pbe_ext.VNIC_TYPE: portbindings.VNIC_DIRECT_PHYSICAL,
}
self._make_port_binding(self.fmt, port['id'], host2, **kwargs)
# this should not change the MAC on the port as the new binding is
# inactive
self.assertEqual(host_1_pf_mac, self._get_port_mac(port['id']))
# Now activate the binding on host2
with mock.patch.object(
mechanism_test.TestMechanismDriver, '_check_port_context'
):
self._activate_port_binding(port['id'], host2)
# Neutron should update the port MAC to the MAC in the host2 binding
self.assertEqual(host_2_pf_mac, self._get_port_mac(port['id']))
# Now activate the binding on host1 again
with mock.patch.object(
mechanism_test.TestMechanismDriver, '_check_port_context'
):
self._activate_port_binding(port['id'], host1)
# Neutron should update the port MAC to the MAC in the host1 binding
self.assertEqual(host_1_pf_mac, self._get_port_mac(port['id']))
# Now delete the inactive binding
response = self._delete_port_binding(port['id'], host2)
self.assertEqual(webob.exc.HTTPNoContent.code, response.status_int)
# the MAC should not change
self.assertEqual(host_1_pf_mac, self._get_port_mac(port['id']))
# Now remove the MAC from the active binding profile
self._update_port_with_binding(
port['id'],
host1,
portbindings.VNIC_DIRECT_PHYSICAL,
binding_profile_mac=None,
)
# so the port should have a generated mac
generated_mac = self._get_port_mac(port['id'])
self.assertNotIn(
generated_mac,
{host_1_pf_mac, host_2_pf_mac}
)
# delete the remaining binding
response = self._delete_port_binding(port['id'], host1)
self.assertEqual(webob.exc.HTTPNoContent.code, response.status_int)
# the MAC should not change as it is already the generated MAC
self.assertEqual(generated_mac, self._get_port_mac(port['id']))
def test_pf_port_unbound_mac_reset(self):
host1 = self.host
host_1_pf_mac = 'b4:96:91:34:f4:aa'
port = self._create_unbound_port()
# neutron generates a MAC for each port created
generated_mac = port[port_def.PORT_MAC_ADDRESS]
# Now lets bind the port as Nova to host1
# Nova, when binds a PF port, provides the current MAC in the
# binding profile
self._update_port_with_binding(
port['id'],
host1,
vnic_type=portbindings.VNIC_DIRECT_PHYSICAL,
binding_profile_mac=host_1_pf_mac
)
current_mac = self._get_port_mac(port['id'])
self.assertNotEqual(generated_mac, current_mac)
# we expect that Neutron updates the port's MAC based on the
# binding
self.assertEqual(host_1_pf_mac, current_mac)
# Now unbound the port
update_body = {
'port': {
'device_owner': '',
'device_id': '',
'binding:host_id': None,
'binding:profile': {},
}
}
with mock.patch.object(
mechanism_test.TestMechanismDriver, '_check_port_context'
):
req = self.new_update_request('ports', update_body, port['id'])
self.assertEqual(200, req.get_response(self.api).status_int)
# Neutron expected to reset the MAC to a generated one so that the
# physical function on host1 is now usable for other ports
self.assertNotEqual(host_1_pf_mac, self._get_port_mac(port['id']))

14
releasenotes/notes/bug-1942329-7687504f9b177f80.yaml Normal file
View File

@ -0,0 +1,14 @@
---
fixes:
- |
`1942329 <https://bugs.launchpad.net/neutron/+bug/1942329>`_
Port binding logic for direct-physical ports has been extended 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
``device_mac_address`` field of the port object in the database with the
value from the active binding profile.
If there are ports bound before `the nova side of this fix is depolyed
<https://review.opendev.org/c/openstack/nova/+/829248>`_ then the VM using
the port needs to be moved or the port needs to be detached and re-attached
to force nova to provide the MAC address of the direct-physical port in the
port binding.