linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] wifi: mac80211: fix EHT issue in ieee80211_determine_chantype
@ 2022-09-29 15:16 greearb
  2022-10-07 12:46 ` Johannes Berg
  0 siblings, 1 reply; 3+ messages in thread
From: greearb @ 2022-09-29 15:16 UTC (permalink / raw)
  To: linux-wireless; +Cc: Ben Greear

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(-)

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;
 		}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] wifi: mac80211: fix EHT issue in ieee80211_determine_chantype
  2022-09-29 15:16 [PATCH] wifi: mac80211: fix EHT issue in ieee80211_determine_chantype greearb
@ 2022-10-07 12:46 ` Johannes Berg
  0 siblings, 0 replies; 3+ messages in thread
From: Johannes Berg @ 2022-10-07 12:46 UTC (permalink / raw)
  To: greearb, linux-wireless

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] wifi: mac80211: fix EHT issue in ieee80211_determine_chantype
@ 2023-03-08 23:27 greearb
  0 siblings, 0 replies; 3+ messages in thread
From: greearb @ 2023-03-08 23:27 UTC (permalink / raw)
  To: linux-wireless; +Cc: Ben Greear

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 | 64 ++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index d5a0416d9c67..0da950732949 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -168,7 +168,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 {
 	struct cfg80211_chan_def vht_chandef;
 	struct ieee80211_sta_ht_cap sta_ht_cap;
-	ieee80211_conn_flags_t ret;
+	ieee80211_conn_flags_t ret = 0;
 	u32 ht_cfreq;
 
 	memset(chandef, 0, sizeof(struct cfg80211_chan_def));
@@ -177,17 +177,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_CONN_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_CONN_DISABLE_HT |
-			      IEEE80211_CONN_DISABLE_VHT |
-			      IEEE80211_CONN_DISABLE_HE |
-			      IEEE80211_CONN_DISABLE_EHT;
-		} else {
-			ret = 0;
+			ret |= IEEE80211_CONN_DISABLE_HT |
+			       IEEE80211_CONN_DISABLE_VHT |
+			       IEEE80211_CONN_DISABLE_HE |
+			       IEEE80211_CONN_DISABLE_EHT;
 		}
 		vht_chandef = *chandef;
 		goto out;
@@ -198,10 +199,10 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			chandef->width = ieee80211_s1g_channel_width(channel);
 		}
 
-		ret = IEEE80211_CONN_DISABLE_HT | IEEE80211_CONN_DISABLE_40MHZ |
-		      IEEE80211_CONN_DISABLE_VHT |
-		      IEEE80211_CONN_DISABLE_80P80MHZ |
-		      IEEE80211_CONN_DISABLE_160MHZ;
+		ret |= IEEE80211_CONN_DISABLE_HT | IEEE80211_CONN_DISABLE_40MHZ |
+		       IEEE80211_CONN_DISABLE_VHT |
+		       IEEE80211_CONN_DISABLE_80P80MHZ |
+		       IEEE80211_CONN_DISABLE_160MHZ;
 		goto out;
 	}
 
@@ -210,10 +211,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_CONN_DISABLE_HT |
-		      IEEE80211_CONN_DISABLE_VHT |
-		      IEEE80211_CONN_DISABLE_HE |
-		      IEEE80211_CONN_DISABLE_EHT;
+		ret |= IEEE80211_CONN_DISABLE_HT |
+		       IEEE80211_CONN_DISABLE_VHT |
+		       IEEE80211_CONN_DISABLE_HE |
+		       IEEE80211_CONN_DISABLE_EHT;
 		goto out;
 	}
 
@@ -234,10 +235,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_CONN_DISABLE_HT |
-		      IEEE80211_CONN_DISABLE_VHT |
-		      IEEE80211_CONN_DISABLE_HE |
-		      IEEE80211_CONN_DISABLE_EHT;
+		ret |= IEEE80211_CONN_DISABLE_HT |
+		       IEEE80211_CONN_DISABLE_VHT |
+		       IEEE80211_CONN_DISABLE_HE |
+		       IEEE80211_CONN_DISABLE_EHT;
 		goto out;
 	}
 
@@ -247,7 +248,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_CONN_DISABLE_VHT;
+		ret |= IEEE80211_CONN_DISABLE_VHT;
 		/* also mark 40 MHz disabled */
 		ret |= IEEE80211_CONN_DISABLE_40MHZ;
 		goto out;
@@ -255,7 +256,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_CONN_DISABLE_VHT;
+		ret |= IEEE80211_CONN_DISABLE_VHT;
 		goto out;
 	}
 
@@ -279,7 +280,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			if (!(conn_flags & IEEE80211_CONN_DISABLE_HE))
 				sdata_info(sdata,
 					   "HE AP VHT information is invalid, disabling HE\n");
-			ret = IEEE80211_CONN_DISABLE_HE | IEEE80211_CONN_DISABLE_EHT;
+			ret |= IEEE80211_CONN_DISABLE_HE | IEEE80211_CONN_DISABLE_EHT;
 			goto out;
 		}
 	} else if (!ieee80211_chandef_vht_oper(&sdata->local->hw,
@@ -289,7 +290,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 		if (!(conn_flags & IEEE80211_CONN_DISABLE_VHT))
 			sdata_info(sdata,
 				   "AP VHT information is invalid, disabling VHT\n");
-		ret = IEEE80211_CONN_DISABLE_VHT;
+		ret |= IEEE80211_CONN_DISABLE_VHT;
 		goto out;
 	}
 
@@ -297,12 +298,11 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 		if (!(conn_flags & IEEE80211_CONN_DISABLE_VHT))
 			sdata_info(sdata,
 				   "AP VHT information is invalid, disabling VHT\n");
-		ret = IEEE80211_CONN_DISABLE_VHT;
+		ret |= IEEE80211_CONN_DISABLE_VHT;
 		goto out;
 	}
 
 	if (cfg80211_chandef_identical(chandef, &vht_chandef)) {
-		ret = 0;
 		goto out;
 	}
 
@@ -310,7 +310,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 		if (!(conn_flags & IEEE80211_CONN_DISABLE_VHT))
 			sdata_info(sdata,
 				   "AP VHT information doesn't match HT, disabling VHT\n");
-		ret = IEEE80211_CONN_DISABLE_VHT;
+		ret |= IEEE80211_CONN_DISABLE_VHT;
 		goto out;
 	}
 
@@ -333,7 +333,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			if (!(conn_flags & IEEE80211_CONN_DISABLE_EHT))
 				sdata_info(sdata,
 					   "AP EHT information is invalid, disabling EHT\n");
-			ret = IEEE80211_CONN_DISABLE_EHT;
+			ret |= IEEE80211_CONN_DISABLE_EHT;
 			goto out;
 		}
 
@@ -341,7 +341,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 			if (!(conn_flags & IEEE80211_CONN_DISABLE_EHT))
 				sdata_info(sdata,
 					   "AP EHT information is incompatible, disabling EHT\n");
-			ret = IEEE80211_CONN_DISABLE_EHT;
+			ret |= IEEE80211_CONN_DISABLE_EHT;
 			goto out;
 		}
 
@@ -389,10 +389,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_CONN_DISABLE_HT |
-			      IEEE80211_CONN_DISABLE_VHT |
-			      IEEE80211_CONN_DISABLE_HE |
-			      IEEE80211_CONN_DISABLE_EHT;
+			ret |= IEEE80211_CONN_DISABLE_HT |
+				IEEE80211_CONN_DISABLE_VHT |
+				IEEE80211_CONN_DISABLE_HE |
+				IEEE80211_CONN_DISABLE_EHT;
 			break;
 		}
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-08 23:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 15:16 [PATCH] wifi: mac80211: fix EHT issue in ieee80211_determine_chantype greearb
2022-10-07 12:46 ` Johannes Berg
2023-03-08 23:27 greearb

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).