diff mbox series

[v2,3/5] dpp: explicitly disconnect station if enrollee is started

Message ID 20240722182932.4091008-3-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series [v2,1/5] dpp: factor out PKEX/DPP start prep into function | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-gitlint success GitLint

Commit Message

James Prestwood July 22, 2024, 6:29 p.m. UTC 
Prior to now the DPP state was required to be disconnected before
DPP would start. This is inconvenient for the user since it requires
extra state checking and/or DBus method calls. Instead model this
case like WSC and issue a disconnect to station if DPP is requested
to start.

The other conditions on stopping DPP are also preserved and no
changes to the configurator role have been made, i.e. being
disconnected while configuring still stops DPP. Similarly any
connection made during enrolling will stop DPP.

It should also be noted that station's autoconfigure setting is also
preserved and set back to its original value upon DPP completing.
---
 src/dpp.c | 195 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 143 insertions(+), 52 deletions(-)

Comments

Denis Kenzior July 24, 2024, 2:30 p.m. UTC | #1 
Hi James,

On 7/22/24 1:29 PM, James Prestwood wrote:
> Prior to now the DPP state was required to be disconnected before
> DPP would start. This is inconvenient for the user since it requires
> extra state checking and/or DBus method calls. Instead model this
> case like WSC and issue a disconnect to station if DPP is requested
> to start.
> 
> The other conditions on stopping DPP are also preserved and no
> changes to the configurator role have been made, i.e. being
> disconnected while configuring still stops DPP. Similarly any
> connection made during enrolling will stop DPP.
> 
> It should also be noted that station's autoconfigure setting is also
> preserved and set back to its original value upon DPP completing.
> ---
>   src/dpp.c | 195 +++++++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 143 insertions(+), 52 deletions(-)
> 

<snip>

> @@ -3734,27 +3740,50 @@ static void dpp_frame_watch(struct dpp_sm *dpp, uint16_t frame_type,
>    *     DPP finishes.
>    *
>    * Other conditions shouldn't ever happen i.e. configuring and going into a
> - * connecting state or enrolling and going to a disconnected/roaming state.
> + * connecting state or enrolling and going to a roaming state.
>    */
>   static void dpp_station_state_watch(enum station_state state, void *user_data)
>   {
>   	struct dpp_sm *dpp = user_data;
>   
> -	if (dpp->state == DPP_STATE_NOTHING)
> +	if (dpp->state == DPP_STATE_NOTHING ||
> +					dpp->interface == DPP_INTERFACE_UNBOUND)

Is this check needed?  It looks like you create the state watch after setting 
state and interface accordingly?

>   		return;
>   

<snip>

> @@ -3927,6 +3947,64 @@ static void dpp_start_presence(struct dpp_sm *dpp, uint32_t *limit_freqs,
>   	dpp_start_offchannel(dpp, dpp->current_freq);
>   }
>   
> +static int dpp_disconnect_started(struct dpp_sm *dpp)
> +{
> +	int ret = 0;

Is initialization needed?

> +	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
> +
> +	/* Unusual case, but an enrollee could still be started */
> +	if (!station)
> +		return false;

Function signature returns int, but you're returning false.

> +
> +	dpp->autoconnect = station_get_autoconnect(station);
> +	station_set_autoconnect(station, false);
> +
> +	switch (station_get_state(station)) {
> +	case STATION_STATE_AUTOCONNECT_QUICK:
> +	case STATION_STATE_AUTOCONNECT_FULL:
> +		/* Should never happen since we just set autoconnect false */
> +		l_warn("Still in autoconnect state after setting false!");
> +
> +		/* fall through */
> +	case STATION_STATE_DISCONNECTED:
> +		ret = false;
> +		goto register_watch;
> +	case STATION_STATE_ROAMING:
> +	case STATION_STATE_FT_ROAMING:
> +	case STATION_STATE_FW_ROAMING:
> +	case STATION_STATE_CONNECTING:
> +	case STATION_STATE_CONNECTING_AUTO:
> +	case STATION_STATE_CONNECTED:
> +	case STATION_STATE_NETCONFIG:
> +		/*
> +		 * For any connected or connection in progress state, start a
> +		 * disconnect
> +		 */
> +		ret = station_disconnect(station);
> +		if (ret < 0)
> +			goto error;
> +
> +		/* fall through */
> +	case STATION_STATE_DISCONNECTING:
> +		l_debug("Delaying DPP start until after disconnect");
> +
> +		ret = true;
> +		goto register_watch;
> +	}
> +
> +error:
> +	l_warn("failed to start disconnecting (%d)", ret);
> +	station_set_autoconnect(station, dpp->autoconnect);
> +
> +	return ret;
> +
> +register_watch:
> +	dpp->station_watch = station_add_state_watch(station,
> +						dpp_station_state_watch,
> +						dpp, NULL);
> +	return ret;
> +}
> +
>   static void dpp_start_enrollee(struct dpp_sm *dpp)
>   {
>   	uint32_t freq = band_channel_to_freq(6, BAND_FREQ_2_4_GHZ);
> @@ -3955,27 +4033,27 @@ static struct l_dbus_message *dpp_dbus_start_enrollee(struct l_dbus *dbus,
>   						void *user_data)
>   {
>   	struct dpp_sm *dpp = user_data;
> -	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
> +	int ret;
>   
>   	if (dpp->state != DPP_STATE_NOTHING ||
>   				dpp->interface != DPP_INTERFACE_UNBOUND)
>   		return dbus_error_busy(message);
>   
> -	/*
> -	 * Station isn't actually required for DPP itself, although this will
> -	 * prevent connecting to the network once configured.
> -	 */
> -	if (station && station_get_connected_network(station)) {
> -		l_warn("cannot be enrollee while connected, please disconnect");
> -		return dbus_error_busy(message);
> -	} else if (!station)
> -		l_debug("No station device, continuing anyways...");
> -
>   	dpp->state = DPP_STATE_PRESENCE;
>   	dpp->role = DPP_CAPABILITY_ENROLLEE;
>   	dpp->interface = DPP_INTERFACE_DPP;
>   
> -	dpp_start_enrollee(dpp);
> +	ret = dpp_disconnect_started(dpp);
> +	if (ret < 0) {
> +		dpp_reset(dpp);
> +		return dbus_error_from_errno(ret, message);
> +	} else if (ret == false)
> +		dpp_start_enrollee(dpp);

I'm super confused here

> +
> +	/*
> +	 * If a disconnect was started/in progress the enrollee will start once
> +	 * that has finished
> +	 */
>   
>   	dpp->pending = l_dbus_message_ref(message);
>   

Regards,
-Denis
James Prestwood July 24, 2024, 2:53 p.m. UTC | #2 
Hi Denis,

On 7/24/24 7:30 AM, Denis Kenzior wrote:
> Hi James,
>
> On 7/22/24 1:29 PM, James Prestwood wrote:
>> Prior to now the DPP state was required to be disconnected before
>> DPP would start. This is inconvenient for the user since it requires
>> extra state checking and/or DBus method calls. Instead model this
>> case like WSC and issue a disconnect to station if DPP is requested
>> to start.
>>
>> The other conditions on stopping DPP are also preserved and no
>> changes to the configurator role have been made, i.e. being
>> disconnected while configuring still stops DPP. Similarly any
>> connection made during enrolling will stop DPP.
>>
>> It should also be noted that station's autoconfigure setting is also
>> preserved and set back to its original value upon DPP completing.
>> ---
>>   src/dpp.c | 195 +++++++++++++++++++++++++++++++++++++++---------------
>>   1 file changed, 143 insertions(+), 52 deletions(-)
>>
>
> <snip>
>
>> @@ -3734,27 +3740,50 @@ static void dpp_frame_watch(struct dpp_sm 
>> *dpp, uint16_t frame_type,
>>    *     DPP finishes.
>>    *
>>    * Other conditions shouldn't ever happen i.e. configuring and 
>> going into a
>> - * connecting state or enrolling and going to a disconnected/roaming 
>> state.
>> + * connecting state or enrolling and going to a roaming state.
>>    */
>>   static void dpp_station_state_watch(enum station_state state, void 
>> *user_data)
>>   {
>>       struct dpp_sm *dpp = user_data;
>>   -    if (dpp->state == DPP_STATE_NOTHING)
>> +    if (dpp->state == DPP_STATE_NOTHING ||
>> +                    dpp->interface == DPP_INTERFACE_UNBOUND)
>
> Is this check needed?  It looks like you create the state watch after 
> setting state and interface accordingly?
>
>>           return;
>
> <snip>
>
>> @@ -3927,6 +3947,64 @@ static void dpp_start_presence(struct dpp_sm 
>> *dpp, uint32_t *limit_freqs,
>>       dpp_start_offchannel(dpp, dpp->current_freq);
>>   }
>>   +static int dpp_disconnect_started(struct dpp_sm *dpp)
>> +{
>> +    int ret = 0;
>
> Is initialization needed?
>
>> +    struct station *station = 
>> station_find(netdev_get_ifindex(dpp->netdev));
>> +
>> +    /* Unusual case, but an enrollee could still be started */
>> +    if (!station)
>> +        return false;
>
> Function signature returns int, but you're returning false.

Yep this is intended.

true for a disconnect has started or is ongoing.
false for already disconnected
< 0 for a fatal error (i.e. station_disconnect() failed)

If this isn't desirable we could do:

-EALREADY for a disconnect is started/ongoing
0 for already disconnected
< 0 for a fatal error

We do a somewhat similar technique with wiphy_radio_work_is_running. But 
if we want to keep the return strictly integers/error codes I'm fine 
with that.

>
>> +
>> +    dpp->autoconnect = station_get_autoconnect(station);
>> +    station_set_autoconnect(station, false);
>> +
>> +    switch (station_get_state(station)) {
>> +    case STATION_STATE_AUTOCONNECT_QUICK:
>> +    case STATION_STATE_AUTOCONNECT_FULL:
>> +        /* Should never happen since we just set autoconnect false */
>> +        l_warn("Still in autoconnect state after setting false!");
>> +
>> +        /* fall through */
>> +    case STATION_STATE_DISCONNECTED:
>> +        ret = false;
>> +        goto register_watch;
>> +    case STATION_STATE_ROAMING:
>> +    case STATION_STATE_FT_ROAMING:
>> +    case STATION_STATE_FW_ROAMING:
>> +    case STATION_STATE_CONNECTING:
>> +    case STATION_STATE_CONNECTING_AUTO:
>> +    case STATION_STATE_CONNECTED:
>> +    case STATION_STATE_NETCONFIG:
>> +        /*
>> +         * For any connected or connection in progress state, start a
>> +         * disconnect
>> +         */
>> +        ret = station_disconnect(station);
>> +        if (ret < 0)
>> +            goto error;
>> +
>> +        /* fall through */
>> +    case STATION_STATE_DISCONNECTING:
>> +        l_debug("Delaying DPP start until after disconnect");
>> +
>> +        ret = true;
>> +        goto register_watch;
>> +    }
>> +
>> +error:
>> +    l_warn("failed to start disconnecting (%d)", ret);
>> +    station_set_autoconnect(station, dpp->autoconnect);
>> +
>> +    return ret;
>> +
>> +register_watch:
>> +    dpp->station_watch = station_add_state_watch(station,
>> +                        dpp_station_state_watch,
>> +                        dpp, NULL);
>> +    return ret;
>> +}
>> +
>>   static void dpp_start_enrollee(struct dpp_sm *dpp)
>>   {
>>       uint32_t freq = band_channel_to_freq(6, BAND_FREQ_2_4_GHZ);
>> @@ -3955,27 +4033,27 @@ static struct l_dbus_message 
>> *dpp_dbus_start_enrollee(struct l_dbus *dbus,
>>                           void *user_data)
>>   {
>>       struct dpp_sm *dpp = user_data;
>> -    struct station *station = 
>> station_find(netdev_get_ifindex(dpp->netdev));
>> +    int ret;
>>         if (dpp->state != DPP_STATE_NOTHING ||
>>                   dpp->interface != DPP_INTERFACE_UNBOUND)
>>           return dbus_error_busy(message);
>>   -    /*
>> -     * Station isn't actually required for DPP itself, although this 
>> will
>> -     * prevent connecting to the network once configured.
>> -     */
>> -    if (station && station_get_connected_network(station)) {
>> -        l_warn("cannot be enrollee while connected, please 
>> disconnect");
>> -        return dbus_error_busy(message);
>> -    } else if (!station)
>> -        l_debug("No station device, continuing anyways...");
>> -
>>       dpp->state = DPP_STATE_PRESENCE;
>>       dpp->role = DPP_CAPABILITY_ENROLLEE;
>>       dpp->interface = DPP_INTERFACE_DPP;
>>   -    dpp_start_enrollee(dpp);
>> +    ret = dpp_disconnect_started(dpp);
>> +    if (ret < 0) {
>> +        dpp_reset(dpp);
>> +        return dbus_error_from_errno(ret, message);
>> +    } else if (ret == false)
>> +        dpp_start_enrollee(dpp);
>
> I'm super confused here
>
>> +
>> +    /*
>> +     * If a disconnect was started/in progress the enrollee will 
>> start once
>> +     * that has finished
>> +     */
>>         dpp->pending = l_dbus_message_ref(message);
>
> Regards,
> -Denis
>
Denis Kenzior July 24, 2024, 3:15 p.m. UTC | #3 
Hi James,

>>> +    /* Unusual case, but an enrollee could still be started */
>>> +    if (!station)
>>> +        return false;
>>
>> Function signature returns int, but you're returning false.
> 
> Yep this is intended.
> 
> true for a disconnect has started or is ongoing.
> false for already disconnected
> < 0 for a fatal error (i.e. station_disconnect() failed)
> 

The typical convention is:
-errno -> error
0 - > success

You can extend this to mean that 0...INT_MAX is a success and return value 
simultaneously.  Mixing integers and bools is not what vast majority of people 
would expect and should be avoided.

> If this isn't desirable we could do:
> 
> -EALREADY for a disconnect is started/ongoing
> 0 for already disconnected
> < 0 for a fatal error

That would be better.  Another alternative is:

int dpp_try_disconnect_station(struct dpp_sm *dpp, bool *out_disconnect_started);

Regards,
-Denis
James Prestwood July 24, 2024, 3:17 p.m. UTC | #4 
Hi Denis,

On 7/24/24 8:15 AM, Denis Kenzior wrote:
> Hi James,
>
>>>> +    /* Unusual case, but an enrollee could still be started */
>>>> +    if (!station)
>>>> +        return false;
>>>
>>> Function signature returns int, but you're returning false.
>>
>> Yep this is intended.
>>
>> true for a disconnect has started or is ongoing.
>> false for already disconnected
>> < 0 for a fatal error (i.e. station_disconnect() failed)
>>
>
> The typical convention is:
> -errno -> error
> 0 - > success
>
> You can extend this to mean that 0...INT_MAX is a success and return 
> value simultaneously.  Mixing integers and bools is not what vast 
> majority of people would expect and should be avoided.
>
>> If this isn't desirable we could do:
>>
>> -EALREADY for a disconnect is started/ongoing
>> 0 for already disconnected
>> < 0 for a fatal error
>
> That would be better.  Another alternative is:
>
> int dpp_try_disconnect_station(struct dpp_sm *dpp, bool 
> *out_disconnect_started);
Ok, I'll do one of these instead.
>
> Regards,
> -Denis
diff mbox series

Patch

diff --git a/src/dpp.c b/src/dpp.c
index 6f05aae9..b04477ca 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -193,6 +193,7 @@  struct dpp_sm {
 	bool roc_started : 1;
 	bool channel_switch : 1;
 	bool mutual_auth : 1;
+	bool autoconnect : 1;
 };
 
 static const char *dpp_role_to_string(enum dpp_capability role)
@@ -469,6 +470,8 @@  static void dpp_free_auth_data(struct dpp_sm *dpp)
 
 static void dpp_reset(struct dpp_sm *dpp)
 {
+	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+
 	if (dpp->uri) {
 		l_free(dpp->uri);
 		dpp->uri = NULL;
@@ -549,12 +552,19 @@  static void dpp_reset(struct dpp_sm *dpp)
 	dpp_property_changed_notify(dpp);
 
 	dpp->interface = DPP_INTERFACE_UNBOUND;
+
+	if (station) {
+		if (dpp->station_watch)
+			station_remove_state_watch(station, dpp->station_watch);
+
+		/* Set the old autoconnect state back to what it was */
+		if (dpp->role == DPP_CAPABILITY_ENROLLEE)
+			station_set_autoconnect(station, dpp->autoconnect);
+	}
 }
 
 static void dpp_free(struct dpp_sm *dpp)
 {
-	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
-
 	dpp_reset(dpp);
 
 	if (dpp->own_asn1) {
@@ -572,13 +582,6 @@  static void dpp_free(struct dpp_sm *dpp)
 		dpp->boot_private = NULL;
 	}
 
-	/*
-	 * Since this is called when the netdev goes down, station may already
-	 * be gone in which case the state watch will automatically go away.
-	 */
-	if (station)
-		station_remove_state_watch(station, dpp->station_watch);
-
 	known_networks_watch_remove(dpp->known_network_watch);
 
 	l_free(dpp);
@@ -3721,6 +3724,9 @@  static void dpp_frame_watch(struct dpp_sm *dpp, uint16_t frame_type,
 					L_UINT_TO_PTR(frame_type), NULL);
 }
 
+static void dpp_start_enrollee(struct dpp_sm *dpp);
+static bool dpp_start_pkex_enrollee(struct dpp_sm *dpp);
+
 /*
  * Station is unaware of DPP's state so we need to handle a few cases here so
  * weird stuff doesn't happen:
@@ -3734,27 +3740,50 @@  static void dpp_frame_watch(struct dpp_sm *dpp, uint16_t frame_type,
  *     DPP finishes.
  *
  * Other conditions shouldn't ever happen i.e. configuring and going into a
- * connecting state or enrolling and going to a disconnected/roaming state.
+ * connecting state or enrolling and going to a roaming state.
  */
 static void dpp_station_state_watch(enum station_state state, void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
 
-	if (dpp->state == DPP_STATE_NOTHING)
+	if (dpp->state == DPP_STATE_NOTHING ||
+					dpp->interface == DPP_INTERFACE_UNBOUND)
 		return;
 
 	switch (state) {
 	case STATION_STATE_DISCONNECTED:
+		/* DPP-initiated disconnect, the enrollee can now start */
+		if (dpp->role == DPP_CAPABILITY_ENROLLEE) {
+			l_debug("Starting enrollee after disconnect");
+
+			if (dpp->interface == DPP_INTERFACE_DPP)
+				dpp_start_enrollee(dpp);
+			else
+				dpp_start_pkex_enrollee(dpp);
+
+			return;
+		}
+
+		break;
 	case STATION_STATE_DISCONNECTING:
+		/* Normal part of disconnecting prior to enrolling */
+		if (dpp->role == DPP_CAPABILITY_ENROLLEE)
+			return;
+
+		/* fall through */
 	case STATION_STATE_ROAMING:
 	case STATION_STATE_FT_ROAMING:
 	case STATION_STATE_FW_ROAMING:
+		/*
+		 * An enrollee will always be disconnected prior to starting
+		 * so a roaming condition should never happen.
+		 */
 		if (L_WARN_ON(dpp->role == DPP_CAPABILITY_ENROLLEE))
-			dpp_reset(dpp);
+			goto reset;
 
 		if (dpp->role == DPP_CAPABILITY_CONFIGURATOR) {
 			l_debug("Disconnected while configuring, stopping DPP");
-			dpp_reset(dpp);
+			goto reset;
 		}
 
 		break;
@@ -3762,28 +3791,22 @@  static void dpp_station_state_watch(enum station_state state, void *user_data)
 	case STATION_STATE_CONNECTED:
 	case STATION_STATE_CONNECTING_AUTO:
 	case STATION_STATE_NETCONFIG:
-		if (L_WARN_ON(dpp->role == DPP_CAPABILITY_CONFIGURATOR))
-			dpp_reset(dpp);
-
-		if (dpp->role == DPP_CAPABILITY_ENROLLEE) {
-			l_debug("Connecting while enrolling, stopping DPP");
-			dpp_reset(dpp);
-		}
-
-		break;
-
-	/*
-	 * Autoconnect states are fine for enrollees. This makes it nicer for
-	 * the user since they don't need to explicity Disconnect() to disable
-	 * autoconnect, then re-enable it if DPP fails.
-	 */
 	case STATION_STATE_AUTOCONNECT_FULL:
 	case STATION_STATE_AUTOCONNECT_QUICK:
-		if (L_WARN_ON(dpp->role == DPP_CAPABILITY_CONFIGURATOR))
-			dpp_reset(dpp);
+		/*
+		 * The user could have issued a connection request during
+		 * enrolling, in which case DPP should be canceled. We should
+		 * never hit this case as a configurator as a disconnect would
+		 * need to come prior.
+		 */
+		L_WARN_ON(dpp->role == DPP_CAPABILITY_CONFIGURATOR);
 
-		break;
+		goto reset;
 	}
+
+reset:
+	l_debug("Resetting DPP after station state change (state=%u)", state);
+	dpp_reset(dpp);
 }
 
 static void dpp_create(struct netdev *netdev)
@@ -3793,7 +3816,6 @@  static void dpp_create(struct netdev *netdev)
 	uint8_t dpp_conf_response_prefix[] = { 0x04, 0x0b };
 	uint8_t dpp_conf_request_prefix[] = { 0x04, 0x0a };
 	uint64_t wdev_id = netdev_get_wdev_id(netdev);
-	struct station *station = station_find(netdev_get_ifindex(netdev));
 
 	dpp->netdev = netdev;
 	dpp->state = DPP_STATE_NOTHING;
@@ -3839,8 +3861,6 @@  static void dpp_create(struct netdev *netdev)
 				sizeof(dpp_conf_request_prefix),
 				dpp_handle_config_request_frame, dpp, NULL);
 
-	dpp->station_watch = station_add_state_watch(station,
-					dpp_station_state_watch, dpp, NULL);
 	dpp->known_network_watch = known_networks_watch_add(
 					dpp_known_network_watch, dpp, NULL);
 
@@ -3927,6 +3947,64 @@  static void dpp_start_presence(struct dpp_sm *dpp, uint32_t *limit_freqs,
 	dpp_start_offchannel(dpp, dpp->current_freq);
 }
 
+static int dpp_disconnect_started(struct dpp_sm *dpp)
+{
+	int ret = 0;
+	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+
+	/* Unusual case, but an enrollee could still be started */
+	if (!station)
+		return false;
+
+	dpp->autoconnect = station_get_autoconnect(station);
+	station_set_autoconnect(station, false);
+
+	switch (station_get_state(station)) {
+	case STATION_STATE_AUTOCONNECT_QUICK:
+	case STATION_STATE_AUTOCONNECT_FULL:
+		/* Should never happen since we just set autoconnect false */
+		l_warn("Still in autoconnect state after setting false!");
+
+		/* fall through */
+	case STATION_STATE_DISCONNECTED:
+		ret = false;
+		goto register_watch;
+	case STATION_STATE_ROAMING:
+	case STATION_STATE_FT_ROAMING:
+	case STATION_STATE_FW_ROAMING:
+	case STATION_STATE_CONNECTING:
+	case STATION_STATE_CONNECTING_AUTO:
+	case STATION_STATE_CONNECTED:
+	case STATION_STATE_NETCONFIG:
+		/*
+		 * For any connected or connection in progress state, start a
+		 * disconnect
+		 */
+		ret = station_disconnect(station);
+		if (ret < 0)
+			goto error;
+
+		/* fall through */
+	case STATION_STATE_DISCONNECTING:
+		l_debug("Delaying DPP start until after disconnect");
+
+		ret = true;
+		goto register_watch;
+	}
+
+error:
+	l_warn("failed to start disconnecting (%d)", ret);
+	station_set_autoconnect(station, dpp->autoconnect);
+
+	return ret;
+
+register_watch:
+	dpp->station_watch = station_add_state_watch(station,
+						dpp_station_state_watch,
+						dpp, NULL);
+	return ret;
+}
+
 static void dpp_start_enrollee(struct dpp_sm *dpp)
 {
 	uint32_t freq = band_channel_to_freq(6, BAND_FREQ_2_4_GHZ);
@@ -3955,27 +4033,27 @@  static struct l_dbus_message *dpp_dbus_start_enrollee(struct l_dbus *dbus,
 						void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
-	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+	int ret;
 
 	if (dpp->state != DPP_STATE_NOTHING ||
 				dpp->interface != DPP_INTERFACE_UNBOUND)
 		return dbus_error_busy(message);
 
-	/*
-	 * Station isn't actually required for DPP itself, although this will
-	 * prevent connecting to the network once configured.
-	 */
-	if (station && station_get_connected_network(station)) {
-		l_warn("cannot be enrollee while connected, please disconnect");
-		return dbus_error_busy(message);
-	} else if (!station)
-		l_debug("No station device, continuing anyways...");
-
 	dpp->state = DPP_STATE_PRESENCE;
 	dpp->role = DPP_CAPABILITY_ENROLLEE;
 	dpp->interface = DPP_INTERFACE_DPP;
 
-	dpp_start_enrollee(dpp);
+	ret = dpp_disconnect_started(dpp);
+	if (ret < 0) {
+		dpp_reset(dpp);
+		return dbus_error_from_errno(ret, message);
+	} else if (ret == false)
+		dpp_start_enrollee(dpp);
+
+	/*
+	 * If a disconnect was started/in progress the enrollee will start once
+	 * that has finished
+	 */
 
 	dpp->pending = l_dbus_message_ref(message);
 
@@ -4111,6 +4189,9 @@  static struct l_dbus_message *dpp_start_configurator_common(
 
 	dpp_property_changed_notify(dpp);
 
+	dpp->station_watch = station_add_state_watch(station,
+					dpp_station_state_watch, dpp, NULL);
+
 	l_debug("DPP Start Configurator: %s", dpp->uri);
 
 	reply = l_dbus_message_new_method_return(message);
@@ -4361,7 +4442,7 @@  static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 	struct dpp_sm *dpp = user_data;
 	const char *key;
 	const char *id;
-	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+	int ret;
 
 	l_debug("");
 
@@ -4369,9 +4450,6 @@  static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 				dpp->interface != DPP_INTERFACE_UNBOUND)
 		return dbus_error_busy(message);
 
-	if (station && station_get_connected_network(station))
-		return dbus_error_busy(message);
-
 	if (!dpp_parse_pkex_args(message, &key, &id))
 		goto invalid_args;
 
@@ -4384,8 +4462,18 @@  static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
 	dpp->state = DPP_STATE_PKEX_EXCHANGE;
 	dpp->interface = DPP_INTERFACE_PKEX;
 
-	if (!dpp_start_pkex_enrollee(dpp))
-		goto invalid_args;
+	ret = dpp_disconnect_started(dpp);
+	if (ret < 0) {
+		dpp_reset(dpp);
+		return dbus_error_from_errno(ret, message);
+	} else if (ret == false)
+		if (!dpp_start_pkex_enrollee(dpp))
+			goto invalid_args;
+
+	/*
+	 * If a disconnect was started/in progress the PKEX enrollee will start
+	 * once that has finished
+	 */
 
 	return l_dbus_message_new_method_return(message);
 
@@ -4470,6 +4558,9 @@  static struct l_dbus_message *dpp_start_pkex_configurator(struct dpp_sm *dpp,
 	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
 	dpp_property_changed_notify(dpp);
 
+	dpp->station_watch = station_add_state_watch(station,
+					dpp_station_state_watch, dpp, NULL);
+
 	if (dpp->pkex_key)
 		l_debug("Starting PKEX configurator for single enrollee");
 	else