| Message ID | 1369883387.2580.38.camel@lbblr-jithu01.broadcom.com |
|---|---|
| State | Accepted |
| Commit | 3f53c006c7d7362cf715ceaeda92c69d91ea7b63 |
| Headers | show |
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. > >
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.
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; }
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);