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). On Mon, Mar 1, 2021 at 3:58 PM Ben Greear <greearb@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxx> > > Series-to: hostap@xxxxxxxxxxxxxxxxxxx > > Series-cc: j@xxxxx, matthewmwang@xxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxx> > Candela Technologies Inc http://www.candelatech.com _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
