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 75a781723b4fae7de4346a69b874fe7c78f67034..4446609b1b2dc22e097157735176b94be2c9e508 100644 (file)
--- 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 8f984e0aac4b6d8f08108c16d4decdfcded477d5..5caa3c4552508f06f8075b04af2c23186354626b 100644 (file)
--- 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 13210aa5152aab8ec643d7d9a3400bbdd9d0fd4a..b9c50bb054356d723465b2dca49daa6737cf22e3 100644 (file)
--- 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 5512ea95f6b051931ec38c7640fb7cdd0bdb6668..9af70df1fa565223c06368cb8cc3c09c889d7ac4 100644 (file)
--- 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 8ebc73ce5fb78c6fc4edf2675aff24e41f969e91..4601eb693dc646b17b07c2fffe761df46da6abe3 100644 (file)
--- 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',