diff mbox

[1/2] Refactor nl80211 mode switch logic

Message ID 1403813587-13731-1-git-send-email-wiley@chromium.org
State Accepted
Headers show

Commit Message

Christopher Wiley June 26, 2014, 8:13 p.m. UTC 
In preparation for another wrinkle around switching into IBSS mode,
refactor existing mode switch logic for simplicity at the expense
of some brevity.

Signed-off-by: Christopher Wiley <wiley@chromium.org>
---
 src/drivers/driver_nl80211.c | 45 ++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

Comments

Jouni Malinen June 28, 2014, 7:49 a.m. UTC | #1 
On Thu, Jun 26, 2014 at 01:13:06PM -0700, Christopher Wiley wrote:
> In preparation for another wrinkle around switching into IBSS mode,
> refactor existing mode switch logic for simplicity at the expense
> of some brevity.

> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> @@ -9071,24 +9072,24 @@ static int wpa_driver_nl80211_set_mode(struct i802_bss *bss,
>  	wpa_printf(MSG_DEBUG, "nl80211: Try mode change after setting "
>  		   "interface down");
>  	for (i = 0; i < 10; i++) {
> +		os_sleep(0, 100000);
>  		res = i802_set_iface_flags(bss, 0);

This part of moving the delay to happen unconditionally before the first
attempt at setting the interface down does not look desired. This would
block all wpa_supplicant operations for 100 ms in a case that is common
with number of drivers. I'm considering this patch with the delay moved
back to after the failure cases:

> -		} else
> +		if (res != 0) {
>  			wpa_printf(MSG_DEBUG, "nl80211: Failed to set "
>  				   "interface down");
> -		os_sleep(0, 100000);
> +			continue;
> +		}

i.e., just before the continue here

> +		/* Try to set the mode again while the interface is down */
> +		mode_switch_res = nl80211_set_mode(drv, drv->ifindex, nlmode);
> +		if (mode_switch_res == -EBUSY) {
> +			wpa_printf(MSG_DEBUG, "nl80211: Delaying mode set "
> +				   "while interface going down.");
> +			continue;
> +		}

and here
Jouni Malinen June 29, 2014, 1:28 p.m. UTC | #2 
On Thu, Jun 26, 2014 at 01:13:06PM -0700, Christopher Wiley wrote:
> In preparation for another wrinkle around switching into IBSS mode,
> refactor existing mode switch logic for simplicity at the expense
> of some brevity.

Thanks, both patches applied.
diff mbox

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index c154ec2..0f4b37c 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -9043,18 +9043,19 @@  static int wpa_driver_nl80211_set_mode(struct i802_bss *bss,
 	int i;
 	int was_ap = is_ap_interface(drv->nlmode);
 	int res;
+	int mode_switch_res;
 
-	res = nl80211_set_mode(drv, drv->ifindex, nlmode);
-	if (res && nlmode == nl80211_get_ifmode(bss))
-		res = 0;
+	mode_switch_res = nl80211_set_mode(drv, drv->ifindex, nlmode);
+	if (mode_switch_res && nlmode == nl80211_get_ifmode(bss))
+		mode_switch_res = 0;
 
-	if (res == 0) {
+	if (mode_switch_res == 0) {
 		drv->nlmode = nlmode;
 		ret = 0;
 		goto done;
 	}
 
-	if (res == -ENODEV)
+	if (mode_switch_res == -ENODEV)
 		return -1;
 
 	if (nlmode == drv->nlmode) {
@@ -9071,24 +9072,24 @@  static int wpa_driver_nl80211_set_mode(struct i802_bss *bss,
 	wpa_printf(MSG_DEBUG, "nl80211: Try mode change after setting "
 		   "interface down");
 	for (i = 0; i < 10; i++) {
+		os_sleep(0, 100000);
 		res = i802_set_iface_flags(bss, 0);
 		if (res == -EACCES || res == -ENODEV)
 			break;
-		if (res == 0) {
-			/* Try to set the mode again while the interface is
-			 * down */
-			ret = nl80211_set_mode(drv, drv->ifindex, nlmode);
-			if (ret == -EACCES)
-				break;
-			res = i802_set_iface_flags(bss, 1);
-			if (res && !ret)
-				ret = -1;
-			else if (ret != -EBUSY)
-				break;
-		} else
+		if (res != 0) {
 			wpa_printf(MSG_DEBUG, "nl80211: Failed to set "
 				   "interface down");
-		os_sleep(0, 100000);
+			continue;
+		}
+		/* Try to set the mode again while the interface is down */
+		mode_switch_res = nl80211_set_mode(drv, drv->ifindex, nlmode);
+		if (mode_switch_res == -EBUSY) {
+			wpa_printf(MSG_DEBUG, "nl80211: Delaying mode set "
+				   "while interface going down.");
+			continue;
+		}
+		ret = mode_switch_res;
+		break;
 	}
 
 	if (!ret) {
@@ -9098,6 +9099,14 @@  static int wpa_driver_nl80211_set_mode(struct i802_bss *bss,
 		drv->ignore_if_down_event = 1;
 	}
 
+	/* Bring the interface back up */
+	res = linux_set_iface_flags(drv->global->ioctl_sock, bss->ifname, 1);
+	if (res != 0) {
+		wpa_printf(MSG_DEBUG, "nl80211: Failed to set "
+			   "interface up after switching mode.");
+		ret = -1;
+	}
+
 done:
 	if (ret) {
 		wpa_printf(MSG_DEBUG, "nl80211: Interface mode change to %d "