dpp: fix incorrect offchannel usage as configurator

Commit Message

James Prestwood April 25, 2023, 8:57 p.m. UTC

When acting as a configurator the enrollee can start on a different
channel than IWD is connected to. IWD will begin the auth process
on this channel but tell the enrollee to transition to the current
channel after the auth request. Since a configurator must be
connected (a requirement IWD enforces) we can assume a channel
transition will always be to the currently connected channel. This
allows us to simply cancel the offchannel request and wait for a
response (rather than start another offchannel).

Doing this improves the DPP performance and reduces the potential
for a lost frame during the channel transition.

This patch also addresses the comment that we should wait for the
auth request ACK before canceling the offchannel. Now a flag is
set and IWD will cancel the offchannel once the ACK is received.
---
 src/dpp.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

Comments

Denis Kenzior April 30, 2023, 6:04 p.m. UTC | #1

Hi James,

On 4/25/23 15:57, James Prestwood wrote:
> When acting as a configurator the enrollee can start on a different
> channel than IWD is connected to. IWD will begin the auth process
> on this channel but tell the enrollee to transition to the current
> channel after the auth request. Since a configurator must be
> connected (a requirement IWD enforces) we can assume a channel
> transition will always be to the currently connected channel. This
> allows us to simply cancel the offchannel request and wait for a
> response (rather than start another offchannel).
> 
> Doing this improves the DPP performance and reduces the potential
> for a lost frame during the channel transition.
> 
> This patch also addresses the comment that we should wait for the
> auth request ACK before canceling the offchannel. Now a flag is
> set and IWD will cancel the offchannel once the ACK is received.
> ---
>   src/dpp.c | 36 +++++++++++++++++++++++++++++++-----
>   1 file changed, 31 insertions(+), 5 deletions(-)
> 

I re-flowed the logic in this patch a little bit (dpp_mlme_notify).  Please 
check that you agree / I didn't screw anything up.

Applied, thanks.

Regards,
-Denis

Patch

diff --git a/src/dpp.c b/src/dpp.c
index 6abf539a..b1f4893e 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -147,6 +147,7 @@  struct dpp_sm {
 
 	bool mcast_support : 1;
 	bool roc_started : 1;
+	bool channel_switch : 1;
 };
 
 static bool dpp_get_started(struct l_dbus *dbus,
@@ -2032,6 +2033,7 @@  static void authenticate_response(struct dpp_sm *dpp, const uint8_t *from,
 		return;
 	}
 
+	dpp->channel_switch = false;
 	dpp->current_freq = dpp->new_freq;
 
 	dpp_send_authenticate_confirm(dpp);
@@ -2112,10 +2114,14 @@  static void dpp_handle_presence_announcement(struct dpp_sm *dpp,
 		return;
 
 	/*
-	 * Should we wait for an ACK then go offchannel?
+	 * Requested the peer to move to another channel for the remainder of
+	 * the protocol. IWD's current logic prohibits a configurator from
+	 * running while not connected, so we can assume here that the new
+	 * frequency is the same of the connected BSS. Wait until an ACK is
+	 * received for the auth request then cancel the offchannel request.
 	 */
 	if (dpp->current_freq != dpp->new_freq)
-		dpp_start_offchannel(dpp, dpp->new_freq);
+		dpp->channel_switch = true;
 }
 
 static void dpp_handle_frame(struct dpp_sm *dpp,
@@ -2211,12 +2217,32 @@  static void dpp_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	if (dpp->state <= DPP_STATE_PRESENCE)
 		return;
 
+
+	if (dpp->frame_cookie != cookie)
+		return;
+
 	/*
-	 * Only want to handle the no-ACK case. Re-transmitting an ACKed
-	 * frame likely wont do any good, at least in the case of DPP.
+	 * Special handling for a channel transition when acting as a
+	 * configurator. The auth request was sent offchannel so we need to
+	 * wait for the ACK before going back to the connected channel.
 	 */
-	if (dpp->frame_cookie != cookie || ack)
+	if (dpp->channel_switch && ack) {
+		if (dpp->offchannel_id) {
+			offchannel_cancel(dpp->wdev_id, dpp->offchannel_id);
+			dpp->offchannel_id = 0;
+		}
+
+		dpp->channel_switch = false;
+
 		return;
+	} else if (ack)
+		/*
+		 * Only want to handle the no-ACK case. Re-transmitting an ACKed
+		 * frame likely wont do any good, at least in the case of DPP.
+		 */
+		return;
+
+
 
 	if (dpp->frame_retry > DPP_FRAME_MAX_RETRIES) {
 		dpp_reset(dpp);