Patchwork [RFC] hostapd: allow configuring driver to VHT

login
register
mail settings
Submitter Johannes Berg
Date Nov. 20, 2012, 9:07 p.m.
Message ID <1353445670-15216-1-git-send-email-johannes@sipsolutions.net>
Download mbox | patch
Permalink /patch/200519/
State Superseded
Headers show

Comments

Johannes Berg - Nov. 20, 2012, 9:07 p.m.
From: Johannes Berg <johannes.berg@intel.com>

Signed-hostap: Johannes Berg <johannes.berg@intel.com>
---
 src/ap/ap_drv_ops.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++-----
 src/ap/ap_drv_ops.h  |  4 +++-
 src/ap/hostapd.c     |  6 +++++-
 src/drivers/driver.h |  9 ++++++++
 4 files changed, 72 insertions(+), 7 deletions(-)
Johannes Berg - Nov. 20, 2012, 9:46 p.m.
>  int hostapd_set_freq(struct hostapd_data *hapd, int mode, int freq,
> -		     int channel, int ht_enabled, int sec_channel_offset)
> +		     int channel, int ht_enabled, int vht_enabled,
> +		     int sec_channel_offset, int vht_oper_chwidth,
> +		     int center_segment0, int center_segment1)

Note I'm just changing this here, and only validate that the HT settings
and VHT settings are compatible.

This probably isn't the right place for this though, I think we should
probably check these things in src/ap/hw_features.c. If anybody wants to
work on that ... I have a bunch of other more important things to worry
about, like parsing the VHT operation IE in mac80211 :-)

johannes
Mahesh Palivela - Nov. 21, 2012, 8:43 a.m.
On 11/21/2012 02:37 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Signed-hostap: Johannes Berg <johannes.berg@intel.com>
> ---
>   src/ap/ap_drv_ops.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++-----
>   src/ap/ap_drv_ops.h  |  4 +++-
>   src/ap/hostapd.c     |  6 +++++-
>   src/drivers/driver.h |  9 ++++++++
>   4 files changed, 72 insertions(+), 7 deletions(-)
>
> diff --git a/src/ap/ap_drv_ops.c b/src/ap/ap_drv_ops.c
> index 02da25b..7b927cf 100644
> --- a/src/ap/ap_drv_ops.c
> +++ b/src/ap/ap_drv_ops.c
> @@ -454,19 +454,69 @@ int hostapd_flush(struct hostapd_data *hapd)
>
>
>   int hostapd_set_freq(struct hostapd_data *hapd, int mode, int freq,
> -		     int channel, int ht_enabled, int sec_channel_offset)
> +		     int channel, int ht_enabled, int vht_enabled,
> +		     int sec_channel_offset, int vht_oper_chwidth,
> +		     int center_segment0, int center_segment1)
>   {
>   	struct hostapd_freq_params data;
> -	if (hapd->driver == NULL)
> -		return 0;
> -	if (hapd->driver->set_freq == NULL)
> -		return 0;
> +	int tmp;
> +
>   	os_memset(&data, 0, sizeof(data));
>   	data.mode = mode;
>   	data.freq = freq;
>   	data.channel = channel;
>   	data.ht_enabled = ht_enabled;
> +	data.vht_enabled = vht_enabled;
>   	data.sec_channel_offset = sec_channel_offset;
> +	data.center_freq1 = freq + sec_channel_offset * 10;
> +	data.center_freq2 = 0;
> +	data.bandwidth = sec_channel_offset ? 40 : 20;
> +
> +	if (data.vht_enabled) switch (vht_oper_chwidth) {
> +	case 0:
> +		if (center_segment1)
> +			return -1;
> +		if (5000 + center_segment0 * 5 != data.center_freq1)
> +			return -1;
> +		break;

instead of case 0, 1 etc, can you define names? can we have name for 5000?

> +	case 3:
> +		data.center_freq2 = 5000 + center_segment1 * 5;
> +		/* fall through */
> +	case 1:
> +		data.bandwidth = 80;
> +		if (vht_oper_chwidth == 1 && center_segment1)
> +			return -1;
> +		if (vht_oper_chwidth == 3 && !center_segment1)
> +			return -1;
> +		if (!sec_channel_offset)
> +			return -1;
> +		/* primary 40 part must match the HT configuration */
> +		tmp = (30 + freq - 5000 - center_segment0 * 5)/20;
> +		tmp /= 2;
> +		if (data.center_freq1 != 5000 +
> +					 center_segment0 * 5 - 20 + 40 * tmp)
> +			return -1;
> +		data.center_freq1 = 5000 + center_segment0 * 5;
> +		break;
> +	case 2:
> +		data.bandwidth = 160;
> +		if (center_segment1)
> +			return -1;
> +		if (!sec_channel_offset)
> +			return -1;
> +		/* primary 40 part must match the HT configuration */
> +		tmp = (70 + freq - 5000 - center_segment0 * 5)/20;
> +		tmp /= 2;
> +		if (data.center_freq1 != 5000 +
> +					 center_segment0 * 5 - 60 + 40 * tmp)
> +			return -1;

Sorry, I couldn't understand your arithmetic. Can you explain with example?

> +		data.center_freq1 = 5000 + center_segment0 * 5;
> +		break;
> +	}
> +	if (hapd->driver == NULL)
> +		return 0;
> +	if (hapd->driver->set_freq == NULL)
> +		return 0;
>   	return hapd->driver->set_freq(hapd->drv_priv, &data);
>   }
>
> diff --git a/src/ap/ap_drv_ops.h b/src/ap/ap_drv_ops.h
> index 9c53b99..768a0ba 100644
> --- a/src/ap/ap_drv_ops.h
> +++ b/src/ap/ap_drv_ops.h
> @@ -55,7 +55,9 @@ int hostapd_get_seqnum(const char *ifname, struct hostapd_data *hapd,
>   		       const u8 *addr, int idx, u8 *seq);
>   int hostapd_flush(struct hostapd_data *hapd);
>   int hostapd_set_freq(struct hostapd_data *hapd, int mode, int freq,
> -		     int channel, int ht_enabled, int sec_channel_offset);
> +		     int channel, int ht_enabled, int vht_enabled,
> +		     int sec_channel_offset, int vht_oper_chwidth,
> +		     int center_segment0, int center_segment1);
>   int hostapd_set_rts(struct hostapd_data *hapd, int rts);
>   int hostapd_set_frag(struct hostapd_data *hapd, int frag);
>   int hostapd_sta_set_flags(struct hostapd_data *hapd, u8 *addr,
> diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
> index 1c968a7..15fb861 100644
> --- a/src/ap/hostapd.c
> +++ b/src/ap/hostapd.c
> @@ -889,7 +889,11 @@ int hostapd_setup_interface_complete(struct hostapd_iface *iface, int err)
>   		if (hostapd_set_freq(hapd, hapd->iconf->hw_mode, iface->freq,
>   				     hapd->iconf->channel,
>   				     hapd->iconf->ieee80211n,
> -				     hapd->iconf->secondary_channel)) {
> +				     hapd->iconf->ieee80211ac,
> +				     hapd->iconf->secondary_channel,
> +				     hapd->iconf->vht_oper_chwidth,
> +				     hapd->iconf->vht_oper_centr_freq_seg0_idx,
> +				     hapd->iconf->vht_oper_centr_freq_seg1_idx)) {
>   			wpa_printf(MSG_ERROR, "Could not set channel for "
>   				   "kernel driver");
>   			return -1;
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> index ccc5a0e..10d639e 100644
> --- a/src/drivers/driver.h
> +++ b/src/drivers/driver.h
> @@ -905,10 +905,19 @@ struct hostapd_freq_params {
>   	int mode;
>   	int freq;
>   	int channel;
> +	/* for HT */
>   	int ht_enabled;
>   	int sec_channel_offset; /* 0 = HT40 disabled, -1 = HT40 enabled,
>   				 * secondary channel below primary, 1 = HT40
>   				 * enabled, secondary channel above primary */
> +
> +	/* for VHT */
> +	int vht_enabled;
> +
> +	/* valid for both HT and VHT, center_freq2 is non-zero
> +	 * only for bandwidth 80 and an 80+80 channel */
> +	int center_freq1, center_freq2;
> +	int bandwidth;
>   };
>
>   enum wpa_driver_if_type {
>
Johannes Berg - Nov. 21, 2012, 9:01 a.m.
On Wed, 2012-11-21 at 14:13 +0530, Mahesh Palivela wrote:

> > +	if (data.vht_enabled) switch (vht_oper_chwidth) {
> > +	case 0:
> > +		if (center_segment1)
> > +			return -1;
> > +		if (5000 + center_segment0 * 5 != data.center_freq1)
> > +			return -1;
> > +		break;
> 
> instead of case 0, 1 etc, can you define names? can we have name for 5000?

Yeah, I guess I should do that.

> > +	case 1:
> > +		data.bandwidth = 80;
> > +		if (vht_oper_chwidth == 1 && center_segment1)
> > +			return -1;
> > +		if (vht_oper_chwidth == 3 && !center_segment1)
> > +			return -1;
> > +		if (!sec_channel_offset)
> > +			return -1;
> > +		/* primary 40 part must match the HT configuration */
> > +		tmp = (30 + freq - 5000 - center_segment0 * 5)/20;
> > +		tmp /= 2;
> > +		if (data.center_freq1 != 5000 +
> > +					 center_segment0 * 5 - 20 + 40 * tmp)
> > +			return -1;
> > +		data.center_freq1 = 5000 + center_segment0 * 5;
> > +		break;
> > +	case 2:
> > +		data.bandwidth = 160;
> > +		if (center_segment1)
> > +			return -1;
> > +		if (!sec_channel_offset)
> > +			return -1;
> > +		/* primary 40 part must match the HT configuration */
> > +		tmp = (70 + freq - 5000 - center_segment0 * 5)/20;
> > +		tmp /= 2;
> > +		if (data.center_freq1 != 5000 +
> > +					 center_segment0 * 5 - 60 + 40 * tmp)
> > +			return -1;
> 
> Sorry, I couldn't understand your arithmetic. Can you explain with example?

Oh, well, it's a little tricky, I had to draw a diagram:

Imagine each - is 5 MHz, so this is a 160 MHz channel, split into the 20
MHz subchannels:

|----|----|----|----|----|----|----|----|

Now each one of these could be the primary 20 MHz channel, which has the
center freq "freq" (in the code.)

center_segment0 points to the middle of the whole thing, so
	freq - (center_segment0 *5+5000)
will be -70, -50, -30, -10, 10, 30, 50, 70

Adding 70 will lead to 0, 20, 40, ..., 140
Divide by 20 will give us 0,...,7
Divide by 2 will give 0 ... 3, mapping 0,1->0, 2,3->1 etc.

data.center_freq1 is the center of the primary 40 MHz channel (as
calculated by the HT information, not VHT)

Now VHT must match this and given the 0..3 index we can calculate what
on the VHT side is the primary 40 MHz channel's center frequency --
which is the 160 MHz channel's center freq, minus 60, plus 40 * the
index.

Yeah, it's tricky ... This is vaguely based on pages 193-195 in the VHT
draft (formulas 22-1 through 22-7 and table 22-7)

johannes
Mahesh Palivela - Nov. 22, 2012, 7 a.m.
On 11/21/2012 02:31 PM, Johannes Berg wrote:
> On Wed, 2012-11-21 at 14:13 +0530, Mahesh Palivela wrote:
>
>>> +	if (data.vht_enabled) switch (vht_oper_chwidth) {
>>> +	case 0:
>>> +		if (center_segment1)
>>> +			return -1;
>>> +		if (5000 + center_segment0 * 5 != data.center_freq1)
>>> +			return -1;
>>> +		break;
>>
>> instead of case 0, 1 etc, can you define names? can we have name for 5000?
>
> Yeah, I guess I should do that.
>

Thanks.

>>> +	case 1:
>>> +		data.bandwidth = 80;
>>> +		if (vht_oper_chwidth == 1 && center_segment1)
>>> +			return -1;
>>> +		if (vht_oper_chwidth == 3 && !center_segment1)
>>> +			return -1;
>>> +		if (!sec_channel_offset)
>>> +			return -1;
>>> +		/* primary 40 part must match the HT configuration */
>>> +		tmp = (30 + freq - 5000 - center_segment0 * 5)/20;
>>> +		tmp /= 2;
>>> +		if (data.center_freq1 != 5000 +
>>> +					 center_segment0 * 5 - 20 + 40 * tmp)
>>> +			return -1;
>>> +		data.center_freq1 = 5000 + center_segment0 * 5;
>>> +		break;
>>> +	case 2:
>>> +		data.bandwidth = 160;
>>> +		if (center_segment1)
>>> +			return -1;
>>> +		if (!sec_channel_offset)
>>> +			return -1;
>>> +		/* primary 40 part must match the HT configuration */
>>> +		tmp = (70 + freq - 5000 - center_segment0 * 5)/20;
>>> +		tmp /= 2;
>>> +		if (data.center_freq1 != 5000 +
>>> +					 center_segment0 * 5 - 60 + 40 * tmp)
>>> +			return -1;
>>
>> Sorry, I couldn't understand your arithmetic. Can you explain with example?
>
> Oh, well, it's a little tricky, I had to draw a diagram:
>
> Imagine each - is 5 MHz, so this is a 160 MHz channel, split into the 20
> MHz subchannels:
>
> |----|----|----|----|----|----|----|----|
>
> Now each one of these could be the primary 20 MHz channel, which has the
> center freq "freq" (in the code.)
>
> center_segment0 points to the middle of the whole thing, so
> 	freq - (center_segment0 *5+5000)
> will be -70, -50, -30, -10, 10, 30, 50, 70
>
> Adding 70 will lead to 0, 20, 40, ..., 140
> Divide by 20 will give us 0,...,7
> Divide by 2 will give 0 ... 3, mapping 0,1->0, 2,3->1 etc.
>
> data.center_freq1 is the center of the primary 40 MHz channel (as
> calculated by the HT information, not VHT)
>
> Now VHT must match this and given the 0..3 index we can calculate what
> on the VHT side is the primary 40 MHz channel's center frequency --
> which is the 160 MHz channel's center freq, minus 60, plus 40 * the
> index.
>
> Yeah, it's tricky ... This is vaguely based on pages 193-195 in the VHT
> draft (formulas 22-1 through 22-7 and table 22-7)
>

Thanks for the detailed write up. It will be good to have this write up 
in code at least briefly.

Patch

diff --git a/src/ap/ap_drv_ops.c b/src/ap/ap_drv_ops.c
index 02da25b..7b927cf 100644
--- a/src/ap/ap_drv_ops.c
+++ b/src/ap/ap_drv_ops.c
@@ -454,19 +454,69 @@  int hostapd_flush(struct hostapd_data *hapd)
 
 
 int hostapd_set_freq(struct hostapd_data *hapd, int mode, int freq,
-		     int channel, int ht_enabled, int sec_channel_offset)
+		     int channel, int ht_enabled, int vht_enabled,
+		     int sec_channel_offset, int vht_oper_chwidth,
+		     int center_segment0, int center_segment1)
 {
 	struct hostapd_freq_params data;
-	if (hapd->driver == NULL)
-		return 0;
-	if (hapd->driver->set_freq == NULL)
-		return 0;
+	int tmp;
+
 	os_memset(&data, 0, sizeof(data));
 	data.mode = mode;
 	data.freq = freq;
 	data.channel = channel;
 	data.ht_enabled = ht_enabled;
+	data.vht_enabled = vht_enabled;
 	data.sec_channel_offset = sec_channel_offset;
+	data.center_freq1 = freq + sec_channel_offset * 10;
+	data.center_freq2 = 0;
+	data.bandwidth = sec_channel_offset ? 40 : 20;
+
+	if (data.vht_enabled) switch (vht_oper_chwidth) {
+	case 0:
+		if (center_segment1)
+			return -1;
+		if (5000 + center_segment0 * 5 != data.center_freq1)
+			return -1;
+		break;
+	case 3:
+		data.center_freq2 = 5000 + center_segment1 * 5;
+		/* fall through */
+	case 1:
+		data.bandwidth = 80;
+		if (vht_oper_chwidth == 1 && center_segment1)
+			return -1;
+		if (vht_oper_chwidth == 3 && !center_segment1)
+			return -1;
+		if (!sec_channel_offset)
+			return -1;
+		/* primary 40 part must match the HT configuration */
+		tmp = (30 + freq - 5000 - center_segment0 * 5)/20;
+		tmp /= 2;
+		if (data.center_freq1 != 5000 +
+					 center_segment0 * 5 - 20 + 40 * tmp)
+			return -1;
+		data.center_freq1 = 5000 + center_segment0 * 5;
+		break;
+	case 2:
+		data.bandwidth = 160;
+		if (center_segment1)
+			return -1;
+		if (!sec_channel_offset)
+			return -1;
+		/* primary 40 part must match the HT configuration */
+		tmp = (70 + freq - 5000 - center_segment0 * 5)/20;
+		tmp /= 2;
+		if (data.center_freq1 != 5000 +
+					 center_segment0 * 5 - 60 + 40 * tmp)
+			return -1;
+		data.center_freq1 = 5000 + center_segment0 * 5;
+		break;
+	}
+	if (hapd->driver == NULL)
+		return 0;
+	if (hapd->driver->set_freq == NULL)
+		return 0;
 	return hapd->driver->set_freq(hapd->drv_priv, &data);
 }
 
diff --git a/src/ap/ap_drv_ops.h b/src/ap/ap_drv_ops.h
index 9c53b99..768a0ba 100644
--- a/src/ap/ap_drv_ops.h
+++ b/src/ap/ap_drv_ops.h
@@ -55,7 +55,9 @@  int hostapd_get_seqnum(const char *ifname, struct hostapd_data *hapd,
 		       const u8 *addr, int idx, u8 *seq);
 int hostapd_flush(struct hostapd_data *hapd);
 int hostapd_set_freq(struct hostapd_data *hapd, int mode, int freq,
-		     int channel, int ht_enabled, int sec_channel_offset);
+		     int channel, int ht_enabled, int vht_enabled,
+		     int sec_channel_offset, int vht_oper_chwidth,
+		     int center_segment0, int center_segment1);
 int hostapd_set_rts(struct hostapd_data *hapd, int rts);
 int hostapd_set_frag(struct hostapd_data *hapd, int frag);
 int hostapd_sta_set_flags(struct hostapd_data *hapd, u8 *addr,
diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
index 1c968a7..15fb861 100644
--- a/src/ap/hostapd.c
+++ b/src/ap/hostapd.c
@@ -889,7 +889,11 @@  int hostapd_setup_interface_complete(struct hostapd_iface *iface, int err)
 		if (hostapd_set_freq(hapd, hapd->iconf->hw_mode, iface->freq,
 				     hapd->iconf->channel,
 				     hapd->iconf->ieee80211n,
-				     hapd->iconf->secondary_channel)) {
+				     hapd->iconf->ieee80211ac,
+				     hapd->iconf->secondary_channel,
+				     hapd->iconf->vht_oper_chwidth,
+				     hapd->iconf->vht_oper_centr_freq_seg0_idx,
+				     hapd->iconf->vht_oper_centr_freq_seg1_idx)) {
 			wpa_printf(MSG_ERROR, "Could not set channel for "
 				   "kernel driver");
 			return -1;
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index ccc5a0e..10d639e 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -905,10 +905,19 @@  struct hostapd_freq_params {
 	int mode;
 	int freq;
 	int channel;
+	/* for HT */
 	int ht_enabled;
 	int sec_channel_offset; /* 0 = HT40 disabled, -1 = HT40 enabled,
 				 * secondary channel below primary, 1 = HT40
 				 * enabled, secondary channel above primary */
+
+	/* for VHT */
+	int vht_enabled;
+
+	/* valid for both HT and VHT, center_freq2 is non-zero
+	 * only for bandwidth 80 and an 80+80 channel */
+	int center_freq1, center_freq2;
+	int bandwidth;
 };
 
 enum wpa_driver_if_type {