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

login
register
mail settings
Submitter Jithu Jance
Date May 30, 2013, 3:09 a.m.
Message ID <1369883387.2580.38.camel@lbblr-jithu01.broadcom.com>
Download mbox | patch
Permalink /patch/247446/
State Accepted
Commit 3f53c006c7d7362cf715ceaeda92c69d91ea7b63
Headers show

Comments

Jithu Jance - May 30, 2013, 3:09 a.m.
Hi Jouni,

Thanks for your suggestions. I have made the changes accordingly. Please
find the patch below.


Signed-hostap: Jithu Jance <jithu@broadcom.com>

---
 src/drivers/driver_nl80211.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

 
@@ -4905,8 +4908,14 @@ 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)) {
+		/*
+		 * For locally generated disconnect, supplicant already
+		 * generates a DEAUTH event. So ignore the event from NL80211.
+		 */
+		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);
Jithu Jance - June 11, 2013, 1:25 p.m.
Hi Jouni,

> 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.
Did you get a chance to check this?


Thanks,

Jithu

> -----Original Message-----
> From: jithu Jance [mailto:jithu@broadcom.com]
> Sent: Thursday, May 30, 2013 8:40 AM
> To: Jouni Malinen
> Cc: hostap
> Subject: Re: [PATCH v3] P2P: Ignore the DEAUTH event from cfg80211 incase
> of locally generated disconnect
> 
> Hi Jouni,
> 
> Thanks for your suggestions. I have made the changes accordingly. Please
> find the patch below.
> 
> 
> Signed-hostap: Jithu Jance <jithu@broadcom.com>
> 
> ---
>  src/drivers/driver_nl80211.c |   15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index f403189..4f7d7e3 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;
> +
>  	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,
> +	ret = wpa_driver_nl80211_mlme(drv, NULL,
> NL80211_CMD_DISCONNECT,
>  				       reason_code, 0);
> +	if (ret)
> +		drv->ignore_next_local_disconnect = 0;
>  }
> 
> 
> @@ -4905,8 +4908,14 @@ 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)) {
> +		/*
> +		 * For locally generated disconnect, supplicant already
> +		 * generates a DEAUTH event. So ignore the event from
> NL80211.
> +		 */
> +		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);
> --
> 1.7.9.5
> 
> 
> 
> Thanks,
> 
> Jithu Jance
> 
> 
> 
> 
> 
> On Thu, 2013-05-30 at 00:38 +0300, Jouni Malinen wrote:
> > 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.
> >
Jouni Malinen - July 20, 2013, 1:12 p.m.
On Thu, May 30, 2013 at 08:39:47AM +0530, jithu Jance wrote:
> Thanks for your suggestions. I have made the changes accordingly. Please
> find the patch below.

Thanks, applied with some changes:

> diff --git 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

>  	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,
> +	ret = wpa_driver_nl80211_mlme(drv, NULL, NL80211_CMD_DISCONNECT,
>  				       reason_code, 0);
> +	if (ret)
> +		drv->ignore_next_local_disconnect = 0;
>  }

This needs to end with "return ret" to avoid compiler warnings and
semi-random behavior on error code.

> 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)) {
> +		/*
> +		 * For locally generated disconnect, supplicant already
> +		 * generates a DEAUTH event. So ignore the event from NL80211.
> +		 */
> +		drv->ignore_next_local_disconnect = 1;
>  		return wpa_driver_nl80211_disconnect(drv, reason_code);
> +	}

I moved ignore_next_local_disconnect setting into
wpa_driver_nl80211_disconnect() so that this is more consistent between
both callers.

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index f403189..4f7d7e3 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;
+
 	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,
+	ret = wpa_driver_nl80211_mlme(drv, NULL, NL80211_CMD_DISCONNECT,
 				       reason_code, 0);
+	if (ret)
+		drv->ignore_next_local_disconnect = 0;
 }