Re: [PATCH 13/15] station: check vendor quirk for BSS TM request candidate list

James Prestwood <[email protected]> Mon, 25 Aug 2025 07:37:25 -0700
Newsgroups dev.linux.lists.iwd
Message-ID <[email protected]>
Hi Marcel,

On 8/23/25 7:48 AM, Marcel Holtmann wrote:
> Hi James,
>
>> If the AP vendor has known issues with the preferred candidate list
>> ignore it and jump directly to requesting a neighbor report.
>> ---
>> src/station.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/station.c b/src/station.c
>> index 4919f89e..3424964d 100644
>> --- a/src/station.c
>> +++ b/src/station.c
>> @@ -64,6 +64,7 @@
>> #include "src/eap-tls-common.h"
>> #include "src/storage.h"
>> #include "src/pmksa.h"
>> +#include "src/vendor_quirks.h"
>>
>> #define STATION_RECENT_NETWORK_LIMIT 5
>> #define STATION_RECENT_FREQS_LIMIT 5
>> @@ -3378,7 +3379,9 @@ static void station_ap_directed_roam(struct station *station,
>> l_timeout_remove(station->roam_trigger_timeout);
>> station->roam_trigger_timeout = NULL;
>>
>> - if (req_mode & WNM_REQUEST_MODE_PREFERRED_CANDIDATE_LIST) {
>> + if ((req_mode & WNM_REQUEST_MODE_PREFERRED_CANDIDATE_LIST) &&
>> + !(station->connected_bss->vendor_quirks &
>> + VENDOR_QUIRK_BAD_BSS_TM_CANDIDATE_LIST)) {
> this is unreadable in the long term. I would either add a variable or add a helper for the quirks.

Agreed. I think a variable makes the most sense. I contemplated 
something like:

vendor_quirk_is_set()

But you'd need to pass in the mask/enum which probably ends up eating up 
more characters anyways. Instead I can just do a local variable:

bool ignore_candidates = station->connected_bss->vendor_quirks & 
VENDOR_QUIRK_BAD_BSS_TM_CANDIDATE_LIST;

>
> As a side note, you might want to actually somewhere print the quirks which are active. Otherwise your debugging in the future will become a nightmare.

Good idea, I could probably do this upon connecting and when roams 
finish. Its probably a bit too verbose for scan results.

Thanks,

James

>
> Regards
>
> Marcel
>