[RFC] mac80211: Don't let regulatory make us deaf

[RFC] mac80211: Don't let regulatory make us deaf

[Paul Stewart]

When regulatory information changes our HT behavior (e.g,
when we get a country code from the AP we have just associated
with), we should use this information to change the power with
which we transmit, and what channels we transmit.  Sometimes
the channel parameters we derive from regulatory information
contradicts the parameters we used in association.  For example,
we could have associated specifying HT40, but the regulatory
rules we apply may forbid HT40 operation.

In the situation above, we should reconfigure ourselves to
transmit in HT20 only, however it makes no sense for us to
disable receive in HT40, since if we associated with these
parameters, the AP has every reason to expect we can and
will receive packets this way.  The code in mac80211 does
not have the capability of sending the appropriate action
frames to signal a change in HT behaviour so the AP has
no clue we can no longer receive frames encoded this way.
In some broken AP implementations, this can leave us
effectively deaf if the AP never retries in lower HT rates.

This change breaks up the channel_type parameter in the
ieee80211_enable_ht function into a separate receive and
transmit part.  It honors the channel flags set by regulatory
in order to configure the rate control algorithm, but uses
the capability flags to configure the channel on the radio,
since these were used in association to set the AP's transmit
rate.

Signed-off-by: Paul Stewart <pstew@chromium.org>
---
 net/mac80211/mlme.c |   30 +++++++++++++++++++-----------
 1 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 52133da..22a1bcd 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -187,7 +187,8 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	u16 ht_opmode;
 	bool enable_ht = true;
 	enum nl80211_channel_type prev_chantype;
-	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
+	enum nl80211_channel_type xmit_channel_type = NL80211_CHAN_NO_HT;
+	enum nl80211_channel_type recv_channel_type = NL80211_CHAN_NO_HT;
 
 	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
 
@@ -220,7 +221,8 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (enable_ht) {
-		channel_type = NL80211_CHAN_HT20;
+		xmit_channel_type = NL80211_CHAN_HT20;
+		recv_channel_type = NL80211_CHAN_HT20;
 
 		if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
 		    !ieee80111_cfg_override_disables_ht40(sdata) &&
@@ -228,26 +230,32 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
 			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
 			case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
+				recv_channel_type = NL80211_CHAN_HT40PLUS;
 				if (!(local->hw.conf.channel->flags &
 				    IEEE80211_CHAN_NO_HT40PLUS))
-					channel_type = NL80211_CHAN_HT40PLUS;
+					xmit_channel_type =
+						NL80211_CHAN_HT40PLUS;
 				break;
 			case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
+				recv_channel_type = NL80211_CHAN_HT40MINUS;
 				if (!(local->hw.conf.channel->flags &
 				    IEEE80211_CHAN_NO_HT40MINUS))
-					channel_type = NL80211_CHAN_HT40MINUS;
+					xmit_channel_type =
+						NL80211_CHAN_HT40MINUS;
 				break;
 			}
 		}
 	}
 
 	if (local->tmp_channel)
-		local->tmp_channel_type = channel_type;
+		local->tmp_channel_type = recv_channel_type;
 
-	if (!ieee80211_set_channel_type(local, sdata, channel_type)) {
+	if (!ieee80211_set_channel_type(local, sdata, recv_channel_type)) {
 		/* can only fail due to HT40+/- mismatch */
-		channel_type = NL80211_CHAN_HT20;
-		WARN_ON(!ieee80211_set_channel_type(local, sdata, channel_type));
+		xmit_channel_type = NL80211_CHAN_HT20;
+		recv_channel_type = NL80211_CHAN_HT20;
+		WARN_ON(!ieee80211_set_channel_type(local, sdata,
+						    recv_channel_type));
 	}
 
 	if (beacon_htcap_ie && (prev_chantype != channel_type)) {
@@ -268,13 +276,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	/* channel_type change automatically detected */
 	ieee80211_hw_config(local, 0);
 
-	if (prev_chantype != channel_type) {
+	if (prev_chantype != xmit_channel_type) {
 		rcu_read_lock();
 		sta = sta_info_get(sdata, bssid);
 		if (sta)
 			rate_control_rate_update(local, sband, sta,
 						 IEEE80211_RC_HT_CHANGED,
-						 channel_type);
+						 xmit_channel_type);
 		rcu_read_unlock();
 
 		if (beacon_htcap_ie)
@@ -287,7 +295,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	/* if bss configuration changed store the new one */
 	if (sdata->ht_opmode_valid != enable_ht ||
 	    sdata->vif.bss_conf.ht_operation_mode != ht_opmode ||
-	    prev_chantype != channel_type) {
+	    prev_chantype != recv_channel_type) {
 		changed |= BSS_CHANGED_HT;
 		sdata->vif.bss_conf.ht_operation_mode = ht_opmode;
 		sdata->ht_opmode_valid = enable_ht;
-- 
1.7.7.3

[RFCv2] mac80211: Don't let regulatory make us deaf

[Paul Stewart]

When regulatory information changes our HT behavior (e.g,
when we get a country code from the AP we have just associated
with), we should use this information to change the power with
which we transmit, and what channels we transmit.  Sometimes
the channel parameters we derive from regulatory information
contradicts the parameters we used in association.  For example,
we could have associated specifying HT40, but the regulatory
rules we apply may forbid HT40 operation.

In the situation above, we should reconfigure ourselves to
transmit in HT20 only, however it makes no sense for us to
disable receive in HT40, since if we associated with these
parameters, the AP has every reason to expect we can and
will receive packets this way.  The code in mac80211 does
not have the capability of sending the appropriate action
frames to signal a change in HT behaviour so the AP has
no clue we can no longer receive frames encoded this way.
In some broken AP implementations, this can leave us
effectively deaf if the AP never retries in lower HT rates.

This change breaks up the channel_type parameter in the
ieee80211_enable_ht function into a separate receive and
transmit part.  It honors the channel flags set by regulatory
in order to configure the rate control algorithm, but uses
the capability flags to configure the channel on the radio,
since these were used in association to set the AP's transmit
rate.

Signed-off-by: Paul Stewart <pstew@chromium.org>
---
 net/mac80211/chan.c        |   24 ++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |    3 +++
 net/mac80211/mlme.c        |   16 ++++++++--------
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index d1f7abd..494580a 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -3,6 +3,7 @@
  */
 
 #include <linux/nl80211.h>
+#include <net/cfg80211.h>
 #include "ieee80211_i.h"
 
 static enum ieee80211_chan_mode
@@ -134,3 +135,26 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 
 	return result;
 }
+
+/*
+ * ieee80211_get_xmit_channel_type returns the channel type we should
+ * use for packet transmission, given the channel capability and
+ * whatever regulatory flags we have been given.
+ */
+enum nl80211_channel_type ieee80211_get_xmit_channel_type(
+				struct ieee80211_local *local,
+				enum nl80211_channel_type channel_type) {
+	switch (channel_type) {
+	case NL80211_CHAN_HT40PLUS:
+		if ((local->hw.conf.channel->flags &
+		    IEEE80211_CHAN_NO_HT40PLUS))
+			return NL80211_CHAN_HT20;
+		break;
+	case NL80211_CHAN_HT40MINUS:
+		if ((local->hw.conf.channel->flags &
+		    IEEE80211_CHAN_NO_HT40MINUS))
+			return NL80211_CHAN_HT20;
+		break;
+	}
+	return channel_type;
+}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 74594f0..0a3c7df 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1468,6 +1468,9 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 				enum nl80211_channel_type chantype);
 enum nl80211_channel_type
 ieee80211_ht_info_to_channel_type(struct ieee80211_ht_info *ht_info);
+enum nl80211_channel_type ieee80211_get_xmit_channel_type(
+					struct ieee80211_local *local,
+					enum nl80211_channel_type channel_type);
 
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 52133da..3bcf4e3 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -188,6 +188,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	bool enable_ht = true;
 	enum nl80211_channel_type prev_chantype;
 	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
+	enum nl80211_channel_type xmit_channel_type;
 
 	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
 
@@ -228,19 +229,18 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
 			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
 			case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
-				if (!(local->hw.conf.channel->flags &
-				    IEEE80211_CHAN_NO_HT40PLUS))
-					channel_type = NL80211_CHAN_HT40PLUS;
+				channel_type = NL80211_CHAN_HT40PLUS;
 				break;
 			case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
-				if (!(local->hw.conf.channel->flags &
-				    IEEE80211_CHAN_NO_HT40MINUS))
-					channel_type = NL80211_CHAN_HT40MINUS;
+				channel_type = NL80211_CHAN_HT40MINUS;
 				break;
 			}
 		}
 	}
 
+	xmit_channel_type =
+		ieee80211_get_xmit_channel_type(local, channel_type);
+
 	if (local->tmp_channel)
 		local->tmp_channel_type = channel_type;
 
@@ -268,13 +268,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	/* channel_type change automatically detected */
 	ieee80211_hw_config(local, 0);
 
-	if (prev_chantype != channel_type) {
+	if (prev_chantype != xmit_channel_type) {
 		rcu_read_lock();
 		sta = sta_info_get(sdata, bssid);
 		if (sta)
 			rate_control_rate_update(local, sband, sta,
 						 IEEE80211_RC_HT_CHANGED,
-						 channel_type);
+						 xmit_channel_type);
 		rcu_read_unlock();
 
 		if (beacon_htcap_ie)
-- 
1.7.7.3

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Mahesh]

On Mon, 20 Feb 2012 21:25:54 -0800, Paul Stewart wrote:
> When regulatory information changes our HT behavior (e.g,
> when we get a country code from the AP we have just associated
> with), we should use this information to change the power with
> which we transmit, and what channels we transmit.  Sometimes
> the channel parameters we derive from regulatory information
> contradicts the parameters we used in association.  For example,
> we could have associated specifying HT40, but the regulatory
> rules we apply may forbid HT40 operation.
>
> In the situation above, we should reconfigure ourselves to
> transmit in HT20 only, however it makes no sense for us to
> disable receive in HT40, since if we associated with these
> parameters, the AP has every reason to expect we can and
> will receive packets this way.  The code in mac80211 does
> not have the capability of sending the appropriate action
> frames to signal a change in HT behaviour so the AP has
> no clue we can no longer receive frames encoded this way.
> In some broken AP implementations, this can leave us
> effectively deaf if the AP never retries in lower HT rates.
>
> This change breaks up the channel_type parameter in the
> ieee80211_enable_ht function into a separate receive and
> transmit part.  It honors the channel flags set by regulatory
> in order to configure the rate control algorithm, but uses
> the capability flags to configure the channel on the radio,
> since these were used in association to set the AP's transmit
> rate.
>
> Signed-off-by: Paul Stewart <pstew@chromium.org>
> ---
>  net/mac80211/chan.c        |   24 ++++++++++++++++++++++++
>  net/mac80211/ieee80211_i.h |    3 +++
>  net/mac80211/mlme.c        |   16 ++++++++--------
>  3 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
> index d1f7abd..494580a 100644
> --- a/net/mac80211/chan.c
> +++ b/net/mac80211/chan.c
> @@ -3,6 +3,7 @@
>   */
>
>  #include <linux/nl80211.h>
> +#include <net/cfg80211.h>
>  #include "ieee80211_i.h"
>
>  static enum ieee80211_chan_mode
> @@ -134,3 +135,26 @@ bool ieee80211_set_channel_type(struct
> ieee80211_local *local,
>
>  	return result;
>  }
> +
> +/*
> + * ieee80211_get_xmit_channel_type returns the channel type we 
> should
> + * use for packet transmission, given the channel capability and
> + * whatever regulatory flags we have been given.
> + */
> +enum nl80211_channel_type ieee80211_get_xmit_channel_type(
> +				struct ieee80211_local *local,
> +				enum nl80211_channel_type channel_type) {
> +	switch (channel_type) {
> +	case NL80211_CHAN_HT40PLUS:
> +		if ((local->hw.conf.channel->flags &
> +		    IEEE80211_CHAN_NO_HT40PLUS))
> +			return NL80211_CHAN_HT20;
> +		break;
> +	case NL80211_CHAN_HT40MINUS:
> +		if ((local->hw.conf.channel->flags &
> +		    IEEE80211_CHAN_NO_HT40MINUS))
> +			return NL80211_CHAN_HT20;
> +		break;
> +	}
> +	return channel_type;
> +}
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 74594f0..0a3c7df 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1468,6 +1468,9 @@ bool ieee80211_set_channel_type(struct
> ieee80211_local *local,
>  				enum nl80211_channel_type chantype);
>  enum nl80211_channel_type
>  ieee80211_ht_info_to_channel_type(struct ieee80211_ht_info 
> *ht_info);
> +enum nl80211_channel_type ieee80211_get_xmit_channel_type(
> +					struct ieee80211_local *local,
> +					enum nl80211_channel_type channel_type);
>
>  #ifdef CONFIG_MAC80211_NOINLINE
>  #define debug_noinline noinline
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 52133da..3bcf4e3 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -188,6 +188,7 @@ static u32 ieee80211_enable_ht(struct
> ieee80211_sub_if_data *sdata,
>  	bool enable_ht = true;
>  	enum nl80211_channel_type prev_chantype;
>  	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
> +	enum nl80211_channel_type xmit_channel_type;
>
>  	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
>
> @@ -228,19 +229,18 @@ static u32 ieee80211_enable_ht(struct
> ieee80211_sub_if_data *sdata,
>  		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
>  			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
>  			case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
> -				if (!(local->hw.conf.channel->flags &
> -				    IEEE80211_CHAN_NO_HT40PLUS))
> -					channel_type = NL80211_CHAN_HT40PLUS;
> +				channel_type = NL80211_CHAN_HT40PLUS;
>  				break;
>  			case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
> -				if (!(local->hw.conf.channel->flags &
> -				    IEEE80211_CHAN_NO_HT40MINUS))
> -					channel_type = NL80211_CHAN_HT40MINUS;
> +				channel_type = NL80211_CHAN_HT40MINUS;
>  				break;
>  			}
>  		}
>  	}
>
> +	xmit_channel_type =
> +		ieee80211_get_xmit_channel_type(local, channel_type);
> +
>  	if (local->tmp_channel)
>  		local->tmp_channel_type = channel_type;
>
> @@ -268,13 +268,13 @@ static u32 ieee80211_enable_ht(struct
> ieee80211_sub_if_data *sdata,
>  	/* channel_type change automatically detected */
>  	ieee80211_hw_config(local, 0);
>
> -	if (prev_chantype != channel_type) {
> +	if (prev_chantype != xmit_channel_type) {
>  		rcu_read_lock();
>  		sta = sta_info_get(sdata, bssid);
>  		if (sta)
>  			rate_control_rate_update(local, sband, sta,
>  						 IEEE80211_RC_HT_CHANGED,
> -						 channel_type);
> +						 xmit_channel_type);
>  		rcu_read_unlock();
>
>  		if (beacon_htcap_ie)


Peter,

When AP sends country code info, it should also obey the same 
regulatory of that country and stick to HT20 in your example? Am I 
missing something here?

---
Thanks,
Mahesh

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
> When regulatory information changes our HT behavior (e.g,
> when we get a country code from the AP we have just associated
> with), we should use this information to change the power with
> which we transmit, and what channels we transmit.  Sometimes
> the channel parameters we derive from regulatory information
> contradicts the parameters we used in association.  For example,
> we could have associated specifying HT40, but the regulatory
> rules we apply may forbid HT40 operation.
> 
> In the situation above, we should reconfigure ourselves to
> transmit in HT20 only, however it makes no sense for us to
> disable receive in HT40, since if we associated with these
> parameters, the AP has every reason to expect we can and
> will receive packets this way.  The code in mac80211 does
> not have the capability of sending the appropriate action
> frames to signal a change in HT behaviour so the AP has
> no clue we can no longer receive frames encoded this way.
> In some broken AP implementations, this can leave us
> effectively deaf if the AP never retries in lower HT rates.
> 
> This change breaks up the channel_type parameter in the
> ieee80211_enable_ht function into a separate receive and
> transmit part.  It honors the channel flags set by regulatory
> in order to configure the rate control algorithm, but uses
> the capability flags to configure the channel on the radio,
> since these were used in association to set the AP's transmit
> rate.

Quite the stupid APs, obviously ...

> +/*
> + * ieee80211_get_xmit_channel_type returns the channel type we should
> + * use for packet transmission, given the channel capability and
> + * whatever regulatory flags we have been given.
> + */
> +enum nl80211_channel_type ieee80211_get_xmit_channel_type(
> +				struct ieee80211_local *local,
> +				enum nl80211_channel_type channel_type) {

code style, please put the brace on the next line

> +	switch (channel_type) {
> +	case NL80211_CHAN_HT40PLUS:
> +		if ((local->hw.conf.channel->flags &
> +		    IEEE80211_CHAN_NO_HT40PLUS))

those extra parentheses seem useless, and maybe indent the IEEE80211_...
another two tabs or so to make it stand out as the continuation of the
statement rather than the if

(see the recent thread about code style here on this list ...)

> @@ -188,6 +188,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>  	bool enable_ht = true;
>  	enum nl80211_channel_type prev_chantype;
>  	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
> +	enum nl80211_channel_type xmit_channel_type;

Maybe you should rename "channel_type" to "rx_channel_type" then, or
"config_channel_type"?
 
>  	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
>  
> @@ -228,19 +229,18 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>  		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
>  			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
>  			case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
> -				if (!(local->hw.conf.channel->flags &
> -				    IEEE80211_CHAN_NO_HT40PLUS))
> -					channel_type = NL80211_CHAN_HT40PLUS;
> +				channel_type = NL80211_CHAN_HT40PLUS;
>  				break;
>  			case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
> -				if (!(local->hw.conf.channel->flags &
> -				    IEEE80211_CHAN_NO_HT40MINUS))
> -					channel_type = NL80211_CHAN_HT40MINUS;
> +				channel_type = NL80211_CHAN_HT40MINUS;
>  				break;
>  			}
>  		}
>  	}
>  
> +	xmit_channel_type =
> +		ieee80211_get_xmit_channel_type(local, channel_type);
> +
>  	if (local->tmp_channel)
>  		local->tmp_channel_type = channel_type;
>  
> @@ -268,13 +268,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>  	/* channel_type change automatically detected */
>  	ieee80211_hw_config(local, 0);
>  
> -	if (prev_chantype != channel_type) {
> +	if (prev_chantype != xmit_channel_type) {
>  		rcu_read_lock();
>  		sta = sta_info_get(sdata, bssid);
>  		if (sta)
>  			rate_control_rate_update(local, sband, sta,
>  						 IEEE80211_RC_HT_CHANGED,
> -						 channel_type);
> +						 xmit_channel_type);
>  		rcu_read_unlock();


Otherwise looks fine to me.

johannes

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Sam Leffler]

On Thu, Feb 23, 2012 at 5:39 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
>> When regulatory information changes our HT behavior (e.g,
>> when we get a country code from the AP we have just associated
>> with), we should use this information to change the power with
>> which we transmit, and what channels we transmit.  Sometimes
>> the channel parameters we derive from regulatory information
>> contradicts the parameters we used in association.  For example,
>> we could have associated specifying HT40, but the regulatory
>> rules we apply may forbid HT40 operation.
>>
>> In the situation above, we should reconfigure ourselves to
>> transmit in HT20 only, however it makes no sense for us to
>> disable receive in HT40, since if we associated with these
>> parameters, the AP has every reason to expect we can and
>> will receive packets this way.  The code in mac80211 does
>> not have the capability of sending the appropriate action
>> frames to signal a change in HT behaviour so the AP has
>> no clue we can no longer receive frames encoded this way.
>> In some broken AP implementations, this can leave us
>> effectively deaf if the AP never retries in lower HT rates.
>>
>> This change breaks up the channel_type parameter in the
>> ieee80211_enable_ht function into a separate receive and
>> transmit part.  It honors the channel flags set by regulatory
>> in order to configure the rate control algorithm, but uses
>> the capability flags to configure the channel on the radio,
>> since these were used in association to set the AP's transmit
>> rate.
>
> Quite the stupid APs, obviously ...

Actually the AP is operating correctly (modulo the question of whether
HT40 may be used in kr).

>
>> +/*
>> + * ieee80211_get_xmit_channel_type returns the channel type we should
>> + * use for packet transmission, given the channel capability and
>> + * whatever regulatory flags we have been given.
>> + */
>> +enum nl80211_channel_type ieee80211_get_xmit_channel_type(
>> +                             struct ieee80211_local *local,
>> +                             enum nl80211_channel_type channel_type) {
>
> code style, please put the brace on the next line
>
>> +     switch (channel_type) {
>> +     case NL80211_CHAN_HT40PLUS:
>> +             if ((local->hw.conf.channel->flags &
>> +                 IEEE80211_CHAN_NO_HT40PLUS))
>
> those extra parentheses seem useless, and maybe indent the IEEE80211_...
> another two tabs or so to make it stand out as the continuation of the
> statement rather than the if
>
> (see the recent thread about code style here on this list ...)
>
>> @@ -188,6 +188,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>>       bool enable_ht = true;
>>       enum nl80211_channel_type prev_chantype;
>>       enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
>> +     enum nl80211_channel_type xmit_channel_type;
>
> Maybe you should rename "channel_type" to "rx_channel_type" then, or
> "config_channel_type"?
>
>>       sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
>>
>> @@ -228,19 +229,18 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>>                   (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
>>                       switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
>>                       case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
>> -                             if (!(local->hw.conf.channel->flags &
>> -                                 IEEE80211_CHAN_NO_HT40PLUS))
>> -                                     channel_type = NL80211_CHAN_HT40PLUS;
>> +                             channel_type = NL80211_CHAN_HT40PLUS;
>>                               break;
>>                       case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
>> -                             if (!(local->hw.conf.channel->flags &
>> -                                 IEEE80211_CHAN_NO_HT40MINUS))
>> -                                     channel_type = NL80211_CHAN_HT40MINUS;
>> +                             channel_type = NL80211_CHAN_HT40MINUS;
>>                               break;
>>                       }
>>               }
>>       }
>>
>> +     xmit_channel_type =
>> +             ieee80211_get_xmit_channel_type(local, channel_type);
>> +
>>       if (local->tmp_channel)
>>               local->tmp_channel_type = channel_type;
>>
>> @@ -268,13 +268,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
>>       /* channel_type change automatically detected */
>>       ieee80211_hw_config(local, 0);
>>
>> -     if (prev_chantype != channel_type) {
>> +     if (prev_chantype != xmit_channel_type) {
>>               rcu_read_lock();
>>               sta = sta_info_get(sdata, bssid);
>>               if (sta)
>>                       rate_control_rate_update(local, sband, sta,
>>                                                IEEE80211_RC_HT_CHANGED,
>> -                                              channel_type);
>> +                                              xmit_channel_type);
>>               rcu_read_unlock();
>
>
> Otherwise looks fine to me.

I believe the root cause is the local sta overriding the AP based on
local regulatory. In general sta's need to honor settings from an AP.

-Sam

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Thu, 2012-02-23 at 06:53 -0800, Sam Leffler wrote:
> On Thu, Feb 23, 2012 at 5:39 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
> >> When regulatory information changes our HT behavior (e.g,
> >> when we get a country code from the AP we have just associated
> >> with), we should use this information to change the power with
> >> which we transmit, and what channels we transmit.  Sometimes
> >> the channel parameters we derive from regulatory information
> >> contradicts the parameters we used in association.  For example,
> >> we could have associated specifying HT40, but the regulatory
> >> rules we apply may forbid HT40 operation.
> >>
> >> In the situation above, we should reconfigure ourselves to
> >> transmit in HT20 only, however it makes no sense for us to
> >> disable receive in HT40, since if we associated with these
> >> parameters, the AP has every reason to expect we can and
> >> will receive packets this way.  The code in mac80211 does
> >> not have the capability of sending the appropriate action
> >> frames to signal a change in HT behaviour so the AP has
> >> no clue we can no longer receive frames encoded this way.
> >> In some broken AP implementations, this can leave us
> >> effectively deaf if the AP never retries in lower HT rates.
> >>
> >> This change breaks up the channel_type parameter in the
> >> ieee80211_enable_ht function into a separate receive and
> >> transmit part.  It honors the channel flags set by regulatory
> >> in order to configure the rate control algorithm, but uses
> >> the capability flags to configure the channel on the radio,
> >> since these were used in association to set the AP's transmit
> >> rate.
> >
> > Quite the stupid APs, obviously ...
> 
> Actually the AP is operating correctly (modulo the question of whether
> HT40 may be used in kr).

Ah, yes. I was thinking the AP was actually saying you can't use 40 MHz,
but it's obviously just saying that you're in KR and our database says
you then can't use 40 Mhz...

johannes

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Paul Stewart]

On Thu, Feb 23, 2012 at 6:59 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Thu, 2012-02-23 at 06:53 -0800, Sam Leffler wrote:
> > On Thu, Feb 23, 2012 at 5:39 AM, Johannes Berg
> > <johannes@sipsolutions.net> wrote:
> > > On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
> > >> When regulatory information changes our HT behavior (e.g,
> > >> when we get a country code from the AP we have just associated
> > >> with), we should use this information to change the power with
> > >> which we transmit, and what channels we transmit.  Sometimes
> > >> the channel parameters we derive from regulatory information
> > >> contradicts the parameters we used in association.  For example,
> > >> we could have associated specifying HT40, but the regulatory
> > >> rules we apply may forbid HT40 operation.
> > >>
> > >> In the situation above, we should reconfigure ourselves to
> > >> transmit in HT20 only, however it makes no sense for us to
> > >> disable receive in HT40, since if we associated with these
> > >> parameters, the AP has every reason to expect we can and
> > >> will receive packets this way.  The code in mac80211 does
> > >> not have the capability of sending the appropriate action
> > >> frames to signal a change in HT behaviour so the AP has
> > >> no clue we can no longer receive frames encoded this way.
> > >> In some broken AP implementations, this can leave us
> > >> effectively deaf if the AP never retries in lower HT rates.
> > >>
> > >> This change breaks up the channel_type parameter in the
> > >> ieee80211_enable_ht function into a separate receive and
> > >> transmit part.  It honors the channel flags set by regulatory
> > >> in order to configure the rate control algorithm, but uses
> > >> the capability flags to configure the channel on the radio,
> > >> since these were used in association to set the AP's transmit
> > >> rate.
> > >
> > > Quite the stupid APs, obviously ...
> >
> > Actually the AP is operating correctly (modulo the question of whether
> > HT40 may be used in kr).
>
> Ah, yes. I was thinking the AP was actually saying you can't use 40 MHz,
> but it's obviously just saying that you're in KR and our database says
> you then can't use 40 Mhz...

More succinctly, Sam is posing the possibility that a STA we should
ignore regulatory on TX as well as RX in this situation, since the AP
clearly negotiated HT40 with us.  This patch does not do this.  If we
are allowed to do that, what if we imposed that regulatory domain
beforehand (ala iw reg set) then associated?  In this scenario we
would have negotiated HT20.  If we then received a beacon where the AP
claimed it supported HT40, should we now allow transmit in HT40
(without having said so to the AP)?  The current version of this patch
prevents that behavior.

>
> johannes
>

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Luis R. Rodriguez]

On Thu, Feb 23, 2012 at 7:14 AM, Paul Stewart <pstew@chromium.org> wrote:
> On Thu, Feb 23, 2012 at 6:59 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>>
>> On Thu, 2012-02-23 at 06:53 -0800, Sam Leffler wrote:
>> > On Thu, Feb 23, 2012 at 5:39 AM, Johannes Berg
>> > <johannes@sipsolutions.net> wrote:
>> > > On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
>> > >> When regulatory information changes our HT behavior (e.g,
>> > >> when we get a country code from the AP we have just associated
>> > >> with), we should use this information to change the power with
>> > >> which we transmit, and what channels we transmit.  Sometimes
>> > >> the channel parameters we derive from regulatory information
>> > >> contradicts the parameters we used in association.  For example,
>> > >> we could have associated specifying HT40, but the regulatory
>> > >> rules we apply may forbid HT40 operation.
>> > >>
>> > >> In the situation above, we should reconfigure ourselves to
>> > >> transmit in HT20 only, however it makes no sense for us to
>> > >> disable receive in HT40, since if we associated with these
>> > >> parameters, the AP has every reason to expect we can and
>> > >> will receive packets this way.  The code in mac80211 does
>> > >> not have the capability of sending the appropriate action
>> > >> frames to signal a change in HT behaviour so the AP has
>> > >> no clue we can no longer receive frames encoded this way.
>> > >> In some broken AP implementations, this can leave us
>> > >> effectively deaf if the AP never retries in lower HT rates.
>> > >>
>> > >> This change breaks up the channel_type parameter in the
>> > >> ieee80211_enable_ht function into a separate receive and
>> > >> transmit part.  It honors the channel flags set by regulatory
>> > >> in order to configure the rate control algorithm, but uses
>> > >> the capability flags to configure the channel on the radio,
>> > >> since these were used in association to set the AP's transmit
>> > >> rate.
>> > >
>> > > Quite the stupid APs, obviously ...
>> >
>> > Actually the AP is operating correctly (modulo the question of whether
>> > HT40 may be used in kr).
>>
>> Ah, yes. I was thinking the AP was actually saying you can't use 40 MHz,
>> but it's obviously just saying that you're in KR and our database says
>> you then can't use 40 Mhz...
>
> More succinctly, Sam is posing the possibility that a STA we should
> ignore regulatory on TX as well as RX in this situation, since the AP
> clearly negotiated HT40 with us.

If the AP was right and we were allowed to use this that'd be fine but
consider the case where the AP is wrong, they'd we'd be doing the
wrong thing. If the AP believes HT40 is allowed but our wireless-regdb
disagrees we trust our wireless-regdb more, by design.

Did I understand your question correctly?

  Luis

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Paul Stewart]

On Thu, Feb 23, 2012 at 5:53 PM, Luis R. Rodriguez <mcgrof@frijolero.org> wrote:
> On Thu, Feb 23, 2012 at 7:14 AM, Paul Stewart <pstew@chromium.org> wrote:
>> On Thu, Feb 23, 2012 at 6:59 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>>>
>>> On Thu, 2012-02-23 at 06:53 -0800, Sam Leffler wrote:
>>> > On Thu, Feb 23, 2012 at 5:39 AM, Johannes Berg
>>> > <johannes@sipsolutions.net> wrote:
>>> > > On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
>>> > >> When regulatory information changes our HT behavior (e.g,
>>> > >> when we get a country code from the AP we have just associated
>>> > >> with), we should use this information to change the power with
>>> > >> which we transmit, and what channels we transmit.  Sometimes
>>> > >> the channel parameters we derive from regulatory information
>>> > >> contradicts the parameters we used in association.  For example,
>>> > >> we could have associated specifying HT40, but the regulatory
>>> > >> rules we apply may forbid HT40 operation.
>>> > >>
>>> > >> In the situation above, we should reconfigure ourselves to
>>> > >> transmit in HT20 only, however it makes no sense for us to
>>> > >> disable receive in HT40, since if we associated with these
>>> > >> parameters, the AP has every reason to expect we can and
>>> > >> will receive packets this way.  The code in mac80211 does
>>> > >> not have the capability of sending the appropriate action
>>> > >> frames to signal a change in HT behaviour so the AP has
>>> > >> no clue we can no longer receive frames encoded this way.
>>> > >> In some broken AP implementations, this can leave us
>>> > >> effectively deaf if the AP never retries in lower HT rates.
>>> > >>
>>> > >> This change breaks up the channel_type parameter in the
>>> > >> ieee80211_enable_ht function into a separate receive and
>>> > >> transmit part.  It honors the channel flags set by regulatory
>>> > >> in order to configure the rate control algorithm, but uses
>>> > >> the capability flags to configure the channel on the radio,
>>> > >> since these were used in association to set the AP's transmit
>>> > >> rate.
>>> > >
>>> > > Quite the stupid APs, obviously ...
>>> >
>>> > Actually the AP is operating correctly (modulo the question of whether
>>> > HT40 may be used in kr).
>>>
>>> Ah, yes. I was thinking the AP was actually saying you can't use 40 MHz,
>>> but it's obviously just saying that you're in KR and our database says
>>> you then can't use 40 Mhz...
>>
>> More succinctly, Sam is posing the possibility that a STA we should
>> ignore regulatory on TX as well as RX in this situation, since the AP
>> clearly negotiated HT40 with us.
>
> If the AP was right and we were allowed to use this that'd be fine but
> consider the case where the AP is wrong, they'd we'd be doing the
> wrong thing. If the AP believes HT40 is allowed but our wireless-regdb
> disagrees we trust our wireless-regdb more, by design.
>
> Did I understand your question correctly?

I think you did.  As such, I'll clean the code up as per Johannes'
suggestion, fix up another call to rate_control_rate_update I found in
rx.c and add a [PATCH] to this thread.

--
Paul

>
>  Luis

[PATCH] mac80211: Don't let regulatory make us deaf

[Paul Stewart]

When regulatory information changes our HT behavior (e.g,
when we get a country code from the AP we have just associated
with), we should use this information to change the power with
which we transmit, and what channels we transmit.  Sometimes
the channel parameters we derive from regulatory information
contradicts the parameters we used in association.  For example,
we could have associated specifying HT40, but the regulatory
rules we apply may forbid HT40 operation.

In the situation above, we should reconfigure ourselves to
transmit in HT20 only, however it makes no sense for us to
disable receive in HT40, since if we associated with these
parameters, the AP has every reason to expect we can and
will receive packets this way.  The code in mac80211 does
not have the capability of sending the appropriate action
frames to signal a change in HT behaviour so the AP has
no clue we can no longer receive frames encoded this way.
In some broken AP implementations, this can leave us
effectively deaf if the AP never retries in lower HT rates.

This change breaks up the channel_type parameter in the
ieee80211_enable_ht function into a separate receive and
transmit part.  It honors the channel flags set by regulatory
in order to configure the rate control algorithm, but uses
the capability flags to configure the channel on the radio,
since these were used in association to set the AP's transmit
rate.

Signed-off-by: Paul Stewart <pstew@chromium.org>
Cc: Sam Leffler <sleffler@chromium.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Luis R Rodriguez <mcgrof@frijolero.org>
---
 net/mac80211/chan.c        |   27 +++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |    3 +++
 net/mac80211/mlme.c        |   32 ++++++++++++++++----------------
 net/mac80211/rx.c          |    8 +++++---
 4 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index d1f7abd..e00ce8c 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -3,6 +3,7 @@
  */
 
 #include <linux/nl80211.h>
+#include <net/cfg80211.h>
 #include "ieee80211_i.h"
 
 static enum ieee80211_chan_mode
@@ -134,3 +135,29 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 
 	return result;
 }
+
+/*
+ * ieee80211_get_tx_channel_type returns the channel type we should
+ * use for packet transmission, given the channel capability and
+ * whatever regulatory flags we have been given.
+ */
+enum nl80211_channel_type ieee80211_get_tx_channel_type(
+				struct ieee80211_local *local,
+				enum nl80211_channel_type channel_type)
+{
+	switch (channel_type) {
+	case NL80211_CHAN_HT40PLUS:
+		if (local->hw.conf.channel->flags &
+				IEEE80211_CHAN_NO_HT40PLUS)
+			return NL80211_CHAN_HT20;
+		break;
+	case NL80211_CHAN_HT40MINUS:
+		if (local->hw.conf.channel->flags &
+				IEEE80211_CHAN_NO_HT40MINUS)
+			return NL80211_CHAN_HT20;
+		break;
+	default:
+		break;
+	}
+	return channel_type;
+}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 74594f0..a385342 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1468,6 +1468,9 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 				enum nl80211_channel_type chantype);
 enum nl80211_channel_type
 ieee80211_ht_info_to_channel_type(struct ieee80211_ht_info *ht_info);
+enum nl80211_channel_type ieee80211_get_tx_channel_type(
+					struct ieee80211_local *local,
+					enum nl80211_channel_type channel_type);
 
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 52133da..4f90cb6 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -187,7 +187,8 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	u16 ht_opmode;
 	bool enable_ht = true;
 	enum nl80211_channel_type prev_chantype;
-	enum nl80211_channel_type channel_type = NL80211_CHAN_NO_HT;
+	enum nl80211_channel_type rx_channel_type = NL80211_CHAN_NO_HT;
+	enum nl80211_channel_type tx_channel_type;
 
 	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
 
@@ -220,7 +221,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (enable_ht) {
-		channel_type = NL80211_CHAN_HT20;
+		rx_channel_type = NL80211_CHAN_HT20;
 
 		if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
 		    !ieee80111_cfg_override_disables_ht40(sdata) &&
@@ -228,29 +229,28 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		    (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
 			switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
 			case IEEE80211_HT_PARAM_CHA_SEC_ABOVE:
-				if (!(local->hw.conf.channel->flags &
-				    IEEE80211_CHAN_NO_HT40PLUS))
-					channel_type = NL80211_CHAN_HT40PLUS;
+				rx_channel_type = NL80211_CHAN_HT40PLUS;
 				break;
 			case IEEE80211_HT_PARAM_CHA_SEC_BELOW:
-				if (!(local->hw.conf.channel->flags &
-				    IEEE80211_CHAN_NO_HT40MINUS))
-					channel_type = NL80211_CHAN_HT40MINUS;
+				rx_channel_type = NL80211_CHAN_HT40MINUS;
 				break;
 			}
 		}
 	}
 
+	tx_channel_type = ieee80211_get_tx_channel_type(local, rx_channel_type);
+
 	if (local->tmp_channel)
-		local->tmp_channel_type = channel_type;
+		local->tmp_channel_type = rx_channel_type;
 
-	if (!ieee80211_set_channel_type(local, sdata, channel_type)) {
+	if (!ieee80211_set_channel_type(local, sdata, rx_channel_type)) {
 		/* can only fail due to HT40+/- mismatch */
-		channel_type = NL80211_CHAN_HT20;
-		WARN_ON(!ieee80211_set_channel_type(local, sdata, channel_type));
+		rx_channel_type = NL80211_CHAN_HT20;
+		WARN_ON(!ieee80211_set_channel_type(local, sdata,
+						    rx_channel_type));
 	}
 
-	if (beacon_htcap_ie && (prev_chantype != channel_type)) {
+	if (beacon_htcap_ie && (prev_chantype != rx_channel_type)) {
 		/*
 		 * Whenever the AP announces the HT mode change that can be
 		 * 40MHz intolerant or etc., it would be safer to stop tx
@@ -268,13 +268,13 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	/* channel_type change automatically detected */
 	ieee80211_hw_config(local, 0);
 
-	if (prev_chantype != channel_type) {
+	if (prev_chantype != tx_channel_type) {
 		rcu_read_lock();
 		sta = sta_info_get(sdata, bssid);
 		if (sta)
 			rate_control_rate_update(local, sband, sta,
 						 IEEE80211_RC_HT_CHANGED,
-						 channel_type);
+						 tx_channel_type);
 		rcu_read_unlock();
 
 		if (beacon_htcap_ie)
@@ -287,7 +287,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 	/* if bss configuration changed store the new one */
 	if (sdata->ht_opmode_valid != enable_ht ||
 	    sdata->vif.bss_conf.ht_operation_mode != ht_opmode ||
-	    prev_chantype != channel_type) {
+	    prev_chantype != rx_channel_type) {
 		changed |= BSS_CHANGED_HT;
 		sdata->vif.bss_conf.ht_operation_mode = ht_opmode;
 		sdata->ht_opmode_valid = enable_ht;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 3ab85c0..a1693cc 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2266,9 +2266,11 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
 
 			sband = rx->local->hw.wiphy->bands[status->band];
 
-			rate_control_rate_update(local, sband, rx->sta,
-						 IEEE80211_RC_SMPS_CHANGED,
-						 local->_oper_channel_type);
+			rate_control_rate_update(
+				local, sband, rx->sta,
+				IEEE80211_RC_SMPS_CHANGED,
+				ieee80211_get_tx_channel_type(
+					local, local->_oper_channel_type));
 			goto handled;
 		}
 		default:
-- 
1.7.7.3

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Sam Leffler]

On Thu, Feb 23, 2012 at 5:53 PM, Luis R. Rodriguez <mcgrof@frijolero.org> wrote:
> On Thu, Feb 23, 2012 at 7:14 AM, Paul Stewart <pstew@chromium.org> wrote:
>> On Thu, Feb 23, 2012 at 6:59 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>>>
>>> On Thu, 2012-02-23 at 06:53 -0800, Sam Leffler wrote:
>>> > On Thu, Feb 23, 2012 at 5:39 AM, Johannes Berg
>>> > <johannes@sipsolutions.net> wrote:
>>> > > On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
>>> > >> When regulatory information changes our HT behavior (e.g,
>>> > >> when we get a country code from the AP we have just associated
>>> > >> with), we should use this information to change the power with
>>> > >> which we transmit, and what channels we transmit.  Sometimes
>>> > >> the channel parameters we derive from regulatory information
>>> > >> contradicts the parameters we used in association.  For example,
>>> > >> we could have associated specifying HT40, but the regulatory
>>> > >> rules we apply may forbid HT40 operation.
>>> > >>
>>> > >> In the situation above, we should reconfigure ourselves to
>>> > >> transmit in HT20 only, however it makes no sense for us to
>>> > >> disable receive in HT40, since if we associated with these
>>> > >> parameters, the AP has every reason to expect we can and
>>> > >> will receive packets this way.  The code in mac80211 does
>>> > >> not have the capability of sending the appropriate action
>>> > >> frames to signal a change in HT behaviour so the AP has
>>> > >> no clue we can no longer receive frames encoded this way.
>>> > >> In some broken AP implementations, this can leave us
>>> > >> effectively deaf if the AP never retries in lower HT rates.
>>> > >>
>>> > >> This change breaks up the channel_type parameter in the
>>> > >> ieee80211_enable_ht function into a separate receive and
>>> > >> transmit part.  It honors the channel flags set by regulatory
>>> > >> in order to configure the rate control algorithm, but uses
>>> > >> the capability flags to configure the channel on the radio,
>>> > >> since these were used in association to set the AP's transmit
>>> > >> rate.
>>> > >
>>> > > Quite the stupid APs, obviously ...
>>> >
>>> > Actually the AP is operating correctly (modulo the question of whether
>>> > HT40 may be used in kr).
>>>
>>> Ah, yes. I was thinking the AP was actually saying you can't use 40 MHz,
>>> but it's obviously just saying that you're in KR and our database says
>>> you then can't use 40 Mhz...
>>
>> More succinctly, Sam is posing the possibility that a STA we should
>> ignore regulatory on TX as well as RX in this situation, since the AP
>> clearly negotiated HT40 with us.
>
> If the AP was right and we were allowed to use this that'd be fine but
> consider the case where the AP is wrong, they'd we'd be doing the
> wrong thing. If the AP believes HT40 is allowed but our wireless-regdb
> disagrees we trust our wireless-regdb more, by design.
>
> Did I understand your question correctly?

Yes, the point is that one must trust the AP otherwise you potentially
have a sta that is not operating correctly within the BSS.  There are
obvious limits (e.g. you don't raise the tx power cap above the h/w
limit) but in this case it makes no sense to have a sta sticking to
HT20 when the AP says the BSS is using HT40.  The local regulatory
state should be used when starting up a master (ibss or infra) but
overriding an AP in most cases will result in confusion.

-Sam

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
> When regulatory information changes our HT behavior (e.g,
> when we get a country code from the AP we have just associated
> with), we should use this information to change the power with
> which we transmit, and what channels we transmit.  Sometimes
> the channel parameters we derive from regulatory information
> contradicts the parameters we used in association.  For example,
> we could have associated specifying HT40, but the regulatory
> rules we apply may forbid HT40 operation.
> 
> In the situation above, we should reconfigure ourselves to
> transmit in HT20 only, however it makes no sense for us to
> disable receive in HT40, since if we associated with these
> parameters, the AP has every reason to expect we can and
> will receive packets this way.

Looking at this in mac80211 (ieee80211_enable_ht) in more detail for
other reasons, I believe our current behaviour is wrong in other cases
as well, not just the regulatory case.

If the AP is advertising HT40, but 40MHz-intolerant at the same time, I
believe we should also configure the channel to HT40, and prohibit using
40 MHz TX for the time in which 40MHz-intolerant is set. Otherwise, when
the AP changes the 40MHz-intolerant flag, we may reconfigure our channel
too late to receive 40MHz transmissions. Also, some devices don't like
having the channel reconfigured while associated and doing so would be
particularly messy when we start talking about multi-channel operation.

I think overall better behaviour would be to set up the HT channel
already in ieee80211_mgd_auth() according to what the AP advertises,
disregarding regulatory information and 40MHz-intolerant bits. While
associated, the only possible changes would be
 1) changes in HT opmode (protection, non-GF/non-HT sta etc.)
 2) 40MHz-intolerant changes (e.g. when such a STA joins)
This would affect both the driver and rate control, and for drivers
having rate control built-in we need to also tell them directly.

Thoughts?

johannes

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Sun, 2012-02-26 at 12:15 +0100, Johannes Berg wrote:
> On Mon, 2012-02-20 at 21:25 -0800, Paul Stewart wrote:
> > When regulatory information changes our HT behavior (e.g,
> > when we get a country code from the AP we have just associated
> > with), we should use this information to change the power with
> > which we transmit, and what channels we transmit.  Sometimes
> > the channel parameters we derive from regulatory information
> > contradicts the parameters we used in association.  For example,
> > we could have associated specifying HT40, but the regulatory
> > rules we apply may forbid HT40 operation.
> > 
> > In the situation above, we should reconfigure ourselves to
> > transmit in HT20 only, however it makes no sense for us to
> > disable receive in HT40, since if we associated with these
> > parameters, the AP has every reason to expect we can and
> > will receive packets this way.
> 
> Looking at this in mac80211 (ieee80211_enable_ht) in more detail for
> other reasons, I believe our current behaviour is wrong in other cases
> as well, not just the regulatory case.
> 
> If the AP is advertising HT40, but 40MHz-intolerant at the same time, I
> believe we should also configure the channel to HT40, and prohibit using
> 40 MHz TX for the time in which 40MHz-intolerant is set. Otherwise, when
> the AP changes the 40MHz-intolerant flag, we may reconfigure our channel
> too late to receive 40MHz transmissions. Also, some devices don't like
> having the channel reconfigured while associated and doing so would be
> particularly messy when we start talking about multi-channel operation.
> 
> I think overall better behaviour would be to set up the HT channel
> already in ieee80211_mgd_auth() according to what the AP advertises,

Correction: ieee80211_mgd_assoc(), during auth we don't yet know what
ciphers we'll use etc. so we don't yet know if HT will be disabled.
However, during auth we also don't care about the channel type, so
there's no problem for multi-channel.

> disregarding regulatory information and 40MHz-intolerant bits. While
> associated, the only possible changes would be
>  1) changes in HT opmode (protection, non-GF/non-HT sta etc.)
>  2) 40MHz-intolerant changes (e.g. when such a STA joins)
> This would affect both the driver and rate control, and for drivers
> having rate control built-in we need to also tell them directly.
> 
> Thoughts?
> 
> johannes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Sun, 2012-02-26 at 12:25 +0100, Johannes Berg wrote:

> > Looking at this in mac80211 (ieee80211_enable_ht) in more detail for
> > other reasons, I believe our current behaviour is wrong in other cases
> > as well, not just the regulatory case.
> > 
> > If the AP is advertising HT40, but 40MHz-intolerant at the same time, I
> > believe we should also configure the channel to HT40, and prohibit using
> > 40 MHz TX for the time in which 40MHz-intolerant is set. Otherwise, when
> > the AP changes the 40MHz-intolerant flag, we may reconfigure our channel
> > too late to receive 40MHz transmissions. Also, some devices don't like
> > having the channel reconfigured while associated and doing so would be
> > particularly messy when we start talking about multi-channel operation.
> > 
> > I think overall better behaviour would be to set up the HT channel
> > already in ieee80211_mgd_auth() according to what the AP advertises,
> 
> Correction: ieee80211_mgd_assoc(), during auth we don't yet know what
> ciphers we'll use etc. so we don't yet know if HT will be disabled.
> However, during auth we also don't care about the channel type, so
> there's no problem for multi-channel.
> 
> > disregarding regulatory information and 40MHz-intolerant bits. While
> > associated, the only possible changes would be
> >  1) changes in HT opmode (protection, non-GF/non-HT sta etc.)
> >  2) 40MHz-intolerant changes (e.g. when such a STA joins)
> > This would affect both the driver and rate control, and for drivers
> > having rate control built-in we need to also tell them directly.


Come to think of it, we could also simply configure the channel to be
the right HT channel during auth, and then still associate as a non-HT
station later. I realise this isn't the best configuration, but given
that this is a corner case (TKIP used on an HT AP!) I think we can live
with that.

Sorry to flood you all with my incoherent ramblings -- anyone have
thoughts about this?

FWIW, in the interest of full disclosure :), part of the reason for this
is that our device hates having the channel reconfigured in the middle
of something, that causes issues with passive channels etc.

More importantly though, I think having to worry about reconfiguring the
channel will also be a big hassle in upcoming multi-channel code.

johannes

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Jouni Malinen]

On Mon, Feb 27, 2012 at 11:34:58AM +0100, Johannes Berg wrote:
> Come to think of it, we could also simply configure the channel to be
> the right HT channel during auth, and then still associate as a non-HT
> station later. I realise this isn't the best configuration, but given
> that this is a corner case (TKIP used on an HT AP!) I think we can live
> with that.

In general, it should be fine to enable HT/HT40 configuration for
receive and just make sure we never transmit using parameters not
allowed by regulatory or something like HT+TKIP rules. However, there
could be some hardware/firmware designs that would refuse HT+TKIP at
lower layer, so skipping the channel (re-)configuration could
potentially cause some problems. I'm not aware of any specific example
of this, though, so I can only speculate that such a thing could exist.

> More importantly though, I think having to worry about reconfiguring the
> channel will also be a big hassle in upcoming multi-channel code.

Agreed.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Mon, 2012-02-27 at 13:32 +0200, Jouni Malinen wrote:
> On Mon, Feb 27, 2012 at 11:34:58AM +0100, Johannes Berg wrote:
> > Come to think of it, we could also simply configure the channel to be
> > the right HT channel during auth, and then still associate as a non-HT
> > station later. I realise this isn't the best configuration, but given
> > that this is a corner case (TKIP used on an HT AP!) I think we can live
> > with that.
> 
> In general, it should be fine to enable HT/HT40 configuration for
> receive and just make sure we never transmit using parameters not
> allowed by regulatory or something like HT+TKIP rules. 

Right, this would be setting the configuration for receive -- in a way.
Remember that we're not telling the AP that we're an HT station (we
don't send HT IEs) so traffic to us still wouldn't be transmitted as HT.
Which is really what we want, since we don't want to suddenly find that
we're being sent TKIP HT frames.

> However, there
> could be some hardware/firmware designs that would refuse HT+TKIP at
> lower layer, so skipping the channel (re-)configuration could
> potentially cause some problems. I'm not aware of any specific example
> of this, though, so I can only speculate that such a thing could exist.

That's a good point, but I don't really think is likely to exist. I can
see this in a full-MAC scenario, but there you get all the relevant
parameters in the nl80211 connect() call so can do the right choices
earlier than mac80211 can with auth/assoc calls.

Of course, if this we actually happen to come across a device that needs
this it could set a flag somehwere and mac80211 could re-configure the
channel to non-HT on the assoc request, but right now I'd rather not
worry about that since we're moving towards multi-channel and have no
indication of such devices existing. Agree?

johannes

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Jouni Malinen]

On Mon, Feb 27, 2012 at 12:38:50PM +0100, Johannes Berg wrote:
> On Mon, 2012-02-27 at 13:32 +0200, Jouni Malinen wrote:
> > However, there
> > could be some hardware/firmware designs that would refuse HT+TKIP at
> > lower layer, so skipping the channel (re-)configuration could
> > potentially cause some problems.

> That's a good point, but I don't really think is likely to exist. I can
> see this in a full-MAC scenario, but there you get all the relevant
> parameters in the nl80211 connect() call so can do the right choices
> earlier than mac80211 can with auth/assoc calls.

Even full MAC designs may end up moving towards auth/assoc calls for
things like IEEE 802.11r.. (And maybe even more so with 802.11ai
eventually.)

> Of course, if this we actually happen to come across a device that needs
> this it could set a flag somehwere and mac80211 could re-configure the
> channel to non-HT on the assoc request, but right now I'd rather not
> worry about that since we're moving towards multi-channel and have no
> indication of such devices existing. Agree?

Yeah, that works for me.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Paul Stewart]

On Mon, Feb 27, 2012 at 5:46 AM, Jouni Malinen <j@w1.fi> wrote:
> On Mon, Feb 27, 2012 at 12:38:50PM +0100, Johannes Berg wrote:
>> On Mon, 2012-02-27 at 13:32 +0200, Jouni Malinen wrote:
>> > However, there
>> > could be some hardware/firmware designs that would refuse HT+TKIP at
>> > lower layer, so skipping the channel (re-)configuration could
>> > potentially cause some problems.
>
>> That's a good point, but I don't really think is likely to exist. I can
>> see this in a full-MAC scenario, but there you get all the relevant
>> parameters in the nl80211 connect() call so can do the right choices
>> earlier than mac80211 can with auth/assoc calls.
>
> Even full MAC designs may end up moving towards auth/assoc calls for
> things like IEEE 802.11r.. (And maybe even more so with 802.11ai
> eventually.)
>
>> Of course, if this we actually happen to come across a device that needs
>> this it could set a flag somehwere and mac80211 could re-configure the
>> channel to non-HT on the assoc request, but right now I'd rather not
>> worry about that since we're moving towards multi-channel and have no
>> indication of such devices existing. Agree?
>
> Yeah, that works for me.

Discussions seem to have come to an end here, however I'm not sure
what we ended up deciding. :-)  I see mention of HT+TKIP -- not sure
I'm familiar enough with what the issues are there -- as well as
pushing the channel configuration earlier and perhaps changes to the
association process.  Since I may not have a complete handle on all
these adjacent issues, where do we want to go with the current change
(fixes a real-life issue) and how do we want to stage that with
respect to the other issues this seems to have brought to the surface?

--
Paul

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Tue, 2012-03-06 at 21:46 -0800, Paul Stewart wrote:

> >> Of course, if this we actually happen to come across a device that needs
> >> this it could set a flag somehwere and mac80211 could re-configure the
> >> channel to non-HT on the assoc request, but right now I'd rather not
> >> worry about that since we're moving towards multi-channel and have no
> >> indication of such devices existing. Agree?
> >
> > Yeah, that works for me.
> 
> Discussions seem to have come to an end here, however I'm not sure
> what we ended up deciding. :-)  I see mention of HT+TKIP -- not sure
> I'm familiar enough with what the issues are there -- as well as
> pushing the channel configuration earlier and perhaps changes to the
> association process.  Since I may not have a complete handle on all
> these adjacent issues, where do we want to go with the current change
> (fixes a real-life issue) and how do we want to stage that with
> respect to the other issues this seems to have brought to the surface?

This discussion really side-tracked somewhat -- we were discussing a
better/different solution. I'm OK with your patch, but since I'll be
touching the code again to address the other things we talked about it'd
be great if you could test after that again, maybe in a week or two?

johannes

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[John W. Linville]

On Wed, Mar 07, 2012 at 08:36:14AM +0100, Johannes Berg wrote:
> On Tue, 2012-03-06 at 21:46 -0800, Paul Stewart wrote:
> 
> > >> Of course, if this we actually happen to come across a device that needs
> > >> this it could set a flag somehwere and mac80211 could re-configure the
> > >> channel to non-HT on the assoc request, but right now I'd rather not
> > >> worry about that since we're moving towards multi-channel and have no
> > >> indication of such devices existing. Agree?
> > >
> > > Yeah, that works for me.
> > 
> > Discussions seem to have come to an end here, however I'm not sure
> > what we ended up deciding. :-)  I see mention of HT+TKIP -- not sure
> > I'm familiar enough with what the issues are there -- as well as
> > pushing the channel configuration earlier and perhaps changes to the
> > association process.  Since I may not have a complete handle on all
> > these adjacent issues, where do we want to go with the current change
> > (fixes a real-life issue) and how do we want to stage that with
> > respect to the other issues this seems to have brought to the surface?
> 
> This discussion really side-tracked somewhat -- we were discussing a
> better/different solution. I'm OK with your patch, but since I'll be
> touching the code again to address the other things we talked about it'd
> be great if you could test after that again, maybe in a week or two?

So, should I wait to merge Paul's patch until after the merge window
(i.e. only merge it for 3.5)?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [RFCv2] mac80211: Don't let regulatory make us deaf

[Johannes Berg]

On Wed, 2012-03-07 at 13:40 -0500, John W. Linville wrote:
> On Wed, Mar 07, 2012 at 08:36:14AM +0100, Johannes Berg wrote:
> > On Tue, 2012-03-06 at 21:46 -0800, Paul Stewart wrote:
> > 
> > > >> Of course, if this we actually happen to come across a device that needs
> > > >> this it could set a flag somehwere and mac80211 could re-configure the
> > > >> channel to non-HT on the assoc request, but right now I'd rather not
> > > >> worry about that since we're moving towards multi-channel and have no
> > > >> indication of such devices existing. Agree?
> > > >
> > > > Yeah, that works for me.
> > > 
> > > Discussions seem to have come to an end here, however I'm not sure
> > > what we ended up deciding. :-)  I see mention of HT+TKIP -- not sure
> > > I'm familiar enough with what the issues are there -- as well as
> > > pushing the channel configuration earlier and perhaps changes to the
> > > association process.  Since I may not have a complete handle on all
> > > these adjacent issues, where do we want to go with the current change
> > > (fixes a real-life issue) and how do we want to stage that with
> > > respect to the other issues this seems to have brought to the surface?
> > 
> > This discussion really side-tracked somewhat -- we were discussing a
> > better/different solution. I'm OK with your patch, but since I'll be
> > touching the code again to address the other things we talked about it'd
> > be great if you could test after that again, maybe in a week or two?
> 
> So, should I wait to merge Paul's patch until after the merge window
> (i.e. only merge it for 3.5)?

I think it's OK to merge for 3.4 too if you wish to do so, although Paul
hasn't sent a [PATCH] yet so ...

johannes