Re: [PATCH v3 2/2] mac80211: do intersection with he mcs and nss set of peer and own

[Kalle Valo <kvalo@codeaurora.org>]

Wen Gong <wgong@codeaurora.org> writes:

> On 2021-09-28 21:02, Johannes Berg wrote:
>> Hi,
>>
>>
>> I had done a bunch of fixups to this patch, but the zero-day build
>> robot
>> correctly reports that:
>>
>>> +	ieee80211_he_mcs_intersection(&own_he_cap.he_mcs_nss_supp.rx_mcs_80,
>>> +				      &he_cap->he_mcs_nss_supp.rx_mcs_80,
>>> +				      &own_he_cap.he_mcs_nss_supp.tx_mcs_80,
>>> +				      &he_cap->he_mcs_nss_supp.tx_mcs_80);
>>
>> the &own_he_cap... parts here will take an __le16 pointer to a possibly
>> unaligned variable - any thoughts how we could fix that?
>>
> Hi Johannes,
>
> Add "__packed" before the "__le16 *" should solve this warning by my
> understand like this:
>
> diff --git a/net/mac80211/he.c b/net/mac80211/he.c
> index c05af7018f79..960fea9646b0 100644
> --- a/net/mac80211/he.c
> +++ b/net/mac80211/he.c
> @@ -52,7 +52,7 @@ ieee80211_update_from_he_6ghz_capa(const struct
> ieee80211_he_6ghz_capa *he_6ghz_
>         sta->sta.he_6ghz_capa = *he_6ghz_capa;
>  }
>
> -static void ieee80211_he_mcs_disable(__le16 *he_mcs)
> +static void ieee80211_he_mcs_disable(__packed __le16 *he_mcs)
>  {
>         u32 i;
>
> @@ -60,8 +60,8 @@ static void ieee80211_he_mcs_disable(__le16 *he_mcs)
>                 *he_mcs |= cpu_to_le16(IEEE80211_HE_MCS_NOT_SUPPORTED
> << i * 2);
>  }
>
> -static void ieee80211_he_mcs_intersection(__le16 *he_own_rx, __le16
> *he_peer_rx,
> -                                         __le16 *he_own_tx, __le16
> *he_peer_tx)
> +static void ieee80211_he_mcs_intersection(__packed __le16 *he_own_rx,
> __packed __le16 *he_peer_rx,
> +                                         __packed __le16 *he_own_tx,
> __packed __le16 *he_peer_tx)
>  {
>         u32 i;
>         u16 own_rx, own_tx, peer_rx, peer_tx;
>
>
>
> net/mac80211/he.c:158:33: warning: taking address of packed member
> 'rx_mcs_80' of class or structure 'ieee80211_he_mcs_nss_supp' may
> result in an unaligned pointer value [-Waddress-of-packed-member]

I don't know what Johannes thinks, but to me that looks like an ugly
hack. Wouldn't use get_unaligned() or similar be cleaner?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k