nm ovs: Enable connection.autoconnect-ports for OVS port

We forgot to enable `connection.autoconnect-ports` for OVS ports in NM
code as the `iface.is_controller()` is false for
`InterfaceType::Other("ovs-port")`

Extend verify count to 10 rounds, as NM takes longer time after enabled
`connection.autoconnect-ports` due to extra reactivations in NM internal.

With this change, we can also skip activation on OVS interface and OVS
port as OVS bridge will activate its subordinates.

Fixed and added integration test case.

Signed-off-by: Gris Ge <[email protected]>

rust/src/lib/nm/profile.rs

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use super::nm_dbus::{NmActiveConnection, NmConnection};
+use super::nm_dbus::{NmActiveConnection, NmConnection, NmIfaceType};
 use super::settings::{
     fix_ip_dhcp_timeout, get_exist_profile, iface_to_nm_connections,
 };
@@ -116,7 +116,6 @@ pub(crate) fn perpare_nm_conns(
 
 // When a new virtual interface is desired, if its controller is also newly
 // created, in NetworkManager, there is no need to activate the subordinates.
-// For OVS stuff, always return false.
 fn can_skip_activation(
     merged_iface: &MergedInterface,
     merged_ifaces: &MergedInterfaces,
@@ -160,16 +159,12 @@ fn can_skip_activation(
     if merged_iface.current.is_none()
         && merged_iface.for_apply.is_some()
         && merged_iface.merged.is_up()
-        && merged_iface.merged.iface_type() != InterfaceType::OvsInterface
     {
         if let Some(desired_iface) = merged_iface.for_apply.as_ref() {
             if let (Some(ctrl_iface), Some(ctrl_type)) = (
                 desired_iface.base_iface().controller.as_deref(),
                 desired_iface.base_iface().controller_type.as_ref(),
             ) {
-                if ctrl_type == &InterfaceType::OvsBridge {
-                    return false;
-                }
                 if let Some(merged_ctrl_iface) =
                     merged_ifaces.get_iface(ctrl_iface, ctrl_type.clone())
                 {
@@ -187,6 +182,11 @@ fn can_skip_activation(
                     }
                 }
             }
+
+            // new OVS port on new OVS bridge can skip activation
+            if nm_conn.iface_type() == Some(&NmIfaceType::OvsPort) {
+                return true;
+            }
         }
     }
     false

rust/src/lib/nm/query_apply/apply.rs

@@ -340,7 +340,9 @@ fn delete_remain_virtual_interface_as_desired(
         .kernel_ifaces
         .values()
         .filter(|i| {
-            i.is_changed() && (i.merged.is_absent() || i.merged.is_down())
+            i.merged.iface_type() != InterfaceType::OvsInterface
+                && i.is_changed()
+                && (i.merged.is_absent() || i.merged.is_down())
         })
         .map(|i| &i.merged)
     {

rust/src/lib/nm/settings/connection.rs

@@ -418,7 +418,9 @@ pub(crate) fn gen_nm_conn_setting(
         nm_conn_set.iface_name = None;
     }
     nm_conn_set.autoconnect = Some(true);
-    nm_conn_set.autoconnect_ports = if iface.is_controller() {
+    nm_conn_set.autoconnect_ports = if iface.is_controller()
+        || iface.iface_type() == InterfaceType::Other("ovs-port".to_string())
+    {
         Some(true)
     } else {
         None

rust/src/lib/query_apply/net_state.rs

@@ -19,6 +19,7 @@ use crate::{
 const DEFAULT_ROLLBACK_TIMEOUT: u32 = 60;
 const VERIFY_RETRY_INTERVAL_MILLISECONDS: u64 = 1000;
 const VERIFY_RETRY_COUNT_DEFAULT: usize = 5;
+const VERIFY_RETRY_COUNT_OVS: usize = 10;
 const VERIFY_RETRY_COUNT_SRIOV_MIN: usize = 30;
 const VERIFY_RETRY_COUNT_SRIOV_MAX: usize = 300;
 const VERIFY_RETRY_COUNT_KERNEL_MODE: usize = 5;
@@ -486,7 +487,13 @@ impl MergedNetworkState {
 
 fn get_proper_verify_retry_count(merged_ifaces: &MergedInterfaces) -> usize {
     match merged_ifaces.get_sriov_vf_count() {
-        0 => VERIFY_RETRY_COUNT_DEFAULT,
+        0 => {
+            if merged_ifaces.has_ovs() {
+                VERIFY_RETRY_COUNT_OVS
+            } else {
+                VERIFY_RETRY_COUNT_DEFAULT
+            }
+        }
         v if v >= 64 => VERIFY_RETRY_COUNT_SRIOV_MAX,
         v if v <= 16 => VERIFY_RETRY_COUNT_SRIOV_MIN,
         v => v as usize / 64 * VERIFY_RETRY_COUNT_SRIOV_MAX,

rust/src/lib/query_apply/ovs.rs

@@ -146,6 +146,18 @@ impl OvsBridgeBondConfig {
     }
 }
 impl MergedInterfaces {
+    pub(crate) fn has_ovs(&self) -> bool {
+        self.kernel_ifaces
+            .values()
+            .chain(self.user_ifaces.values())
+            .any(|i| {
+                i.for_apply.as_ref().map(|i| {
+                    i.iface_type() == InterfaceType::OvsBridge
+                        || i.iface_type() == InterfaceType::OvsInterface
+                }) == Some(true)
+            })
+    }
+
     // This function remove extra(undesired) ovs patch port from post-apply
     // current, so it will not interfere with verification
     pub(crate) fn process_allow_extra_ovs_patch_ports_for_verify(

tests/integration/nm/ovs_test.py

@@ -2,7 +2,6 @@
 
 import pytest
 
-import yaml
 
 import libnmstate
 from libnmstate.schema import Interface
@@ -18,6 +17,7 @@
 from ..testlib.ovslib import Bridge
 from ..testlib.retry import retry_till_true_or_timeout
 from ..testlib.dummy import nm_unmanaged_dummy
+from ..testlib.yaml import load_yaml
 
 
 BRIDGE0 = "brtest0"
@@ -526,7 +526,7 @@ def test_do_not_touch_ovs_port_when_not_desired_internal_iface(
 
 
 def test_gc_on_ovs_dpdk():
-    desired_state = yaml.load(
+    desired_state = load_yaml(
         """---
         interfaces:
         - name: ovs0
@@ -545,8 +545,7 @@ def test_gc_on_ovs_dpdk():
               datapath: "netdev"
             port:
             - name: ovs0
-        """,
-        Loader=yaml.SafeLoader,
+        """
     )
     confs = libnmstate.generate_configurations(desired_state)["NetworkManager"]
     ovs_iface_conf = [conf for conf in confs if conf[0].startswith("ovs0-if")][
@@ -567,7 +566,7 @@ def unmaaged_dummy1():
 
 
 def test_ovs_bond_auto_managed_ignored_port(unmaaged_dummy1, eth1_up):
-    desired_state = yaml.load(
+    desired_state = load_yaml(
         f"""---
         interfaces:
         - name: br0
@@ -582,20 +581,55 @@ def test_ovs_bond_auto_managed_ignored_port(unmaaged_dummy1, eth1_up):
                   - name: {DUMMY1}
                   - name: eth1
         """,
-        Loader=yaml.SafeLoader,
     )
     try:
         libnmstate.apply(desired_state)
         assertlib.assert_state_match(desired_state)
     finally:
         libnmstate.apply(
-            yaml.load(
+            load_yaml(
+                """---
+                interfaces:
+                - name: br0
+                  type: ovs-bridge
+                  state: absent
+                """,
+            )
+        )
+
+
+def test_ovs_port_has_autoconnect_ports(eth1_up):
+    desired_state = load_yaml(
+        """---
+        interfaces:
+        - name: ovs0
+          type: ovs-interface
+        - name: br0
+          type: ovs-bridge
+          state: up
+          bridge:
+            port:
+            - name: ovs0
+            - name: eth1
+        """,
+    )
+    try:
+        libnmstate.apply(desired_state)
+        assert (
+            cmdlib.exec_cmd(
+                "nmcli -g connection.autoconnect-slaves "
+                "c show ovs0-port".split(),
+            )[1].strip()
+            == "1"
+        )
+    finally:
+        libnmstate.apply(
+            load_yaml(
                 """---
                 interfaces:
                 - name: br0
                   type: ovs-bridge
                   state: absent
                 """,
-                Loader=yaml.SafeLoader,
             )
         )