[PATCH 0/5] ath: simplify radar / beaconing flag settings

[PATCH 0/5] ath: simplify radar / beaconing flag settings

[Luis R. Rodriguez]

John, Johannes,

these patches start cleaning up the flag usage through the reg-notifier
but the main motiviation here as to help clarify the case in which
DFS is supported and we don't need to apply some paranoid flags. I've
determined its best to clean this up on cfg80211 (unifying flags)
and deal with the DFS case in cfg80211 as well (I add the documentation,
but expect Januz to send a respective change later for DFS).

Given that the later patches cfg80211 / other drivers I'll let you
guys decide what tree these bubble up through.

This series simplifies the flag application setup. Just a heads up I
noticed that broadcom and rtl drivers share a lot of this code so
eventually we may want to just export these and share them somehow.

Luis R. Rodriguez (5):
  ath: fix logic on ath_reg_apply_active_scan_flags()
  ath: clean up ath_reg_apply_active_scan_flags()
  ath: rename ath_reg_apply_active_scan_flags() to
    ath_reg_apply_ir_flags()
  ath: simplify ath_reg_apply_beaconing_flags()
  ath: move the channel for ath_reg_apply_beaconing_flags() into helper

 drivers/net/wireless/ath/regd.c | 174 ++++++++++++++++++++++------------------
 1 file changed, 94 insertions(+), 80 deletions(-)

-- 
1.8.4.rc3

[PATCH 1/5] ath: fix logic on ath_reg_apply_active_scan_flags()

[Luis R. Rodriguez]

The existing logic removes the passive scan flag from
channels 12 and 13 when a regulatory hint coming from
something other than a country IE has been passed. This
is incorrect, the original intention was to ensure we
always have passive scan enabled for these two channels
for a specific set of custom world regulatory domains.

Cc: smihir@qti.qualcomm.com
Cc: tushnimb@qca.qualcomm.com
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/net/wireless/ath/regd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index c00687e..1b4a532 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -255,16 +255,16 @@ ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 		return;
 
 	/*
-	 * If no country IE has been received always enable active scan
-	 * on these channels. This is only done for specific regulatory SKUs
+	 * If no country IE has been received always enable passive scan
+	 * and no-ibss on these channels. This is only done for specific
+	 * regulatory SKUs.
 	 */
 	if (initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 		ch = &sband->channels[11]; /* CH 12 */
-		if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
-			ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
+		ch->flags |= IEEE80211_CHAN_PASSIVE_SCAN;
+
 		ch = &sband->channels[12]; /* CH 13 */
-		if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
-			ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
+		ch->flags |= IEEE80211_CHAN_PASSIVE_SCAN;
 		return;
 	}
 
-- 
1.8.4.rc3

[PATCH 2/5] ath: clean up ath_reg_apply_active_scan_flags()

[Luis R. Rodriguez]

The routine ath_reg_apply_active_scan_flags() can be a bit
hard to read, this cleans it up by adding helpers for the
two cases of clearing IR flags or adding them. This approach
also makes no assumptions on the index of channels 12 and 13
so it should be portable accross different drivers.

Cc: smihir@qti.qualcomm.com
Cc: tushnimb@qca.qualcomm.com
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/net/wireless/ath/regd.c | 101 +++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 1b4a532..48e2368 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -177,6 +177,47 @@ static bool ath_is_radar_freq(u16 center_freq)
 	return (center_freq >= 5260 && center_freq <= 5700);
 }
 
+static void ath_force_clear_no_ir_chan(struct wiphy *wiphy,
+				       struct ieee80211_channel *ch)
+{
+	const struct ieee80211_reg_rule *reg_rule;
+
+	reg_rule = freq_reg_info(wiphy, ch->center_freq);
+	if (IS_ERR(reg_rule))
+		return;
+
+	if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
+		if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
+			ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
+}
+
+static void ath_force_clear_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
+{
+	struct ieee80211_channel *ch;
+
+	ch = ieee80211_get_channel(wiphy, center_freq);
+	if (!ch)
+		return;
+
+	ath_force_clear_no_ir_chan(wiphy, ch);
+}
+
+static void ath_force_no_ir_chan(struct ieee80211_channel *ch)
+{
+	ch->flags |= IEEE80211_CHAN_PASSIVE_SCAN;
+}
+
+static void ath_force_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
+{
+	struct ieee80211_channel *ch;
+
+	ch = ieee80211_get_channel(wiphy, center_freq);
+	if (!ch)
+		return;
+
+	ath_force_no_ir_chan(ch);
+}
+
 /*
  * N.B: These exception rules do not apply radar freqs.
  *
@@ -241,54 +282,38 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 
 }
 
-/* Allows active scan scan on Ch 12 and 13 */
+/**
+ * ath_reg_apply_active_scan_flags()
+ * @wiphy: the wiphy to use
+ * @initiator: the regulatory hint initiator
+ *
+ * If no country IE has been received always enable passive scan
+ * and no-ibss on these channels. This is only done for specific
+ * regulatory SKUs.
+ *
+ * If a country IE has been received check its rule for this
+ * channel first before enabling active scan. The passive scan
+ * would have been enforced by the initial processing of our
+ * custom regulatory domain.
+ */
 static void
 ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 				enum nl80211_reg_initiator initiator)
 {
 	struct ieee80211_supported_band *sband;
-	struct ieee80211_channel *ch;
-	const struct ieee80211_reg_rule *reg_rule;
 
 	sband = wiphy->bands[IEEE80211_BAND_2GHZ];
 	if (!sband)
 		return;
 
-	/*
-	 * If no country IE has been received always enable passive scan
-	 * and no-ibss on these channels. This is only done for specific
-	 * regulatory SKUs.
-	 */
-	if (initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-		ch = &sband->channels[11]; /* CH 12 */
-		ch->flags |= IEEE80211_CHAN_PASSIVE_SCAN;
-
-		ch = &sband->channels[12]; /* CH 13 */
-		ch->flags |= IEEE80211_CHAN_PASSIVE_SCAN;
-		return;
-	}
-
-	/*
-	 * If a country IE has been received check its rule for this
-	 * channel first before enabling active scan. The passive scan
-	 * would have been enforced by the initial processing of our
-	 * custom regulatory domain.
-	 */
-
-	ch = &sband->channels[11]; /* CH 12 */
-	reg_rule = freq_reg_info(wiphy, ch->center_freq);
-	if (!IS_ERR(reg_rule)) {
-		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
-			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
-				ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
-	}
-
-	ch = &sband->channels[12]; /* CH 13 */
-	reg_rule = freq_reg_info(wiphy, ch->center_freq);
-	if (!IS_ERR(reg_rule)) {
-		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
-			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
-				ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
+	switch(initiator) {
+	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
+		ath_force_clear_no_ir_freq(wiphy, 2467);
+		ath_force_clear_no_ir_freq(wiphy, 2472);
+		break;
+	default:
+		ath_force_no_ir_freq(wiphy, 2467);
+		ath_force_no_ir_freq(wiphy, 2472);
 	}
 }
 
-- 
1.8.4.rc3

[PATCH 3/5] ath: rename ath_reg_apply_active_scan_flags() to ath_reg_apply_ir_flags()

[Luis R. Rodriguez]

This also applies the no-ibss flag to the channels or clears it.
The idea here is to clarify no initiated radiation should be
allowed on these channels.

Cc: smihir@qti.qualcomm.com
Cc: tushnimb@qca.qualcomm.com
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/net/wireless/ath/regd.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 48e2368..2188fdd 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -189,6 +189,10 @@ static void ath_force_clear_no_ir_chan(struct wiphy *wiphy,
 	if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 		if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
 			ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
+
+	if (!(reg_rule->flags & IEEE80211_CHAN_NO_IBSS))
+		if (ch->flags & IEEE80211_CHAN_NO_IBSS)
+			ch->flags &= ~IEEE80211_CHAN_NO_IBSS;
 }
 
 static void ath_force_clear_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
@@ -205,6 +209,7 @@ static void ath_force_clear_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
 static void ath_force_no_ir_chan(struct ieee80211_channel *ch)
 {
 	ch->flags |= IEEE80211_CHAN_PASSIVE_SCAN;
+	ch->flags |= IEEE80211_CHAN_NO_IBSS;
 }
 
 static void ath_force_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
@@ -283,7 +288,7 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 }
 
 /**
- * ath_reg_apply_active_scan_flags()
+ * ath_reg_apply_ir_flags()
  * @wiphy: the wiphy to use
  * @initiator: the regulatory hint initiator
  *
@@ -297,7 +302,7 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
  * custom regulatory domain.
  */
 static void
-ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
+ath_reg_apply_ir_flags(struct wiphy *wiphy,
 				enum nl80211_reg_initiator initiator)
 {
 	struct ieee80211_supported_band *sband;
@@ -364,7 +369,7 @@ static void ath_reg_apply_world_flags(struct wiphy *wiphy,
 		break;
 	case 0x68:
 		ath_reg_apply_beaconing_flags(wiphy, initiator);
-		ath_reg_apply_active_scan_flags(wiphy, initiator);
+		ath_reg_apply_ir_flags(wiphy, initiator);
 		break;
 	}
 }
-- 
1.8.4.rc3

[PATCH 4/5] ath: simplify ath_reg_apply_beaconing_flags()

[Luis R. Rodriguez]

Simplify ath_reg_apply_beaconing_flags() by making use of
thew new no-ir helper.

Cc: smihir@qti.qualcomm.com
Cc: tushnimb@qca.qualcomm.com
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/net/wireless/ath/regd.c | 37 ++++++++++---------------------------
 1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 2188fdd..941e65b 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -224,13 +224,11 @@ static void ath_force_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
 }
 
 /*
- * N.B: These exception rules do not apply radar freqs.
+ * These exception rules do not apply radar frequencies.
  *
- * - We enable adhoc (or beaconing) if allowed by 11d
- * - We enable active scan if the channel is allowed by 11d
+ * - We enable initiating radiation if the country IE says its fine:
  * - If no country IE has been processed and a we determine we have
- *   received a beacon on a channel we can enable active scan and
- *   adhoc (or beaconing).
+ *   received a beacon on a channel we can enable initiating radiation.
  */
 static void
 ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
@@ -238,7 +236,6 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 {
 	enum ieee80211_band band;
 	struct ieee80211_supported_band *sband;
-	const struct ieee80211_reg_rule *reg_rule;
 	struct ieee80211_channel *ch;
 	unsigned int i;
 
@@ -257,27 +254,13 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 			    (ch->flags & IEEE80211_CHAN_RADAR))
 				continue;
 
-			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-				reg_rule = freq_reg_info(wiphy, ch->center_freq);
-				if (IS_ERR(reg_rule))
-					continue;
-				/*
-				 * If 11d had a rule for this channel ensure
-				 * we enable adhoc/beaconing if it allows us to
-				 * use it. Note that we would have disabled it
-				 * by applying our static world regdomain by
-				 * default during init, prior to calling our
-				 * regulatory_hint().
-				 */
-				if (!(reg_rule->flags &
-				    NL80211_RRF_NO_IBSS))
-					ch->flags &=
-					  ~IEEE80211_CHAN_NO_IBSS;
-				if (!(reg_rule->flags &
-				    NL80211_RRF_PASSIVE_SCAN))
-					ch->flags &=
-					  ~IEEE80211_CHAN_PASSIVE_SCAN;
-			} else {
+			/*
+			 * If the country IE says initiating radiation
+			 * is OK we trust that.
+			 */
+			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
+				ath_force_clear_no_ir_chan(wiphy, ch);
+			else {
 				if (ch->beacon_found)
 					ch->flags &= ~(IEEE80211_CHAN_NO_IBSS |
 					  IEEE80211_CHAN_PASSIVE_SCAN);
-- 
1.8.4.rc3

[PATCH 5/5] ath: move the channel for ath_reg_apply_beaconing_flags() into helper

[Luis R. Rodriguez]

While at it convert this into a switch statement, this
makes it easier and manage.

Cc: smihir@qti.qualcomm.com
Cc: tushnimb@qca.qualcomm.com
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/net/wireless/ath/regd.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 941e65b..14d3614 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -223,6 +223,26 @@ static void ath_force_no_ir_freq(struct wiphy *wiphy, u16 center_freq)
 	ath_force_no_ir_chan(ch);
 }
 
+static void
+__ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
+				enum nl80211_reg_initiator initiator,
+				struct ieee80211_channel *ch)
+{
+	if (ath_is_radar_freq(ch->center_freq) ||
+	    (ch->flags & IEEE80211_CHAN_RADAR))
+		return;
+
+	switch (initiator) {
+	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
+		ath_force_clear_no_ir_chan(wiphy, ch);
+		break;
+	default:
+		if (ch->beacon_found)
+			ch->flags &= ~(IEEE80211_CHAN_NO_IBSS |
+				       IEEE80211_CHAN_PASSIVE_SCAN);
+	}
+}
+
 /*
  * These exception rules do not apply radar frequencies.
  *
@@ -240,34 +260,15 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 	unsigned int i;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
-
 		if (!wiphy->bands[band])
 			continue;
-
 		sband = wiphy->bands[band];
-
 		for (i = 0; i < sband->n_channels; i++) {
-
 			ch = &sband->channels[i];
+			__ath_reg_apply_beaconing_flags(wiphy, initiator, ch);
 
-			if (ath_is_radar_freq(ch->center_freq) ||
-			    (ch->flags & IEEE80211_CHAN_RADAR))
-				continue;
-
-			/*
-			 * If the country IE says initiating radiation
-			 * is OK we trust that.
-			 */
-			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
-				ath_force_clear_no_ir_chan(wiphy, ch);
-			else {
-				if (ch->beacon_found)
-					ch->flags &= ~(IEEE80211_CHAN_NO_IBSS |
-					  IEEE80211_CHAN_PASSIVE_SCAN);
-			}
 		}
 	}
-
 }
 
 /**
-- 
1.8.4.rc3

RE: [PATCH 2/5] ath: clean up ath_reg_apply_active_scan_flags()

[Shete, Mihir]

> -----Original Message-----
> From: Luis R. Rodriguez [mailto:mcgrof@gmail.com] On Behalf Of Luis R.
> Rodriguez
> Sent: Monday, October 21, 2013 10:45 PM
> To: linville@tuxdriver.com; johannes@sipsolutions.net
> Cc: linux-wireless@vger.kernel.org; janusz.dziedzic@tieto.com; Shete, Mihir;
> Bhattacharyya, Tushnim; Luis R. Rodriguez
> Subject: [PATCH 2/5] ath: clean up ath_reg_apply_active_scan_flags()
> 
> The routine ath_reg_apply_active_scan_flags() can be a bit hard to read, this
> cleans it up by adding helpers for the two cases of clearing IR flags or adding
> them. This approach also makes no assumptions on the index of channels 12
> and 13 so it should be portable accross different drivers.
> 
> Cc: smihir@qti.qualcomm.com
> Cc: tushnimb@qca.qualcomm.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
> ---
>  drivers/net/wireless/ath/regd.c | 101 +++++++++++++++++++++++++-------
> --------
>  1 file changed, 63 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/regd.c
> b/drivers/net/wireless/ath/regd.c index 1b4a532..48e2368 100644
> --- a/drivers/net/wireless/ath/regd.c
> +++ b/drivers/net/wireless/ath/regd.c
> @@ -177,6 +177,47 @@ static bool ath_is_radar_freq(u16 center_freq)
>  	return (center_freq >= 5260 && center_freq <= 5700);  }
> 
> +static void ath_force_clear_no_ir_chan(struct wiphy *wiphy,
> +				       struct ieee80211_channel *ch) {
> +	const struct ieee80211_reg_rule *reg_rule;
> +
> +	reg_rule = freq_reg_info(wiphy, ch->center_freq);

Do we not require to provide the frequency in KHZ here?

Re: [PATCH 2/5] ath: clean up ath_reg_apply_active_scan_flags()

[Luis R. Rodriguez]

On Tue, Oct 29, 2013 at 5:19 PM, Shete, Mihir <smihir@qti.qualcomm.com> wrote:
>
>
>> -----Original Message-----
>> From: Luis R. Rodriguez [mailto:mcgrof@gmail.com] On Behalf Of Luis R.
>> Rodriguez
>> Sent: Monday, October 21, 2013 10:45 PM
>> To: linville@tuxdriver.com; johannes@sipsolutions.net
>> Cc: linux-wireless@vger.kernel.org; janusz.dziedzic@tieto.com; Shete, Mihir;
>> Bhattacharyya, Tushnim; Luis R. Rodriguez
>> Subject: [PATCH 2/5] ath: clean up ath_reg_apply_active_scan_flags()
>>
>> The routine ath_reg_apply_active_scan_flags() can be a bit hard to read, this
>> cleans it up by adding helpers for the two cases of clearing IR flags or adding
>> them. This approach also makes no assumptions on the index of channels 12
>> and 13 so it should be portable accross different drivers.
>>
>> Cc: smihir@qti.qualcomm.com
>> Cc: tushnimb@qca.qualcomm.com
>> Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
>> ---
>>  drivers/net/wireless/ath/regd.c | 101 +++++++++++++++++++++++++-------
>> --------
>>  1 file changed, 63 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/regd.c
>> b/drivers/net/wireless/ath/regd.c index 1b4a532..48e2368 100644
>> --- a/drivers/net/wireless/ath/regd.c
>> +++ b/drivers/net/wireless/ath/regd.c
>> @@ -177,6 +177,47 @@ static bool ath_is_radar_freq(u16 center_freq)
>>       return (center_freq >= 5260 && center_freq <= 5700);  }
>>
>> +static void ath_force_clear_no_ir_chan(struct wiphy *wiphy,
>> +                                    struct ieee80211_channel *ch) {
>> +     const struct ieee80211_reg_rule *reg_rule;
>> +
>> +     reg_rule = freq_reg_info(wiphy, ch->center_freq);
>
> Do we not require to provide the frequency in KHZ here?

Good catch! This has been beautifully broken for eons. And guess, what
all those other drivers that copied the code also have this broken.
I'll just send a follow up patch to fix this on top of this patch for
each driver as they're all broken. In a way this seems to be begging
to just share a reg_notifier between drivers as a few drivers copied
the same behaviour from ath9k.

  Luis