[PATCH v2] Reject auths during explicit roam requests
Ben Greear
greearb at candelatech.com
Tue Mar 2 01:36:42 GMT 2021
On 3/1/21 5:25 PM, Matthew Wang wrote:
> Thanks Ben, the patch is a good idea, but doesn't exactly address my
> issue. The D-Bus/wpa_cli roam command sets the reassociate bit, and
> this short circuits the roam logic in wpa_supplicant_need_to_roam (it
> just returns true at the top of the function instead of going into
> need_to_roam_within_ess).
Yeah, I believe your application would only want to temporarily fully restrict
the auto-roaming, where I wanted a way to do it for the duration of my
scenario (and then would re-load a new supplicant config afterwards).
Seems to me that you would want to disable roam-based-on-scan results for a bit longer
though if you are explicitly trying a roam based on logic that should know better
than supplicant (otherwise, why bother, just let supplicant do the job).
Thanks,
Ben
>
> On Mon, Mar 1, 2021 at 3:58 PM Ben Greear <greearb at candelatech.com> wrote:
>>
>> On 3/1/21 3:34 PM, Matthew Wang wrote:
>>> The roam D-Bus and wpa_cli commands flip the reassociate bit before
>>> calling wpa_supplicant_connect. wpa_supplicant connect eventually aborts
>>> ongoing scans, which causes scan results to be reported. Since the
>>> reassociate bit is set, this will trigger a connection attempt based on
>>> the aborted scan's scan results and cancel the initial connetion
>>> request. This often causes wpa_supplicant to reassociate to the same AP
>>> it is currently associated to.
>>>
>>> Add a roam_in_progress flag to indicate that we're currently attempting
>>> to roam via D-Bus so that we don't initate another connection attempt.
>>
>> I had similar issues, in case it helps, here is my patch:
>>
>> https://github.com/greearb/hostap-ct/commit/50b1c17d0a782dc4d354ac7366500763611ea5af
>>
>> I first relaxed thresholds by default, and then allowed configuring them. When something
>> beyond hostapd is controlling roaming, I set the thresholds so that it would never automatically
>> roam based on scan results.
>>
>> Lord git only knows how much of this was built on previous patches in my tree...
>>
>> Thanks,
>> Ben
>>
>>
>>>
>>> Signed-off-by: Matthew Wang <matthewmwang at chromium.org>
>>> Series-to: hostap at lists.infradead.org
>>> Series-cc: j at w1.fi, matthewmwang at chromium.org
>>> Change-Id: Ibeb6cceddabd10cab6938411f54ed63f4d4428c1
>>> ---
>>> wpa_supplicant/ctrl_iface.c | 9 +++++++++
>>> wpa_supplicant/dbus/dbus_new_handlers.c | 9 +++++++++
>>> wpa_supplicant/sme.c | 7 +++++++
>>> wpa_supplicant/wpa_supplicant_i.h | 1 +
>>> 4 files changed, 26 insertions(+)
>>>
>>> diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
>>> index 0d4c2bc8f0b..048fe72d472 100644
>>> --- a/wpa_supplicant/ctrl_iface.c
>>> +++ b/wpa_supplicant/ctrl_iface.c
>>> @@ -5686,6 +5686,15 @@ static int wpa_supplicant_ctrl_iface_roam(struct wpa_supplicant *wpa_s,
>>> wpa_s->reassociate = 1;
>>> wpa_supplicant_connect(wpa_s, bss, ssid);
>>>
>>> + /*
>>> + * Indicate that an explicitly requested roam is in progress so scan
>>> + * results that come in before the 'sme-connect' radio work gets
>>> + * executed do not override the original connection attempt.
>>> + */
>>> + if (radio_work_pending(wpa_s, "sme-connect")) {
>>> + wpa_s->roam_in_progress = true;
>>> + }
>>> +
>>> return 0;
>>> #endif /* CONFIG_NO_SCAN_PROCESSING */
>>> }
>>> diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
>>> index 7d20f2123a8..6803558accd 100644
>>> --- a/wpa_supplicant/dbus/dbus_new_handlers.c
>>> +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
>>> @@ -2005,6 +2005,15 @@ DBusMessage * wpas_dbus_handler_roam(DBusMessage *message,
>>> wpa_s->reassociate = 1;
>>> wpa_supplicant_connect(wpa_s, bss, ssid);
>>>
>>> + /*
>>> + * Indicate that an explicitly requested roam is in progress so scan
>>> + * results that come in before the 'sme-connect' radio work gets
>>> + * executed do not override the original connection attempt.
>>> + */
>>> + if (radio_work_pending(wpa_s, "sme-connect")) {
>>> + wpa_s->roam_in_progress = true;
>>> + }
>>> +
>>> return NULL;
>>> #endif /* CONFIG_NO_SCAN_PROCESSING */
>>> }
>>> diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c
>>> index 522c8297f7e..dde80863a98 100644
>>> --- a/wpa_supplicant/sme.c
>>> +++ b/wpa_supplicant/sme.c
>>> @@ -941,6 +941,8 @@ static void sme_auth_start_cb(struct wpa_radio_work *work, int deinit)
>>> struct wpa_connect_work *cwork = work->ctx;
>>> struct wpa_supplicant *wpa_s = work->wpa_s;
>>>
>>> + wpa_s->roam_in_progress = false;
>>> +
>>> if (deinit) {
>>> if (work->started)
>>> wpa_s->connect_work = NULL;
>>> @@ -981,6 +983,11 @@ void sme_authenticate(struct wpa_supplicant *wpa_s,
>>> return;
>>> }
>>>
>>> + if (wpa_s->roam_in_progress) {
>>> + wpa_dbg(wpa_s, MSG_DEBUG,
>>> + "SME: Reject sme_authenticate() in favor of explicit roam request");
>>> + return;
>>> + }
>>> if (radio_work_pending(wpa_s, "sme-connect")) {
>>> /*
>>> * The previous sme-connect work might no longer be valid due to
>>> diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
>>> index 7d14b627d89..b6c339ab711 100644
>>> --- a/wpa_supplicant/wpa_supplicant_i.h
>>> +++ b/wpa_supplicant/wpa_supplicant_i.h
>>> @@ -621,6 +621,7 @@ struct wpa_supplicant {
>>> u8 pending_bssid[ETH_ALEN]; /* If wpa_state == WPA_ASSOCIATING, this
>>> * field contains the target BSSID. */
>>> int reassociate; /* reassociation requested */
>>> + bool roam_in_progress; /* roam in progress */
>>> unsigned int reassoc_same_bss:1; /* reassociating to the same BSS */
>>> unsigned int reassoc_same_ess:1; /* reassociating to the same ESS */
>>> int disconnected; /* all connections disabled; i.e., do no reassociate
>>>
>>
>>
>> --
>> Ben Greear <greearb at candelatech.com>
>> Candela Technologies Inc http://www.candelatech.com
>
--
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc http://www.candelatech.com
More information about the Hostap
mailing list