Received-SPF: pass (google.com: domain of linux-wireless+bounces-518-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <50a6f31a-ef28-496c-acf4-0521034e43a1@quicinc.com>
Date: Wed, 6 Dec 2023 16:07:03 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] wifi: mac80211: Fix SMPS action frame ht cap check
Content-Language: en-US
To: Allen Ye <allen.ye@mediatek.com>, <linux-wireless@vger.kernel.org>
CC: Johannes Berg <johannes.berg@intel.com>, Felix Fietkau <nbd@nbd.name>,
        Lorenzo Bianconi <lorenzo.bianconi@redhat.com>,
        Evelyn Tsai
	<evelyn.tsai@mediatek.com>,
        "Money . Wang" <money.wang@mediatek.com>,
        <linux-mediatek@lists.infradead.org>,
        Thomas Wu <quic_wthomas@quicinc.com>
References: <20231206024708.24831-1-allen.ye@mediatek.com>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20231206024708.24831-1-allen.ye@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 12/5/2023 6:47 PM, Allen Ye wrote:
> From: "Allen.Ye" <allen.ye@mediatek.com>
> 
> Since there is no HT BSS in 6GHz, the HT Cap check would stop 6G HE/EHT
> BSS from processing the HT action frame for SM Power Save which can be
> also used in an HE BSS. Therefore, we remove the HT Cap check in 6GHz and
> add the HE check accordingly.
> 
> Signed-off-by: Money.Wang <money.wang@mediatek.com>
> Signed-off-by: Allen.Ye <allen.ye@mediatek.com>
> ---
>  net/mac80211/rx.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 64352e4e6d00..f8cd14dc58ec 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -3482,7 +3482,8 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
>  	switch (mgmt->u.action.category) {
>  	case WLAN_CATEGORY_HT:
>  		/* reject HT action frames from stations not supporting HT */
> -		if (!rx->link_sta->pub->ht_cap.ht_supported)
> +		if (status->band != NL80211_BAND_6GHZ &&
> +		    !rx->link_sta->pub->ht_cap.ht_supported)

we had found the same issue and were preparing a patch that changed this to:
+		if (!rx->link_sta->pub->ht_cap.ht_supported &&
+		    !rx->link_sta->pub->he_cap.has_he)

I see you added the has_he check below, but curious if it is better to
do it here to short circuit the tests that follow

>  			goto invalid;
>  
>  		if (sdata->vif.type != NL80211_IFTYPE_STATION &&
> @@ -3502,6 +3503,12 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
>  			enum ieee80211_smps_mode smps_mode;
>  			struct sta_opmode_info sta_opmode = {};
>  
> +			if (status->band == NL80211_BAND_6GHZ &&
> +			    rx->link_sta->pub->he_cap.has_he &&
> +			    !(rx->link_sta->pub->he_cap.he_cap_elem.mac_cap_info[5] &
> +			    IEEE80211_HE_MAC_CAP5_HE_DYNAMIC_SM_PS))
> +				goto invalid;
> +
>  			if (sdata->vif.type != NL80211_IFTYPE_AP &&
>  			    sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
>  				goto handled;