From bb6702b1d094901f495bb9ed49f4aec628b4c013 Mon Sep 17 00:00:00 2001 From: vcandappa Date: Mon, 30 Aug 2021 16:27:42 +0530 Subject: [PATCH] Update sriov config service to handle nic partitioned PF Create VFs using udev rules are required for attaching and removing PF from a VM. On a reboot, when udev rules trigger VF creation script and when a VF is attached to vfio-pci driver, some of the VFs on that PF are not initialized properly, resulting in VF not found issue. Though it looks like driver issue, this patch avoids this scenarios by removing the nic-paritioned VF from the udev rule, as when one of the VF is attached to host, the PF cannot be used by the VM. Change-Id: Ic6738de42b9aa0346f48060c143c5f779b67867e (cherry picked from commit fcf3d9cd126f30ecfe166aa5271a2f212a51a5a6) --- os_net_config/sriov_config.py | 23 +++++- os_net_config/tests/test_sriov_config.py | 95 ++++++++++++++++++++++++ os_net_config/tests/test_utils.py | 39 ++++++++++ os_net_config/utils.py | 27 +++++++ 4 files changed, 182 insertions(+), 2 deletions(-) diff --git a/os_net_config/sriov_config.py b/os_net_config/sriov_config.py index 2adfbdb0..8f4d88ed 100644 --- a/os_net_config/sriov_config.py +++ b/os_net_config/sriov_config.py @@ -265,6 +265,24 @@ def udev_monitor_stop(observer): observer.stop() +def is_partitioned_pf(dev_name: str) -> bool: + """Check if any nic-partition(VF) is already used + + Given a PF device, returns True if any VFs of this + device are in-use. + """ + sriov_map = _get_sriov_map() + for config in sriov_map: + devtype = config.get('device_type', None) + if devtype == 'vf': + name = config.get('device', {}).get('name') + vf_name = config.get('name') + if dev_name == name: + logger.warning("%s has VF(%s) used by host" % (name, vf_name)) + return True + return False + + def configure_sriov_pf(execution_from_cli=False, restart_openvswitch=False): observer = udev_monitor_setup() udev_monitor_start(observer) @@ -282,8 +300,9 @@ def configure_sriov_pf(execution_from_cli=False, restart_openvswitch=False): if item.get('link_mode') == "legacy": # Add a udev rule to configure the VF's when PF's are # released by a guest - add_udev_rule_for_legacy_sriov_pf(item['name'], - item['numvfs']) + if not is_partitioned_pf(item['name']): + add_udev_rule_for_legacy_sriov_pf(item['name'], + item['numvfs']) # When configuring vdpa, we need to configure switchdev before # we create the VFs vendor_id = get_vendor_id(item['name']) diff --git a/os_net_config/tests/test_sriov_config.py b/os_net_config/tests/test_sriov_config.py index a51696e6..09ccba7e 100644 --- a/os_net_config/tests/test_sriov_config.py +++ b/os_net_config/tests/test_sriov_config.py @@ -319,6 +319,101 @@ class TestSriovConfig(base.TestCase): self.assertEqual(10, sriov_config.get_numvfs('p2p1')) self.assertEqual(12, sriov_config.get_numvfs('p2p2')) + def test_configure_sriov_pf_nicpart(self): + """Test the udev rules created for legacy mode of SR-IOV PF + + In this case, the VF(s) are already bound/attached + """ + + self.setUp_pf_stubs() + + exp_udev_content = '# This file is autogenerated by os-net-config\n'\ + 'KERNEL=="p2p2", RUN+="/bin/os-net-config-sriov -n %k:12"\n' + exp_actions = [ + 'udev_monitor_setup', + 'udev_monitor_start', + 'set_numvfs', + 'get_numvfs', + '_wait_for_vf_creation_stub', + 'get_numvfs', + 'reload_udev_rules', + 'set_numvfs', + 'get_numvfs', + '_wait_for_vf_creation_stub', + 'get_numvfs', + 'udev_monitor_stop', + ] + + pf_config = [{"device_type": "pf", "name": "p2p1", "numvfs": 10, + "promisc": "on", "link_mode": "legacy"}, + {"device_type": "pf", "name": "p2p2", "numvfs": 12, + "promisc": "off", "link_mode": "legacy"}, + {"device": {"name": "p2p1", "vfid": 0}, + "device_type": "vf", "name": "p2p1v0", "max_tx_rate": 0, + "min_tx_rate": 0, "pci_address": "0000:18:0a.0", + "trust": "on", "spoofcheck": "off"}] + + for ifname in ['p2p1', 'p2p2']: + self._write_numvfs(ifname) + + self._action_order = [] + utils.write_yaml_config(sriov_config._SRIOV_CONFIG_FILE, pf_config) + sriov_config.configure_logger(debug=True) + sriov_config.configure_sriov_pf() + self.assertEqual(exp_actions, self._action_order) + f = open(sriov_config._UDEV_LEGACY_RULE_FILE, 'r') + self.assertEqual(exp_udev_content, f.read()) + self.assertEqual(12, sriov_config.get_numvfs('p2p2')) + + def test_configure_sriov_pf_non_nicpart(self): + """Test the udev rules created for legacy mode of SR-IOV PF + + In this case, the nic partitioned VF(s) are not attached + """ + + self.setUp_pf_stubs() + + exp_udev_content = '# This file is autogenerated by os-net-config\n'\ + 'KERNEL=="p2p1", RUN+="/bin/os-net-config-sriov -n %k:10"\n'\ + 'KERNEL=="p2p2", RUN+="/bin/os-net-config-sriov -n %k:12"\n' + exp_actions = [ + 'udev_monitor_setup', + 'udev_monitor_start', + 'reload_udev_rules', + 'set_numvfs', + 'get_numvfs', + '_wait_for_vf_creation_stub', + 'get_numvfs', + 'reload_udev_rules', + 'set_numvfs', + 'get_numvfs', + '_wait_for_vf_creation_stub', + 'get_numvfs', + 'udev_monitor_stop', + ] + + pf_config = [{"device_type": "pf", "name": "p2p1", "numvfs": 10, + "promisc": "on", "link_mode": "legacy"}, + {"device_type": "pf", "name": "p2p2", "numvfs": 12, + "promisc": "off", "link_mode": "legacy"}, + {"device": {"name": "eno3", "vfid": 1}, + "device_type": "vf", "name": "eno3v0", "max_tx_rate": 0, + "min_tx_rate": 0, "pci_address": "0000:18:0e.1", + "trust": "on", "spoofcheck": "off"}] + + for ifname in ['p2p1', 'p2p2']: + self._write_numvfs(ifname) + + self._action_order = [] + utils.write_yaml_config(sriov_config._SRIOV_CONFIG_FILE, pf_config) + sriov_config.configure_logger(debug=True) + sriov_config.configure_sriov_pf() + self.assertEqual(exp_actions, self._action_order) + f = open(sriov_config._UDEV_LEGACY_RULE_FILE, 'r') + self.assertEqual(exp_udev_content, f.read()) + self.assertEqual(10, sriov_config.get_numvfs('p2p1')) + self.assertEqual(12, sriov_config.get_numvfs('p2p2')) + def test_configure_vdpa_pf(self): """Test the vdpa PF diff --git a/os_net_config/tests/test_utils.py b/os_net_config/tests/test_utils.py index cec15b55..1ef60816 100644 --- a/os_net_config/tests/test_utils.py +++ b/os_net_config/tests/test_utils.py @@ -75,6 +75,11 @@ dpdk { } ''' +udev_content = '# This file is autogenerated by os-net-config\n'\ + 'KERNEL=="eth1", RUN+="/bin/os-net-config-sriov -n %k:10"\n' \ + 'KERNEL=="eth2", RUN+="/bin/os-net-config-sriov -n %k:10"\n' +UDEV_FILE = '/tmp/70-os-net-config-sriov.' + class TestUtils(base.TestCase): @@ -83,6 +88,7 @@ class TestUtils(base.TestCase): rand = str(int(random.random() * 100000)) utils._DPDK_MAPPING_FILE = '/tmp/dpdk_mapping_' + rand + '.yaml' sriov_config._SRIOV_CONFIG_FILE = '/tmp/sriov_config_' + rand + '.yaml' + sriov_config._UDEV_LEGACY_RULE_FILE = UDEV_FILE + rand def tearDown(self): super(TestUtils, self).tearDown() @@ -90,6 +96,8 @@ class TestUtils(base.TestCase): os.remove(utils._DPDK_MAPPING_FILE) if os.path.isfile(sriov_config._SRIOV_CONFIG_FILE): os.remove(sriov_config._SRIOV_CONFIG_FILE) + if os.path.isfile(sriov_config._UDEV_LEGACY_RULE_FILE): + os.remove(sriov_config._UDEV_LEGACY_RULE_FILE) def test_ordered_active_nics(self): @@ -240,6 +248,37 @@ class TestUtils(base.TestCase): 'device': {"name": "eth1", "vfid": 2}}] self.assertListEqual(test_sriov_vf_map, sriov_vf_map) + def test_udev_rule_for_sriov_vf(self): + def get_numvfs_stub(pf_name): + return 10 + self.stub_out('os_net_config.sriov_config.get_numvfs', + get_numvfs_stub) + utils.update_sriov_pf_map('eth1', 10, False) + utils.update_sriov_pf_map('eth2', 10, False) + utils.update_sriov_vf_map('eth1', 2, 'eth1_2') + contents = utils.get_file_data(sriov_config._SRIOV_CONFIG_FILE) + sriov_vf_map = yaml.safe_load(contents) if contents else [] + self.assertEqual(3, len(sriov_vf_map)) + + test_sriov_vf_map = {'device': {'name': 'eth1', 'vfid': 2}, + 'device_type': 'vf', 'max_tx_rate': 0, + 'min_tx_rate': 0, 'name': 'eth1_2'} + udev_file = open(sriov_config._UDEV_LEGACY_RULE_FILE, "w") + udev_file.write(udev_content) + udev_file.close() + + # sriov_config.add_udev_rule_for_legacy_sriov_pf("eth1", 10) + for dev in sriov_vf_map: + if(dev.keys() == test_sriov_vf_map.keys()): + self.assertEqual(test_sriov_vf_map, dev) + + expect = '# This file is autogenerated by os-net-config\n'\ + 'KERNEL=="eth2", RUN+="/bin/os-net-config-sriov -n %k:10"\n' + utils.nicpart_udev_rules_check() + f = open(sriov_config._UDEV_LEGACY_RULE_FILE, 'r') + self.assertEqual(expect, f.read()) + f.close() + def test_update_sriov_vf_map_complete_new(self): utils.update_sriov_vf_map('eth1', 2, 'eth1_2', vlan_id=10, qos=5, spoofcheck="on", trust="on", state="enable", diff --git a/os_net_config/utils.py b/os_net_config/utils.py index 8f7ed07b..d7451ecb 100644 --- a/os_net_config/utils.py +++ b/os_net_config/utils.py @@ -588,6 +588,32 @@ def _get_vf_name_from_map(pf_name, vfid): return item['name'] +def nicpart_udev_rules_check(): + """Clean-up UDEV rules on initial deployment + + After writing sriov_config.yaml file, clean-up + UDEV rule(s) of PF for which VFs are used by host + """ + if not os.path.exists(sriov_config._UDEV_LEGACY_RULE_FILE): + return + + udev = '^KERNEL=="(.*)", RUN.*' + udev_reg = re.compile(udev, 0) + + with open(sriov_config._UDEV_LEGACY_RULE_FILE, "r") as fp: + rules = fp.readlines() + + with open(sriov_config._UDEV_LEGACY_RULE_FILE, "w") as fp: + for line in rules: + match = udev_reg.match(line) + if match: + dev_name = match.group(1) + if not sriov_config.is_partitioned_pf(dev_name): + fp.write(line) + else: + fp.write(line) + + def _configure_sriov_config_service(): """Generate the sriov_config.service @@ -609,6 +635,7 @@ def configure_sriov_pfs(execution_from_cli=False, restart_openvswitch=False): def configure_sriov_vfs(): logger.info("Configuring VFs now") sriov_config.configure_sriov_vf() + nicpart_udev_rules_check() def get_vf_devname(pf_name, vfid):