From b8e3c6928c2a22a828b85a6780de3cea1107aa85 Mon Sep 17 00:00:00 2001 From: Gris Ge Date: Fri, 24 Dec 2021 16:34:30 +0800 Subject: [PATCH] sriov: New way to wait VF been created By checking `/sys/class/net//device/virtfn/net/` we could tell the interface name of VF. The old VF name guessing method is removed. The `verify_sriov_vf()` is created to collect all VF interfaces name, then comparing it with `total_vfs` count and check whether they exists in current state. The test indicate kernel/udev does not require extra time to remove VF interface when decreasing SR-IOV VF count. Signed-off-by: Gris Ge --- libnmstate/ifaces/ethernet.py | 100 +++++++++++++++------------------- libnmstate/ifaces/ifaces.py | 44 +-------------- libnmstate/nispor/ethernet.py | 17 ++++++ libnmstate/nm/profiles.py | 1 - libnmstate/nm/sriov.py | 5 +- 5 files changed, 67 insertions(+), 100 deletions(-) diff --git a/libnmstate/ifaces/ethernet.py b/libnmstate/ifaces/ethernet.py index 84fc665..720deab 100644 --- a/libnmstate/ifaces/ethernet.py +++ b/libnmstate/ifaces/ethernet.py @@ -17,10 +17,9 @@ # along with this program. If not, see . # +from libnmstate.error import NmstateVerificationError from libnmstate.schema import Ethernet -from libnmstate.schema import Interface from libnmstate.schema import InterfaceType -from libnmstate.schema import InterfaceState from .base_iface import BaseIface @@ -30,7 +29,7 @@ MULTIPORT_PCI_DEVICE_PREFIX = "n" class EthernetIface(BaseIface): - IS_GENERATED_VF_METADATA = "_is_generated_vf" + VF_IFACE_NAME_METADATA = "_vf_iface_name" def __init__(self, info, save_to_disk=True): super().__init__(info, save_to_disk) @@ -56,10 +55,6 @@ class EthernetIface(BaseIface): state = super().state_for_verify() _capitalize_sriov_vf_mac(state) EthernetIface._canonicalize(state) - if self.is_generated_vf: - # The VF state is unpredictable when PF is changing total_vfs count - # Just don't verify generated VF state. - state.pop(Interface.STATE, None) return state @property @@ -102,54 +97,6 @@ class EthernetIface(BaseIface): def duplex(self): return self.raw.get(Ethernet.CONFIG_SUBTREE, {}).get(Ethernet.DUPLEX) - def create_sriov_vf_ifaces(self): - # Currently there is not a way to see the relation between a SR-IOV PF - # and its VFs. Broadcom BCM57416 follows a different name pattern for - # PF and VF, therefore it needs to be parsed if present. - # - # PF name: ens2f0np0 - # VF name: ens2f0v0 - # - # The different name pattern is due to: - # 1. The `n` is for `multi-port PCI device` support. - # 2. The `p*` is `phys_port_name` provided by the BCM driver - # `bnxt_en`. - # - # If the NIC is following the standard pattern "pfname+v+vfid", this - # split will not touch it and the vf_pattern will be the PF name. - # Ref: https://bugzilla.redhat.com/1959679 - vf_pattern = self.name - multiport_pattern = ( - MULTIPORT_PCI_DEVICE_PREFIX + BNXT_DRIVER_PHYS_PORT_PREFIX - ) - if len(self.name.split(multiport_pattern)) == 2: - vf_pattern = self.name.split(multiport_pattern)[0] - - vf_ifaces = [ - EthernetIface( - { - # According to manpage of systemd.net-naming-scheme(7), - # SRIOV VF interface will have v{slot} in device name. - # Currently, nmstate has no intention to support - # user-defined udev rule on SRIOV interface naming policy. - Interface.NAME: f"{vf_pattern}v{i}", - Interface.TYPE: InterfaceType.ETHERNET, - # VF will be in DOWN state initialy - Interface.STATE: InterfaceState.DOWN, - } - ) - for i in range(0, self.sriov_total_vfs) - ] - # The generated vf metadata cannot be part of the original dict. - for vf in vf_ifaces: - vf._info[EthernetIface.IS_GENERATED_VF_METADATA] = True - - return vf_ifaces - - @property - def is_generated_vf(self): - return self._info.get(EthernetIface.IS_GENERATED_VF_METADATA) is True - def remove_vfs_entry_when_total_vfs_decreased(self): vfs_count = len( self._info[Ethernet.CONFIG_SUBTREE] @@ -173,6 +120,16 @@ class EthernetIface(BaseIface): def check_total_vfs_matches_vf_list(self, total_vfs): return total_vfs == len(self.sriov_vfs) + def to_dict(self): + info = super().to_dict() + for vf_info in ( + info.get(Ethernet.CONFIG_SUBTREE, {}) + .get(Ethernet.SRIOV_SUBTREE, {}) + .get(Ethernet.SRIOV.VFS_SUBTREE, []) + ): + vf_info.pop(EthernetIface.VF_IFACE_NAME_METADATA, None) + return info + def _capitalize_sriov_vf_mac(state): vfs = ( @@ -184,3 +141,36 @@ def _capitalize_sriov_vf_mac(state): vf_mac = vf.get(Ethernet.SRIOV.VFS.MAC_ADDRESS) if vf_mac: vf[Ethernet.SRIOV.VFS.MAC_ADDRESS] = vf_mac.upper() + + +def verify_sriov_vf(iface, cur_ifaces): + """ + Checking whether VF interface is been created + """ + if not ( + iface.is_up + and (iface.is_desired or iface.is_changed) + and iface.type == InterfaceType.ETHERNET + and iface.sriov_total_vfs > 0 + ): + return + cur_iface = cur_ifaces.get_iface(iface.name, iface.type) + if not cur_iface: + # Other verification will raise proper exception when current interface + # is missing + return + cur_vf_names = [] + for sriov_vf in cur_iface.sriov_vfs: + if sriov_vf.get(EthernetIface.VF_IFACE_NAME_METADATA): + cur_vf_names.append(sriov_vf[EthernetIface.VF_IFACE_NAME_METADATA]) + + if len(cur_vf_names) != iface.sriov_total_vfs: + raise NmstateVerificationError( + f"Found VF ports count does not match desired " + f"{iface.sriov_total_vfs}, current is: {','.join(cur_vf_names)}" + ) + for cur_vf_name in cur_vf_names: + if not cur_ifaces.get_iface(cur_vf_name, InterfaceType.ETHERNET): + raise NmstateVerificationError( + f"VF interface {cur_vf_name} of PF {iface.name} not found" + ) diff --git a/libnmstate/ifaces/ifaces.py b/libnmstate/ifaces/ifaces.py index be295e3..4bd6792 100644 --- a/libnmstate/ifaces/ifaces.py +++ b/libnmstate/ifaces/ifaces.py @@ -33,6 +33,7 @@ from .base_iface import BaseIface from .bond import BondIface from .dummy import DummyIface from .ethernet import EthernetIface +from .ethernet import verify_sriov_vf from .infiniband import InfiniBandIface from .linux_bridge import LinuxBridgeIface from .macvlan import MacVlanIface @@ -151,7 +152,6 @@ class Ifaces: self._kernel_ifaces[iface.name] = iface self._create_virtual_port() - self._create_sriov_vfs_when_changed() self._mark_vf_interface_as_absent_when_sriov_vf_decrease() self._validate_unknown_port() self._validate_unknown_parent() @@ -241,35 +241,6 @@ class Ifaces: for iface in new_ifaces: self._kernel_ifaces[iface.name] = iface - def _create_sriov_vfs_when_changed(self): - """ - When plugin set the TOTAL_VFS of PF, it might take 1 seconds or - more to have the VFs to be ready. - Nmstate should use verification retry to make sure VFs are full ready. - To do that, we include VFs into desire state. - """ - new_ifaces = [] - for iface in self.all_ifaces(): - if ( - iface.is_up - and (iface.is_desired or iface.is_changed) - and iface.type == InterfaceType.ETHERNET - and iface.sriov_total_vfs > 0 - ): - for new_iface in iface.create_sriov_vf_ifaces(): - cur_iface = self._kernel_ifaces.get(new_iface.name) - if cur_iface and cur_iface.is_desired: - raise NmstateNotSupportedError( - "Does not support changing SR-IOV PF interface " - "along with VF interface in the single desire " - f"state: PF {iface.name}, VF {cur_iface.name}" - ) - else: - new_iface.mark_as_desired() - new_ifaces.append(new_iface) - for new_iface in new_ifaces: - self._kernel_ifaces[new_iface.name] = new_iface - def _mark_vf_interface_as_absent_when_sriov_vf_decrease(self): """ When SRIOV TOTAL_VFS decreased, we should mark certain VF interfaces @@ -647,6 +618,7 @@ class Ifaces: cur_ifaces._remove_ignore_interfaces(self._ignored_ifaces) self._remove_ignore_interfaces(self._ignored_ifaces) for iface in self.all_ifaces(): + verify_sriov_vf(iface, cur_ifaces) if iface.is_desired: if ( iface.is_virtual @@ -700,18 +672,6 @@ class Ifaces: cur_iface.state_for_verify(), ) ) - elif ( - iface.type == InterfaceType.ETHERNET and iface.is_sriov - ): - if not cur_iface.check_total_vfs_matches_vf_list( - iface.sriov_total_vfs - ): - raise NmstateVerificationError( - "The NIC exceeded the waiting time for " - "verification and it is failing because " - "the `total_vfs` does not match the VF " - "list length." - ) def gen_dns_metadata(self, dns_state, route_state): iface_metadata = dns_state.gen_metadata(self, route_state) diff --git a/libnmstate/nispor/ethernet.py b/libnmstate/nispor/ethernet.py index d6969b9..e4b82ff 100644 --- a/libnmstate/nispor/ethernet.py +++ b/libnmstate/nispor/ethernet.py @@ -17,10 +17,13 @@ # along with this program. If not, see . # +import os + from libnmstate.schema import Ethernet from libnmstate.schema import InterfaceType from .base_iface import NisporPluginBaseIface +from libnmstate.ifaces.ethernet import EthernetIface class NisporPluginEthernetIface(NisporPluginBaseIface): @@ -35,6 +38,9 @@ class NisporPluginEthernetIface(NisporPluginBaseIface): for vf in self.np_iface.sr_iov.vfs: vf_infos.append( { + EthernetIface.VF_IFACE_NAME_METADATA: _get_vf_name( + self.np_iface.name, vf.vf_id + ), Ethernet.SRIOV.VFS.ID: vf.vf_id, Ethernet.SRIOV.VFS.MAC_ADDRESS: vf.mac.upper(), Ethernet.SRIOV.VFS.SPOOF_CHECK: vf.spoof_check, @@ -76,3 +82,14 @@ def np_ethtool_link_mode_to_nmstate(np_link_mode): info[Ethernet.SPEED] = np_link_mode.speed return info + + +def _get_vf_name(pf_name, vf_id): + try: + vf_names = os.listdir( + f"/sys/class/net/{pf_name}/device/virtfn{vf_id}/net/" + ) + if len(vf_names) >= 1: + return vf_names[0] + except Exception: + return "" diff --git a/libnmstate/nm/profiles.py b/libnmstate/nm/profiles.py index 3b0b6be..9e3020b 100644 --- a/libnmstate/nm/profiles.py +++ b/libnmstate/nm/profiles.py @@ -71,7 +71,6 @@ class NmProfiles: for iface in sorted( list(net_state.ifaces.all_ifaces()), key=attrgetter("name") ) - if not getattr(iface, "is_generated_vf", None) ] for profile in all_profiles: diff --git a/libnmstate/nm/sriov.py b/libnmstate/nm/sriov.py index a7e387c..e0bf60b 100644 --- a/libnmstate/nm/sriov.py +++ b/libnmstate/nm/sriov.py @@ -102,8 +102,9 @@ def _create_sriov_vfs_from_config(vfs_config, sriov_setting, vf_ids_to_add): def _set_nm_attribute(vf_object, key, value): - nm_attr, nm_variant = SRIOV_NMSTATE_TO_NM_MAP[key] - vf_object.set_attribute(nm_attr, nm_variant(value)) + if key in SRIOV_NMSTATE_TO_NM_MAP: + nm_attr, nm_variant = SRIOV_NMSTATE_TO_NM_MAP[key] + vf_object.set_attribute(nm_attr, nm_variant(value)) def _remove_sriov_vfs_in_setting(vfs_config, sriov_setting, vf_ids_to_remove): -- 2.27.0