From d410b928c8f2a22d42d1974b62ab5b3164861184 Mon Sep 17 00:00:00 2001

From: Gris Ge <fge@redhat.com>

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 <fge@redhat.com>

---

 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