Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c79dca8b-ae49-a015-8ce9-e22c8dd1ceda@quicinc.com>
Date:   Tue, 12 Jul 2022 15:23:05 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 07/12] mac80211: S1G capabilities information element in
 probe request
Content-Language: en-US
To:     Kieran Frewen <kieran.frewen@morsemicro.com>,
        <johannes@sipsolutions.net>
CC:     <linux-wireless@vger.kernel.org>,
        Bassem Dawood <bassem@morsemicro.com>
References: <20220711010816.45927-1-kieran.frewen@morsemicro.com>
 <20220711010816.45927-8-kieran.frewen@morsemicro.com>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20220711010816.45927-8-kieran.frewen@morsemicro.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/10/2022 6:08 PM, Kieran Frewen wrote:
> Add the missing S1G capabilities information element to probe requests.
> 
> Signed-off-by: Kieran Frewen <kieran.frewen@morsemicro.com>
> Signed-off-by: Bassem Dawood <bassem@morsemicro.com>
> ---
>   net/mac80211/util.c | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
> 
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index bccc3a309ed0..51a2c1dee360 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -1832,6 +1832,43 @@ static int ieee80211_build_preq_ies_band(struct ieee80211_sub_if_data *sdata,
>   	rate_flags = ieee80211_chandef_rate_flags(chandef);
>   	shift = ieee80211_chandef_get_shift(chandef);
>   
> +	/* For direct mac80211 scan (probe request), add S1G IE and consider its override bits */
> +	if (band == NL80211_BAND_S1GHZ) {
> +		struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
> +		struct ieee80211_s1g_cap s1g_capab;
> +
> +		*pos++ = WLAN_EID_S1G_CAPABILITIES;
> +		*pos++ = sizeof(s1g_capab);
> +

why create s1g_capab on the stack and then copy into place?
why not create it directly in place and avoid the copy?

> +		memcpy(s1g_capab.capab_info,
> +		       sband->s1g_cap.cap,
> +		       sizeof(sband->s1g_cap.cap));
> +		memcpy(s1g_capab.supp_mcs_nss,
> +		       sband->s1g_cap.nss_mcs,
> +		       sizeof(sband->s1g_cap.nss_mcs));
> +
> +		/* override the capability info */
> +		for (i = 0; i < sizeof(ifmgd->s1g_capa.capab_info); i++) {
> +			u8 mask = ifmgd->s1g_capa_mask.capab_info[i];
> +
> +			s1g_capab.capab_info[i] &= ~mask;
> +			s1g_capab.capab_info[i] |= ifmgd->s1g_capa.capab_info[i] & mask;
> +		}
> +
> +		/* then MCS and NSS set */
> +		for (i = 0; i < sizeof(ifmgd->s1g_capa.supp_mcs_nss); i++) {
> +			u8 mask = ifmgd->s1g_capa_mask.supp_mcs_nss[i];
> +
> +			s1g_capab.supp_mcs_nss[i] &= ~mask;
> +			s1g_capab.supp_mcs_nss[i] |= ifmgd->s1g_capa.supp_mcs_nss[i] & mask;
> +		}
> +
> +		memcpy(pos, &s1g_capab, sizeof(s1g_capab));
> +		pos += sizeof(s1g_capab);
> +		/* No more IEs relevant for S1G */
> +		goto done;
> +	}
> +
>   	num_rates = 0;
>   	for (i = 0; i < sband->n_bitrates; i++) {
>   		if ((BIT(i) & rate_mask) == 0)