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

[Jithu Jance]

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 at 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 at 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 at 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.
> >