Patchwork [v2] P2P: Ignore the DEAUTH event from cfg80211 incase of locally generated disconnect

login
register
mail settings
Submitter Jithu Jance
Date May 29, 2013, 12:58 p.m.
Message ID <1369832304.2580.33.camel@lbblr-jithu01.broadcom.com>
Download mbox | patch
Permalink /patch/247263/
State Superseded
Headers show

Comments

Jithu Jance - May 29, 2013, 12:58 p.m.
Hi Jouni,
 
> > Is it safe to ignore this DEAUTH event since the
> > wpa_supplicant_deathenticate function already generated a local DEAUTH
> > event?
> 
> Yes and that is already done with some drivers.

>  Actually even better would be to just handle this
> automatically within driver_nl80211.c. It should be clear there that
> cfg80211 will send a disconnection event after disconnection request.

Would the below patch be sufficient enough?

[PATCH] Ignore the Disconnect event that is generated as a result of
 local disconnect 
hostap: Jithu Jance <jithu@broadcom.com>

---
 src/drivers/driver_nl80211.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

 
@@ -4905,8 +4908,10 @@ static int
wpa_driver_nl80211_deauthenticate(struct i802_bss *bss,
 					     const u8 *addr, int reason_code)
 {
 	struct wpa_driver_nl80211_data *drv = bss->drv;
-	if (!(drv->capa.flags & WPA_DRIVER_FLAGS_SME))
+	if (!(drv->capa.flags & WPA_DRIVER_FLAGS_SME)) {
+		drv->ignore_next_local_disconnect = 1;
 		return wpa_driver_nl80211_disconnect(drv, reason_code);
+	}
 	wpa_printf(MSG_DEBUG, "%s(addr=" MACSTR " reason_code=%d)",
 		   __func__, MAC2STR(addr), reason_code);
 	nl80211_mark_disconnected(drv);
Jouni Malinen - May 29, 2013, 9:38 p.m.
On Wed, May 29, 2013 at 06:28:24PM +0530, jithu Jance wrote:
> Would the below patch be sufficient enough?

It may be. I need to do some experiments to see if I can come up with
cases where this would be an issue.

> [PATCH] Ignore the Disconnect event that is generated as a result of
>  local disconnect 
> hostap: Jithu Jance <jithu@broadcom.com>

Could you please repost with "Signed-hostap" tag?

> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> @@ -4892,12 +4892,15 @@ nla_put_failure:
> +	int ret = 0;

No need to initialize ret here.

> -	drv->ignore_next_local_disconnect = 0;
>  	/* Disconnect command doesn't need BSSID - it uses cached value */
> -	return wpa_driver_nl80211_mlme(drv, NULL, NL80211_CMD_DISCONNECT,
> -				       reason_code, 0);
> +	if ((ret = wpa_driver_nl80211_mlme(drv, NULL, NL80211_CMD_DISCONNECT,
> +				       reason_code, 0)) < 0)
> +		drv->ignore_next_local_disconnect = 0;

I'd prefer following style:
    ret = wpa_driver....;
    if (ret < 0)
	drv->ignore_next...;

> @@ -4905,8 +4908,10 @@ static int
> wpa_driver_nl80211_deauthenticate(struct i802_bss *bss,
>  					     const u8 *addr, int reason_code)
>  {
>  	struct wpa_driver_nl80211_data *drv = bss->drv;
> -	if (!(drv->capa.flags & WPA_DRIVER_FLAGS_SME))
> +	if (!(drv->capa.flags & WPA_DRIVER_FLAGS_SME)) {
> +		drv->ignore_next_local_disconnect = 1;
>  		return wpa_driver_nl80211_disconnect(drv, reason_code);
> +	}

Would be a good to add a comment here describing why this is needed.

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index f403189..0dee35a 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -4892,12 +4892,15 @@  nla_put_failure:
 static int wpa_driver_nl80211_disconnect(struct wpa_driver_nl80211_data
*drv,
 					 int reason_code)
 {
+	int ret = 0;
 	wpa_printf(MSG_DEBUG, "%s(reason_code=%d)", __func__, reason_code);
 	nl80211_mark_disconnected(drv);
-	drv->ignore_next_local_disconnect = 0;
 	/* Disconnect command doesn't need BSSID - it uses cached value */
-	return wpa_driver_nl80211_mlme(drv, NULL, NL80211_CMD_DISCONNECT,
-				       reason_code, 0);
+	if ((ret = wpa_driver_nl80211_mlme(drv, NULL, NL80211_CMD_DISCONNECT,
+				       reason_code, 0)) < 0)
+		drv->ignore_next_local_disconnect = 0;
+
+	return ret;
 }