From d410b928c8f2a22d42d1974b62ab5b3164861184 Mon Sep 17 00:00:00 2001 From: Gris Ge Date: Thu, 23 Feb 2023 13:06:01 +0800 Subject: [PATCH] nm: Fix error on SR-IOV When SR-IOV VF naming scheme is like `ens1f0v0`, nmstate will delete the VF NM connection when applying this state: ```yml --- interfaces: - name: ens1f0 type: ethernet state: up ethernet: sr-iov: total-vfs: 1 - name: ens1f0v0 type: ethernet state: up ipv4: enabled: false ipv6: enabled: false ``` This is because `delete_other_profiles()` is checking `self._nm_profile()` from active NM profile instead of newly created one. The fix is using newly created profile `self._nm_simple_conn`. We also have race problem when activating PF along with VF, PF activation might delete VF NIC which cause VF activation failed. To workaround that, we activate PF first via `NmProfile.ACTION_SRIOV_PF` and wait on it before start VF activation. Also problem found during SR-IOV investigations is we do extra un-required modification to `NM.SettingOvsExternalIDs` even it is not mentioned in desired. We skip overriding `NM.SettingOvsExternalIDs` when not desired. Existing test case can cover the use cases. Signed-off-by: Gris Ge --- libnmstate/ifaces/ifaces.py | 18 +++++++++++++++++- libnmstate/netapplier.py | 20 +++++++++++--------- libnmstate/nm/connection.py | 2 +- libnmstate/nm/profile.py | 12 ++++++++++-- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/libnmstate/ifaces/ifaces.py b/libnmstate/ifaces/ifaces.py index 828ff578..470dc0e6 100644 --- a/libnmstate/ifaces/ifaces.py +++ b/libnmstate/ifaces/ifaces.py @@ -157,6 +157,23 @@ class Ifaces: def has_vf_count_change_and_missing_eth(self): return self._has_vf_count_change() and self._has_missing_veth() + def has_sriov_iface(self): + for iface in self.all_kernel_ifaces.values(): + if (iface.is_desired or iface.is_changed) and iface.is_up: + cur_iface = self._cur_kernel_ifaces.get(iface.name) + if ( + cur_iface + and cur_iface.raw.get(Ethernet.CONFIG_SUBTREE, {}).get( + Ethernet.SRIOV_SUBTREE, {} + ) + ) or iface.original_desire_dict.get( + Ethernet.CONFIG_SUBTREE, {} + ).get( + Ethernet.SRIOV_SUBTREE, {} + ): + return True + return False + def _has_vf_count_change(self): for iface in self.all_kernel_ifaces.values(): cur_iface = self._cur_kernel_ifaces.get(iface.name) @@ -664,7 +681,6 @@ class Ifaces: return None def get_cur_iface(self, iface_name, iface_type): - iface = self._cur_kernel_ifaces.get(iface_name) if iface and iface_type in (None, InterfaceType.UNKNOWN, iface.type): return iface diff --git a/libnmstate/netapplier.py b/libnmstate/netapplier.py index ae909126..50a70a9c 100644 --- a/libnmstate/netapplier.py +++ b/libnmstate/netapplier.py @@ -104,7 +104,7 @@ def apply( pf_net_state, verify_change, save_to_disk, - has_sriov_pf=True, + VERIFY_RETRY_COUNT_SRIOV, ) # Refresh the current state current_state = show_with_plugins( @@ -120,8 +120,16 @@ def apply( current_state, save_to_disk, ) + + if net_state.ifaces.has_sriov_iface(): + # If SR-IOV is present, the verification timeout is being increased + # to avoid timeouts due to slow drivers like i40e. + verify_retry = VERIFY_RETRY_COUNT_SRIOV + else: + verify_retry = VERIFY_RETRY_COUNT + _apply_ifaces_state( - plugins, net_state, verify_change, save_to_disk, has_sriov_pf=False + plugins, net_state, verify_change, save_to_disk, verify_retry ) if commit: destroy_checkpoints(plugins, checkpoints) @@ -154,7 +162,7 @@ def rollback(*, checkpoint=None): def _apply_ifaces_state( - plugins, net_state, verify_change, save_to_disk, has_sriov_pf=False + plugins, net_state, verify_change, save_to_disk, verify_retry ): for plugin in plugins: # Do not allow plugin to modify the net_state for future verification @@ -163,12 +171,6 @@ def _apply_ifaces_state( verified = False if verify_change: - if has_sriov_pf: - # If SR-IOV is present, the verification timeout is being increased - # to avoid timeouts due to slow drivers like i40e. - verify_retry = VERIFY_RETRY_COUNT_SRIOV - else: - verify_retry = VERIFY_RETRY_COUNT for _ in range(verify_retry): try: _verify_change(plugins, net_state) diff --git a/libnmstate/nm/connection.py b/libnmstate/nm/connection.py index 1fbb380b..6448e372 100644 --- a/libnmstate/nm/connection.py +++ b/libnmstate/nm/connection.py @@ -240,7 +240,7 @@ def create_new_nm_simple_conn(iface, nm_profile): InterfaceType.OVS_PORT, ) or iface.type == InterfaceType.OVS_BRIDGE - ): + ) and OvsDB.OVS_DB_SUBTREE in iface.original_desire_dict: nm_setting = create_ovsdb_external_ids_setting( iface_info.get(OvsDB.OVS_DB_SUBTREE, {}) ) diff --git a/libnmstate/nm/profile.py b/libnmstate/nm/profile.py index 53eaebed..ad1ad19f 100644 --- a/libnmstate/nm/profile.py +++ b/libnmstate/nm/profile.py @@ -56,6 +56,7 @@ ROUTE_REMOVED = "_route_removed" class NmProfile: # For unmanged iface and desired to down ACTION_ACTIVATE_FIRST = "activate_first" + ACTION_SRIOV_PF = "activate_sriov_pf" ACTION_DEACTIVATE = "deactivate" ACTION_DEACTIVATE_FIRST = "deactivate_first" ACTION_DELETE_DEVICE = "delete_device" @@ -77,6 +78,7 @@ class NmProfile: ACTION_ACTIVATE_FIRST, ACTION_DEACTIVATE_FIRST, ACTION_TOP_CONTROLLER, + ACTION_SRIOV_PF, ACTION_NEW_IFACES, ACTION_OTHER_CONTROLLER, ACTION_NEW_OVS_PORT, @@ -181,6 +183,11 @@ class NmProfile: else: self._add_action(NmProfile.ACTION_NEW_IFACES) else: + if ( + self._nm_dev.props.capabilities + & NM.DeviceCapabilities.SRIOV + ): + self._add_action(NmProfile.ACTION_SRIOV_PF) if self._iface.type == InterfaceType.OVS_PORT: self._add_action(NmProfile.ACTION_MODIFIED_OVS_PORT) if self._iface.type == InterfaceType.OVS_INTERFACE: @@ -462,6 +469,7 @@ class NmProfile: def do_action(self, action): if action in ( + NmProfile.ACTION_SRIOV_PF, NmProfile.ACTION_MODIFIED, NmProfile.ACTION_MODIFIED_OVS_PORT, NmProfile.ACTION_MODIFIED_OVS_IFACE, @@ -559,8 +567,8 @@ class NmProfile: or nm_profile.get_connection_type() == self._nm_iface_type ) and ( - self._nm_profile is None - or nm_profile.get_uuid() != self._nm_profile.get_uuid() + self._nm_simple_conn is None + or nm_profile.get_uuid() != self._nm_simple_conn.get_uuid() ) ): ProfileDelete( -- 2.39.2