| Message ID | fa542692f791312bdf7ede16e68f7879315a2f5b.1467614677.git.jithu@broadcom.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Aug 16, 2016 at 10:43:44AM +0530, Jithu Jance wrote: > For "device_ap_sme" devices, the ap_sta_disconnect call in > supplicant results in two calls to wpa_driver_nl80211_sta_remove. > > ap_sta_disconnect > hostapd_drv_sta_deauth > wpa_driver_nl80211_sta_remove > ap_sta_disconnect > ap_sta_deauth_cb_timeout > ap_sta_remove > hostapd_drv_sta_remove > > The ap_sta_deauth_cb_timeout is invoked immediately (timeout of [0,0]) for > device_ap_sme devices. The hostapd_drv_sta_deauth call can be avoided > for devices without WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS set. Hmm.. That does indeed seem to be the case, but it should be noted that there is a difference in calling hostapd_drv_sta_remove() and hostapd_drv_sta_deauth(): the former does not pass the reason code to the driver wrapper while the latter does. > diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c > @@ -1198,7 +1198,8 @@ void ap_sta_disconnect(struct hostapd_data *hapd, struct sta_info *sta, > if (sta == NULL && addr) > sta = ap_get_sta(hapd, addr); > > - if (addr) > + if (addr && (hapd->iface->drv_flags & > + WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS)) > hostapd_drv_sta_deauth(hapd, addr, reason); > > if (sta == NULL) This is the former case and if this is made conditional, all drivers that do not set WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS would lose the reason code when they get only the second call from ap_sta_remove(). I don't think this is acceptable. In addition, the sta == NULL case would return from ap_sta_disconnect() without even registering the ap_sta_disassoc_cb_timeout() callback at all. That does not sound correct either, i.e., this condition on skipping the hostapd_drv_sta_deauth() call should likely apply only if sta != NULL. For the reason code disappearing issue, one could consider extending hostapd_drv_sta_remove() support passing a reason code to the driver, but I'm not really sure this is the correct thing to do.. In other words, I think I'd rather leave this as-is. Other than debug logs showing some warnings, are there any real issues noticeable by external devices that this patch is fixing?
On Sun, Sep 18, 2016 at 12:34 AM, Jouni Malinen <j@w1.fi> wrote: > > This is the former case and if this is made conditional, all drivers > that do not set WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS would lose the reason > code when they get only the second call from ap_sta_remove(). I don't > think this is acceptable. > > In addition, the sta == NULL case would return from ap_sta_disconnect() > without even registering the ap_sta_disassoc_cb_timeout() callback at > all. That does not sound correct either, i.e., this condition on > skipping the hostapd_drv_sta_deauth() call should likely apply only if > sta != NULL. > > For the reason code disappearing issue, one could consider extending > hostapd_drv_sta_remove() support passing a reason code to the driver, > but I'm not really sure this is the correct thing to do.. In other > words, I think I'd rather leave this as-is. Thanks Jouni for detailed explanation. >Other than debug logs showing some warnings, are there any real issues noticeable by external devices that this patch is fixing? Yes. For STA devices supporting firmware roam, the first deauth clears the assoc and kicks the firmware to scan and search for similar profile networks. Now the second deauth from AP/GO pre-empts the join process. For e.g In P2P case, the deauth following WPS EAP-FAIL will cause the P2P GC to disassociate. Now for cases, where GC tries to connect back immediately, the P2P GO would have moved to authenticated state internally and the second deauth from supplicant pre-empts this join process. Do you see any other solution to avoid this second deauth? - Jithu Jance
diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c index c36842b..c9b6578 100644 --- a/src/ap/sta_info.c +++ b/src/ap/sta_info.c @@ -1198,7 +1198,8 @@ void ap_sta_disconnect(struct hostapd_data *hapd, struct sta_info *sta, if (sta == NULL && addr) sta = ap_get_sta(hapd, addr); - if (addr) + if (addr && (hapd->iface->drv_flags & + WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS)) hostapd_drv_sta_deauth(hapd, addr, reason); if (sta == NULL)
Hi Jouni, For "device_ap_sme" devices, the ap_sta_disconnect call in supplicant results in two calls to wpa_driver_nl80211_sta_remove. ap_sta_disconnect > hostapd_drv_sta_deauth > wpa_driver_nl80211_sta_remove ap_sta_disconnect > ap_sta_deauth_cb_timeout > ap_sta_remove > hostapd_drv_sta_remove The ap_sta_deauth_cb_timeout is invoked immediately (timeout of [0,0]) for device_ap_sme devices. The hostapd_drv_sta_deauth call can be avoided for devices without WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS set. ##### Sample logs +++++++++++++++++++++++++++++++++++++++++++++++++ Example logs D/wpa_supplicant(16311): p2p-wlan0-0: IEEE 802.1X: Force disconnection after EAP-Failure D/wpa_supplicant(16311): nl80211: sta_remove -> DEL_STATION p2p-wlan0-0 02:90:4c:74:40:14 --> 0 (Success) <====================== 1st call D/wpa_supplicant(16311): ap_sta_disconnect: reschedule ap_handle_timer timeout for 02:90:4c:74:40:14 (5 seconds - AP_MAX_INACTIVITY_AFTER_DEAUTH) D/wpa_supplicant(16311): IEEE 802.1X: 02:90:4c:74:40:14 BE_AUTH entering state IDLE D/wpa_supplicant(16311): IEEE 802.1X: 02:90:4c:74:40:14 AUTH_PAE entering state INITIALIZE D/wpa_supplicant(16311): EAP: EAP entering state DISABLED D/wpa_supplicant(16311): Removing STA 02:90:4c:74:40:14 from kernel driver D/wpa_supplicant(16311): nl80211: sta_remove -> DEL_STATION p2p-wlan0-0 02:90:4c:74:40:14 --> 0 (Success) <====================== 2nd call D/wpa_supplicant(16311): hostapd_logger: STA 02:90:4c:74:40:14 - MLME-DEAUTHENTICATE.indication(02:90:4c:74:40:14, 23) D/wpa_supplicant(16311): hostapd_logger: STA 02:90:4c:74:40:14 - MLME-DELETEKEYS.request(02:90:4c:74:40:14) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Signed-off-by: Jithu Jance <jithujance@gmail.com> --- src/ap/sta_info.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)