| Message ID | 1473085991-5073-14-git-send-email-andrei.otcheretianski@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Sep 05, 2016 at 05:33:07PM +0300, andrei.otcheretianski@intel.com wrote: > In some cases, after a sudden AP disappearing and reconnection to > another AP in the same ESS, if another scan ocures, wpa_supplicant might > try to roam to the old AP (if it was better ranked than the new one) > because it is still saved in BSS list and the blacklist entry was > cleared in previous reconnect. This attempt is going to fail because > the AP is not present anymore and it'll cause long disconnections. > > Remove an AP that is probably out of range from BSS list to avoid > such disconnections. What is that "probably out of range" expectation based on? > @@ -2617,6 +2626,9 @@ static void wpa_supplicant_event_disassoc_finish(struct wpa_supplicant *wpa_s, > last_ssid = wpa_s->current_ssid; > wpa_supplicant_mark_disassoc(wpa_s); > > + if (curr) > + wpa_bss_remove(wpa_s, curr, "Connection to AP lost"); This gets called when the AP is sending out a Deauthentication/Disassociation frame. If we receive that frame, obviously the AP is not out of range.. I don't see why we would remove a BSS entry for an AP from which we are able to receive a frame. Maybe the AP selection after the scan should better take into account the relative times of when the BSS entries have been last updated if an old scan result is causing problems for roaming. For the case where we can clearly identify that the driver is indicating that the AP is not reachable anymore (e.g., missing beacons, etc.), it might be more reasonable to remove the cached BSS entry from wpa_supplicant before the entry expires due to scans not being able to update it.
> -----Original Message----- > From: Jouni Malinen [mailto:j@w1.fi] > Sent: Tuesday, September 27, 2016 00:22 > To: Otcheretianski, Andrei <andrei.otcheretianski@intel.com> > Cc: hostap@lists.infradead.org; Spinadel, David <david.spinadel@intel.com> > Subject: Re: [PATCH 14/18] wpa_supplicant: remove disconnected APs from > BSS table > > On Mon, Sep 05, 2016 at 05:33:07PM +0300, andrei.otcheretianski@intel.com > wrote: > > In some cases, after a sudden AP disappearing and reconnection to > > another AP in the same ESS, if another scan ocures, wpa_supplicant > > might try to roam to the old AP (if it was better ranked than the new > > one) because it is still saved in BSS list and the blacklist entry was > > cleared in previous reconnect. This attempt is going to fail because > > the AP is not present anymore and it'll cause long disconnections. > > > > Remove an AP that is probably out of range from BSS list to avoid such > > disconnections. > > What is that "probably out of range" expectation based on? Because the event is locally generated and the reason is DISASSOC_DUE_TO_INACTIVITY > > > @@ -2617,6 +2626,9 @@ static void > wpa_supplicant_event_disassoc_finish(struct wpa_supplicant *wpa_s, > > last_ssid = wpa_s->current_ssid; > > wpa_supplicant_mark_disassoc(wpa_s); > > > > + if (curr) > > + wpa_bss_remove(wpa_s, curr, "Connection to AP lost"); > > This gets called when the AP is sending out a Deauthentication/Disassociation > frame. If we receive that frame, obviously the AP is not out of range.. I don't > see why we would remove a BSS entry for an AP from which we are able to > receive a frame. It is also called for locally generated DEAUTH, otherwise curr==NULL. > > Maybe the AP selection after the scan should better take into account the > relative times of when the BSS entries have been last updated if an old scan > result is causing problems for roaming. Prioritizing by timestamp may break other roaming considerations.. > > For the case where we can clearly identify that the driver is indicating that > the AP is not reachable anymore (e.g., missing beacons, etc.), it might be > more reasonable to remove the cached BSS entry from wpa_supplicant > before the entry expires due to scans not being able to update it. This is exactly what we do. Please note that there is also mac80211's patch that clears the bss cache entry ("mac80211: remove disconnected APs from BSS table"). > > -- > Jouni Malinen PGP id EFC895FA
On Tue, Sep 27, 2016 at 07:25:58AM +0000, Otcheretianski, Andrei wrote: > > From: Jouni Malinen [mailto:j@w1.fi] > > What is that "probably out of range" expectation based on? > > Because the event is locally generated and the reason is DISASSOC_DUE_TO_INACTIVITY I had to look at mac80211 to find this behavior.. I think it would be nicer if there were a more explicit driver notification of the disconnection happening due to the AP becoming unreachable since another driver could use this specific reason code to indicate something completely different. Anyway, this can do for now. > > This gets called when the AP is sending out a Deauthentication/Disassociation > > frame. If we receive that frame, obviously the AP is not out of range.. I don't > > see why we would remove a BSS entry for an AP from which we are able to > > receive a frame. > > It is also called for locally generated DEAUTH, otherwise curr==NULL. Somehow I missed this being in the same function.. Based on that review of mac80211 behavior, I applied this with additional notes pointing out why this indicates the AP being likely out of range with mac80211-based drivers.
diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c index 3687a2e..3a8778d 100644 --- a/wpa_supplicant/bss.c +++ b/wpa_supplicant/bss.c @@ -214,8 +214,8 @@ static void wpa_bss_update_pending_connect(struct wpa_supplicant *wpa_s, } -static void wpa_bss_remove(struct wpa_supplicant *wpa_s, struct wpa_bss *bss, - const char *reason) +void wpa_bss_remove(struct wpa_supplicant *wpa_s, struct wpa_bss *bss, + const char *reason) { if (wpa_s->last_scan_res) { unsigned int i; diff --git a/wpa_supplicant/bss.h b/wpa_supplicant/bss.h index f7f72f3..84e8fb0 100644 --- a/wpa_supplicant/bss.h +++ b/wpa_supplicant/bss.h @@ -113,6 +113,8 @@ void wpa_bss_update_start(struct wpa_supplicant *wpa_s); void wpa_bss_update_scan_res(struct wpa_supplicant *wpa_s, struct wpa_scan_res *res, struct os_reltime *fetch_time); +void wpa_bss_remove(struct wpa_supplicant *wpa_s, struct wpa_bss *bss, + const char *reason); void wpa_bss_update_end(struct wpa_supplicant *wpa_s, struct scan_info *info, int new_scan); int wpa_bss_init(struct wpa_supplicant *wpa_s); diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index 1bb646d..63fba4d 100644 --- a/wpa_supplicant/events.c +++ b/wpa_supplicant/events.c @@ -2530,6 +2530,7 @@ static void wpa_supplicant_event_disassoc_finish(struct wpa_supplicant *wpa_s, struct wpa_bss *fast_reconnect = NULL; struct wpa_ssid *fast_reconnect_ssid = NULL; struct wpa_ssid *last_ssid; + struct wpa_bss *curr = NULL; authenticating = wpa_s->wpa_state == WPA_AUTHENTICATING; os_memcpy(prev_pending_bssid, wpa_s->pending_bssid, ETH_ALEN); @@ -2545,6 +2546,14 @@ static void wpa_supplicant_event_disassoc_finish(struct wpa_supplicant *wpa_s, return; } + if (!wpa_s->disconnected && wpa_s->wpa_state >= WPA_AUTHENTICATING && + reason_code == WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY && + locally_generated) + /* Remove the inactive AP (which is probably out of range) from + * BSS list after marking disassoc. + */ + curr = wpa_s->current_bss; + if (could_be_psk_mismatch(wpa_s, reason_code, locally_generated)) { wpa_msg(wpa_s, MSG_INFO, "WPA: 4-Way Handshake failed - " "pre-shared key may be incorrect"); @@ -2617,6 +2626,9 @@ static void wpa_supplicant_event_disassoc_finish(struct wpa_supplicant *wpa_s, last_ssid = wpa_s->current_ssid; wpa_supplicant_mark_disassoc(wpa_s); + if (curr) + wpa_bss_remove(wpa_s, curr, "Connection to AP lost"); + if (authenticating && (wpa_s->drv_flags & WPA_DRIVER_FLAGS_SME)) { sme_disassoc_while_authenticating(wpa_s, prev_pending_bssid); wpa_s->current_ssid = last_ssid;