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 37b9935b64f..8b0bf3bc8ba 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/common/exceptions.py @@ -27,7 +27,7 @@ class InvalidDeviceError(SriovNicError): class IpCommandError(SriovNicError): - message = _("ip command failed: %(reason)s") + message = _("ip command failed on device %(dev_name)s: %(reason)s") class IpCommandOperationNotSupportedError(SriovNicError): @@ -36,7 +36,3 @@ 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 8e5a97de13f..42f2a5b3a39 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import glob import os import re @@ -36,6 +37,7 @@ class PciOsWrapper(object): NUMVFS_PATH = "/sys/class/net/%s/device/sriov_numvfs" 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): @@ -67,17 +69,16 @@ class PciOsWrapper(object): return os.path.isdir(cls.DEVICE_PATH % dev_name) @classmethod - def is_assigned_vf(cls, dev_name, vf_index, ip_link_show_output): + def is_assigned_vf(cls, dev_name, vf_index): """Check if VF is assigned. Checks if a given vf index of a given device name is assigned by checking the relevant path in the system: VF is assigned if: Direct VF: PCI_PATH does not exist. - Macvtap VF: macvtap@ interface exists in ip link show + Macvtap VF: upper_macvtap path exists. @param dev_name: pf network device name @param vf_index: vf index - @param ip_link_show_output: 'ip link show' output """ if not cls.pf_device_exists(dev_name): @@ -87,21 +88,10 @@ class PciOsWrapper(object): return False path = cls.PCI_PATH % (dev_name, vf_index) - - try: - ifname_list = os.listdir(path) - except OSError: - # PCI_PATH does not exist means that the DIRECT VF assigned + if not os.path.isdir(path): return True - - # 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, ip_link_show_output): - return True - return False + upper_macvtap_path = os.path.join(path, "*", cls.MAC_VTAP_PREFIX) + return bool(glob.glob(upper_macvtap_path)) @classmethod def get_numvfs(cls, dev_name): @@ -169,9 +159,8 @@ 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, ls): + if not PciOsWrapper.is_assigned_vf(self.dev_name, vf_index): continue vf_to_pci_slot_mapping[vf_index] = pci_slot if vf_to_pci_slot_mapping: @@ -263,8 +252,7 @@ class EmbSwitch(object): vf_index = self.pci_slot_map.get(pci_slot) mac = None if vf_index is not None: - ls = self.pci_dev_wrapper.link_show() - if PciOsWrapper.is_assigned_vf(self.dev_name, vf_index, ls): + if PciOsWrapper.is_assigned_vf(self.dev_name, vf_index): macs = self.pci_dev_wrapper.get_assigned_macs([vf_index]) mac = macs.get(vf_index) return mac 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 f84a4583194..42f875620cf 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/pci_lib.py @@ -39,11 +39,9 @@ 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' @@ -71,8 +69,8 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): raise exc.IpCommandOperationNotSupportedError( dev_name=self.dev_name) else: - raise exc.IpCommandDeviceError(dev_name=self.dev_name, - reason=str(e)) + 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. @@ -84,8 +82,8 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): out = self._as_root([], "link", ("show", self.dev_name)) except Exception as e: LOG.exception("Failed executing ip command") - raise exc.IpCommandDeviceError(dev_name=self.dev_name, - reason=e) + raise exc.IpCommandError(dev_name=self.dev_name, + reason=e) vf_to_mac_mapping = {} vf_lines = self._get_vf_link_show(vf_list, out) if vf_lines: @@ -106,8 +104,8 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): out = self._as_root([], "link", ("show", self.dev_name)) except Exception as e: LOG.exception("Failed executing ip command") - raise exc.IpCommandDeviceError(dev_name=self.dev_name, - reason=e) + raise exc.IpCommandError(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]) @@ -187,29 +185,3 @@ class PciDeviceIPWrapper(ip_lib.IPWrapper): "for %(device)s", {'line': vf_line, 'device': self.dev_name}) return vf_details - - def link_show(self): - try: - out = self._as_root([], "link", ("show", )) - except Exception as e: - LOG.error("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]+@ 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 - """ - for line in ip_link_show_output.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 91497e07988..703493745f3 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 @@ -130,10 +130,7 @@ 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)]),\ - mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "pci_lib.PciDeviceIPWrapper.link_show", - return_value=''): + return_value=[(self.ASSIGNED_MAC, self.PCI_SLOT)]): result = self.eswitch_mgr.get_assigned_devices_info() self.assertIn(self.ASSIGNED_MAC, list(result)[0]) self.assertIn(self.PCI_SLOT, list(result)[0]) @@ -343,9 +340,6 @@ class TestESwitchManagerApi(base.BaseTestCase): mock.patch('neutron.plugins.ml2.drivers.mech_sriov.agent.' 'pci_lib.PciDeviceIPWrapper.get_assigned_macs', return_value=mac_address), \ - mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "pci_lib.PciDeviceIPWrapper.link_show", - return_value=''), \ mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." "eswitch_manager.PciOsWrapper.pf_device_exists", return_value=True): @@ -457,10 +451,7 @@ 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), \ - mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "pci_lib.PciDeviceIPWrapper.link_show", - return_value=''): + return_value=True): result = emb_switch.get_assigned_devices_info() self.assertIn(self.ASSIGNED_MAC, list(result)[0]) self.assertIn(self.PCI_SLOT, list(result)[0]) @@ -475,10 +466,7 @@ 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),\ - mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "pci_lib.PciDeviceIPWrapper.link_show", - return_value=''): + return_value=True): devices_info = emb_switch.get_assigned_devices_info() for device_info in devices_info: mac = device_info[0] @@ -489,10 +477,7 @@ 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), \ - mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "pci_lib.PciDeviceIPWrapper.link_show", - return_value=''): + return_value=False): result = self.emb_switch.get_assigned_devices_info() self.assertFalse(result) @@ -609,10 +594,7 @@ 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),\ - mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "pci_lib.PciDeviceIPWrapper.link_show", - return_value=''): + return_value=True): result = self.emb_switch.get_pci_device(self.PCI_SLOT) self.assertEqual(self.ASSIGNED_MAC, result) @@ -687,44 +669,43 @@ class TestPciOsWrapper(base.BaseTestCase): self.assertEqual([], esm.PciOsWrapper.scan_vf_devices(self.DEV_NAME)) - @mock.patch("os.listdir", side_effect=OSError()) - def test_is_assigned_vf_true(self, *args): - with mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." - "eswitch_manager.PciOsWrapper.pf_device_exists", - return_value=True): - self.assertTrue(esm.PciOsWrapper.is_assigned_vf( - self.DEV_NAME, self.VF_INDEX, '')) + def _mock_assign_vf(self, dir_exists): + with mock.patch("os.path.isdir", + return_value=dir_exists), \ + mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent." + "eswitch_manager.PciOsWrapper.pf_device_exists", + return_value=True): + result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME, + self.VF_INDEX) + self.assertEqual(not dir_exists, result) - @mock.patch("os.path.exists", return_value=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_true(self): + self._mock_assign_vf(True) - @mock.patch("os.path.exists", return_value=True) - @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 test_is_assigned_vf_false(self): + self._mock_assign_vf(False) - @mock.patch("os.path.exists", return_value=True) - @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) + 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.path.exists", return_value=False) @mock.patch("os.listdir", return_value=["eth0", "eth1"]) def test_is_assigned_vf_pf_disappeared(self, list_dir_mock, *args): self.assertFalse(esm.PciOsWrapper.is_assigned_vf( - self.DEV_NAME, self.VF_INDEX, '')) + self.DEV_NAME, self.VF_INDEX)) self.assertFalse(list_dir_mock.called) def test_pf_device_exists_with_no_dir(self): 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 ad958d54822..0cfda31a009 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 @@ -38,17 +38,6 @@ 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') - MACVTAP_LINK_SHOW2 = ('64: macvtap2@p1p2_1: 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)) - IP_LINK_SHOW_WITH_MACVTAP2 = '\n'.join((VF_LINK_SHOW, MACVTAP_LINK_SHOW2)) MAC_MAPPING = { 0: "fa:16:3e:b4:81:ac", @@ -72,7 +61,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.IpCommandDeviceError, + self.assertRaises(exc.IpCommandError, self.pci_wrapper.get_assigned_macs, [self.VF_INDEX]) @@ -94,7 +83,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.IpCommandDeviceError, + self.assertRaises(exc.IpCommandError, self.pci_wrapper.get_vf_state, self.VF_INDEX) @@ -108,7 +97,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.IpCommandDeviceError, + self.assertRaises(exc.IpCommandError, self.pci_wrapper.set_vf_state, self.VF_INDEX, True) @@ -123,7 +112,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.IpCommandDeviceError, + self.assertRaises(exc.IpCommandError, self.pci_wrapper.set_vf_spoofcheck, self.VF_INDEX, True) @@ -143,7 +132,7 @@ class TestPciLib(base.BaseTestCase): else: with mock.patch.object(self.pci_wrapper, "_as_root", side_effect=Exception()): - self.assertRaises(exc.IpCommandDeviceError, + self.assertRaises(exc.IpCommandError, self.pci_wrapper.set_vf_rate, self.VF_INDEX, rate, @@ -174,22 +163,3 @@ class TestPciLib(base.BaseTestCase): self.pci_wrapper.set_vf_state, self.VF_INDEX, state=True) - - def test_is_macvtap_assigned(self): - self.assertTrue(pci_lib.PciDeviceIPWrapper.is_macvtap_assigned( - 'enp129s0f1', self.IP_LINK_SHOW_WITH_MACVTAP)) - - def test_is_macvtap_assigned_interface_with_underscore(self): - self.assertTrue(pci_lib.PciDeviceIPWrapper.is_macvtap_assigned( - 'p1p2_1', self.IP_LINK_SHOW_WITH_MACVTAP2)) - - def test_is_macvtap_assigned_not_assigned(self): - self.assertFalse(pci_lib.PciDeviceIPWrapper.is_macvtap_assigned( - 'enp129s0f2', self.IP_LINK_SHOW_WITH_MACVTAP)) - - def test_link_show_command_failed(self): - with mock.patch.object(pci_lib.PciDeviceIPWrapper, - "_as_root") as mock_as_root: - mock_as_root.side_effect = Exception() - self.assertRaises(exc.IpCommandError, - self.pci_wrapper.link_show) diff --git a/releasenotes/notes/sriov-agent-kernel-3.13-removed-support-8bb00902dd607746.yaml b/releasenotes/notes/sriov-agent-kernel-3.13-removed-support-8bb00902dd607746.yaml new file mode 100644 index 00000000000..d6be602bf89 --- /dev/null +++ b/releasenotes/notes/sriov-agent-kernel-3.13-removed-support-8bb00902dd607746.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + SR-IOV agent code no longer supports old kernels (<3.13) for MacVtap + ports. This change is not expected to affect existing deployments since + most OS distributions already have the relevant kernel patches. + In addition, latest major release of all Supported distributions already + have a newer kernel.