linux-kernel - Re: [PATCH 1/2] wifi: mac80211: add EHT 320 MHz support for mesh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <D5Q10DQM9OC7.3VR13L1UINUVF@gmail.com>
Date: Tue, 19 Nov 2024 09:52:32 +0100
From: "Nicolas Escande" <nico.escande@...il.com>
To: "Aditya Kumar Singh" <quic_adisi@...cinc.com>, "Johannes Berg"
 <johannes@...solutions.net>, "Kalle Valo" <kvalo@...nel.org>
Cc: <linux-wireless@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 "Sathishkumar Muruganandam" <quic_murugana@...cinc.com>
Subject: Re: [PATCH 1/2] wifi: mac80211: add EHT 320 MHz support for mesh

On Tue Nov 19, 2024 at 5:27 AM CET, Aditya Kumar Singh wrote:
> From: Sathishkumar Muruganandam <quic_murugana@...cinc.com>
>
> Currently, ieee80211_ie_build_he_oper() lacks support for 320 MHz handling
> (already noted as a TODO). This is because 320 MHz is not included in
> IEEE 802.11-ax. However, IEEE 802.11-be introduces 320 MHz support and if
> the chandef indicates a 320 MHz bandwidth and is used directly as it is, it
> will result in an incorrect HE Operation Information Element.
>
> In order to support EHT 320 MHz, HE Operation Element should indicate
> bandwidth as 160 MHz only. In EHT Operation IE, the correct bandwidth will
> be present. Devices capable of EHT can parse EHT Information Element and
> connect in 320 MHz and other HE capable devices can parse HE and can
> connect in 160 MHz.
>
> Add support to downgrade the bandwidth in ieee80211_ie_build_he_oper()
> during 320 MHz operation and advertise it.
>
> Signed-off-by: Sathishkumar Muruganandam <quic_murugana@...cinc.com>
> Signed-off-by: Aditya Kumar Singh <quic_adisi@...cinc.com>
> ---
>  net/mac80211/util.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index a4e1301cc999d24d6ab1bd899742a2ff04229040..c88ce537aaa7ea5d59dc42e5f30805c6eb4a5c6d 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -2762,6 +2762,7 @@ u8 *ieee80211_ie_build_he_oper(u8 *pos, const struct cfg80211_chan_def *chandef)
>  {
>  	struct ieee80211_he_operation *he_oper;
>  	struct ieee80211_he_6ghz_oper *he_6ghz_op;
> +	struct cfg80211_chan_def he_chandef;
>  	u32 he_oper_params;
>  	u8 ie_len = 1 + sizeof(struct ieee80211_he_operation);
>  
> @@ -2793,27 +2794,33 @@ u8 *ieee80211_ie_build_he_oper(u8 *pos, const struct cfg80211_chan_def *chandef)
>  	if (chandef->chan->band != NL80211_BAND_6GHZ)
>  		goto out;
>  
> +	cfg80211_chandef_create(&he_chandef, chandef->chan, NL80211_CHAN_NO_HT);
> +	he_chandef.center_freq1 = chandef->center_freq1;
> +	he_chandef.center_freq2 = chandef->center_freq2;
> +	he_chandef.width = chandef->width;
> +
>  	/* TODO add VHT operational */
>  	he_6ghz_op = (struct ieee80211_he_6ghz_oper *)pos;
>  	he_6ghz_op->minrate = 6; /* 6 Mbps */
>  	he_6ghz_op->primary =
> -		ieee80211_frequency_to_channel(chandef->chan->center_freq);
> +		ieee80211_frequency_to_channel(he_chandef.chan->center_freq);
>  	he_6ghz_op->ccfs0 =
> -		ieee80211_frequency_to_channel(chandef->center_freq1);
> -	if (chandef->center_freq2)
> +		ieee80211_frequency_to_channel(he_chandef.center_freq1);
> +	if (he_chandef.center_freq2)
>  		he_6ghz_op->ccfs1 =
> -			ieee80211_frequency_to_channel(chandef->center_freq2);
> +			ieee80211_frequency_to_channel(he_chandef.center_freq2);
>  	else
>  		he_6ghz_op->ccfs1 = 0;
>  
> -	switch (chandef->width) {
> +	switch (he_chandef.width) {
>  	case NL80211_CHAN_WIDTH_320:
> -		/*
> -		 * TODO: mesh operation is not defined over 6GHz 320 MHz
> -		 * channels.
> +		/* Downgrade EHT 320 MHz BW to 160 MHz for HE and set new
> +		 * center_freq1
>  		 */
> -		WARN_ON(1);
> -		break;
> +		ieee80211_chandef_downgrade(&he_chandef, NULL);
> +		he_6ghz_op->ccfs0 =
> +			ieee80211_frequency_to_channel(he_chandef.center_freq1);
I don't understand the full implication of those two calls but I made 320MHz
work by doing the same as for 160 but with +/- = 16 instead of 8.
> +		fallthrough;
>  	case NL80211_CHAN_WIDTH_160:
>  		/* Convert 160 MHz channel width to new style as interop
>  		 * workaround.
> @@ -2821,7 +2828,7 @@ u8 *ieee80211_ie_build_he_oper(u8 *pos, const struct cfg80211_chan_def *chandef)
>  		he_6ghz_op->control =
>  			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ;
>  		he_6ghz_op->ccfs1 = he_6ghz_op->ccfs0;
> -		if (chandef->chan->center_freq < chandef->center_freq1)
> +		if (he_chandef.chan->center_freq < he_chandef.center_freq1)
>  			he_6ghz_op->ccfs0 -= 8;
>  		else
>  			he_6ghz_op->ccfs0 += 8;

Something along this lines seemed to be enough:

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index a4e1301cc999..f24479f74813 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2808,18 +2808,15 @@ u8 *ieee80211_ie_build_he_oper(u8 *pos, const struct cfg80211_chan_def *chandef)
 
 	switch (chandef->width) {
 	case NL80211_CHAN_WIDTH_320:
-		/*
-		 * TODO: mesh operation is not defined over 6GHz 320 MHz
-		 * channels.
-		 */
-		WARN_ON(1);
-		break;
+		if (chandef->chan->center_freq < chandef->center_freq1)
+			he_6ghz_op->ccfs0 -= 16;
+		else
+			he_6ghz_op->ccfs0 += 16;
+		fallthrough;
 	case NL80211_CHAN_WIDTH_160:
 		/* Convert 160 MHz channel width to new style as interop
 		 * workaround.
 		 */
-		he_6ghz_op->control =
-			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ;
 		he_6ghz_op->ccfs1 = he_6ghz_op->ccfs0;
 		if (chandef->chan->center_freq < chandef->center_freq1)
 			he_6ghz_op->ccfs0 -= 8;
-- 
2.47.0