diff mbox series

ACS: new hw_mode HOSTAPD_MODE_IEEE80211AG for dual band's channels

Message ID 1568885512-6507-1-git-send-email-neojou@gmail.com
State Changes Requested
Headers show
Series ACS: new hw_mode HOSTAPD_MODE_IEEE80211AG for dual band's channels | expand

Commit Message

Neo Jou Sept. 19, 2019, 9:31 a.m. UTC 
From: Neo Jou <neojou@gmail.com>

A new hw_mode HOSTAPD_MODE_IEEE80211AG is proposed for ACS to
compute 2.4G's and 5G's channels.

At before, in hostapd.conf, hw_mode=a is set for 5G only, and
hw_mode=g is set for 2.4G only. Also hw_mode=any is special case
only with drivers with which offloaded ACS is used. So we would
like to suggest a new hw_mode for ACS to compute 2.4G and 5G band
both when the driver doesn't support offloaded ACS.

In this patch:

1. A new hw_mode, HOSTAPD_MODE_IEEE80211AG is proposed, and conf->hw
   will be set to this mode when "hw_mode=ag" is set in hostapd.conf.

2. If the driver supports 2.4G (g) and 5G (a) band, then the hw_mode
   HOSTAPD_MODE_IEEE80211AG is added by
   wpa_driver_nl80211_postprocess_mode() with all channels/rates which
   the driver supports in 2.4G and 5G bands. (If this interface only
   supports 2.4G or 5G single band, not dual band, then this hw mode
   won't be created for the interface)

3. After ACS process is completed, iface->current_mode will be set
   again to HOSTAPD_MODE_IEEE80211G or HOSTAPD_MODE_IEEE80211A,
   depends on the computed ideal channel.

This HOSTAPD_MODE_IEEE80211AG is only for ACS, so it fails to
intiailize interface when hw_mode=ag and channel is not 0 (not ACS)
in hostapd.conf.

Signed-off-by: Neo Jou <neojou@gmail.com>
---
 hostapd/config_file.c             |  2 ++
 src/ap/hw_features.c              | 24 +++++++++++++
 src/common/defs.h                 |  1 +
 src/drivers/driver_nl80211_capa.c | 71 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+)

Comments

Jouni Malinen Dec. 25, 2019, 9:19 p.m. UTC | #1 
On Thu, Sep 19, 2019 at 05:31:52PM +0800, neojou@gmail.com wrote:
> A new hw_mode HOSTAPD_MODE_IEEE80211AG is proposed for ACS to
> compute 2.4G's and 5G's channels.
> 
> At before, in hostapd.conf, hw_mode=a is set for 5G only, and
> hw_mode=g is set for 2.4G only. Also hw_mode=any is special case
> only with drivers with which offloaded ACS is used. So we would
> like to suggest a new hw_mode for ACS to compute 2.4G and 5G band
> both when the driver doesn't support offloaded ACS.

Why would this need a new hw_mode=ag instead of extending the existing
hw_mode=any to cover the not-offloaded-ACS case?

> 1. A new hw_mode, HOSTAPD_MODE_IEEE80211AG is proposed, and conf->hw
>    will be set to this mode when "hw_mode=ag" is set in hostapd.conf.

I'd like to drop this part. The existing hw_mode=any and freqlist
parameters (if needed to exclude some frequencies) should work fine here
and I'd rather keep offloaded and not offloaded cases using the exact
same configuration so that the upper layer components would not need to
know whether the driver offloads ACS.

> diff --git a/src/ap/hw_features.c b/src/ap/hw_features.c
> @@ -832,6 +832,30 @@ int hostapd_acs_completed(struct hostapd_iface *iface, int err)
> +		if (iface->current_mode->mode == HOSTAPD_MODE_IEEE80211AG) {
> +			int i;
> +			enum hostapd_hw_mode target_mode;
> +
> +			if (iface->conf->channel<=14)
> +				target_mode = HOSTAPD_MODE_IEEE80211G;
> +			else
> +				target_mode = HOSTAPD_MODE_IEEE80211A;

This does not really work since there are channels 1..14 in the 5 GHz
band as well. Much more of the ACS functionality is now using
frequencies instead of channel numbers (which are unique only in
combination with the operating class) and whatever changes are done for
supporting multi-band operations should really be done in a way that
does not open such ambiguous checks even if the 5 GHz channels in that
range are not commonly used.

> diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c
> @@ -1832,6 +1873,36 @@ wpa_driver_nl80211_postprocess_modes(struct hostapd_hw_modes *modes,
> +	/* add AG mode if 2.4GHz and 5GHz are all supported */
...

Why would this be needed? I don't really like this part at all..
Shouldn't the upper layer code be able to find the matching mode and
channel from the existing HOSTAPD_MODE_IEEE80211G/_IEEE80211A entries?
Neo Jou Jan. 3, 2020, 2:55 a.m. UTC | #2 
Thanks for the kind reply.

 This change looks superfluous and not good. Please abandon it.
 I am thinking for another solution, as
    https://patchwork.ozlabs.org/patch/1217027/
 and would like to ask for the advise and help.

 I agree that " Shouldn't the upper layer code be able to find the
matching mode and
 channel from the existing HOSTAPD_MODE_IEEE80211G/_IEEE80211A entries".
 At first I met problem at Android VTS test, since "the upper layer
code" doesn't consider
 this situation...  The issue was recorded at:
 https://issuetracker.google.com/issues/145944392
 That's why there comes this temporary solution,  just to make Android
VTS pass...
 https://android-review.googlesource.com/c/platform/external/wpa_supplicant_8/+/1180867

 I am thinking if there is any general approach for not only QCA, but other
 Wifi vendor also. It seems hard to decide which OUI hostapd can use as
 the general OUI for hostapd to send Linux wiphy vendor command...

 Any guidance or advice would be appreciated.


 BR,
 Neo Jou
Jouni Malinen Jan. 4, 2020, 9:49 p.m. UTC | #3 
On Fri, Jan 03, 2020 at 10:55:23AM +0800, Neo Jou wrote:
>  I agree that " Shouldn't the upper layer code be able to find the
> matching mode and
>  channel from the existing HOSTAPD_MODE_IEEE80211G/_IEEE80211A entries".
>  At first I met problem at Android VTS test, since "the upper layer
> code" doesn't consider
>  this situation...  The issue was recorded at:
>  https://issuetracker.google.com/issues/145944392
>  That's why there comes this temporary solution,  just to make Android
> VTS pass...
>  https://android-review.googlesource.com/c/platform/external/wpa_supplicant_8/+/1180867

That issue sounds like a valid issue, but that proposed change does not
look at all like a good way of handling this. If hw_mode=any does not
work with not offloaded ACS, it would be appropriate to extend that to
work rather than try to somehow hide the fact that this functionality
has not yet been implemented or try to make upper layer components have
to be aware of such difference in capabilities.

>  I am thinking if there is any general approach for not only QCA, but other
>  Wifi vendor also. It seems hard to decide which OUI hostapd can use as
>  the general OUI for hostapd to send Linux wiphy vendor command...

I'm not sure I follow what you mean here. hostapd has two available ACS
options today: internal one using channel survey mechanism and an
offloaded (to the driver) if the driver advertises support for that.
There should be no need to be deciding which OUI to use since that
happens automatically based on driver capabilities. All this really
needs is straightforward extension of the not offloaded ACS to provide
same level of functionality as the offloaded case.
diff mbox series

Patch

diff --git a/hostapd/config_file.c b/hostapd/config_file.c
index 0d340d2..88b5a99 100644
--- a/hostapd/config_file.c
+++ b/hostapd/config_file.c
@@ -3112,6 +3112,8 @@  static int hostapd_config_fill(struct hostapd_config *conf,
 			conf->hw_mode = HOSTAPD_MODE_IEEE80211AD;
 		else if (os_strcmp(pos, "any") == 0)
 			conf->hw_mode = HOSTAPD_MODE_IEEE80211ANY;
+		else if (os_strcmp(pos, "ag") == 0)
+			conf->hw_mode = HOSTAPD_MODE_IEEE80211AG;
 		else {
 			wpa_printf(MSG_ERROR, "Line %d: unknown hw_mode '%s'",
 				   line, pos);
diff --git a/src/ap/hw_features.c b/src/ap/hw_features.c
index c1f19e2..285ccfe 100644
--- a/src/ap/hw_features.c
+++ b/src/ap/hw_features.c
@@ -832,6 +832,30 @@  int hostapd_acs_completed(struct hostapd_iface *iface, int err)
 			hostapd_hw_get_freq(iface->bss[0],
 					    iface->conf->channel),
 			iface->conf->channel);
+		if (iface->current_mode->mode == HOSTAPD_MODE_IEEE80211AG) {
+			int i;
+			enum hostapd_hw_mode target_mode;
+
+			if (iface->conf->channel<=14)
+				target_mode = HOSTAPD_MODE_IEEE80211G;
+			else
+				target_mode = HOSTAPD_MODE_IEEE80211A;
+
+			for (i = 0; i < iface->num_hw_features; i++) {
+				struct hostapd_hw_modes *mode = &iface->hw_features[i];
+				if (mode->mode == target_mode) {
+					iface->current_mode = mode;
+					break;
+				}
+			}
+
+			if (iface->current_mode->mode == HOSTAPD_MODE_IEEE80211AG) {
+				wpa_printf(MSG_ERROR, "ACS error - can not decide A band or G band");
+				wpa_msg(iface->bss[0]->msg_ctx, MSG_INFO, ACS_EVENT_FAILED);
+				hostapd_notify_bad_chans(iface);
+				goto out;
+			}
+		}
 		break;
 	case HOSTAPD_CHAN_ACS:
 		wpa_printf(MSG_ERROR, "ACS error - reported complete, but no result available");
diff --git a/src/common/defs.h b/src/common/defs.h
index 4faf1c8..b531bc0 100644
--- a/src/common/defs.h
+++ b/src/common/defs.h
@@ -349,6 +349,7 @@  enum hostapd_hw_mode {
 	HOSTAPD_MODE_IEEE80211A,
 	HOSTAPD_MODE_IEEE80211AD,
 	HOSTAPD_MODE_IEEE80211ANY,
+	HOSTAPD_MODE_IEEE80211AG,
 	NUM_HOSTAPD_MODES
 };
 
diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c
index 8318b10..afc1570 100644
--- a/src/drivers/driver_nl80211_capa.c
+++ b/src/drivers/driver_nl80211_capa.c
@@ -1743,6 +1743,43 @@  static int phy_info_handler(struct nl_msg *msg, void *arg)
 	return NL_SKIP;
 }
 
+static int
+wpa_driver_nl80211_hw_mode_append(struct hostapd_hw_modes *mode,
+				  struct hostapd_hw_modes *mode11)
+{
+	struct hostapd_channel_data *new_channels;
+	int *new_rates;
+
+	if ((mode11->num_channels == 0) || (mode11->num_rates == 0)) {
+		wpa_printf(MSG_ERROR, "%s: num_channels or num_rates is 0", __func__);
+		return -1;
+	}
+
+	new_channels = os_realloc_array(mode->channels,
+					mode->num_channels + mode11->num_channels,
+					sizeof(struct hostapd_channel_data));
+	if (new_channels == NULL)
+		return -1;
+
+	memcpy(&new_channels[mode->num_channels], mode11->channels,
+		mode11->num_channels * sizeof(struct hostapd_channel_data));
+	mode->channels = new_channels;
+	mode->num_channels += mode11->num_channels;
+
+	new_rates = os_realloc_array(mode->rates, mode->num_rates + mode11->num_rates,
+				     sizeof(int));
+	if ( new_rates == NULL) {
+		os_free(mode->channels);
+		return -1;
+	}
+
+	memcpy(&new_rates[mode->num_rates], mode11->rates,
+		mode11->num_rates * sizeof(int));
+	mode->rates = new_rates;
+	mode->num_rates += mode11->num_rates;
+
+	return 0;
+}
 
 static struct hostapd_hw_modes *
 wpa_driver_nl80211_postprocess_modes(struct hostapd_hw_modes *modes,
@@ -1751,6 +1788,8 @@  wpa_driver_nl80211_postprocess_modes(struct hostapd_hw_modes *modes,
 	u16 m;
 	struct hostapd_hw_modes *mode11g = NULL, *nmodes, *mode;
 	int i, mode11g_idx = -1;
+	struct hostapd_hw_modes *mode11a = NULL;
+	int mode11a_idx = -1;
 
 	/* heuristic to set up modes */
 	for (m = 0; m < *num_modes; m++) {
@@ -1778,6 +1817,8 @@  wpa_driver_nl80211_postprocess_modes(struct hostapd_hw_modes *modes,
 			return modes; /* 802.11b already included */
 		if (modes[m].mode == HOSTAPD_MODE_IEEE80211G)
 			mode11g_idx = m;
+		if (modes[m].mode == HOSTAPD_MODE_IEEE80211A)
+			mode11a_idx = m;
 	}
 
 	if (mode11g_idx < 0)
@@ -1832,6 +1873,36 @@  wpa_driver_nl80211_postprocess_modes(struct hostapd_hw_modes *modes,
 	wpa_printf(MSG_DEBUG, "nl80211: Added 802.11b mode based on 802.11g "
 		   "information");
 
+	if (mode11a_idx < 0) {
+		wpa_printf(MSG_DEBUG, "nl80211: NO 5G band");
+		return modes; /* 5GHz band not supported at all */
+	}
+
+	/* add AG mode if 2.4GHz and 5GHz are all supported */
+	nmodes = os_realloc_array(modes, *num_modes + 1, sizeof(*nmodes));
+	if (nmodes == NULL)
+		return modes; /* Could not add 802.11b mode */
+
+	mode = &nmodes[*num_modes];
+	os_memset(mode, 0, sizeof(*mode));
+	(*num_modes)++;
+	modes = nmodes;
+	mode11g = &modes[mode11g_idx];
+	mode11a = &modes[mode11a_idx];
+
+	mode->mode = HOSTAPD_MODE_IEEE80211AG;
+	mode->num_channels = 0;
+	mode->num_rates = 0;
+        if ((wpa_driver_nl80211_hw_mode_append(mode, mode11g) < 0) ||
+	    (wpa_driver_nl80211_hw_mode_append(mode, mode11a) < 0)) {
+		wpa_printf(MSG_ERROR, "nl80211: Added 802.11a+g failed");
+		(*num_modes)--;
+		return modes; /* Could not add 802.11g mode */
+	}
+
+	wpa_printf(MSG_DEBUG, "nl80211: Added 802.11a+g mode based on 802.11g/a "
+		   "information");
+
 	return modes;
 }