diff mbox series

[RFC AP: ACS: select primary channel for 5G-band based on interference factor

Message ID 1603715517413.92611@itmh.ru
State Changes Requested
Headers show
Series [RFC AP: ACS: select primary channel for 5G-band based on interference factor | expand

Commit Message

Марков Михаил Александрович Oct. 26, 2020, 12:31 p.m. UTC 
Make ACS algorithm select the best channel within a bandwidth as primary
channel instead of just picking the first one.

The reason i had to add acs_adjust_sec_chan is because even if i use VHT80
ieee80211n_allowed_ht40_channel_pair -> allowed_ht40_channel_pair
check fails and i get fallback to 20 MHz.

Signed-off-by: Markov Mikhail <markov.mikhail@itmh.ru>
---
 src/ap/acs.c       | 42 ++++++++++++++++++++++++++++++++++--------
 src/ap/ap_config.h |  1 +
 2 files changed, 35 insertions(+), 8 deletions(-)

Comments

Jouni Malinen Feb. 27, 2021, 4:19 p.m. UTC | #1 
On Mon, Oct 26, 2020 at 12:31:59PM +0000, Марков Михаил Александрович wrote:
> Make ACS algorithm select the best channel within a bandwidth as primary
> channel instead of just picking the first one.
> 
> The reason i had to add acs_adjust_sec_chan is because even if i use VHT80
> ieee80211n_allowed_ht40_channel_pair -> allowed_ht40_channel_pair
> check fails and i get fallback to 20 MHz.

Can you please share hostapd debug logs for a case with and without this
patch highlighting why this is needed?

> diff --git a/src/ap/acs.c b/src/ap/acs.c
> +static void acs_adjust_sec_chan(struct hostapd_config *conf)
> +{
> +	const int ht40_plus_channels[] = { 36, 44, 52, 60, 100, 108, 116, 124, 132, 140,
> +								149, 157, 165, 184, 192 };
> +	int i, sec_chan = -1;
> +
> +	for (i = 0; i < ARRAY_SIZE(ht40_plus_channels); i++) {
> +		if (conf->channel == ht40_plus_channels[i]) {
> +		    sec_chan = 1;
> +		    break;
> +		}
> +	}
> +
> +	conf->secondary_channel = sec_chan;
>  }

So this function would force conf->secondary_channel to be != 0.

> @@ -961,8 +985,10 @@ static void acs_study(struct hostapd_iface *iface)
> -	if (iface->conf->ieee80211ac || iface->conf->ieee80211ax)
> +	if (iface->conf->ieee80211ac || iface->conf->ieee80211ax) {
>  		acs_adjust_center_freq(iface);
> +		acs_adjust_sec_chan(iface->conf);
> +	}

And it would be called unconditionally for VHT and HE.. Is that really
correct? What if the configuration is requesting 20 MHz channel
bandwidth to be used?
Марков Михаил Александрович Feb. 27, 2021, 6:41 p.m. UTC | #2 
>Can you please share hostapd debug logs for a case with and without this
>patch highlighting why this is needed?
With https://pastebin.com/WVeeBiDD
Without https://pastebin.com/VLZhJf3S
Both times VHT80 requested.

>And it would be called unconditionally for VHT and HE.. Is that really
>correct? What if the configuration is requesting 20 MHz channel
>bandwidth to be used?
I can not say for sure how would it work for HE, but if i use HT20 it
works correctly. Here is `iw dev' output:
phy#3
	Interface wlan1
		ifindex 12
		wdev 0x300000002
		addr 7a:f0:82:62:19:11
		ssid xxx
		type AP
		channel 40 (5200 MHz), width: 20 MHz, center1: 5200 MHz
		txpower 20.00 dBm
		multicast TXQ:
			qsz-byt	qsz-pkt	flows	drops	marks	...
			0	0	0	0	0

I do realize that setting "conf->secondary_channel to be != 0" could be
not the best decision, but `allowed_ht40_channel_pair' check messes
everything. That's why i tried to RFC.
diff mbox series

Patch

diff --git a/src/ap/acs.c b/src/ap/acs.c
index aa2ceb0d1..1ac37271d 100644
--- a/src/ap/acs.c
+++ b/src/ap/acs.c
@@ -642,9 +642,10 @@  acs_find_ideal_chan_mode(struct hostapd_iface *iface,
 			 int n_chans, u32 bw,
 			 struct hostapd_channel_data **rand_chan,
 			 struct hostapd_channel_data **ideal_chan,
+			 struct hostapd_channel_data **first_chan,
 			 long double *ideal_factor)
 {
-	struct hostapd_channel_data *chan, *adj_chan = NULL;
+	struct hostapd_channel_data *chan, *adj_chan = NULL, *pri_chan;
 	long double factor;
 	int i, j;
 	unsigned int k;
@@ -653,7 +654,7 @@  acs_find_ideal_chan_mode(struct hostapd_iface *iface,
 		double total_weight;
 		struct acs_bias *bias, tmp_bias;
 
-		chan = &mode->channels[i];
+		pri_chan = chan = &mode->channels[i];
 
 		/* Since in the current ACS implementation the first channel is
 		 * always a primary channel, skip channels not available as
@@ -727,6 +728,9 @@  acs_find_ideal_chan_mode(struct hostapd_iface *iface,
 			if (acs_usable_chan(adj_chan)) {
 				factor += adj_chan->interference_factor;
 				total_weight += 1;
+
+				if (adj_chan->interference_factor < pri_chan->interference_factor)
+					pri_chan = adj_chan;
 			}
 		}
 
@@ -805,7 +809,8 @@  acs_find_ideal_chan_mode(struct hostapd_iface *iface,
 		if (acs_usable_chan(chan) &&
 		    (!*ideal_chan || factor < *ideal_factor)) {
 			*ideal_factor = factor;
-			*ideal_chan = chan;
+			*ideal_chan = pri_chan;
+			*first_chan = chan;
 		}
 
 		/* This channel would at least be usable */
@@ -825,7 +830,7 @@  static struct hostapd_channel_data *
 acs_find_ideal_chan(struct hostapd_iface *iface)
 {
 	struct hostapd_channel_data *ideal_chan = NULL,
-		*rand_chan = NULL;
+		*rand_chan = NULL, *first_chan = NULL;
 	long double ideal_factor = 0;
 	int i;
 	int n_chans = 1;
@@ -866,14 +871,16 @@  acs_find_ideal_chan(struct hostapd_iface *iface)
 		mode = &iface->hw_features[i];
 		if (!hostapd_hw_skip_mode(iface, mode))
 			acs_find_ideal_chan_mode(iface, mode, n_chans, bw,
-						 &rand_chan, &ideal_chan,
+						 &rand_chan, &ideal_chan, &first_chan,
 						 &ideal_factor);
 	}
 
 	if (ideal_chan) {
 		wpa_printf(MSG_DEBUG, "ACS: Ideal channel is %d (%d MHz) with total interference factor of %Lg",
 			   ideal_chan->chan, ideal_chan->freq, ideal_factor);
-		return ideal_chan;
+		iface->conf->bw_first_chan = first_chan->chan;
+		/* Primary channel selection belongs only to 5G band. */
+		return is_24ghz_mode(iface->current_mode->mode)? first_chan : ideal_chan;
 	}
 
 	return rand_chan;
@@ -905,7 +912,24 @@  static void acs_adjust_center_freq(struct hostapd_iface *iface)
 	}
 
 	hostapd_set_oper_centr_freq_seg0_idx(iface->conf,
-					     iface->conf->channel + offset);
+						iface->conf->bw_first_chan + offset);
+}
+
+
+static void acs_adjust_sec_chan(struct hostapd_config *conf)
+{
+	const int ht40_plus_channels[] = { 36, 44, 52, 60, 100, 108, 116, 124, 132, 140,
+								149, 157, 165, 184, 192 };
+	int i, sec_chan = -1;
+
+	for (i = 0; i < ARRAY_SIZE(ht40_plus_channels); i++) {
+		if (conf->channel == ht40_plus_channels[i]) {
+		    sec_chan = 1;
+		    break;
+		}
+	}
+
+	conf->secondary_channel = sec_chan;
 }
 
 
@@ -961,8 +985,10 @@  static void acs_study(struct hostapd_iface *iface)
 	iface->conf->channel = ideal_chan->chan;
 	iface->freq = ideal_chan->freq;
 
-	if (iface->conf->ieee80211ac || iface->conf->ieee80211ax)
+	if (iface->conf->ieee80211ac || iface->conf->ieee80211ax) {
 		acs_adjust_center_freq(iface);
+		acs_adjust_sec_chan(iface->conf);
+	}
 
 	err = 0;
 fail:
diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index bada04c3e..18d662c6e 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -909,6 +909,7 @@  struct hostapd_config {
 	int fragm_threshold;
 	u8 op_class;
 	u8 channel;
+	u8 bw_first_chan;
 	int enable_edmg;
 	u8 edmg_channel;
 	u8 acs;