Re: [PATCH 2/6] scan: allow 'faked' scan_bss results

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]

Hi James,

On 3/10/21 2:27 PM, James Prestwood wrote:
> When a full mac roam occurs IWD has no idea about how the
> roam occurred, including any scan results associated with
> it. IWD just gets a roam event (basically CMD_CONNECT) and
> will need to 'fake' a scan result to provide to station.
> 
> To allow this scan_bss_rank_compare was exposed, and
> scan_bss_addr_eq was also modified to work with queue's.
> This will allow station to insert a 'faked' scan_bss result
> into its list as if it did a prior scan. A new frame type
> was added, SCAN_BSS_NONE, which will indicate this scan
> result was obtained via other means (aka faked). This may
> not be strictly required but it makes things clearer
> for cleanup, and avoids frame type specific routines.
> ---
>   src/p2p.c  | 3 +++
>   src/scan.c | 7 ++++++-
>   src/scan.h | 8 ++++++--
>   3 files changed, 15 insertions(+), 3 deletions(-)
> 

<snip>

> diff --git a/src/scan.h b/src/scan.h
> index 355b4b5a..dc7f4688 100644
> --- a/src/scan.h
> +++ b/src/scan.h
> @@ -43,6 +43,7 @@ enum scan_bss_frame_type {
>   	SCAN_BSS_PROBE_RESP,
>   	SCAN_BSS_PROBE_REQ,
>   	SCAN_BSS_BEACON,
> +	SCAN_BSS_NONE,

Ah, I'm not a huge fan of adding enum entries that don't represent anything. 
Given that this is a huge special case that only station cares about, can we 
just fake it by setting SCAN_BSS_PROBE_RESP?

>   };
>   
>   struct scan_bss {
> @@ -115,9 +116,11 @@ static inline int scan_bss_addr_cmp(const struct scan_bss *a1,
>   	return memcmp(a1->addr, a2->addr, sizeof(a1->addr));
>   }
>   
> -static inline bool scan_bss_addr_eq(const struct scan_bss *a1,
> -					const struct scan_bss *a2)
> +static inline bool scan_bss_addr_eq(const void *a, const void *b)
>   {
> +	const struct scan_bss *a1 = a;
> +	const struct scan_bss *a2 = b;
> +

Not really against this, but wouldn't you use bss_match_bssid inside station.c?

>   	return !memcmp(a1->addr, a2->addr, sizeof(a1->addr));
>   }
>   
> @@ -153,6 +156,7 @@ uint64_t scan_get_triggered_time(uint64_t wdev_id, uint32_t id);
>   
>   void scan_bss_free(struct scan_bss *bss);
>   int scan_bss_rank_compare(const void *a, const void *b, void *user);
> +void scan_bss_compute_rank(struct scan_bss *bss);

Okay, but don't you also want to parse the IEs obtained from GET_STATION / 
NEW_STATION?  i.e. scan_parse_bss_information_elements() or similar?

So maybe a better approach would be another constructor that takes mac, rssi, 
ies and frequency?

>   
>   int scan_bss_get_rsn_info(const struct scan_bss *bss, struct ie_rsn_info *info);
>   
> 

Regards,
-Denis