Merge "Reduce IP link show calls for SR-IOV scan loop"

neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py

@@ -63,7 +63,7 @@ class PciOsWrapper(object):
         return vf_list
 
     @classmethod
-    def is_assigned_vf(cls, dev_name, vf_index):
+    def is_assigned_vf(cls, dev_name, vf_index, ip_link_show_output):
         """Check if VF is assigned.
 
         Checks if a given vf index of a given device name is assigned
@@ -73,6 +73,7 @@ class PciOsWrapper(object):
             Macvtap VF: macvtap@<vf interface> interface exists in ip link show
         @param dev_name: pf network device name
         @param vf_index: vf index
+        @param ip_link_show_output: 'ip link show' output
         """
         path = cls.PCI_PATH % (dev_name, vf_index)
 
@@ -86,7 +87,8 @@ class PciOsWrapper(object):
         # for macvtap interface. Therefore we workaround it
         # by parsing ip link show and checking if macvtap interface exists
         for ifname in ifname_list:
-            if pci_lib.PciDeviceIPWrapper.is_macvtap_assigned(ifname):
+            if pci_lib.PciDeviceIPWrapper.is_macvtap_assigned(
+                    ifname, ip_link_show_output):
                 return True
         return False
 
@@ -138,8 +140,9 @@ class EmbSwitch(object):
         """
         vf_to_pci_slot_mapping = {}
         assigned_devices_info = []
+        ls = self.pci_dev_wrapper.link_show()
         for pci_slot, vf_index in self.pci_slot_map.items():
-            if not PciOsWrapper.is_assigned_vf(self.dev_name, vf_index):
+            if not PciOsWrapper.is_assigned_vf(self.dev_name, vf_index, ls):
                 continue
             vf_to_pci_slot_mapping[vf_index] = pci_slot
         if vf_to_pci_slot_mapping:
@@ -230,7 +233,8 @@ class EmbSwitch(object):
         vf_index = self.pci_slot_map.get(pci_slot)
         mac = None
         if vf_index is not None:
-            if PciOsWrapper.is_assigned_vf(self.dev_name, vf_index):
+            ls = pci_lib.PciDeviceIPWrapper.link_show()
+            if PciOsWrapper.is_assigned_vf(self.dev_name, vf_index, ls):
                 macs = self.pci_dev_wrapper.get_assigned_macs([vf_index])
                 mac = macs.get(vf_index)
         return mac

neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py

@@ -186,22 +186,26 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper):
         return vf_details
 
     @classmethod
-    def is_macvtap_assigned(cls, ifname):
+    def link_show(cls):
+        try:
+            out = cls._execute([], "link", ("show", ), run_as_root=True)
+        except Exception as e:
+            LOG.error(_LE("Failed executing ip command: %s"), e)
+            raise exc.IpCommandError(reason=e)
+        return out
+
+    @classmethod
+    def is_macvtap_assigned(cls, ifname, ip_link_show_output):
         """Check if vf has macvtap interface assigned
 
         Parses the output of ip link show command and checks
         if macvtap[0-9]+@<vf interface> regex matches the
         output.
         @param ifname: vf interface name
+        @param ip_link_show_output: 'ip link show' result to parse
         @return: True on match otherwise False
         """
-        try:
+        for line in ip_link_show_output.splitlines():
-            out = cls._execute([], "link", ("show", ), run_as_root=True)
-        except Exception as e:
-            LOG.error(_LE("Failed executing ip command: %s"), e)
-            raise exc.IpCommandError(reason=e)
-
-        for line in out.splitlines():
             pattern_match = cls.MACVTAP_REG_EX.match(line)
             if pattern_match:
                 if ifname == pattern_match.group('vf_interface'):

neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py

@@ -99,7 +99,10 @@ class TestESwitchManagerApi(base.BaseTestCase):
     def test_get_assigned_devices_info(self):
         with mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
                         "eswitch_manager.EmbSwitch.get_assigned_devices_info",
-                        return_value=[(self.ASSIGNED_MAC, self.PCI_SLOT)]):
+                        return_value=[(self.ASSIGNED_MAC, self.PCI_SLOT)]),\
+                mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
+                           "pci_lib.PciDeviceIPWrapper.link_show",
+                           return_value=''):
             result = self.eswitch_mgr.get_assigned_devices_info()
             self.assertIn(self.ASSIGNED_MAC, list(result)[0])
             self.assertIn(self.PCI_SLOT, list(result)[0])
@@ -267,7 +270,10 @@ class TestESwitchManagerApi(base.BaseTestCase):
                 as set_rate_mock, \
                 mock.patch('neutron.plugins.ml2.drivers.mech_sriov.agent.'
                            'pci_lib.PciDeviceIPWrapper.get_assigned_macs',
-                           return_value=mac_address):
+                           return_value=mac_address), \
+                mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
+                           "pci_lib.PciDeviceIPWrapper.link_show",
+                           return_value=''):
             self.eswitch_mgr._clear_rate(pci_slot, rate_type)
             if passed:
                 set_rate_mock.assert_called_once_with(pci_slot, rate_type, 0)
@@ -335,7 +341,10 @@ class TestEmbSwitch(base.BaseTestCase):
                         return_value={0: self.ASSIGNED_MAC}),\
                 mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
                            "eswitch_manager.PciOsWrapper.is_assigned_vf",
-                           return_value=True):
+                           return_value=True), \
+                mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
+                           "pci_lib.PciDeviceIPWrapper.link_show",
+                           return_value=''):
             result = emb_switch.get_assigned_devices_info()
             self.assertIn(self.ASSIGNED_MAC, list(result)[0])
             self.assertIn(self.PCI_SLOT, list(result)[0])
@@ -350,7 +359,10 @@ class TestEmbSwitch(base.BaseTestCase):
                         return_value=self.VF_TO_MAC_MAPPING),\
                 mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
                            "eswitch_manager.PciOsWrapper.is_assigned_vf",
-                           return_value=True):
+                           return_value=True),\
+                mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
+                           "pci_lib.PciDeviceIPWrapper.link_show",
+                           return_value=''):
             devices_info = emb_switch.get_assigned_devices_info()
             for device_info in devices_info:
                 mac = device_info[0]
@@ -361,7 +373,10 @@ class TestEmbSwitch(base.BaseTestCase):
     def test_get_assigned_devices_empty(self):
         with mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
                         "eswitch_manager.PciOsWrapper.is_assigned_vf",
-                        return_value=False):
+                        return_value=False), \
+             mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
+                        "pci_lib.PciDeviceIPWrapper.link_show",
+                        return_value=''):
             result = self.emb_switch.get_assigned_devices_info()
             self.assertFalse(result)
 
@@ -478,7 +493,10 @@ class TestEmbSwitch(base.BaseTestCase):
                         return_value={0: self.ASSIGNED_MAC}),\
                 mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
                            "eswitch_manager.PciOsWrapper.is_assigned_vf",
-                           return_value=True):
+                           return_value=True),\
+                mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
+                           "pci_lib.PciDeviceIPWrapper.link_show",
+                           return_value=''):
             result = self.emb_switch.get_pci_device(self.PCI_SLOT)
             self.assertEqual(self.ASSIGNED_MAC, result)
 
@@ -556,21 +574,21 @@ class TestPciOsWrapper(base.BaseTestCase):
     @mock.patch("os.listdir", side_effect=OSError())
     def test_is_assigned_vf_true(self, *args):
         self.assertTrue(esm.PciOsWrapper.is_assigned_vf(
-            self.DEV_NAME, self.VF_INDEX))
+            self.DEV_NAME, self.VF_INDEX, ''))
 
     @mock.patch("os.listdir", return_value=[DEV_NAME, "eth1"])
     @mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
                 "PciDeviceIPWrapper.is_macvtap_assigned", return_value=False)
     def test_is_assigned_vf_false(self, *args):
         self.assertFalse(esm.PciOsWrapper.is_assigned_vf(
-            self.DEV_NAME, self.VF_INDEX))
+            self.DEV_NAME, self.VF_INDEX, ''))
 
     @mock.patch("os.listdir", return_value=["eth0", "eth1"])
     @mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
                 "PciDeviceIPWrapper.is_macvtap_assigned", return_value=True)
     def test_is_assigned_vf_macvtap(
         self, mock_is_macvtap_assigned, *args):
-        esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, self.VF_INDEX)
+        esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, self.VF_INDEX, '')
         mock_is_macvtap_assigned.called_with(self.VF_INDEX, "eth0")
 
     @mock.patch("os.listdir", side_effect=OSError())
@@ -578,5 +596,5 @@ class TestPciOsWrapper(base.BaseTestCase):
                 "PciDeviceIPWrapper.is_macvtap_assigned")
     def test_is_assigned_vf_macvtap_failure(
         self, mock_is_macvtap_assigned, *args):
-        esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, self.VF_INDEX)
+        esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, self.VF_INDEX, '')
         self.assertFalse(mock_is_macvtap_assigned.called)

neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_pci_lib.py

@@ -176,30 +176,20 @@ class TestPciLib(base.BaseTestCase):
                               state=True)
 
     def test_is_macvtap_assigned(self):
-        with mock.patch.object(pci_lib.PciDeviceIPWrapper,
+        self.assertTrue(pci_lib.PciDeviceIPWrapper.is_macvtap_assigned(
-                               "_execute") as mock_exec:
+            'enp129s0f1', self.IP_LINK_SHOW_WITH_MACVTAP))
-            mock_exec.return_value = self.IP_LINK_SHOW_WITH_MACVTAP
-            self.assertTrue(
-                pci_lib.PciDeviceIPWrapper.is_macvtap_assigned('enp129s0f1'))
 
     def test_is_macvtap_assigned_interface_with_underscore(self):
-        with mock.patch.object(pci_lib.PciDeviceIPWrapper,
+        self.assertTrue(pci_lib.PciDeviceIPWrapper.is_macvtap_assigned(
-                               "_execute") as mock_exec:
+            'p1p2_1', self.IP_LINK_SHOW_WITH_MACVTAP2))
-            mock_exec.return_value = self.IP_LINK_SHOW_WITH_MACVTAP2
-            self.assertTrue(
-                pci_lib.PciDeviceIPWrapper.is_macvtap_assigned('p1p2_1'))
 
     def test_is_macvtap_assigned_not_assigned(self):
-        with mock.patch.object(pci_lib.PciDeviceIPWrapper,
+        self.assertFalse(pci_lib.PciDeviceIPWrapper.is_macvtap_assigned(
-                               "_execute") as mock_exec:
+            'enp129s0f2', self.IP_LINK_SHOW_WITH_MACVTAP))
-            mock_exec.return_value = self.IP_LINK_SHOW_WITH_MACVTAP
-            self.assertFalse(
-                pci_lib.PciDeviceIPWrapper.is_macvtap_assigned('enp129s0f2'))
 
-    def test_is_macvtap_assigned_failed(self):
+    def test_link_show_command_failed(self):
         with mock.patch.object(pci_lib.PciDeviceIPWrapper,
                                "_execute") as mock_exec:
             mock_exec.side_effect = Exception()
             self.assertRaises(exc.IpCommandError,
-                              pci_lib.PciDeviceIPWrapper.is_macvtap_assigned,
+                              pci_lib.PciDeviceIPWrapper.link_show)
-                              'enp129s0f3')