diff mbox series

wifi: mac80211: fix EHT issue in ieee80211_determine_chantype

Message ID 20220929151638.9823-1-greearb@candelatech.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series wifi: mac80211: fix EHT issue in ieee80211_determine_chantype | expand

Commit Message

Ben Greear Sept. 29, 2022, 3:16 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

The IEEE80211_STA_DISABLE_EHT flag was not enabled when it should
be, so the bandwidth change failed, and STA disconnected.

Use an OR approach instead of just assinging the flags, to
allow easier setting of certain flags such as the EHT
one.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/mlme.c | 63 ++++++++++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

Comments

Johannes Berg Oct. 7, 2022, 12:46 p.m. UTC | #1 
On Thu, 2022-09-29 at 08:16 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> The IEEE80211_STA_DISABLE_EHT flag was not enabled when it should
> be, so the bandwidth change failed, and STA disconnected.
> 
> Use an OR approach instead of just assinging the flags, to

typo - assigning

> allow easier setting of certain flags such as the EHT
> one.

This *looked* reasonable, but doesn't apply in any way? Can you rebase
on wireless/main please?

> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>  net/mac80211/mlme.c | 63 ++++++++++++++++++++++-----------------------
>  1 file changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 011827137da7..9ac99f48c63b 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -167,7 +167,8 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>  	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
>  	struct cfg80211_chan_def vht_chandef;
>  	struct ieee80211_sta_ht_cap sta_ht_cap;
> -	u32 ht_cfreq, ret;
> +	u32 ht_cfreq;
> +	u32 ret = 0;

Could keep that on one line still? Any particular reason not to?
 
>  
>  	if (cfg80211_chandef_identical(chandef, &vht_chandef)) {
> -		ret = 0;
>  		goto out;
>  	}

can drop the braces now

johannes
diff mbox series

Patch

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 011827137da7..9ac99f48c63b 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -167,7 +167,8 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	struct cfg80211_chan_def vht_chandef;
 	struct ieee80211_sta_ht_cap sta_ht_cap;
-	u32 ht_cfreq, ret;
+	u32 ht_cfreq;
+	u32 ret = 0;
 
 	memset(chandef, 0, sizeof(struct cfg80211_chan_def));
 	chandef->chan = channel;
@@ -175,17 +176,18 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	chandef->center_freq1 = channel->center_freq;
 	chandef->freq1_offset = channel->freq_offset;
 
+	if (!eht_oper)
+		ret |= IEEE80211_STA_DISABLE_EHT;
+
 	if (channel->band == NL80211_BAND_6GHZ) {
 		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, eht_oper,
 						    chandef)) {
 			mlme_dbg(sdata,
 				 "bad 6 GHz operation, disabling HT/VHT/HE/EHT\n");
-			ret = IEEE80211_STA_DISABLE_HT |
-			      IEEE80211_STA_DISABLE_VHT |
-			      IEEE80211_STA_DISABLE_HE |
-			      IEEE80211_STA_DISABLE_EHT;
-		} else {
-			ret = 0;
+			ret |= IEEE80211_STA_DISABLE_HT |
+			       IEEE80211_STA_DISABLE_VHT |
+			       IEEE80211_STA_DISABLE_HE |
+			       IEEE80211_STA_DISABLE_EHT;
 		}
 		vht_chandef = *chandef;
 		goto out;
@@ -196,10 +198,10 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			chandef->width = ieee80211_s1g_channel_width(channel);
 		}
 
-		ret = IEEE80211_STA_DISABLE_HT | IEEE80211_STA_DISABLE_40MHZ |
-		      IEEE80211_STA_DISABLE_VHT |
-		      IEEE80211_STA_DISABLE_80P80MHZ |
-		      IEEE80211_STA_DISABLE_160MHZ;
+		ret |= IEEE80211_STA_DISABLE_HT | IEEE80211_STA_DISABLE_40MHZ |
+		       IEEE80211_STA_DISABLE_VHT |
+		       IEEE80211_STA_DISABLE_80P80MHZ |
+		       IEEE80211_STA_DISABLE_160MHZ;
 		goto out;
 	}
 
@@ -208,10 +210,10 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 
 	if (!ht_oper || !sta_ht_cap.ht_supported) {
 		mlme_dbg(sdata, "HT operation missing / HT not supported\n");
-		ret = IEEE80211_STA_DISABLE_HT |
-		      IEEE80211_STA_DISABLE_VHT |
-		      IEEE80211_STA_DISABLE_HE |
-		      IEEE80211_STA_DISABLE_EHT;
+		ret |= IEEE80211_STA_DISABLE_HT |
+		       IEEE80211_STA_DISABLE_VHT |
+		       IEEE80211_STA_DISABLE_HE |
+		       IEEE80211_STA_DISABLE_EHT;
 		goto out;
 	}
 
@@ -232,10 +234,10 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			   "Wrong control channel: center-freq: %d ht-cfreq: %d ht->primary_chan: %d band: %d - Disabling HT\n",
 			   channel->center_freq, ht_cfreq,
 			   ht_oper->primary_chan, channel->band);
-		ret = IEEE80211_STA_DISABLE_HT |
-		      IEEE80211_STA_DISABLE_VHT |
-		      IEEE80211_STA_DISABLE_HE |
-		      IEEE80211_STA_DISABLE_EHT;
+		ret |= IEEE80211_STA_DISABLE_HT |
+		       IEEE80211_STA_DISABLE_VHT |
+		       IEEE80211_STA_DISABLE_HE |
+		       IEEE80211_STA_DISABLE_EHT;
 		goto out;
 	}
 
@@ -245,7 +247,7 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	} else {
 		mlme_dbg(sdata, "40 MHz not supported\n");
 		/* 40 MHz (and 80 MHz) must be supported for VHT */
-		ret = IEEE80211_STA_DISABLE_VHT;
+		ret |= IEEE80211_STA_DISABLE_VHT;
 		/* also mark 40 MHz disabled */
 		ret |= IEEE80211_STA_DISABLE_40MHZ;
 		goto out;
@@ -253,7 +255,7 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 
 	if (!vht_oper || !sband->vht_cap.vht_supported) {
 		mlme_dbg(sdata, "VHT operation missing / VHT not supported\n");
-		ret = IEEE80211_STA_DISABLE_VHT;
+		ret |= IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
 
@@ -276,7 +278,7 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE))
 				sdata_info(sdata,
 					   "HE AP VHT information is invalid, disabling HE\n");
-			ret = IEEE80211_STA_DISABLE_HE | IEEE80211_STA_DISABLE_EHT;
+			ret |= IEEE80211_STA_DISABLE_HE | IEEE80211_STA_DISABLE_EHT;
 			goto out;
 		}
 	} else if (!ieee80211_chandef_vht_oper(&sdata->local->hw,
@@ -286,7 +288,7 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
 				   "AP VHT information is invalid, disabling VHT\n");
-		ret = IEEE80211_STA_DISABLE_VHT;
+		ret |= IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
 
@@ -294,12 +296,11 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
 				   "AP VHT information is invalid, disabling VHT\n");
-		ret = IEEE80211_STA_DISABLE_VHT;
+		ret |= IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
 
 	if (cfg80211_chandef_identical(chandef, &vht_chandef)) {
-		ret = 0;
 		goto out;
 	}
 
@@ -307,14 +308,12 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
 				   "AP VHT information doesn't match HT, disabling VHT\n");
-		ret = IEEE80211_STA_DISABLE_VHT;
+		ret |= IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
 
 	*chandef = vht_chandef;
 
-	ret = 0;
-
 out:
 	/*
 	 * When tracking the current AP, don't do any further checks if the
@@ -354,10 +353,10 @@  ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 					tracking ? 0 :
 						   IEEE80211_CHAN_DISABLED)) {
 		if (WARN_ON(chandef->width == NL80211_CHAN_WIDTH_20_NOHT)) {
-			ret = IEEE80211_STA_DISABLE_HT |
-			      IEEE80211_STA_DISABLE_VHT |
-			      IEEE80211_STA_DISABLE_HE |
-			      IEEE80211_STA_DISABLE_EHT;
+			ret |= IEEE80211_STA_DISABLE_HT |
+			       IEEE80211_STA_DISABLE_VHT |
+			       IEEE80211_STA_DISABLE_HE |
+			       IEEE80211_STA_DISABLE_EHT;
 			break;
 		}