From b987bf5f1eb65fe8534d01b9de2a7b74fc16f84d Mon Sep 17 00:00:00 2001 From: Dino Madarang Date: Wed, 1 Nov 2017 09:10:16 -0700 Subject: NSB: Fix standalone.model.Libvirt SR-IOV modeling Fixed standalone.model.Libvirt SR-IOV XML interface modeling, acording to [1]: - All PCI attributes now are printed in hexadecimal format. - The PCI address is now added in the correct section, 'interface'. network_services.utils.PciAddress was refactored to accept both 'domain: bus:slot:function' and 'bus:slot:function' format inputs. This class is used as input in the previous class, Libvirt, to print in XML the PCI address of a SR-IOV interface. network_services.utils.PciAddress.parse_address is now deprecated. Instead the class standard instantiation must be used: libvirt_obj = utils.PciAddress(text_with_address) A deprecation decorator is implemented along with this patch. This decorator is used for the first time in the previously mentioned function. This decorator stores every decorated function name and deprecation message and raises a logging warning message the first time this function is used. [1] https://goo.gl/so2Mrp Change-Id: I22e95c488e27d6e2a8fdf6c1a07faab275fa6bba Signed-off-by: Dino Simeon Madarang Reviewed-by: Alain Jebara Reviewed-by: Deepak S Reviewed-by: Ross Brattain Reviewed-by: Rodolfo Alonso Hernandez (cherry picked from commit 6cfec77db6b95af5b31b741d513955ee3dfa3bb2) --- .../benchmark/contexts/standalone/test_model.py | 261 ++++++++++++++------- .../benchmark/contexts/standalone/test_sriov.py | 56 ++--- tests/unit/network_services/test_utils.py | 83 ++++++- yardstick/benchmark/contexts/standalone/model.py | 105 ++++++--- .../benchmark/contexts/standalone/ovs_dpdk.py | 19 +- yardstick/benchmark/contexts/standalone/sriov.py | 21 +- yardstick/network_services/utils.py | 52 ++-- 7 files changed, 407 insertions(+), 190 deletions(-) diff --git a/tests/unit/benchmark/contexts/standalone/test_model.py b/tests/unit/benchmark/contexts/standalone/test_model.py index 6899a0af6..31ec2b7d1 100644 --- a/tests/unit/benchmark/contexts/standalone/test_model.py +++ b/tests/unit/benchmark/contexts/standalone/test_model.py @@ -16,70 +16,137 @@ # Unittest for yardstick.benchmark.contexts.standalone.model - from __future__ import absolute_import +import copy import os import unittest -import errno import mock -from yardstick.common import constants as consts -from yardstick.benchmark.contexts.standalone.model import Libvirt -from yardstick.benchmark.contexts.standalone.model import StandaloneContextHelper -from yardstick.benchmark.contexts.standalone import model -from yardstick.network_services.utils import PciAddress +from xml.etree import ElementTree +from yardstick.benchmark.contexts.standalone import model +from yardstick.network_services import utils +from yardstick.network_services.helpers import cpu + + +XML_SAMPLE = """ + + + + +""" + +XML_SAMPLE_INTERFACE = """ + + + + + + +""" class ModelLibvirtTestCase(unittest.TestCase): + def setUp(self): + self.xml = ElementTree.ElementTree( + element=ElementTree.fromstring(XML_SAMPLE)) + self.pci_address_str = '0001:04:03.2' + self.pci_address = utils.PciAddress(self.pci_address_str) + self.mac = '00:00:00:00:00:01' + self._mock_write_xml = mock.patch.object(ElementTree.ElementTree, + 'write') + self.mock_write_xml = self._mock_write_xml.start() + + self.addCleanup(self._cleanup) + + def _cleanup(self): + self._mock_write_xml.stop() + def test_check_if_vm_exists_and_delete(self): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) + ssh_mock.execute = mock.Mock(return_value=(0, "a", "")) ssh.return_value = ssh_mock - result = Libvirt.check_if_vm_exists_and_delete("vm_0", ssh_mock) - self.assertIsNone(result) + # NOTE(ralonsoh): this test doesn't cover function execution. + model.Libvirt.check_if_vm_exists_and_delete("vm_0", ssh_mock) def test_virsh_create_vm(self): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) + ssh_mock.execute = mock.Mock(return_value=(0, "a", "")) ssh.return_value = ssh_mock - result = Libvirt.virsh_create_vm(ssh_mock, "vm_0") - self.assertIsNone(result) + # NOTE(ralonsoh): this test doesn't cover function execution. + model.Libvirt.virsh_create_vm(ssh_mock, "vm_0") def test_virsh_destroy_vm(self): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) + ssh_mock.execute = mock.Mock(return_value=(0, "a", "")) ssh.return_value = ssh_mock - result = Libvirt.virsh_destroy_vm("vm_0", ssh_mock) - self.assertIsNone(result) - - @mock.patch('yardstick.benchmark.contexts.standalone.model.ET') - def test_add_interface_address(self, mock_et): - pci_address = PciAddress.parse_address("0000:00:04.0", multi_line=True) - result = Libvirt.add_interface_address("", pci_address) - self.assertIsNotNone(result) - - @mock.patch('yardstick.benchmark.contexts.standalone.model.Libvirt.add_interface_address') - @mock.patch('yardstick.benchmark.contexts.standalone.model.ET') - def test_add_ovs_interfaces(self, mock_et, mock_add_interface_address): - pci_address = PciAddress.parse_address("0000:00:04.0", multi_line=True) - result = Libvirt.add_ovs_interface("/usr/local", 0, "0000:00:04.0", - "00:00:00:00:00:01", "xml") - self.assertIsNone(result) - - @mock.patch('yardstick.benchmark.contexts.standalone.model.Libvirt.add_interface_address') - @mock.patch('yardstick.benchmark.contexts.standalone.model.ET') - def test_add_sriov_interfaces(self, mock_et, mock_add_interface_address): - pci_address = PciAddress.parse_address("0000:00:04.0", multi_line=True) - result = Libvirt.add_sriov_interfaces("0000:00:05.0", "0000:00:04.0", - "00:00:00:00:00:01", "xml") - self.assertIsNone(result) + # NOTE(ralonsoh): this test doesn't cover function execution. + model.Libvirt.virsh_destroy_vm("vm_0", ssh_mock) + + def test_add_interface_address(self): + xml = ElementTree.ElementTree( + element=ElementTree.fromstring(XML_SAMPLE_INTERFACE)) + interface = xml.find('devices').find('interface') + result = model.Libvirt._add_interface_address(interface, self.pci_address) + self.assertEqual('pci', result.get('type')) + self.assertEqual('0x{}'.format(self.pci_address.domain), + result.get('domain')) + self.assertEqual('0x{}'.format(self.pci_address.bus), + result.get('bus')) + self.assertEqual('0x{}'.format(self.pci_address.slot), + result.get('slot')) + self.assertEqual('0x{}'.format(self.pci_address.function), + result.get('function')) + + def test_add_ovs_interfaces(self): + xml_input = mock.Mock() + with mock.patch.object(ElementTree, 'parse', return_value=self.xml) \ + as mock_parse: + xml = copy.deepcopy(self.xml) + mock_parse.return_value = xml + model.Libvirt.add_ovs_interface( + '/usr/local', 0, self.pci_address_str, self.mac, xml_input) + mock_parse.assert_called_once_with(xml_input) + self.mock_write_xml.assert_called_once_with(xml_input) + interface = xml.find('devices').find('interface') + self.assertEqual('vhostuser', interface.get('type')) + mac = interface.find('mac') + self.assertEqual(self.mac, mac.get('address')) + source = interface.find('source') + self.assertEqual('unix', source.get('type')) + self.assertEqual('/usr/local/var/run/openvswitch/dpdkvhostuser0', + source.get('path')) + self.assertEqual('client', source.get('mode')) + _model = interface.find('model') + self.assertEqual('virtio', _model.get('type')) + driver = interface.find('driver') + self.assertEqual('4', driver.get('queues')) + host = driver.find('host') + self.assertEqual('off', host.get('mrg_rxbuf')) + self.assertIsNotNone(interface.find('address')) + + def test_add_sriov_interfaces(self): + xml_input = mock.Mock() + with mock.patch.object(ElementTree, 'parse', return_value=self.xml) \ + as mock_parse: + xml = copy.deepcopy(self.xml) + mock_parse.return_value = xml + vf_pci = '0001:05:04.2' + model.Libvirt.add_sriov_interfaces( + self.pci_address_str, vf_pci, self.mac, xml_input) + mock_parse.assert_called_once_with(xml_input) + self.mock_write_xml.assert_called_once_with(xml_input) + interface = xml.find('devices').find('interface') + self.assertEqual('yes', interface.get('managed')) + self.assertEqual('hostdev', interface.get('type')) + mac = interface.find('mac') + self.assertEqual(self.mac, mac.get('address')) + source = interface.find('source') + self.assertIsNotNone(source.find('address')) + self.assertIsNotNone(interface.find('address')) def test_create_snapshot_qemu(self): result = "/var/lib/libvirt/images/0.qcow2" @@ -88,15 +155,15 @@ class ModelLibvirtTestCase(unittest.TestCase): ssh_mock.execute = \ mock.Mock(return_value=(0, "a", "")) ssh.return_value = ssh_mock - image = Libvirt.create_snapshot_qemu(ssh_mock, "0", "ubuntu.img") + image = model.Libvirt.create_snapshot_qemu(ssh_mock, "0", "ubuntu.img") self.assertEqual(image, result) - @mock.patch("yardstick.benchmark.contexts.standalone.model.Libvirt.pin_vcpu_for_perf") - @mock.patch("yardstick.benchmark.contexts.standalone.model.Libvirt.create_snapshot_qemu") - @mock.patch('yardstick.benchmark.contexts.standalone.model.open') - @mock.patch('yardstick.benchmark.contexts.standalone.model.write_file') - def test_build_vm_xml(self, mock_open, mock_write_file, mock_create_snapshot_qemu, - mock_pin_vcpu_for_perf): + @mock.patch.object(model.Libvirt, 'pin_vcpu_for_perf') + @mock.patch.object(model.Libvirt, 'create_snapshot_qemu') + def test_build_vm_xml(self, mock_create_snapshot_qemu, + *args): + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. result = [4] with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) @@ -105,7 +172,7 @@ class ModelLibvirtTestCase(unittest.TestCase): ssh.return_value = ssh_mock mock_create_snapshot_qemu.return_value = "0.img" - status = Libvirt.build_vm_xml(ssh_mock, {}, "test", "vm_0", 0) + status = model.Libvirt.build_vm_xml(ssh_mock, {}, "test", "vm_0", 0) self.assertEqual(status[0], result[0]) def test_update_interrupts_hugepages_perf(self): @@ -114,19 +181,21 @@ class ModelLibvirtTestCase(unittest.TestCase): ssh_mock.execute = \ mock.Mock(return_value=(0, "a", "")) ssh.return_value = ssh_mock - status = Libvirt.update_interrupts_hugepages_perf(ssh_mock) - self.assertIsNone(status) + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. + model.Libvirt.update_interrupts_hugepages_perf(ssh_mock) + + @mock.patch.object(cpu.CpuSysCores, 'get_core_socket') + def test_pin_vcpu_for_perf(self, mock_get_core_socket): + mock_get_core_socket.return_value = { + 'cores_per_socket': 1, + 'thread_per_core': 1, + '0': [1, 2] + } + # NOTE(ralonsoh): this test doesn't cover function execution. This + # function needs more tests. + model.Libvirt.pin_vcpu_for_perf(mock.Mock()) - @mock.patch("yardstick.benchmark.contexts.standalone.model.CpuSysCores") - @mock.patch("yardstick.benchmark.contexts.standalone.model.Libvirt.update_interrupts_hugepages_perf") - def test_pin_vcpu_for_perf(self, mock_update_interrupts_hugepages_perf, mock_CpuSysCores): - with mock.patch("yardstick.ssh.SSH") as ssh: - ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) - ssh.return_value = ssh_mock - status = Libvirt.pin_vcpu_for_perf(ssh_mock, "vm_0", 4) - self.assertIsNotNone(status) class StandaloneContextHelperTestCase(unittest.TestCase): @@ -148,7 +217,7 @@ class StandaloneContextHelperTestCase(unittest.TestCase): } def setUp(self): - self.helper = StandaloneContextHelper() + self.helper = model.StandaloneContextHelper() def test___init__(self): self.assertIsNone(self.helper.file_path) @@ -159,8 +228,9 @@ class StandaloneContextHelperTestCase(unittest.TestCase): ssh_mock.execute = \ mock.Mock(return_value=(1, "a", "")) ssh.return_value = ssh_mock - status = StandaloneContextHelper.install_req_libs(ssh_mock) - self.assertIsNone(status) + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. + model.StandaloneContextHelper.install_req_libs(ssh_mock) def test_get_kernel_module(self): with mock.patch("yardstick.ssh.SSH") as ssh: @@ -168,10 +238,12 @@ class StandaloneContextHelperTestCase(unittest.TestCase): ssh_mock.execute = \ mock.Mock(return_value=(1, "i40e", "")) ssh.return_value = ssh_mock - status = StandaloneContextHelper.get_kernel_module(ssh_mock, "05:00.0", None) - self.assertEqual(status, "i40e") + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. + model.StandaloneContextHelper.get_kernel_module( + ssh_mock, "05:00.0", None) - @mock.patch('yardstick.benchmark.contexts.standalone.model.StandaloneContextHelper.get_kernel_module') + @mock.patch.object(model.StandaloneContextHelper, 'get_kernel_module') def test_get_nic_details(self, mock_get_kernel_module): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) @@ -179,8 +251,10 @@ class StandaloneContextHelperTestCase(unittest.TestCase): mock.Mock(return_value=(1, "i40e ixgbe", "")) ssh.return_value = ssh_mock mock_get_kernel_module.return_value = "i40e" - status = StandaloneContextHelper.get_nic_details(ssh_mock, self.NETWORKS, "dpdk-devbind.py") - self.assertIsNotNone(status) + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. + model.StandaloneContextHelper.get_nic_details( + ssh_mock, self.NETWORKS, 'dpdk-devbind.py') def test_get_virtual_devices(self): pattern = "PCI_SLOT_NAME=0000:05:00.0" @@ -189,8 +263,10 @@ class StandaloneContextHelperTestCase(unittest.TestCase): ssh_mock.execute = \ mock.Mock(return_value=(1, pattern, "")) ssh.return_value = ssh_mock - status = StandaloneContextHelper.get_virtual_devices(ssh_mock, "0000:00:05.0") - self.assertIsNotNone(status) + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. + model.StandaloneContextHelper.get_virtual_devices( + ssh_mock, '0000:00:05.0') def _get_file_abspath(self, filename): curr_path = os.path.dirname(os.path.abspath(__file__)) @@ -204,40 +280,48 @@ class StandaloneContextHelperTestCase(unittest.TestCase): def test_parse_pod_file(self): self.helper.file_path = self._get_file_abspath("dummy") - self.assertRaises(IOError, self.helper.parse_pod_file, self.helper.file_path) + self.assertRaises(IOError, self.helper.parse_pod_file, + self.helper.file_path) self.helper.file_path = self._get_file_abspath(self.NODE_SAMPLE) - self.assertRaises(TypeError, self.helper.parse_pod_file, self.helper.file_path) + self.assertRaises(TypeError, self.helper.parse_pod_file, + self.helper.file_path) self.helper.file_path = self._get_file_abspath(self.NODE_SRIOV_SAMPLE) self.assertIsNotNone(self.helper.parse_pod_file(self.helper.file_path)) def test_get_mac_address(self): - status = StandaloneContextHelper.get_mac_address() + status = model.StandaloneContextHelper.get_mac_address() self.assertIsNotNone(status) @mock.patch('yardstick.ssh.SSH') - def test_get_mgmt_ip(self, mock_ssh): + def test_get_mgmt_ip(self, *args): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(1, "1.2.3.4 00:00:00:00:00:01", "")) + ssh_mock.execute = mock.Mock( + return_value=(1, "1.2.3.4 00:00:00:00:00:01", "")) ssh.return_value = ssh_mock - status = StandaloneContextHelper.get_mgmt_ip(ssh_mock, "00:00:00:00:00:01", "1.1.1.1/24", {}) + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. + status = model.StandaloneContextHelper.get_mgmt_ip( + ssh_mock, "00:00:00:00:00:01", "1.1.1.1/24", {}) self.assertIsNotNone(status) @mock.patch('yardstick.ssh.SSH') - def test_get_mgmt_ip_no(self, mock_ssh): + def test_get_mgmt_ip_no(self, *args): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) ssh_mock.execute = \ mock.Mock(return_value=(1, "", "")) ssh.return_value = ssh_mock - + # NOTE(ralonsoh): this test doesn't cover function execution. This test + # should also check mocked function calls. model.WAIT_FOR_BOOT = 0 - status = StandaloneContextHelper.get_mgmt_ip(ssh_mock, "99", "1.1.1.1/24", {}) + status = model.StandaloneContextHelper.get_mgmt_ip( + ssh_mock, "99", "1.1.1.1/24", {}) self.assertIsNone(status) + class ServerTestCase(unittest.TestCase): NETWORKS = { @@ -257,6 +341,7 @@ class ServerTestCase(unittest.TestCase): 'cidr': '152.16.40.10/24', 'gateway_ip': '152.16.100.20'} } + def setUp(self): self.server = model.Server() @@ -282,7 +367,8 @@ class ServerTestCase(unittest.TestCase): 'xe1': ['public_0'], } } - status = self.server.generate_vnf_instance({}, self.NETWORKS, "1.1.1.1/24", 'vm_0', vnf, '00:00:00:00:00:01') + status = self.server.generate_vnf_instance( + {}, self.NETWORKS, '1.1.1.1/24', 'vm_0', vnf, '00:00:00:00:00:01') self.assertIsNotNone(status) class OvsDeployTestCase(unittest.TestCase): @@ -312,14 +398,17 @@ class OvsDeployTestCase(unittest.TestCase): self.assertIsNotNone(self.ovs_deploy.connection) @mock.patch('yardstick.benchmark.contexts.standalone.model.os') - def test_prerequisite(self, mock_ssh): + def test_prerequisite(self, *args): + # NOTE(ralonsoh): this test should check mocked function calls. self.ovs_deploy.helper = mock.Mock() self.assertIsNone(self.ovs_deploy.prerequisite()) @mock.patch('yardstick.benchmark.contexts.standalone.model.os') - def test_prerequisite(self, mock_ssh): + def test_prerequisite_2(self, *args): + # NOTE(ralonsoh): this test should check mocked function calls. Rename + # this test properly. self.ovs_deploy.helper = mock.Mock() - self.ovs_deploy.connection.execute = \ - mock.Mock(return_value=(1, "1.2.3.4 00:00:00:00:00:01", "")) + self.ovs_deploy.connection.execute = mock.Mock( + return_value=(1, '1.2.3.4 00:00:00:00:00:01', '')) self.ovs_deploy.prerequisite = mock.Mock() self.assertIsNone(self.ovs_deploy.ovs_deploy()) diff --git a/tests/unit/benchmark/contexts/standalone/test_sriov.py b/tests/unit/benchmark/contexts/standalone/test_sriov.py index 50ae5fe13..3ea673abc 100644 --- a/tests/unit/benchmark/contexts/standalone/test_sriov.py +++ b/tests/unit/benchmark/contexts/standalone/test_sriov.py @@ -17,12 +17,10 @@ from __future__ import absolute_import import os import unittest -import errno import mock -from yardstick.common import constants as consts +from yardstick import ssh from yardstick.benchmark.contexts.standalone import sriov -from yardstick.network_services.utils import PciAddress class SriovContextTestCase(unittest.TestCase): @@ -66,6 +64,9 @@ class SriovContextTestCase(unittest.TestCase): @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt') @mock.patch('yardstick.benchmark.contexts.standalone.model.Server') def test___init__(self, mock_helper, mock_libvirt, mock_server): + # pylint: disable=unused-argument + # NOTE(ralonsoh): this test doesn't cover function execution. + # The pylint exception should be removed. self.sriov.helper = mock_helper self.sriov.vnf_node = mock_server self.assertIsNone(self.sriov.file_path) @@ -75,14 +76,11 @@ class SriovContextTestCase(unittest.TestCase): self.sriov.helper.parse_pod_file = mock.Mock(return_value=[{}, {}, {}]) self.assertIsNone(self.sriov.init(self.ATTRS)) - @mock.patch('yardstick.ssh.SSH') + @mock.patch.object(ssh, 'SSH', return_value=(0, "a", "")) def test_deploy(self, mock_ssh): - with mock.patch("yardstick.ssh.SSH") as ssh: - ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) - ssh.return_value = ssh_mock - + # pylint: disable=unused-argument + # NOTE(ralonsoh): this test doesn't cover function execution. + # The pylint exception should be removed. self.sriov.vm_deploy = False self.assertIsNone(self.sriov.deploy()) @@ -94,20 +92,16 @@ class SriovContextTestCase(unittest.TestCase): self.sriov.wait_for_vnfs_to_start = mock.Mock(return_value={}) self.assertIsNone(self.sriov.deploy()) - @mock.patch('yardstick.ssh.SSH') + @mock.patch.object(ssh, 'SSH', return_value=(0, "a", "")) @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt') def test_undeploy(self, mock_libvirt, mock_ssh): - with mock.patch("yardstick.ssh.SSH") as ssh: - ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) - ssh.return_value = ssh_mock - + # pylint: disable=unused-argument + # NOTE(ralonsoh): the pylint exception should be removed. self.sriov.vm_deploy = False self.assertIsNone(self.sriov.undeploy()) self.sriov.vm_deploy = True - self.sriov.connection = ssh_mock + self.sriov.connection = mock_ssh self.sriov.vm_names = ['vm_0', 'vm_1'] self.sriov.drivers = ['vm_0', 'vm_1'] self.assertIsNone(self.sriov.undeploy()) @@ -246,27 +240,27 @@ class SriovContextTestCase(unittest.TestCase): self.sriov.drivers = [] self.sriov.networks = self.NETWORKS self.sriov.helper.get_mac_address = mock.Mock(return_value="") - self.sriov.get_vf_data = mock.Mock(return_value="") + self.sriov._get_vf_data = mock.Mock(return_value="") self.assertIsNone(self.sriov.configure_nics_for_sriov()) + @mock.patch.object(ssh, 'SSH', return_value=(0, "a", "")) @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt') - def test__enable_interfaces(self, mock_libvirt): - with mock.patch("yardstick.ssh.SSH") as ssh: - ssh_mock = mock.Mock(autospec=ssh.SSH) - ssh_mock.execute = \ - mock.Mock(return_value=(0, "a", "")) - ssh.return_value = ssh_mock + def test__enable_interfaces(self, mock_libvirt, mock_ssh): + # pylint: disable=unused-argument + # NOTE(ralonsoh): the pylint exception should be removed. self.sriov.vm_deploy = True - self.sriov.connection = ssh_mock + self.sriov.connection = mock_ssh self.sriov.vm_names = ['vm_0', 'vm_1'] self.sriov.drivers = [] self.sriov.networks = self.NETWORKS - self.sriov.get_vf_data = mock.Mock(return_value="") + self.sriov._get_vf_data = mock.Mock(return_value="") self.assertIsNone(self.sriov._enable_interfaces(0, 0, ["private_0"], 'test')) @mock.patch('yardstick.benchmark.contexts.standalone.model.Server') @mock.patch('yardstick.benchmark.contexts.standalone.sriov.Libvirt') def test_setup_sriov_context(self, mock_libvirt, mock_server): + # pylint: disable=unused-argument + # NOTE(ralonsoh): the pylint exception should be removed. with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) ssh_mock.execute = \ @@ -296,7 +290,7 @@ class SriovContextTestCase(unittest.TestCase): self.sriov.vnf_node.generate_vnf_instance = mock.Mock(return_value={}) self.assertIsNotNone(self.sriov.setup_sriov_context()) - def test_get_vf_data(self): + def test__get_vf_data(self): with mock.patch("yardstick.ssh.SSH") as ssh: ssh_mock = mock.Mock(autospec=ssh.SSH) ssh_mock.execute = \ @@ -318,5 +312,7 @@ class SriovContextTestCase(unittest.TestCase): } } self.sriov.networks = self.NETWORKS - self.sriov.helper.get_virtual_devices = mock.Mock(return_value={"0000:00:01.0": ""}) - self.assertIsNotNone(self.sriov.get_vf_data("", "0000:00:01.0", "00:00:00:00:00:01", "if0")) + self.sriov.helper.get_virtual_devices = mock.Mock( + return_value={'0000:00:01.0': ''}) + self.assertIsNotNone(self.sriov._get_vf_data( + '0000:00:01.0', '00:00:00:00:00:01', 'if0')) diff --git a/tests/unit/network_services/test_utils.py b/tests/unit/network_services/test_utils.py index 993adbeae..bf98a4474 100644 --- a/tests/unit/network_services/test_utils.py +++ b/tests/unit/network_services/test_utils.py @@ -15,8 +15,6 @@ # Unittest for yardstick.network_services.utils -from __future__ import absolute_import - import os import unittest import mock @@ -62,3 +60,84 @@ class UtilsTestCase(unittest.TestCase): ssh.return_value = ssh_mock tool_path = utils.provision_tool(ssh_mock, self.DPDK_PATH) self.assertEqual(tool_path, self.DPDK_PATH) + + +class PciAddressTestCase(unittest.TestCase): + + PCI_ADDRESS_DBSF = '000A:07:03.2' + PCI_ADDRESS_BSF = '06:02.1' + PCI_ADDRESS_DBSF_MULTILINE_1 = '0001:08:04.3\nother text\n' + PCI_ADDRESS_DBSF_MULTILINE_2 = 'first line\n 0001:08:04.3 \nother text\n' + # Will match and return the first address found. + PCI_ADDRESS_DBSF_MULTILINE_3 = ' 0001:08:04.1 \n 05:03.1 \nother\n' + PCI_ADDRESS_BSF_MULTILINE_1 = 'first line\n 08:04.3 \n 0002:05:03.1\n' + BAD_INPUT_1 = 'no address found' + BAD_INPUT_2 = '001:08:04.1' + BAD_INPUT_3 = '08:4.1' + + def test_pciaddress_dbsf(self): + pci_address = utils.PciAddress(PciAddressTestCase.PCI_ADDRESS_DBSF) + self.assertEqual('000a', pci_address.domain) + self.assertEqual('07', pci_address.bus) + self.assertEqual('03', pci_address.slot) + self.assertEqual('2', pci_address.function) + + def test_pciaddress_bsf(self): + pci_address = utils.PciAddress(PciAddressTestCase.PCI_ADDRESS_BSF) + self.assertEqual('0000', pci_address.domain) + self.assertEqual('06', pci_address.bus) + self.assertEqual('02', pci_address.slot) + self.assertEqual('1', pci_address.function) + + def test_pciaddress_dbsf_multiline_1(self): + pci_address = utils.PciAddress( + PciAddressTestCase.PCI_ADDRESS_DBSF_MULTILINE_1) + self.assertEqual('0001', pci_address.domain) + self.assertEqual('08', pci_address.bus) + self.assertEqual('04', pci_address.slot) + self.assertEqual('3', pci_address.function) + + def test_pciaddress_dbsf_multiline_2(self): + pci_address = utils.PciAddress( + PciAddressTestCase.PCI_ADDRESS_DBSF_MULTILINE_2) + self.assertEqual('0001', pci_address.domain) + self.assertEqual('08', pci_address.bus) + self.assertEqual('04', pci_address.slot) + self.assertEqual('3', pci_address.function) + + def test_pciaddress_dbsf_multiline_3(self): + pci_address = utils.PciAddress( + PciAddressTestCase.PCI_ADDRESS_DBSF_MULTILINE_3) + self.assertEqual('0001', pci_address.domain) + self.assertEqual('08', pci_address.bus) + self.assertEqual('04', pci_address.slot) + self.assertEqual('1', pci_address.function) + + def test_pciaddress_bsf_multiline_1(self): + pci_address = utils.PciAddress( + PciAddressTestCase.PCI_ADDRESS_BSF_MULTILINE_1) + self.assertEqual('0000', pci_address.domain) + self.assertEqual('08', pci_address.bus) + self.assertEqual('04', pci_address.slot) + self.assertEqual('3', pci_address.function) + + def test_pciaddress_bad_input_no_address(self): + with self.assertRaises(ValueError) as exception: + utils.PciAddress(PciAddressTestCase.BAD_INPUT_1) + self.assertEqual('Invalid PCI address: {}'.format( + PciAddressTestCase.BAD_INPUT_1), str(exception.exception)) + + def test_pciaddress_bad_input_dbsf_bad_formatted(self): + # In this test case, the domain has only 3 characters instead of 4. + pci_address = utils.PciAddress( + PciAddressTestCase.BAD_INPUT_2) + self.assertEqual('0000', pci_address.domain) + self.assertEqual('08', pci_address.bus) + self.assertEqual('04', pci_address.slot) + self.assertEqual('1', pci_address.function) + + def test_pciaddress_bad_input_bsf_bad_formatted(self): + with self.assertRaises(ValueError) as exception: + utils.PciAddress(PciAddressTestCase.BAD_INPUT_3) + self.assertEqual('Invalid PCI address: {}'.format( + PciAddressTestCase.BAD_INPUT_3), str(exception.exception)) diff --git a/yardstick/benchmark/contexts/standalone/model.py b/yardstick/benchmark/contexts/standalone/model.py index 45ae3c18a..a8943c3f6 100644 --- a/yardstick/benchmark/contexts/standalone/model.py +++ b/yardstick/benchmark/contexts/standalone/model.py @@ -94,33 +94,63 @@ class Libvirt(object): cmd_template = "virsh list --name | grep -i %s" status = connection.execute(cmd_template % vm_name)[0] if status == 0: - LOG.info("VM '%s' is already present.. destroying" % vm_name) + LOG.info("VM '%s' is already present... destroying", vm_name) connection.execute("virsh destroy %s" % vm_name) @staticmethod def virsh_create_vm(connection, cfg): err = connection.execute("virsh create %s" % cfg)[0] - LOG.info("VM create status: %s" % (err)) + LOG.info("VM create status: %s", err) @staticmethod def virsh_destroy_vm(vm_name, connection): connection.execute("virsh destroy %s" % vm_name) @staticmethod - def add_interface_address(interface, pci_address): + def _add_interface_address(interface, pci_address): + """Add a PCI 'address' XML node + +
+ + Refence: https://software.intel.com/en-us/articles/ + configure-sr-iov-network-virtual-functions-in-linux-kvm + """ vm_pci = ET.SubElement(interface, 'address') vm_pci.set('type', 'pci') - vm_pci.set('domain', '0x%s' % pci_address.domain) - vm_pci.set('bus', '0x%s' % pci_address.bus) - vm_pci.set('slot', '0x%s' % pci_address.slot) - vm_pci.set('function', '0x%s' % pci_address.function) + vm_pci.set('domain', '0x{}'.format(pci_address.domain)) + vm_pci.set('bus', '0x{}'.format(pci_address.bus)) + vm_pci.set('slot', '0x{}'.format(pci_address.slot)) + vm_pci.set('function', '0x{}'.format(pci_address.function)) return vm_pci @classmethod def add_ovs_interface(cls, vpath, port_num, vpci, vports_mac, xml): - vhost_path = '{0}/var/run/openvswitch/dpdkvhostuser{1}' + """Add a DPDK OVS 'interface' XML node in 'devices' node + + + + + + + + + +
+ + ... + + + Reference: http://docs.openvswitch.org/en/latest/topics/dpdk/ + vhost-user/ + """ + + vhost_path = ('{0}/var/run/openvswitch/dpdkvhostuser{1}'. + format(vpath, port_num)) root = ET.parse(xml) - pci_address = PciAddress.parse_address(vpci.strip(), multi_line=True) + pci_address = PciAddress(vpci.strip()) device = root.find('devices') interface = ET.SubElement(device, 'interface') @@ -130,7 +160,7 @@ class Libvirt(object): source = ET.SubElement(interface, 'source') source.set('type', 'unix') - source.set('path', vhost_path.format(vpath, port_num)) + source.set('path', vhost_path) source.set('mode', 'client') model = ET.SubElement(interface, 'model') @@ -142,14 +172,35 @@ class Libvirt(object): host = ET.SubElement(driver, 'host') host.set('mrg_rxbuf', 'off') - cls.add_interface_address(interface, pci_address) + cls._add_interface_address(interface, pci_address) root.write(xml) @classmethod - def add_sriov_interfaces(cls, vm_pci, vf_pci, vfmac, xml): + def add_sriov_interfaces(cls, vm_pci, vf_pci, vf_mac, xml): + """Add a SR-IOV 'interface' XML node in 'devices' node + + + + +
+ + +
+ + ... + + + Reference: https://access.redhat.com/documentation/en-us/ + red_hat_enterprise_linux/6/html/ + virtualization_host_configuration_and_guest_installation_guide/ + sect-virtualization_host_configuration_and_guest_installation_guide + -sr_iov-how_sr_iov_libvirt_works + """ + root = ET.parse(xml) - pci_address = PciAddress.parse_address(vf_pci.strip(), multi_line=True) device = root.find('devices') interface = ET.SubElement(device, 'interface') @@ -157,18 +208,15 @@ class Libvirt(object): interface.set('type', 'hostdev') mac = ET.SubElement(interface, 'mac') - mac.set('address', vfmac) - source = ET.SubElement(interface, 'source') + mac.set('address', vf_mac) - addr = ET.SubElement(source, "address") - addr.set('domain', "0x0") - addr.set('bus', "{0}".format(pci_address.bus)) - addr.set('function', "{0}".format(pci_address.function)) - addr.set('slot', "0x{0}".format(pci_address.slot)) - addr.set('type', "pci") + source = ET.SubElement(interface, 'source') + addr = ET.SubElement(source, 'address') + pci_address = PciAddress(vf_pci.strip()) + cls._add_interface_address(addr, pci_address) - pci_vm_address = PciAddress.parse_address(vm_pci.strip(), multi_line=True) - cls.add_interface_address(interface, pci_vm_address) + pci_vm_address = PciAddress(vm_pci.strip()) + cls._add_interface_address(interface, pci_vm_address) root.write(xml) @@ -192,7 +240,7 @@ class Libvirt(object): vcpu = int(cpu) * int(threads) numa_cpus = '0-%s' % (vcpu - 1) hw_socket = flavor.get('hw_socket', '0') - cpuset = Libvirt.pin_vcpu_for_perf(connection, vm_name, vcpu, hw_socket) + cpuset = Libvirt.pin_vcpu_for_perf(connection, hw_socket) mac = StandaloneContextHelper.get_mac_address(0x00) image = cls.create_snapshot_qemu(connection, index, @@ -216,7 +264,7 @@ class Libvirt(object): connection.execute("echo never > /sys/kernel/mm/transparent_hugepage/enabled") @classmethod - def pin_vcpu_for_perf(cls, connection, vm_name, cpu, socket="0"): + def pin_vcpu_for_perf(cls, connection, socket='0'): threads = "" sys_obj = CpuSysCores(connection) soc_cpu = sys_obj.get_core_socket() @@ -244,9 +292,6 @@ class StandaloneContextHelper(object): if connection.execute(cmd_template % pkg)[0]: connection.execute("apt-get update") connection.execute("apt-get -y install %s" % pkg) - else: - # all installed - return @staticmethod def get_kernel_module(connection, pci, driver): @@ -283,7 +328,7 @@ class StandaloneContextHelper(object): 'interface': str(interface), 'driver': driver }) - LOG.info("{0}".format(networks)) + LOG.info(networks) return networks @@ -352,7 +397,7 @@ class StandaloneContextHelper(object): while not mgmtip and times: connection.execute("fping -c 1 -g %s > /dev/null 2>&1" % cidr) out = connection.execute("ip neighbor | grep '%s'" % mac)[1] - LOG.info("fping -c 1 -g %s > /dev/null 2>&1" % cidr) + LOG.info("fping -c 1 -g %s > /dev/null 2>&1", cidr) if out.strip(): mgmtip = str(out.split(" ")[0]).strip() client = ssh.SSH.from_node(node, overrides={"ip": mgmtip}) diff --git a/yardstick/benchmark/contexts/standalone/ovs_dpdk.py b/yardstick/benchmark/contexts/standalone/ovs_dpdk.py index fcb7bb66c..a6c35de53 100644 --- a/yardstick/benchmark/contexts/standalone/ovs_dpdk.py +++ b/yardstick/benchmark/contexts/standalone/ovs_dpdk.py @@ -118,7 +118,7 @@ class OvsDpdkContext(Context): self.connection.execute(cmd) bind_cmd = "{dpdk_nic_bind} --force -b {driver} {port}" phy_driver = "vfio-pci" - for key, port in self.networks.items(): + for _, port in self.networks.items(): vpci = port.get("phy_port") self.connection.execute(bind_cmd.format(dpdk_nic_bind=self.dpdk_nic_bind, driver=phy_driver, port=vpci)) @@ -170,7 +170,7 @@ class OvsDpdkContext(Context): ] ordered_network = OrderedDict(self.networks) - for index, (key, vnf) in enumerate(ordered_network.items()): + for index, vnf in enumerate(ordered_network.values()): if ovs_ver >= [2, 7, 0]: dpdk_args = " options:dpdk-devargs=%s" % vnf.get("phy_port") dpdk_list.append(ovs_add_port.format(br='br0', port='dpdk%s' % vnf.get("port_num", 0), @@ -263,7 +263,7 @@ class OvsDpdkContext(Context): # Bind nics back to kernel bind_cmd = "{dpdk_nic_bind} --force -b {driver} {port}" - for key, port in self.networks.items(): + for port in self.networks.values(): vpci = port.get("phy_port") phy_driver = port.get("driver") self.connection.execute(bind_cmd.format(dpdk_nic_bind=self.dpdk_nic_bind, @@ -328,17 +328,17 @@ class OvsDpdkContext(Context): def configure_nics_for_ovs_dpdk(self): portlist = OrderedDict(self.networks) - for key, ports in portlist.items(): + for key in portlist: mac = StandaloneContextHelper.get_mac_address() portlist[key].update({'mac': mac}) self.networks = portlist - LOG.info("Ports %s" % self.networks) + LOG.info("Ports %s", self.networks) def _enable_interfaces(self, index, vfs, cfg): vpath = self.ovs_properties.get("vpath", "/usr/local") vf = self.networks[vfs[0]] port_num = vf.get('port_num', 0) - vpci = PciAddress.parse_address(vf['vpci'].strip(), multi_line=True) + vpci = PciAddress(vf['vpci'].strip()) # Generate the vpci for the interfaces slot = index + port_num + 10 vf['vpci'] = \ @@ -357,9 +357,10 @@ class OvsDpdkContext(Context): # 1. Check and delete VM if already exists Libvirt.check_if_vm_exists_and_delete(vm_name, self.connection) - vcpu, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, cfg, vm_name, index) + _, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, + cfg, vm_name, index) # 2: Cleanup already available VMs - for idx, (vkey, vfs) in enumerate(OrderedDict(vnf["network_ports"]).items()): + for vkey, vfs in OrderedDict(vnf["network_ports"]).items(): if vkey == "mgmt": continue self._enable_interfaces(index, vfs, cfg) @@ -367,7 +368,7 @@ class OvsDpdkContext(Context): # copy xml to target... self.connection.put(cfg, cfg) - # FIXME: launch through libvirt + # NOTE: launch through libvirt LOG.info("virsh create ...") Libvirt.virsh_create_vm(self.connection, cfg) diff --git a/yardstick/benchmark/contexts/standalone/sriov.py b/yardstick/benchmark/contexts/standalone/sriov.py index 69825fbbf..9d8423b5f 100644 --- a/yardstick/benchmark/contexts/standalone/sriov.py +++ b/yardstick/benchmark/contexts/standalone/sriov.py @@ -110,7 +110,7 @@ class SriovContext(Context): Libvirt.check_if_vm_exists_and_delete(vm, self.connection) # Bind nics back to kernel - for key, ports in self.networks.items(): + for ports in self.networks.values(): # enable VFs for given... build_vfs = "echo 0 > /sys/bus/pci/devices/{0}/sriov_numvfs" self.connection.execute(build_vfs.format(ports.get('phy_port'))) @@ -170,8 +170,7 @@ class SriovContext(Context): def configure_nics_for_sriov(self): vf_cmd = "ip link set {0} vf 0 mac {1}" - for key, ports in self.networks.items(): - vf_pci = [] + for ports in self.networks.values(): host_driver = ports.get('driver') if host_driver not in self.drivers: self.connection.execute("rmmod %svf" % host_driver) @@ -187,19 +186,19 @@ class SriovContext(Context): if interface is not None: self.connection.execute(vf_cmd.format(interface, mac)) - vf_pci = self.get_vf_data('vf_pci', ports.get('phy_port'), mac, interface) + vf_pci = self._get_vf_data(ports.get('phy_port'), mac, interface) ports.update({ 'vf_pci': vf_pci, 'mac': mac }) - LOG.info("Ports %s" % self.networks) + LOG.info('Ports %s', self.networks) def _enable_interfaces(self, index, idx, vfs, cfg): vf_spoofchk = "ip link set {0} vf 0 spoofchk off" vf = self.networks[vfs[0]] - vpci = PciAddress.parse_address(vf['vpci'].strip(), multi_line=True) + vpci = PciAddress(vf['vpci'].strip()) # Generate the vpci for the interfaces slot = index + idx + 10 vf['vpci'] = \ @@ -222,7 +221,7 @@ class SriovContext(Context): # 1. Check and delete VM if already exists Libvirt.check_if_vm_exists_and_delete(vm_name, self.connection) - vcpu, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, cfg, vm_name, index) + _, mac = Libvirt.build_vm_xml(self.connection, self.vm_flavor, cfg, vm_name, index) # 2: Cleanup already available VMs for idx, (vkey, vfs) in enumerate(OrderedDict(vnf["network_ports"]).items()): if vkey == "mgmt": @@ -232,7 +231,7 @@ class SriovContext(Context): # copy xml to target... self.connection.put(cfg, cfg) - # FIXME: launch through libvirt + # NOTE: launch through libvirt LOG.info("virsh create ...") Libvirt.virsh_create_vm(self.connection, cfg) @@ -246,15 +245,15 @@ class SriovContext(Context): return nodes - def get_vf_data(self, key, value, vfmac, pfif): + def _get_vf_data(self, value, vfmac, pfif): vf_data = { "mac": vfmac, "pf_if": pfif } vfs = StandaloneContextHelper.get_virtual_devices(self.connection, value) for k, v in vfs.items(): - m = PciAddress.parse_address(k.strip(), multi_line=True) - m1 = PciAddress.parse_address(value.strip(), multi_line=True) + m = PciAddress(k.strip()) + m1 = PciAddress(value.strip()) if m.bus == m1.bus: vf_data.update({"vf_pci": str(v)}) break diff --git a/yardstick/network_services/utils.py b/yardstick/network_services/utils.py index eac3c814f..7a1815eb9 100644 --- a/yardstick/network_services/utils.py +++ b/yardstick/network_services/utils.py @@ -22,6 +22,7 @@ from oslo_config import cfg from oslo_config.cfg import NoSuchOptError from oslo_utils import encodeutils + NSB_ROOT = "/opt/nsb_bin" CONF = cfg.CONF @@ -43,30 +44,37 @@ HEXADECIMAL = "[0-9a-zA-Z]" class PciAddress(object): + """Class to handle PCI addresses. - PCI_PATTERN_STR = HEXADECIMAL.join([ - "(", - "{4}):(", # domain (4 bytes) - "{2}):(", # bus (2 bytes) - "{2}).(", # function (2 bytes) - ")", # slot (1 byte) - ]) + A PCI address could be written in two ways: + - Simple BDF notation: + 00:00.0 # bus:device.function + - With domain extension. + 0000:00:00.0 # domain:bus:device.function - PCI_PATTERN = re.compile(PCI_PATTERN_STR) + Note: in Libvirt, 'device' is called 'slot'. - @classmethod - def parse_address(cls, text, multi_line=False): - if multi_line: - text = text.replace(os.linesep, '') - match = cls.PCI_PATTERN.search(text) - return cls(match.group(0)) + Reference: https://wiki.xenproject.org/wiki/ + Bus:Device.Function_(BDF)_Notation + """ + PCI_PATTERN_STR = ( + r"((?P[0-9a-zA-Z]{4}):)?(?P[0-9a-zA-Z]{2}):" + r"(?P[0-9a-zA-Z]{2})\.(?P[0-9a-zA-Z]{1})") def __init__(self, address): - super(PciAddress, self).__init__() - match = self.PCI_PATTERN.match(address) + pci_pattern = re.compile(self.PCI_PATTERN_STR) + match = pci_pattern.search(address) if not match: raise ValueError('Invalid PCI address: {}'.format(address)) - self.address = address + + self._domain = (match.group('domain') or '0000').lower() + self._bus = match.group('bus').lower() + self._slot = match.group('slot').lower() + self._function = match.group('function').lower() + self.address = '{:0>4}:{:0>2}:{:0>2}.{:1}'.format(self.domain, + self.bus, + self.slot, + self.function) self.match = match def __repr__(self): @@ -74,22 +82,22 @@ class PciAddress(object): @property def domain(self): - return self.match.group(1) + return self._domain @property def bus(self): - return self.match.group(2) + return self._bus @property def slot(self): - return self.match.group(3) + return self._slot @property def function(self): - return self.match.group(4) + return self._function def values(self): - return [self.match.group(n) for n in range(1, 5)] + return [self._domain, self._bus, self._slot, self._function] def get_nsb_option(option, default=None): -- cgit 1.2.3-korg