openstack/neutron
Code Issues Proposed changes

Merge "Update port MAC from binding profile for PFs"

Browse Source
This commit is contained in:
Zuul 2022-04-25 12:54:29 +00:00 committed by Gerrit Code Review
commit cab15b15e2
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

@ -1955,7 +1955,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.