Hi,
looks like I initially only replied in private by accident.
On Fri, 2024-08-02 at 13:30 +0300, Jouni Malinen wrote:
> On Mon, Apr 29, 2024 at 01:51:49PM +0200, benjamin@xxxxxxxxxxxxxxxx wrote:
> > Change the logic a bit to not directly use the result of the
> > wpa_drv_get_bss_trans_status call and instead use the same selection
> > logic as usual but taking into account the driver rejections.
> >
> > This changes the logic in minor ways. The main change is that this
> > aligns the ordering of BSSs to be identical in all cases. More
> > precisely, we'll select the best BSS as found by find_better_target.
> >
> > Beyond that, it also means that in the case of an non-abridged BTM
> > request we'll also consider candidates that were found through the scan
> > and not in the neighbor report. In this case, the driver will not have a
> > chance to reject them.
>
> This feels a bit scary especially if the updated design has not actually
> been tested with a driver that uses the driver-based mechanism for
> updating candidate lists (which I'm assuming has not been done here).
Yes, it is a bit scary. Honestly, I am not able to test the code. Maybe
I missed it, but I didn't even find a driver that provides the nl80211
API.
> And there is something strange here:
>
> > diff --git a/wpa_supplicant/wnm_sta.c b/wpa_supplicant/wnm_sta.c
> > #ifdef CONFIG_MBO
> > -static struct wpa_bss *
> > -get_mbo_transition_candidate(struct wpa_supplicant *wpa_s,
> > +static void
> > +fetch_drv_mbo_candidate_info(struct wpa_supplicant *wpa_s,
> > enum mbo_transition_reject_reason *reason)
> > {
>
> > + if (nei->preference_present && nei->preference == 0)
> > continue;
> > +
> > + /* FIXME: Should we go through wpa_scan_res_match? */
>
> Well, that is not the strange part, but again, something that makes me
> unhappy about applying this without full testing.
>
> > +#else
> > +static void
> > +fetch_drv_mbo_transition_candidate_info(struct wpa_supplicant *wpa_s)
> > +{
> > }
> > #endif /* CONFIG_MBO */
>
> That is the strange part.. What was that supposed to be? That same
> function with empty payload? Now the function name is different and so
> is the list of arguments. This would obviously not work without
> CONFIG_MBO defined.
Oops, yep, looks like I messed up the function rename and such.
> If this is just to skip the functionality within the function, defining
> the function once with the actually body within #ifdef CONFIG_MBO would
> be much cleaner.
Sure, that makes sense!
Benjamin
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
