SR-IOV: Fix macvtap assigned vf check when kernel < 3.13

when creating macvtap passthrough to SR-IOV VF in Kernel >= 3.13 an
upper_macvtap symbolic link is created. For Kernel < 3.13 the only
way to know it is by parsing the ip link show output and look for
macvtap[0-9]+@<vf ifname>.

This patch used the ip link show command to detects
macvtap assigned vf so that detection of macvtap assigned vf
will work on all kernels

Closes-Bug: #1523083
Change-Id: Icbc8d6af5c00d1453095e04dd779210dc7244c7f

neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py

@@ -26,7 +26,7 @@ class InvalidDeviceError(SriovNicError):
 
 
 class IpCommandError(SriovNicError):
-    message = _("ip command failed on device %(dev_name)s: %(reason)s")
+    message = _("ip command failed: %(reason)s")
 
 
 class IpCommandOperationNotSupportedError(SriovNicError):
@@ -35,3 +35,7 @@ class IpCommandOperationNotSupportedError(SriovNicError):
 
 class InvalidPciSlotError(SriovNicError):
     message = _("Invalid pci slot %(pci_slot)s")
+
+
+class IpCommandDeviceError(SriovNicError):
+    message = _("ip command failed on device %(dev_name)s: %(reason)s")

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

@@ -13,7 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import glob
 import os
 import re
 
@@ -36,7 +35,6 @@ class PciOsWrapper(object):
     PCI_PATH = "/sys/class/net/%s/device/virtfn%s/net"
     VIRTFN_FORMAT = r"^virtfn(?P<vf_index>\d+)"
     VIRTFN_REG_EX = re.compile(VIRTFN_FORMAT)
-    MAC_VTAP_PREFIX = "upper_macvtap*"
 
     @classmethod
     def scan_vf_devices(cls, dev_name):
@@ -75,15 +73,25 @@ class PciOsWrapper(object):
         by checking the relevant path in the system:
         VF is assigned if:
             Direct VF: PCI_PATH does not exist.
-            Macvtap VF: upper_macvtap path exists.
+            Macvtap VF: macvtap@<vf interface> interface exists in ip link show
         @param dev_name: pf network device name
         @param vf_index: vf index
         """
         path = cls.PCI_PATH % (dev_name, vf_index)
-        if not os.path.isdir(path):
+
+        try:
+            ifname_list = os.listdir(path)
+        except OSError:
+            # PCI_PATH does not exist means that the DIRECT VF assigend
             return True
-        upper_macvtap_path = os.path.join(path, "*", cls.MAC_VTAP_PREFIX)
+
-        return bool(glob.glob(upper_macvtap_path))
+        # Note(moshele) kernel < 3.13 doesn't create symbolic link
+        # 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):
+                return True
+        return False
 
 
 class EmbSwitch(object):

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

@@ -34,9 +34,11 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper):
     MAC_PATTERN = r"MAC\s+(?P<mac>[a-fA-F0-9:]+),"
     STATE_PATTERN = r"\s+link-state\s+(?P<state>\w+)"
     ANY_PATTERN = ".*,"
+    MACVTAP_PATTERN = r".*macvtap[0-9]+@(?P<vf_interface>[a-zA-Z0-9]+):"
 
     VF_LINE_FORMAT = VF_PATTERN + MAC_PATTERN + ANY_PATTERN + STATE_PATTERN
     VF_DETAILS_REG_EX = re.compile(VF_LINE_FORMAT)
+    MACVTAP_REG_EX = re.compile(MACVTAP_PATTERN)
 
     IP_LINK_OP_NOT_SUPPORTED = 'RTNETLINK answers: Operation not supported'
 
@@ -68,7 +70,7 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper):
                 raise exc.IpCommandOperationNotSupportedError(
                     dev_name=self.dev_name)
             else:
-                raise exc.IpCommandError(dev_name=self.dev_name,
+                raise exc.IpCommandDeviceError(dev_name=self.dev_name,
                                                reason=str(e))
 
     def get_assigned_macs(self, vf_list):
@@ -81,7 +83,7 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper):
             out = self._as_root([], "link", ("show", self.dev_name))
         except Exception as e:
             LOG.exception(_LE("Failed executing ip command"))
-            raise exc.IpCommandError(dev_name=self.dev_name,
+            raise exc.IpCommandDeviceError(dev_name=self.dev_name,
                                            reason=e)
         vf_to_mac_mapping = {}
         vf_lines = self._get_vf_link_show(vf_list, out)
@@ -104,7 +106,7 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper):
             out = self._as_root([], "link", ("show", self.dev_name))
         except Exception as e:
             LOG.exception(_LE("Failed executing ip command"))
-            raise exc.IpCommandError(dev_name=self.dev_name,
+            raise exc.IpCommandDeviceError(dev_name=self.dev_name,
                                            reason=e)
         vf_lines = self._get_vf_link_show([vf_index], out)
         if vf_lines:
@@ -181,3 +183,26 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper):
                             "for %(device)s"),
                         {'line': vf_line, 'device': self.dev_name})
         return vf_details
+
+    @classmethod
+    def is_macvtap_assigned(cls, ifname):
+        """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
+        @return: True on match otherwise False
+        """
+        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)
+
+        for line in out.splitlines():
+            pattern_match = cls.MACVTAP_REG_EX.match(line)
+            if pattern_match:
+                if ifname == pattern_match.group('vf_interface'):
+                    return True
+        return False

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

@@ -19,7 +19,6 @@ import os
 import mock
 import testtools
 
-
 from neutron.plugins.ml2.drivers.mech_sriov.agent.common \
     import exceptions as exc
 from neutron.plugins.ml2.drivers.mech_sriov.agent import eswitch_manager as esm
@@ -457,31 +456,30 @@ class TestPciOsWrapper(base.BaseTestCase):
                               esm.PciOsWrapper.scan_vf_devices,
                               self.DEV_NAME)
 
-    def _mock_assign_vf(self, dir_exists):
+    @mock.patch("os.listdir", side_effect=OSError())
-        with mock.patch("os.path.isdir",
+    def test_is_assigned_vf_true(self, *args):
-                        return_value=dir_exists):
+        self.assertTrue(esm.PciOsWrapper.is_assigned_vf(
-            result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME,
+            self.DEV_NAME, self.VF_INDEX))
-                                                     self.VF_INDEX)
-            self.assertEqual(not dir_exists, result)
 
-    def test_is_assigned_vf_true(self):
+    @mock.patch("os.listdir", return_value=[DEV_NAME, "eth1"])
-        self._mock_assign_vf(True)
+    @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))
 
-    def test_is_assigned_vf_false(self):
+    @mock.patch("os.listdir", return_value=["eth0", "eth1"])
-        self._mock_assign_vf(False)
+    @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)
+        mock_is_macvtap_assigned.called_with(self.VF_INDEX, "eth0")
 
-    def _mock_assign_vf_macvtap(self, macvtap_exists):
+    @mock.patch("os.listdir", side_effect=OSError())
-        def _glob(file_path):
+    @mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
-            return ["upper_macvtap0"] if macvtap_exists else []
+                "PciDeviceIPWrapper.is_macvtap_assigned")
-
+    def test_is_assigned_vf_macvtap_failure(
-        with mock.patch("os.path.isdir", return_value=True),\
+        self, mock_is_macvtap_assigned, *args):
-                mock.patch("glob.glob", side_effect=_glob):
+        esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, self.VF_INDEX)
-            result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME,
+        self.assertFalse(mock_is_macvtap_assigned.called)
-                                                     self.VF_INDEX)
-            self.assertEqual(macvtap_exists, result)
-
-    def test_is_assigned_vf_macvtap_true(self):
-        self._mock_assign_vf_macvtap(True)
-
-    def test_is_assigned_vf_macvtap_false(self):
-        self._mock_assign_vf_macvtap(False)

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

@@ -37,6 +37,12 @@ class TestPciLib(base.BaseTestCase):
                       ' checking off, link-state enable')
     VF_LINK_SHOW = '\n'.join((PF_LINK_SHOW, PF_MAC, VF_0_LINK_SHOW,
                               VF_1_LINK_SHOW, VF_2_LINK_SHOW))
+    MACVTAP_LINK_SHOW = ('63: macvtap1@enp129s0f1: <BROADCAST,MULTICAST> mtu '
+                         '1500 qdisc  noop state DOWN mode DEFAULT group '
+                         'default qlen 500 link/ether 4a:9b:6d:de:65:b5 brd '
+                         'ff:ff:ff:ff:ff:ff')
+
+    IP_LINK_SHOW_WITH_MACVTAP = '\n'.join((VF_LINK_SHOW, MACVTAP_LINK_SHOW))
 
     MAC_MAPPING = {
         0: "fa:16:3e:b4:81:ac",
@@ -60,7 +66,7 @@ class TestPciLib(base.BaseTestCase):
         with mock.patch.object(self.pci_wrapper,
                                "_as_root") as mock_as_root:
             mock_as_root.side_effect = Exception()
-            self.assertRaises(exc.IpCommandError,
+            self.assertRaises(exc.IpCommandDeviceError,
                               self.pci_wrapper.get_assigned_macs,
                               [self.VF_INDEX])
 
@@ -82,7 +88,7 @@ class TestPciLib(base.BaseTestCase):
         with mock.patch.object(self.pci_wrapper,
                                "_as_root") as mock_as_root:
             mock_as_root.side_effect = Exception()
-            self.assertRaises(exc.IpCommandError,
+            self.assertRaises(exc.IpCommandDeviceError,
                               self.pci_wrapper.get_vf_state,
                               self.VF_INDEX)
 
@@ -96,7 +102,7 @@ class TestPciLib(base.BaseTestCase):
         with mock.patch.object(self.pci_wrapper,
                                "_as_root") as mock_as_root:
             mock_as_root.side_effect = Exception()
-            self.assertRaises(exc.IpCommandError,
+            self.assertRaises(exc.IpCommandDeviceError,
                               self.pci_wrapper.set_vf_state,
                               self.VF_INDEX,
                               True)
@@ -111,7 +117,7 @@ class TestPciLib(base.BaseTestCase):
         with mock.patch.object(self.pci_wrapper,
                                "_execute") as mock_exec:
             mock_exec.side_effect = Exception()
-            self.assertRaises(exc.IpCommandError,
+            self.assertRaises(exc.IpCommandDeviceError,
                               self.pci_wrapper.set_vf_spoofcheck,
                               self.VF_INDEX,
                               True)
@@ -128,7 +134,7 @@ class TestPciLib(base.BaseTestCase):
         with mock.patch.object(self.pci_wrapper,
                                "_execute") as mock_exec:
             mock_exec.side_effect = Exception()
-            self.assertRaises(exc.IpCommandError,
+            self.assertRaises(exc.IpCommandDeviceError,
                               self.pci_wrapper.set_vf_max_rate,
                               self.VF_INDEX,
                               1000)
@@ -142,3 +148,25 @@ class TestPciLib(base.BaseTestCase):
                               self.pci_wrapper.set_vf_state,
                               self.VF_INDEX,
                               state=True)
+
+    def test_is_macvtap_assigned(self):
+        with mock.patch.object(pci_lib.PciDeviceIPWrapper,
+                               "_execute") as mock_exec:
+            mock_exec.return_value = self.IP_LINK_SHOW_WITH_MACVTAP
+            self.assertTrue(
+                pci_lib.PciDeviceIPWrapper.is_macvtap_assigned('enp129s0f1'))
+
+    def test_is_macvtap_assigned_not_assigned(self):
+        with mock.patch.object(pci_lib.PciDeviceIPWrapper,
+                               "_execute") as mock_exec:
+            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):
+        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,
+                              'enp129s0f3')

releasenotes/notes/macvtap_assigned_vf_check-f4d07660ffd82a24.yaml

@@ -0,0 +1,3 @@
+---
+fixes:
+  - Fix SR-IOV agent macvtap assigned VF check when linux kernel < 3.13