diff mbox series

[RFC,1/4] ACS: extract bw40/80/160 freqs out of acs_usable_bwXXX_chan

Message ID 20220324141920.317515-2-nico.escande@gmail.com
State RFC
Headers show
Series ACS: better channel selection for 40/80/160 MHz | expand

Commit Message

Nicolas Escande March 24, 2022, 2:19 p.m. UTC 
This extracts the 3 lists of allowed channels for 40/80/160MHz bandwidth
out of their respective functions. It also ads for each segment the
frequency of the segment's last channel & the number of the segment's
"center" channel.

This is preparativie work to allow selecting a channel which is not the
first of the segment for 40/80/160MHz

Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
---
 src/ap/acs.c | 67 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 17 deletions(-)

Comments

Jouni Malinen April 24, 2022, 1:42 p.m. UTC | #1 
On Thu, Mar 24, 2022 at 03:19:17PM +0100, Nicolas Escande wrote:
> diff --git a/src/ap/acs.c b/src/ap/acs.c
> +const struct {
> +	int first;
> +	int last;
> +	int center_chan;
> +} bw40_seg[] = { { 5180, 5200, 38 }, { 5220, 5240, 46 }, { 5260, 5280, 54 },

This would make the bw40_seg visible to out her files.. Shouldn't this
have "static" somewhere if the list is moved to the file global level
from the function?

>  static int acs_usable_bw40_chan(const struct hostapd_channel_data *chan)
> -	for (i = 0; i < ARRAY_SIZE(allowed); i++)
> -		if (chan->freq == allowed[i])
> +	for (i = 0; i < ARRAY_SIZE(bw40_seg); i++)
> +		if (chan->freq == bw40_seg[i].first)
>  			return 1;
>  
>  	return 0;

And with this type of design, it would seem cleaner to replace these
separate acs_usable_bw*_chan() functions with a single one that takes
the appropriate table (e.g., bw40_seg) as an argument to avoid
duplicated implementation. This would also apply for the patch 2/4
acs_get_bw*_center_chan() functions.

It might be easier to have an explicit termination entry in the array
(e.g., first = last = center_chan = -1) instead of depending on
ARRAY_SIZE() for that. And the struct could be named as well for this to
work cleanly.
Nicolas Escande April 26, 2022, 8:34 a.m. UTC | #2 
On Sun Apr 24, 2022 at 3:42 PM CEST, Jouni Malinen wrote:
> On Thu, Mar 24, 2022 at 03:19:17PM +0100, Nicolas Escande wrote:
> > diff --git a/src/ap/acs.c b/src/ap/acs.c
> > +const struct {
> > +	int first;
> > +	int last;
> > +	int center_chan;
> > +} bw40_seg[] = { { 5180, 5200, 38 }, { 5220, 5240, 46 }, { 5260, 5280, 54 },
>
> This would make the bw40_seg visible to out her files.. Shouldn't this
> have "static" somewhere if the list is moved to the file global level
> from the function?

Yes

>
> >  static int acs_usable_bw40_chan(const struct hostapd_channel_data *chan)
> > -	for (i = 0; i < ARRAY_SIZE(allowed); i++)
> > -		if (chan->freq == allowed[i])
> > +	for (i = 0; i < ARRAY_SIZE(bw40_seg); i++)
> > +		if (chan->freq == bw40_seg[i].first)
> >  			return 1;
> >  
> >  	return 0;
>
> And with this type of design, it would seem cleaner to replace these
> separate acs_usable_bw*_chan() functions with a single one that takes
> the appropriate table (e.g., bw40_seg) as an argument to avoid
> duplicated implementation. This would also apply for the patch 2/4
> acs_get_bw*_center_chan() functions.
>
> It might be easier to have an explicit termination entry in the array
> (e.g., first = last = center_chan = -1) instead of depending on
> ARRAY_SIZE() for that. And the struct could be named as well for this to
> work cleanly.

Sure, fine by me, I'll do it like that.

>
> -- 
> Jouni Malinen                                            PGP id EFC895FA
diff mbox series

Patch

diff --git a/src/ap/acs.c b/src/ap/acs.c
index e2ac1d0d3..40df377f0 100644
--- a/src/ap/acs.c
+++ b/src/ap/acs.c
@@ -371,47 +371,80 @@  acs_survey_chan_interference_factor(struct hostapd_iface *iface,
 }
 
 
+const struct {
+	int first;
+	int last;
+	int center_chan;
+} bw40_seg[] = { { 5180, 5200, 38 }, { 5220, 5240, 46 }, { 5260, 5280, 54 },
+		 { 5300, 5320, 62 }, { 5500, 5520, 102 }, { 5540, 5560, 110 },
+		 { 5580, 5600, 110 }, { 5620, 5640, 126}, { 5660, 5680, 134 },
+		 { 5700, 5720, 142 }, { 5745, 5765, 151 }, { 5785, 5805, 159 },
+		 { 5825, 5845, 167 }, { 5865, 5885, 175 },
+		 { 5955, 5975, 3 }, { 5995, 6015, 11 }, { 6035, 6055, 19 },
+		 { 6075, 6095, 27 }, { 6115, 6135, 35 }, { 6155, 6175, 43 },
+		 { 6195, 6215, 51 }, { 6235, 6255, 59 }, { 6275, 6295, 67 },
+		 { 6315, 6335, 75 }, { 6355, 6375, 83 }, { 6395, 6415, 91 },
+		 { 6435, 6455, 99 }, { 6475, 6495, 107 }, { 6515, 6535, 115 },
+		 { 6555, 6575, 123 }, { 6595, 6615, 131 }, { 6635, 6655, 139 },
+		 { 6675, 6695, 147 }, { 6715, 6735, 155 }, { 6755, 6775, 163 },
+		 { 6795, 6815, 171 }, { 6835, 6855, 179 }, { 6875, 6895, 187 },
+		 { 6915, 6935, 195 }, { 6955, 6975, 203 }, { 6995, 7015, 211 },
+		 { 7035, 7055, 219 }, { 7075, 7115, 227} };
+
 static int acs_usable_bw40_chan(const struct hostapd_channel_data *chan)
 {
-	const int allowed[] = { 5180, 5220, 5260, 5300, 5500, 5540, 5580, 5620,
-				5660, 5745, 5785, 4920, 4960, 5955, 5995, 6035,
-				6075, 6115, 6155, 6195, 6235, 6275, 6315, 6355,
-				6395, 6435, 6475, 6515, 6555, 6595, 6635, 6675,
-				6715, 6755, 6795, 6835, 6875, 6915, 6955, 6995,
-				7035, 7075 };
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(allowed); i++)
-		if (chan->freq == allowed[i])
+	for (i = 0; i < ARRAY_SIZE(bw40_seg); i++)
+		if (chan->freq == bw40_seg[i].first)
 			return 1;
 
 	return 0;
 }
 
 
+const struct {
+	int first;
+	int last;
+	int center_chan;
+} bw80_seg[] = { { 5180, 5240, 42 }, { 5260, 5320, 58 }, { 5500, 5560, 106 },
+		 { 5580, 5640, 122 }, { 5660, 5720, 138 }, { 5745, 5805, 155 },
+		 { 5825, 5885, 171},
+		 { 5955, 6015, 7 }, { 6035, 6095, 23 }, { 6115, 6175, 39 },
+		 { 6195, 6255, 55 }, { 6275, 6335, 71 }, { 6355, 6415, 87 },
+		 { 6435, 6495, 103 }, { 6515, 6575, 119 }, { 6595, 6655, 135 },
+		 { 6675, 6735, 151 }, { 6755, 6815, 167 }, { 6835, 6895, 183 },
+		 { 6915, 6975, 199 }, { 6995, 7055, 215 } };
+
+
 static int acs_usable_bw80_chan(const struct hostapd_channel_data *chan)
 {
-	const int allowed[] = { 5180, 5260, 5500, 5580, 5660, 5745, 5955, 6035,
-				6115, 6195, 6275, 6355, 6435, 6515, 6595, 6675,
-				6755, 6835, 6915, 6995 };
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(allowed); i++)
-		if (chan->freq == allowed[i])
+	for (i = 0; i < ARRAY_SIZE(bw80_seg); i++)
+		if (chan->freq == bw80_seg[i].first)
 			return 1;
 
 	return 0;
 }
 
 
+const struct {
+	int first;
+	int last;
+	int center_chan;
+} bw160_seg[] = { { 5180, 5320, 50 }, { 5500, 5640, 114 }, {5745, 5885, 163 },
+		  { 5955, 6095, 15 }, { 6115, 6255, 47 }, { 6275, 6415, 79 },
+		  { 6435, 6575, 111 }, { 6595, 6735, 143 },
+		  { 6755, 6895, 175 }, { 6915, 7055, 207 } };
+
+
 static int acs_usable_bw160_chan(const struct hostapd_channel_data *chan)
 {
-	const int allowed[] = { 5180, 5500, 5955, 6115, 6275, 6435, 6595, 6755,
-				6915 };
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(allowed); i++)
-		if (chan->freq == allowed[i])
+	for (i = 0; i < ARRAY_SIZE(bw160_seg); i++)
+		if (chan->freq == bw160_seg[i].first)
 			return 1;
 
 	return 0;