net: conn_mgr: Fix disconnect with CONN_MGR_IF_NO_AUTO_DOWN unset

Connection manager enforces non-blocking disconnect() behavior, yet in
case CONN_MGR_IF_NO_AUTO_DOWN flag is not set, it'd put the interface
down right after, disrupting the disconnect process.

As putting the interface down can be handled in the corresponding event
handler as well, when the interface is actually disconnected, remove the
conn_mgr_conn_if_auto_admin_down() call from conn_mgr_if_disconnect().
To make this work with persistence flag, introduce a new internal flag
indicating that the interface is in active disconnect.

Finally, since it isn't really necessary that disconnect() API call is
non-blocking, remove that requirement.

Signed-off-by: Robert Lubos <[email protected]>

Author: rlubos

include/zephyr/net/conn_mgr_connectivity.h

@@ -90,6 +90,12 @@ enum conn_mgr_if_flag {
 	CONN_MGR_IF_NO_AUTO_DOWN,
 
 /** @cond INTERNAL_HIDDEN */
+	/**
+	 * Internal flag indicating that the interface is in active (application initiated)
+	 * disconnect.
+	 */
+	CONN_MGR_IF_DISCONNECTING,
+
 	/* Total number of flags - must be at the end of the enum */
 	CONN_MGR_NUM_IF_FLAGS,
 /** @endcond */

include/zephyr/net/conn_mgr_connectivity_impl.h

@@ -57,8 +57,6 @@ struct conn_mgr_conn_api {
 	 * stop any in-progress attempts to associate to a network, the bound iface pointed to by
 	 * if_conn->iface.
 	 *
-	 * Must be non-blocking.
-	 *
 	 * Called by @ref conn_mgr_if_disconnect.
 	 */
 	int (*disconnect)(struct conn_mgr_conn_binding *const binding);

subsys/net/conn_mgr/conn_mgr_connectivity.c

@@ -41,6 +41,8 @@ int conn_mgr_if_connect(struct net_if *iface)
 		}
 	}
 
+	conn_mgr_if_set_flag(iface, CONN_MGR_IF_DISCONNECTING, false);
+
 	status = api->connect(binding);
 
 out:
@@ -49,8 +51,6 @@ int conn_mgr_if_connect(struct net_if *iface)
 	return status;
 }
 
-static void conn_mgr_conn_if_auto_admin_down(struct net_if *iface);
-
 int conn_mgr_if_disconnect(struct net_if *iface)
 {
 	struct conn_mgr_conn_binding *binding;
@@ -75,21 +75,13 @@ int conn_mgr_if_disconnect(struct net_if *iface)
 		goto out;
 	}
 
+	conn_mgr_if_set_flag(iface, CONN_MGR_IF_DISCONNECTING, true);
+
 	status = api->disconnect(binding);
 
 out:
 	conn_mgr_binding_unlock(binding);
 
-	/* Since the connectivity implementation will not automatically attempt to reconnect after
-	 * a call to conn_mgr_if_disconnect, conn_mgr_conn_if_auto_admin_down should be called.
-	 *
-	 * conn_mgr_conn_handle_iface_down will only call conn_mgr_conn_if_auto_admin_down if
-	 * persistence is disabled. To ensure conn_mgr_conn_if_auto_admin_down is called in all
-	 * cases, we must call it directly from here. If persistence is disabled, this will result
-	 * in conn_mgr_conn_if_auto_admin_down being called twice, but that is not an issue.
-	 */
-	conn_mgr_conn_if_auto_admin_down(iface);
-
 	return status;
 }
 
@@ -306,11 +298,16 @@ static void conn_mgr_conn_handle_iface_down(struct net_if *iface)
 		return;
 	}
 
-	/* If the iface is persistent, we expect it to try to reconnect, so nothing else to do */
-	if (conn_mgr_if_get_flag(iface, CONN_MGR_IF_PERSISTENT)) {
+	/* If the iface is persistent, we expect it to try to reconnect, unless
+	 * disconnect was explicitly initiated by the application.
+	 */
+	if (conn_mgr_if_get_flag(iface, CONN_MGR_IF_PERSISTENT) &&
+	    !conn_mgr_if_get_flag(iface, CONN_MGR_IF_DISCONNECTING)) {
 		return;
 	}
 
+	conn_mgr_if_set_flag(iface, CONN_MGR_IF_DISCONNECTING, false);
+
 	/* Otherwise, we do not expect the iface to reconnect, and we should call
 	 * conn_mgr_conn_if_auto_admin_down
 	 */