openstack/neutron
Code Issues Proposed changes

Checking pci_slot to avoid changing staus to BUILD forever

Browse Source 
Currently when sriov agent is enabled and migrating a non-sriov
instance, non-sriov port status is frequently set to BUILD
instead of ACTIVE.
This is because the 'binding_activate' function in sriov-nic-agent sets it
BUILD with get_device_details_from_port_id(as it calls _get_new_status).

This patch checks network_ports in binding_activate and
skip binding port if it is not sriov port

Closes-Bug: #2072154
Change-Id: I2d7702e17c75c96ca2f29749dccab77cb2f4bcf4
(cherry picked from commit a311606fcd)
This commit is contained in:
Seyeong Kim 2024-07-04 06:23:59 +00:00 committed by Lajos Katona
parent df02d0dfec
commit 59bc8e476f
2 changed files with 62 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py
View File

@ -112,17 +112,39 @@ class SriovNicSwitchRpcCallbacks(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
def binding_activate(self, context, **kwargs):
if kwargs.get('host') != self.agent.conf.host:
return
LOG.debug("binding activate for port %s", kwargs.get('port_id'))
port_id = kwargs.get('port_id')
def _is_port_id_in_network(network_port, port_id):
for network_id, ports in network_port.items():
for port in ports:
if port['port_id'] == port_id:
return True
return False
is_port_id_sriov = _is_port_id_in_network(
self.agent.network_ports, port_id
)
if is_port_id_sriov:
LOG.debug("binding activate for port %s", port_id)
device_details = self.agent.get_device_details_from_port_id(
kwargs.get('port_id'))
port_id)
mac = device_details.get('mac_address')
binding_profile = device_details.get('profile')
if binding_profile:
pci_slot = binding_profile.get('pci_slot')
self.agent.activated_bindings.add((mac, pci_slot))
else:
LOG.warning("binding_profile not found for port %s.",
kwargs.get('port_id'))
LOG.warning(
"binding_profile not found for port %s.",
port_id
)
else:
LOG.warning(
"This port is not SRIOV, skip binding for port %s.",
port_id
)
def binding_deactivate(self, context, **kwargs):
if kwargs.get('host') != self.agent.conf.host:

30
neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py
View File

@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import collections
import copy
from unittest import mock
@ -460,6 +461,7 @@ class FakeAgent(object):
self.activated_bindings = set()
self.conf = mock.Mock()
self.conf.host = 'host1'
self.network_ports = collections.defaultdict(list)
class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
@ -528,6 +530,12 @@ class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
}
kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host)
kwargs['context'] = self.context
self.agent.network_ports['network_id'].append({
'port_id': fake_port['id'],
'device': 'fake_device'
})
self.sriov_rpc_callback.binding_activate(**kwargs)
# Assert agent.activated_binding set contains the new binding
self.assertIn((fake_port['mac_address'],
@ -538,10 +546,32 @@ class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
fake_port = self._create_fake_port()
kwargs = self._create_fake_bindings(fake_port, 'other-host')
kwargs['context'] = self.context
self.agent.network_ports[self.agent.conf.host].append({
'port_id': fake_port['id'],
'device': 'fake_device'
})
self.sriov_rpc_callback.binding_activate(**kwargs)
# Assert no bindings were added
self.assertEqual(set(), self.agent.activated_bindings)
def test_binding_activate_port_not_in_network(self):
fake_port = self._create_fake_port()
kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host)
kwargs['context'] = self.context
self.agent.network_ports['network_id'] = []
with mock.patch.object(sriov_nic_agent.LOG,
'warning') as mock_warning:
self.sriov_rpc_callback.binding_activate(**kwargs)
# Check that the warning message was logged
expected_msg = (
"This port is not SRIOV, skip binding for port %s."
)
mock_warning.assert_called_once_with(expected_msg, fake_port['id'])
def test_binding_deactivate(self):
# binding_deactivate() basically does nothing
# call it with both the agent's host and other host to cover