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 fbf8a5492ca..2c960748f46 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py +++ b/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") diff --git a/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py b/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py index dc4c0e72f01..ea0fac3d0be 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py +++ b/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\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@ 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): 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 e5403e2e4bc..307418207dc 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py +++ b/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[a-fA-F0-9:]+)," STATE_PATTERN = r"\s+link-state\s+(?P\w+)" ANY_PATTERN = ".*," + MACVTAP_PATTERN = r".*macvtap[0-9]+@(?P[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,8 +70,8 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): raise exc.IpCommandOperationNotSupportedError( dev_name=self.dev_name) else: - raise exc.IpCommandError(dev_name=self.dev_name, - reason=str(e)) + raise exc.IpCommandDeviceError(dev_name=self.dev_name, + reason=str(e)) def get_assigned_macs(self, vf_list): """Get assigned mac addresses for vf list. @@ -81,8 +83,8 @@ 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, - reason=e) + raise exc.IpCommandDeviceError(dev_name=self.dev_name, + reason=e) vf_to_mac_mapping = {} vf_lines = self._get_vf_link_show(vf_list, out) if vf_lines: @@ -104,8 +106,8 @@ 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, - reason=e) + raise exc.IpCommandDeviceError(dev_name=self.dev_name, + reason=e) vf_lines = self._get_vf_link_show([vf_index], out) if vf_lines: vf_details = self._parse_vf_link_show(vf_lines[0]) @@ -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]+@ 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 diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py index 248426e9ddd..bdaf9f3aa9e 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py +++ b/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): - with mock.patch("os.path.isdir", - return_value=dir_exists): - result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, - self.VF_INDEX) - self.assertEqual(not dir_exists, result) + @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)) - def test_is_assigned_vf_true(self): - self._mock_assign_vf(True) + @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)) - def test_is_assigned_vf_false(self): - self._mock_assign_vf(False) + @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) + mock_is_macvtap_assigned.called_with(self.VF_INDEX, "eth0") - def _mock_assign_vf_macvtap(self, macvtap_exists): - def _glob(file_path): - return ["upper_macvtap0"] if macvtap_exists else [] - - with mock.patch("os.path.isdir", return_value=True),\ - mock.patch("glob.glob", side_effect=_glob): - result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, - 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) + @mock.patch("os.listdir", side_effect=OSError()) + @mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib." + "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) + self.assertFalse(mock_is_macvtap_assigned.called) 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 d6e385be0bb..c6b892e77c5 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 @@ -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: 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') diff --git a/releasenotes/notes/macvtap_assigned_vf_check-f4d07660ffd82a24.yaml b/releasenotes/notes/macvtap_assigned_vf_check-f4d07660ffd82a24.yaml new file mode 100644 index 00000000000..c74a57134ad --- /dev/null +++ b/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