From: Roman Bogorodskiy Date: Wed, 24 Jun 2015 12:40:35 +0000 (+0200) Subject: sriov: update port state even if ip link fails X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=9c466f4d0effa4686ca6744d7b9d015857830cb7;p=openstack-build%2Fneutron-build.git sriov: update port state even if ip link fails Some SRIOV drivers/devices don't support link state setting, meaning that 'ip link' fails like this when trying to set state: # ip l set dev p2p1 vf 6 state disable RTNETLINK answers: Operation not supported The sriov-nic-agent tries to do that in SriovNicSwitchAgent.treat_device() and fails because of non-zero exit status from 'ip link' and, therefore, doesn't reach the code that updates the actual port status, so port could hang in a BUILD state even if binding was successful. This patch fixes problem of nova not being able to successfully bind or cleanup such a port. It does not fix a case when user manually updates admin_state_up for a port via API, it's subject to a separate fix. Also, replace LOG.exception with LOG.warning for set_device_state() as the exception would be logged by PciDeviceIPWrapper.set_vf_state() anyway. Closes-bug: #1468332 Change-Id: Ifa7c128d369ced60b5986aa0ed92527868f7efab --- diff --git a/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py b/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py index 75a781723..4446609b1 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py @@ -28,5 +28,9 @@ class IpCommandError(SriovNicError): message = _("ip command failed on device %(dev_name)s: %(reason)s") +class IpCommandOperationNotSupportedError(SriovNicError): + message = _("Operation not supported on device %(dev_name)s") + + class InvalidPciSlotError(SriovNicError): message = _("Invalid pci slot %(pci_slot)s") diff --git a/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py b/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py index 8f984e0aa..5caa3c455 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py @@ -38,6 +38,8 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): VF_LINE_FORMAT = VF_PATTERN + MAC_PATTERN + ANY_PATTERN + STATE_PATTERN VF_DETAILS_REG_EX = re.compile(VF_LINE_FORMAT) + IP_LINK_OP_NOT_SUPPORTED = 'RTNETLINK answers: Operation not supported' + class LinkState(object): ENABLE = "enable" DISABLE = "disable" @@ -46,6 +48,29 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): super(PciDeviceIPWrapper, self).__init__() self.dev_name = dev_name + def _set_feature(self, vf_index, feature, value): + """Sets vf feature + + Checks if the feature is not supported or there's some + general error during ip link invocation and raises + exception accordingly. + + :param vf_index: vf index + :param feature: name of a feature to be passed to ip link, + such as 'state' or 'spoofchk' + :param value: value of the feature setting + """ + try: + self._as_root([], "link", ("set", self.dev_name, "vf", + str(vf_index), feature, value)) + except Exception as e: + if self.IP_LINK_OP_NOT_SUPPORTED in str(e): + raise exc.IpCommandOperationNotSupportedError( + dev_name=self.dev_name) + else: + raise exc.IpCommandError(dev_name=self.dev_name, + reason=str(e)) + def get_assigned_macs(self, vf_list): """Get assigned mac addresses for vf list. @@ -97,14 +122,7 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): """ status_str = self.LinkState.ENABLE if state else \ self.LinkState.DISABLE - - try: - self._as_root([], "link", ("set", self.dev_name, "vf", - str(vf_index), "state", status_str)) - except Exception as e: - LOG.exception(_LE("Failed executing ip command")) - raise exc.IpCommandError(dev_name=self.dev_name, - reason=e) + self._set_feature(vf_index, "state", status_str) def set_vf_spoofcheck(self, vf_index, enabled): """sets vf spoofcheck @@ -114,13 +132,7 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): False to disable """ setting = "on" if enabled else "off" - - try: - self._as_root('', "link", ("set", self.dev_name, "vf", - str(vf_index), "spoofchk", setting)) - except Exception as e: - raise exc.IpCommandError(dev_name=self.dev_name, - reason=str(e)) + self._set_feature(vf_index, "spoofchk", setting) def set_vf_max_rate(self, vf_index, max_tx_rate): """sets vf max rate. @@ -128,14 +140,7 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): @param vf_index: vf index @param max_tx_rate: vf max tx rate in Mbps """ - try: - self._as_root([], "link", ("set", self.dev_name, "vf", - str(vf_index), "rate", - str(max_tx_rate))) - except Exception as e: - LOG.exception(_LE("Failed executing ip command")) - raise exc.IpCommandError(dev_name=self.dev_name, - reason=e) + self._set_feature(vf_index, "rate", str(max_tx_rate)) def _get_vf_link_show(self, vf_list, link_show_out): """Get link show output for VFs diff --git a/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py b/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py index 13210aa51..b9c50bb05 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py @@ -197,8 +197,11 @@ class SriovNicSwitchAgent(object): try: self.eswitch_mgr.set_device_state(device, pci_slot, admin_state_up) + except exc.IpCommandOperationNotSupportedError: + LOG.warning(_LW("Device %s does not support state change"), + device) except exc.SriovNicError: - LOG.exception(_LE("Failed to set device %s state"), device) + LOG.warning(_LW("Failed to set device %s state"), device) return if admin_state_up: # update plugin about port status diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_pci_lib.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_pci_lib.py index 5512ea95f..9af70df1f 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_pci_lib.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_pci_lib.py @@ -131,3 +131,13 @@ class TestPciLib(base.BaseTestCase): self.pci_wrapper.set_vf_max_rate, self.VF_INDEX, 1000) + + def test_set_vf_state_not_supported(self): + with mock.patch.object(self.pci_wrapper, + "_execute") as mock_exec: + mock_exec.side_effect = Exception( + pci_lib.PciDeviceIPWrapper.IP_LINK_OP_NOT_SUPPORTED) + self.assertRaises(exc.IpCommandOperationNotSupportedError, + self.pci_wrapper.set_vf_state, + self.VF_INDEX, + state=True) diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py index 8ebc73ce5..4601eb693 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py @@ -18,6 +18,7 @@ import mock from oslo_config import cfg from neutron.plugins.ml2.drivers.mech_sriov.agent.common import config # noqa +from neutron.plugins.ml2.drivers.mech_sriov.agent.common import exceptions from neutron.plugins.ml2.drivers.mech_sriov.agent import sriov_nic_agent from neutron.tests import base @@ -220,6 +221,31 @@ class TestSriovAgent(base.BaseTestCase): False) self.assertTrue(agent.plugin_rpc.update_device_up.called) + def test_treat_device_ip_link_state_not_supported(self): + agent = self.agent + agent.plugin_rpc = mock.Mock() + agent.eswitch_mgr = mock.Mock() + agent.eswitch_mgr.device_exists.return_value = True + agent.eswitch_mgr.set_device_state.side_effect = ( + exceptions.IpCommandOperationNotSupportedError( + dev_name='aa:bb:cc:dd:ee:ff')) + + agent.treat_device('aa:bb:cc:dd:ee:ff', '1:2:3:0', + admin_state_up=True) + self.assertTrue(agent.plugin_rpc.update_device_up.called) + + def test_treat_device_set_device_state_exception(self): + agent = self.agent + agent.plugin_rpc = mock.Mock() + agent.eswitch_mgr = mock.Mock() + agent.eswitch_mgr.device_exists.return_value = True + agent.eswitch_mgr.set_device_state.side_effect = ( + exceptions.SriovNicError()) + + agent.treat_device('aa:bb:cc:dd:ee:ff', '1:2:3:0', + admin_state_up=True) + self.assertFalse(agent.plugin_rpc.update_device_up.called) + def test_treat_devices_added_updated_admin_state_up_false(self): agent = self.agent mock_details = {'device': 'aa:bb:cc:dd:ee:ff',