[PATCH v2 1/6] network: make clearing network blacklist a separate operation

[PATCH v2 1/6] network: make clearing network blacklist a separate operation

[James Prestwood]

This adds a new API network_clear_blacklist() and removes this
functionality from network_connected(). This is done to support BSS
iteration when netconfig is enabled. Since a call to
network_connected() will happen prior to netconfig completing we
cannot clear the blacklist until netconfig has either passed or
failed.
---
 src/network.c | 9 ++++++---
 src/network.h | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/network.c b/src/network.c
index 4602a110..588ea5ae 100644
--- a/src/network.c
+++ b/src/network.c
@@ -168,6 +168,11 @@ static bool network_secret_check_cacheable(void *data, void *user_data)
 	return false;
 }
 
+void network_clear_blacklist(struct network *network)
+{
+	l_queue_clear(network->blacklist, NULL);
+}
+
 void network_connected(struct network *network)
 {
 	enum security security = network_get_security(network);
@@ -198,8 +203,6 @@ void network_connected(struct network *network)
 	l_queue_foreach_remove(network->secrets,
 				network_secret_check_cacheable, network);
 
-	l_queue_clear(network->blacklist, NULL);
-
 	network->provisioning_hidden = false;
 }
 
@@ -207,7 +210,7 @@ void network_disconnected(struct network *network)
 {
 	network_settings_close(network);
 
-	l_queue_clear(network->blacklist, NULL);
+	network_clear_blacklist(network);
 
 	if (network->provisioning_hidden)
 		station_hide_network(network->station, network);
diff --git a/src/network.h b/src/network.h
index 849051dd..061f47ca 100644
--- a/src/network.h
+++ b/src/network.h
@@ -74,6 +74,7 @@ bool network_bss_update(struct network *network, struct scan_bss *bss);
 const char *network_bss_get_path(const struct network *network,
 						const struct scan_bss *bss);
 bool network_bss_list_isempty(struct network *network);
+void network_clear_blacklist(struct network *network);
 
 const char *__network_path_append_bss(const char *network_path,
 					const struct scan_bss *bss);
-- 
2.34.1

[PATCH v2 2/6] station: fix DBus reply for Connect() with netconfig

[James Prestwood]

When netconfig is enabled the DBus reply was being sent in
station_connect_ok(), before netconfig had even started. This would
result in a call to Connect() succeeding from a DBus perspective but
really netconfig still needed to complete before IWD transitioned
to a connected state.

Fixes: 72e7d3ceb83d ("station: Handle NETCONFIG_EVENT_FAILED")
---
 src/station.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

v2:
 * Broke this out into its own commit. I believe this is how the original
   change was supposed to operate i.e. send the reply only once fully
   connected.

diff --git a/src/station.c b/src/station.c
index 2b6a18f8..4180b7a4 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1795,6 +1795,13 @@ static void station_enter_state(struct station *station,
 		periodic_scan_stop(station);
 		break;
 	case STATION_STATE_CONNECTED:
+		if (station->connect_pending) {
+			struct l_dbus_message *reply =
+				l_dbus_message_new_method_return(
+						station->connect_pending);
+			dbus_pending_reply(&station->connect_pending, reply);
+		}
+
 		l_dbus_object_add_interface(dbus,
 					netdev_get_path(station->netdev),
 					IWD_STATION_DIAGNOSTIC_INTERFACE,
@@ -3581,13 +3588,6 @@ static void station_connect_ok(struct station *station)
 
 	l_debug("");
 
-	if (station->connect_pending) {
-		struct l_dbus_message *reply =
-			l_dbus_message_new_method_return(
-						station->connect_pending);
-		dbus_pending_reply(&station->connect_pending, reply);
-	}
-
 	/*
 	 * Get a neighbor report now so future roams can avoid waiting for
 	 * a report at that time
-- 
2.34.1

[PATCH v2 3/6] station: include netconfig as part of the BSS retry logic

[James Prestwood]

A netconfig failure results in a failed connection which restarts
autoconnect and prevents IWD from retrying the connection on any
other BSS's within the network as a whole. When autoconnect restarts
IWD will scan and choose the "best" BSS which is likely the same as
the prior attempt. If that BSS is somehow misconfigured as far as
DHCP goes, it will likely fail indefinitely and in turn cause IWD to
retry indefinitely.

To improve this netconfig has been adopted into the IWD's BSS retry
logic. If netconfig fails this will not result in IWD transitioning
to a disconnected state, and instead the BSS will be network
blacklisted and the next will be tried. Only once all BSS's have been
tried will IWD go into a disconnected state and start autoconnect
over.
---
 src/station.c | 85 +++++++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

v2:
 * Removed network_disconnected() call on failure
 * Utilize network_clear_blacklist() and retain the existing call site
   for network_connected()

diff --git a/src/station.c b/src/station.c
index 4180b7a4..a4c3e7d1 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1795,6 +1795,8 @@ static void station_enter_state(struct station *station,
 		periodic_scan_stop(station);
 		break;
 	case STATION_STATE_CONNECTED:
+		network_clear_blacklist(station->connected_network);
+
 		if (station->connect_pending) {
 			struct l_dbus_message *reply =
 				l_dbus_message_new_method_return(
@@ -2221,6 +2223,26 @@ static void station_early_neighbor_report_cb(struct netdev *netdev, int err,
 				&station->roam_freqs);
 }
 
+static bool station_try_next_bss(struct station *station)
+{
+	struct scan_bss *next;
+	int ret;
+
+	next = network_bss_select(station->connected_network, false);
+
+	if (!next)
+		return false;
+
+	ret = __station_connect_network(station, station->connected_network,
+						next, station->state);
+	if (ret < 0)
+		return false;
+
+	l_debug("Attempting to connect to next BSS "MAC, MAC_STR(next->addr));
+
+	return true;
+}
+
 static bool station_can_fast_transition(struct station *station,
 					struct handshake_state *hs,
 					struct scan_bss *bss)
@@ -2263,28 +2285,26 @@ static bool station_can_fast_transition(struct station *station,
 	return true;
 }
 
-static void station_disconnect_on_error_cb(struct netdev *netdev, bool success,
-					void *user_data)
+static void station_disconnect_on_netconfig_failed(struct netdev *netdev,
+							bool success,
+							void *user_data)
 {
 	struct station *station = user_data;
-	bool continue_autoconnect;
 
-	station_enter_state(station, STATION_STATE_DISCONNECTED);
-
-	continue_autoconnect = station->state == STATION_STATE_CONNECTING_AUTO;
+	if (station_try_next_bss(station))
+		return;
 
-	if (continue_autoconnect) {
-		if (station_autoconnect_next(station) < 0) {
-			l_debug("Nothing left on autoconnect list");
-			station_enter_state(station,
-					STATION_STATE_AUTOCONNECT_FULL);
-		}
+	if (station->connect_pending) {
+		struct l_dbus_message *reply = dbus_error_failed(
+						station->connect_pending);
 
-		return;
+		dbus_pending_reply(&station->connect_pending, reply);
 	}
 
-	if (station->autoconnect)
-		station_enter_state(station, STATION_STATE_AUTOCONNECT_QUICK);
+	station_reset_connection_state(station);
+
+	station_enter_state(station, STATION_STATE_DISCONNECTED);
+	station_enter_state(station, STATION_STATE_AUTOCONNECT_FULL);
 }
 
 static void station_netconfig_event_handler(enum netconfig_event event,
@@ -2297,23 +2317,20 @@ static void station_netconfig_event_handler(enum netconfig_event event,
 		station_enter_state(station, STATION_STATE_CONNECTED);
 		break;
 	case NETCONFIG_EVENT_FAILED:
-		if (station->connect_pending) {
-			struct l_dbus_message *reply = dbus_error_failed(
-						station->connect_pending);
+		station_debug_event(station, "netconfig-failed");
 
-			dbus_pending_reply(&station->connect_pending, reply);
-		}
+		netconfig_reset(station->netconfig);
 
 		if (station->state == STATION_STATE_NETCONFIG)
 			network_connect_failed(station->connected_network,
 						false);
 
+		network_blacklist_add(station->connected_network,
+						station->connected_bss);
+
 		netdev_disconnect(station->netdev,
-					station_disconnect_on_error_cb,
+					station_disconnect_on_netconfig_failed,
 					station);
-		station_reset_connection_state(station);
-
-		station_enter_state(station, STATION_STATE_DISCONNECTING);
 		break;
 	default:
 		l_error("station: Unsupported netconfig event: %d.", event);
@@ -3416,26 +3433,6 @@ static void station_event_channel_switched(struct station *station,
 	network_bss_update(network, station->connected_bss);
 }
 
-static bool station_try_next_bss(struct station *station)
-{
-	struct scan_bss *next;
-	int ret;
-
-	next = network_bss_select(station->connected_network, false);
-
-	if (!next)
-		return false;
-
-	ret = __station_connect_network(station, station->connected_network,
-						next, station->state);
-	if (ret < 0)
-		return false;
-
-	l_debug("Attempting to connect to next BSS "MAC, MAC_STR(next->addr));
-
-	return true;
-}
-
 static bool station_retry_owe_default_group(struct station *station)
 {
 	/*
-- 
2.34.1

[PATCH v2 4/6] auto-t: allow configurable DBus timeout/callbacks on connect{_bssid}

[James Prestwood]

Let the caller specify the method timeout if there is an expectation
that it could take a long time.

For the conventional connect call (not the "bssid" debug variant) let
them pass their own callback handlers. This is useful if we don't
want to wait for the connect call to finish, but later get some
indication that it did finish either successfully or not.
---
 autotests/util/iwd.py | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index 9091807a..37eb4943 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -112,8 +112,8 @@ class AsyncOpAbstract(object):
         self._is_completed = True
         self._exception = _convert_dbus_ex(ex)
 
-    def _wait_for_async_op(self):
-        ctx.non_block_wait(lambda s: s._is_completed, 30, self, exception=None)
+    def _wait_for_async_op(self, timeout=50):
+        ctx.non_block_wait(lambda s: s._is_completed, timeout, self, exception=None)
 
         self._is_completed = False
         if self._exception is not None:
@@ -280,8 +280,15 @@ class StationDebug(IWDDBusAbstract):
     def autoconnect(self):
         return self._properties['AutoConnect']
 
-    def connect_bssid(self, address):
-        self._iface.ConnectBssid(dbus.ByteArray.fromhex(address.replace(':', '')))
+    def connect_bssid(self, address, wait=True):
+        self._iface.ConnectBssid(
+            dbus.ByteArray.fromhex(address.replace(':', '')),
+            reply_handler=self._success,
+            error_handler=self._failure
+        )
+
+        if wait:
+            self._wait_for_async_op()
 
     def roam(self, address):
         self._iface.Roam(dbus.ByteArray.fromhex(address.replace(':', '')))
@@ -870,8 +877,8 @@ class Device(IWDDBusAbstract):
     def stop_adhoc(self):
         self._prop_proxy.Set(IWD_DEVICE_INTERFACE, 'Mode', 'station')
 
-    def connect_bssid(self, address):
-        self._station_debug.connect_bssid(address)
+    def connect_bssid(self, address, wait=True):
+        self._station_debug.connect_bssid(address, wait=wait)
 
     def roam(self, address):
         self._station_debug.roam(address)
@@ -999,7 +1006,7 @@ class Network(IWDDBusAbstract):
     def extended_service_set(self):
         return self._properties['ExtendedServiceSet']
 
-    def connect(self, wait=True):
+    def connect(self, wait=True, timeout=50, reply_handler=None, error_handler=None):
         '''
             Connect to the network. Request the device implied by the object
             path to connect to specified network.
@@ -1014,12 +1021,19 @@ class Network(IWDDBusAbstract):
             @rtype: void
         '''
 
+        if not reply_handler:
+            reply_handler = self._success
+
+        if not error_handler:
+            error_handler = self._failure
+
         self._iface.Connect(dbus_interface=self._iface_name,
-                            reply_handler=self._success,
-                            error_handler=self._failure)
+                            reply_handler=reply_handler,
+                            error_handler=error_handler,
+                            timeout=timeout)
 
         if wait:
-            self._wait_for_async_op()
+            self._wait_for_async_op(timeout=timeout)
 
     def __str__(self, prefix = ''):
         return prefix + 'Network:\n' \
-- 
2.34.1

[PATCH v2 5/6] auto-t: update several tests to work with netconfig refactor

[James Prestwood]

Since the method return to Connect() and ConnectBssid() come after
netconfig some tests needed to be updated since they were waiting
for the method return before continuing. For timeout-based tests
specifically this caused them to fail since before they expected
the return to come before the connection was actually completed.
---
 autotests/testNetconfig/static_test.py        | 15 +++-----
 autotests/testNetconfig/timeout_test.py       | 35 +++++++++++++------
 .../testNetconfigRoam/netconfig_roam_test.py  |  2 +-
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/autotests/testNetconfig/static_test.py b/autotests/testNetconfig/static_test.py
index 94307a8c..61037e0e 100644
--- a/autotests/testNetconfig/static_test.py
+++ b/autotests/testNetconfig/static_test.py
@@ -91,16 +91,11 @@ class Test(unittest.TestCase):
         # using the same static config.  The new client's ACD client should
         # detect an IP conflict and not allow the device to reach the
         # "connected" state although the DBus .Connect call will succeed.
-        ordered_network.network_object.connect()
-        self.assertEqual(dev2.state, iwd.DeviceState.connecting)
-        try:
-            # We should either stay in "connecting" indefinitely or move to
-            # "disconnecting"
-            condition = 'obj.state != DeviceState.connecting'
-            iwd_ns0_1.wait_for_object_condition(dev2, condition, max_wait=21)
-            self.assertEqual(dev2.state, iwd.DeviceState.disconnecting)
-        except TimeoutError:
-            dev2.disconnect()
+        with self.assertRaises(iwd.FailedEx):
+            ordered_network.network_object.connect(timeout=500)
+
+        condition = 'obj.state == DeviceState.disconnected'
+        iwd_ns0_1.wait_for_object_condition(dev2, condition, max_wait=21)
 
         iwd_ns0_1.unregister_psk_agent(psk_agent_ns0_1)
         del dev2
diff --git a/autotests/testNetconfig/timeout_test.py b/autotests/testNetconfig/timeout_test.py
index a15706e3..00b03df4 100644
--- a/autotests/testNetconfig/timeout_test.py
+++ b/autotests/testNetconfig/timeout_test.py
@@ -8,6 +8,8 @@ from iwd import PSKAgent
 from iwd import NetworkType
 
 class Test(unittest.TestCase):
+    def connect_failure(self, ex):
+        self.failure_triggered = True
 
     def test_netconfig_timeout(self):
         IWD.copy_to_storage('autoconnect.psk', name='ap-ns1.psk')
@@ -27,23 +29,34 @@ class Test(unittest.TestCase):
         condition = 'not obj.connected'
         wd.wait_for_object_condition(ordered_network.network_object, condition)
 
-        ordered_network.network_object.connect()
+        self.failure_triggered = False
 
-        condition = 'obj.state == DeviceState.connecting'
-        wd.wait_for_object_condition(device, condition)
+        # Set our error handler here so we can check if it fails
+        ordered_network.network_object.connect(
+            wait=False,
+            timeout=1000,
+            error_handler=self.connect_failure
+        )
+
+        # IWD should attempt to try both BSS's with both failing netconfig.
+        # Then the autoconnect list should be exhausted, and IWD should
+        # transition to a disconnected state, then proceed to full autoconnect.
+        device.wait_for_event("netconfig-failed", timeout=1000)
+        device.wait_for_event("netconfig-failed", timeout=1000)
+        device.wait_for_event("disconnected")
 
-        device.wait_for_event("connecting (netconfig)")
+        device.wait_for_event("autoconnect_full")
 
-        # Netconfig should fail, and IWD should disconnect
-        from_condition = 'obj.state == DeviceState.connecting'
-        to_condition = 'obj.state == DeviceState.disconnecting'
-        wd.wait_for_object_change(device, from_condition, to_condition, max_wait=60)
+        # The connect call should have failed
+        self.assertTrue(self.failure_triggered)
 
-        # Autoconnect should then try again
-        condition = 'obj.state == DeviceState.connecting'
+        condition = "obj.scanning"
+        wd.wait_for_object_condition(device, condition)
+        condition = "not obj.scanning"
         wd.wait_for_object_condition(device, condition)
 
-        device.wait_for_event("connecting (netconfig)")
+        # IWD should attempt to connect, but it will of course fail again.
+        device.wait_for_event("netconfig-failed", timeout=1000)
 
         device.disconnect()
         condition = 'obj.state == DeviceState.disconnected'
diff --git a/autotests/testNetconfigRoam/netconfig_roam_test.py b/autotests/testNetconfigRoam/netconfig_roam_test.py
index 63e5eabf..b7a7012f 100644
--- a/autotests/testNetconfigRoam/netconfig_roam_test.py
+++ b/autotests/testNetconfigRoam/netconfig_roam_test.py
@@ -19,7 +19,7 @@ class Test(unittest.TestCase):
 
         device = wd.list_devices(1)[0]
         device.get_ordered_network('TestFT', full_scan=True)
-        device.connect_bssid(self.bss_hostapd[1].bssid)
+        device.connect_bssid(self.bss_hostapd[1].bssid, wait=False)
 
         self.bss_hostapd[1].wait_for_event(f'AP-STA-CONNECTED {device.address}')
         device.wait_for_event("connecting (netconfig)")
-- 
2.34.1

[PATCH v2 6/6] doc: add note about timeouts to Network.Connect()

[James Prestwood]

Since netconfig is now part of the Connect() call from a DBus
perspective add a note indicating that this method has the potential
to take a very long time if there are issues with DHCP.
---
 doc/network-api.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/network-api.txt b/doc/network-api.txt
index 8bc6eea6..d96fcede 100644
--- a/doc/network-api.txt
+++ b/doc/network-api.txt
@@ -11,6 +11,12 @@ Methods		void Connect()
 			the object path to connect to specified network.
 			Connecting to WEP networks is not supported.
 
+			Note: When [General].EnableNetworkConfiguration is set
+			to true a call to Connect() has the potential to take
+			a significant amount of time. Specifically if DHCP is
+			either slow, or is unable to complete. The timeout for
+			DHCP is roughly 30 seconds per BSS.
+
 			Possible errors: net.connman.iwd.Aborted
 					 net.connman.iwd.Busy
 					 net.connman.iwd.Failed
-- 
2.34.1

Re: [PATCH v2 1/6] network: make clearing network blacklist a separate operation

[Denis Kenzior]

Hi James,

On 5/28/25 3:10 PM, James Prestwood wrote:
> This adds a new API network_clear_blacklist() and removes this
> functionality from network_connected(). This is done to support BSS
> iteration when netconfig is enabled. Since a call to
> network_connected() will happen prior to netconfig completing we
> cannot clear the blacklist until netconfig has either passed or
> failed.
> ---
>   src/network.c | 9 ++++++---
>   src/network.h | 1 +
>   2 files changed, 7 insertions(+), 3 deletions(-)
> 

All applied, thanks.

Regards,
-Denis