[PATCH v2] iwd: Always disconnect connection completely

["VAUTRIN Emmanuel (Canal Plus Prestataire)" <Emmanuel.VAUTRIN@cpexterne.org>]

[-- Attachment #1: Type: text/plain, Size: 6852 bytes --]

Before being able to connect to a new network, finish disconnecting
the old connection. The network can change while the
cm_network_disconnect_cb is scheduled.

Commit 024309a9e04a ("wifi: Reset disconnecting status of any network")
Commit 795883e98eba ("wifi: Check valid network in disconnect callback")
Commit dd86f09107e8 ("wifi: Always disconnect connection completely")
---
 plugins/iwd.c | 115 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 103 insertions(+), 12 deletions(-)

diff --git a/plugins/iwd.c b/plugins/iwd.c
index ac3d1e1787ad..f65148b691e7 100644
--- a/plugins/iwd.c
+++ b/plugins/iwd.c
@@ -82,6 +82,10 @@ struct iwd_device {
         char *mode;
 
         struct connman_device *device;
+
+       char *network;
+       char *pending_network;
+       bool disconnecting;
 };
 
 struct iwd_network {
@@ -219,12 +223,6 @@ static void update_network_connected(struct iwd_network *iwdn)
         connman_network_set_connected(iwdn->network, true);
 }
 
-static void update_network_disconnected(struct iwd_network *iwdn)
-{
-       DBG("interface name %s", iwdn->name);
-       connman_network_set_connected(iwdn->network, false);
-}
-
 static void cm_network_connect_cb(DBusMessage *message, void *user_data)
 {
         const char *path = user_data;
@@ -253,21 +251,94 @@ static void cm_network_connect_cb(DBusMessage *message, void *user_data)
         update_network_connected(iwdn);
 }
 
+static void abort_pending_network(struct iwd_device *iwdd,
+               enum connman_network_error error)
+{
+       struct iwd_network *iwdn;
+
+       if (!iwdd->pending_network)
+               return;
+
+       iwdn = g_hash_table_lookup(networks, iwdd->pending_network);
+       if (iwdn)
+               connman_network_set_error(iwdn->network, error);
+
+       g_free(iwdd->pending_network);
+       iwdd->pending_network = NULL;
+}
+
 static int cm_network_connect(struct connman_network *network)
 {
         struct iwd_network *iwdn = connman_network_get_data(network);
+       struct iwd_device *iwdd;
+       int err;
 
-       if (!iwdn)
+       if (!iwdn || !iwdn->iwdd)
                 return -EINVAL;
 
+       iwdd = iwdn->iwdd;
+       if (iwdd->disconnecting) {
+               if (g_strcmp0(iwdn->path, iwdd->pending_network)) {
+                       abort_pending_network(iwdd,
+                                       CONNMAN_NETWORK_ERROR_CONNECT_FAIL);
+                       iwdd->pending_network = g_strdup(iwdn->path);
+               }
+               return -EINPROGRESS;
+       }
+
         if (!g_dbus_proxy_method_call(iwdn->proxy, "Connect",
                         NULL, cm_network_connect_cb,
-                       g_strdup(iwdn->path), g_free))
-               return -EIO;
+                       g_strdup(iwdn->path), g_free)) {
+               err = -EIO;
+               goto out;
+       }
 
         connman_network_set_associating(iwdn->network, true);
 
-       return -EINPROGRESS;
+       g_free(iwdd->network);
+       iwdd->network = g_strdup(iwdn->path);
+
+       err = -EINPROGRESS;
+
+out:
+       abort_pending_network(iwdd, CONNMAN_NETWORK_ERROR_UNKNOWN);
+       return err;
+}
+
+static void update_network_disconnected(struct iwd_network *iwdn)
+{
+       struct iwd_device *iwdd;
+
+       if (!iwdn || !iwdn->iwdd)
+               return;
+
+       iwdd = iwdn->iwdd;
+
+       DBG("interface name %s", iwdn->name);
+       connman_network_set_connected(iwdn->network, false);
+
+       iwdd->disconnecting = false;
+
+       if (g_strcmp0(iwdn->path, iwdd->network)) {
+               if (!g_strcmp0(iwdn->path, iwdd->pending_network)) {
+                       abort_pending_network(iwdd,
+                                       CONNMAN_NETWORK_ERROR_CONNECT_FAIL);
+               }
+               DBG("current wifi network has changed since disconnection");
+               return;
+       }
+
+       g_free(iwdd->network);
+       iwdd->network = NULL;
+
+       if (iwdd->pending_network) {
+               struct iwd_network *iwdn_pending =
+                       g_hash_table_lookup(networks, iwdd->pending_network);
+               if (!iwdn_pending)
+                       return;
+
+               cm_network_connect(iwdn_pending->network);
+       }
 }
 
 static void cm_network_disconnect_cb(DBusMessage *message, void *user_data)
@@ -279,6 +350,9 @@ static void cm_network_disconnect_cb(DBusMessage *message, void *user_data)
         if (!iwdn)
                 return;
 
+       if(iwdn->iwdd)
+               iwdn->iwdd->disconnecting = false;
+
         if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_ERROR) {
                 const char *dbus_error = dbus_message_get_error_name(message);
 
@@ -303,11 +377,16 @@ static int cm_network_disconnect(struct connman_network *network)
 {
         struct iwd_network *iwdn = connman_network_get_data(network);
         struct iwd_station *iwds;
+       struct iwd_device *iwdd;
 
-       if (!iwdn && !iwdn->iwdd)
+       if (!iwdn || !iwdn->iwdd)
                 return -EINVAL;
 
-       iwds = g_hash_table_lookup(stations, iwdn->iwdd->path);
+       iwdd = iwdn->iwdd;
+       if (iwdd->disconnecting)
+               return -EALREADY;
+
+       iwds = g_hash_table_lookup(stations, iwdd->path);
         if (!iwds)
                 return -EIO;
 
@@ -317,6 +396,8 @@ static int cm_network_disconnect(struct connman_network *network)
                         NULL, cm_network_disconnect_cb, g_strdup(iwdn->path), g_free))
                 return -EIO;
 
+       iwdd->disconnecting = true;
+
         return 0;
 }
 
@@ -515,6 +596,12 @@ static void device_powered_cb(const DBusError *error, void *user_data)
         }
 
         connman_device_set_powered(iwdd->device, cbd->powered);
+
+       if(!cbd->powered) {
+               abort_pending_network(iwdd,
+                                       CONNMAN_NETWORK_ERROR_CONNECT_FAIL);
+               iwdd->disconnecting = false;
+       }
 out:
         g_free(cbd->path);
         g_free(cbd);
@@ -1309,6 +1396,10 @@ static void device_free(gpointer data)
         g_free(iwdd->adapter);
         g_free(iwdd->name);
         g_free(iwdd->address);
+
+       g_free(iwdd->network);
+       g_free(iwdd->pending_network);
+
         g_free(iwdd);
 }
 
-- 
2.25.1
Please find the well formatted path as attachment.

> I don't have any fallouts in there. This could be just because I use a
> different compiler with different settings. Please report those
> failures. It's not good to paper over them in the long run.
Probably, by my side, I am building via Yocto.
I will send you the related patch.

Emmanuel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-iwd-Always-disconnect-connection-completely.patch --]
[-- Type: text/x-patch; name="v2-0001-iwd-Always-disconnect-connection-completely.patch", Size: 5435 bytes --]

From 2526604133f8bf47da3234650f06ff27fa01ecb1 Mon Sep 17 00:00:00 2001
From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN@cpexterne.org>
Date: Wed, 19 Jan 2022 10:38:07 +0100
Subject: [PATCH v2] iwd: Always disconnect connection completely

Before being able to connect to a new network, finish disconnecting
the old connection. The network can change while the
cm_network_disconnect_cb is scheduled.

Commit 024309a9e04a ("wifi: Reset disconnecting status of any network")
Commit 795883e98eba ("wifi: Check valid network in disconnect callback")
Commit dd86f09107e8 ("wifi: Always disconnect connection completely")
---
 plugins/iwd.c | 115 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 103 insertions(+), 12 deletions(-)

diff --git a/plugins/iwd.c b/plugins/iwd.c
index ac3d1e1787ad..f65148b691e7 100644
--- a/plugins/iwd.c
+++ b/plugins/iwd.c
@@ -82,6 +82,10 @@ struct iwd_device {
 	char *mode;
 
 	struct connman_device *device;
+
+	char *network;
+	char *pending_network;
+	bool disconnecting;
 };
 
 struct iwd_network {
@@ -219,12 +223,6 @@ static void update_network_connected(struct iwd_network *iwdn)
 	connman_network_set_connected(iwdn->network, true);
 }
 
-static void update_network_disconnected(struct iwd_network *iwdn)
-{
-	DBG("interface name %s", iwdn->name);
-	connman_network_set_connected(iwdn->network, false);
-}
-
 static void cm_network_connect_cb(DBusMessage *message, void *user_data)
 {
 	const char *path = user_data;
@@ -253,21 +251,94 @@ static void cm_network_connect_cb(DBusMessage *message, void *user_data)
 	update_network_connected(iwdn);
 }
 
+static void abort_pending_network(struct iwd_device *iwdd,
+		enum connman_network_error error)
+{
+	struct iwd_network *iwdn;
+
+	if (!iwdd->pending_network)
+		return;
+
+	iwdn = g_hash_table_lookup(networks, iwdd->pending_network);
+	if (iwdn)
+		connman_network_set_error(iwdn->network, error);
+
+	g_free(iwdd->pending_network);
+	iwdd->pending_network = NULL;
+}
+
 static int cm_network_connect(struct connman_network *network)
 {
 	struct iwd_network *iwdn = connman_network_get_data(network);
+	struct iwd_device *iwdd;
+	int err;
 
-	if (!iwdn)
+	if (!iwdn || !iwdn->iwdd)
 		return -EINVAL;
 
+	iwdd = iwdn->iwdd;
+	if (iwdd->disconnecting) {
+		if (g_strcmp0(iwdn->path, iwdd->pending_network)) {
+			abort_pending_network(iwdd,
+					CONNMAN_NETWORK_ERROR_CONNECT_FAIL);
+			iwdd->pending_network = g_strdup(iwdn->path);
+		}
+		return -EINPROGRESS;
+	}
+
 	if (!g_dbus_proxy_method_call(iwdn->proxy, "Connect",
 			NULL, cm_network_connect_cb,
-			g_strdup(iwdn->path), g_free))
-		return -EIO;
+			g_strdup(iwdn->path), g_free)) {
+		err = -EIO;
+		goto out;
+	}
 
 	connman_network_set_associating(iwdn->network, true);
 
-	return -EINPROGRESS;
+	g_free(iwdd->network);
+	iwdd->network = g_strdup(iwdn->path);
+
+	err = -EINPROGRESS;
+
+out:
+	abort_pending_network(iwdd, CONNMAN_NETWORK_ERROR_UNKNOWN);
+	return err;
+}
+
+static void update_network_disconnected(struct iwd_network *iwdn)
+{
+	struct iwd_device *iwdd;
+
+	if (!iwdn || !iwdn->iwdd)
+		return;
+
+	iwdd = iwdn->iwdd;
+
+	DBG("interface name %s", iwdn->name);
+	connman_network_set_connected(iwdn->network, false);
+
+	iwdd->disconnecting = false;
+
+	if (g_strcmp0(iwdn->path, iwdd->network)) {
+		if (!g_strcmp0(iwdn->path, iwdd->pending_network)) {
+			abort_pending_network(iwdd,
+					CONNMAN_NETWORK_ERROR_CONNECT_FAIL);
+		}
+		DBG("current wifi network has changed since disconnection");
+		return;
+	}
+
+	g_free(iwdd->network);
+	iwdd->network = NULL;
+
+	if (iwdd->pending_network) {
+		struct iwd_network *iwdn_pending =
+			g_hash_table_lookup(networks, iwdd->pending_network);
+		if (!iwdn_pending)
+			return;
+
+		cm_network_connect(iwdn_pending->network);
+	}
 }
 
 static void cm_network_disconnect_cb(DBusMessage *message, void *user_data)
@@ -279,6 +350,9 @@ static void cm_network_disconnect_cb(DBusMessage *message, void *user_data)
 	if (!iwdn)
 		return;
 
+	if(iwdn->iwdd)
+		iwdn->iwdd->disconnecting = false;
+
 	if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_ERROR) {
 		const char *dbus_error = dbus_message_get_error_name(message);
 
@@ -303,11 +377,16 @@ static int cm_network_disconnect(struct connman_network *network)
 {
 	struct iwd_network *iwdn = connman_network_get_data(network);
 	struct iwd_station *iwds;
+	struct iwd_device *iwdd;
 
-	if (!iwdn && !iwdn->iwdd)
+	if (!iwdn || !iwdn->iwdd)
 		return -EINVAL;
 
-	iwds = g_hash_table_lookup(stations, iwdn->iwdd->path);
+	iwdd = iwdn->iwdd;
+	if (iwdd->disconnecting)
+		return -EALREADY;
+
+	iwds = g_hash_table_lookup(stations, iwdd->path);
 	if (!iwds)
 		return -EIO;
 
@@ -317,6 +396,8 @@ static int cm_network_disconnect(struct connman_network *network)
 			NULL, cm_network_disconnect_cb, g_strdup(iwdn->path), g_free))
 		return -EIO;
 
+	iwdd->disconnecting = true;
+
 	return 0;
 }
 
@@ -515,6 +596,12 @@ static void device_powered_cb(const DBusError *error, void *user_data)
 	}
 
 	connman_device_set_powered(iwdd->device, cbd->powered);
+
+	if(!cbd->powered) {
+		abort_pending_network(iwdd,
+					CONNMAN_NETWORK_ERROR_CONNECT_FAIL);
+		iwdd->disconnecting = false;
+	}
 out:
 	g_free(cbd->path);
 	g_free(cbd);
@@ -1309,6 +1396,10 @@ static void device_free(gpointer data)
 	g_free(iwdd->adapter);
 	g_free(iwdd->name);
 	g_free(iwdd->address);
+
+	g_free(iwdd->network);
+	g_free(iwdd->pending_network);
+
 	g_free(iwdd);
 }
 
-- 
2.25.1