[PATCH 1/5] cfg80211: introduce regulatory flags controlling bw

[PATCH 1/5] cfg80211: introduce regulatory flags controlling bw

[Arik Nemtsov]

Allow setting bandwidth related regulatory flags. These flags are mapped
to the corresponding channel flags in the specified range.
Make sure the new flags are consulted when calculating the maximum
bandwidth allowed by a regulatory-rule.

Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
channel.

Change-Id: Ic120a291ef7a56f2efa800e77e9689c6c421affa
Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
Reviewed-on: https://gerrit.rds.intel.com/32263
Tested-by: IWL Jenkins
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
---
 include/uapi/linux/nl80211.h | 12 ++++++++++++
 net/wireless/reg.c           | 30 ++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index fb0efa1..e87f187 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2572,6 +2572,11 @@ enum nl80211_sched_scan_match_attr {
  * @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated
  *	base on contiguous rules and wider channels will be allowed to cross
  *	multiple contiguous/overlapping frequency ranges.
+ * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT
+ * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation
+ * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation
+ * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
+ * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
  */
 enum nl80211_reg_rule_flags {
 	NL80211_RRF_NO_OFDM		= 1<<0,
@@ -2584,11 +2589,18 @@ enum nl80211_reg_rule_flags {
 	NL80211_RRF_NO_IR		= 1<<7,
 	__NL80211_RRF_NO_IBSS		= 1<<8,
 	NL80211_RRF_AUTO_BW		= 1<<11,
+	NL80211_RRF_GO_CONCURRENT	= 1<<12,
+	NL80211_RRF_NO_HT40MINUS	= 1<<13,
+	NL80211_RRF_NO_HT40PLUS		= 1<<14,
+	NL80211_RRF_NO_80MHZ		= 1<<15,
+	NL80211_RRF_NO_160MHZ		= 1<<16,
 };
 
 #define NL80211_RRF_PASSIVE_SCAN	NL80211_RRF_NO_IR
 #define NL80211_RRF_NO_IBSS		NL80211_RRF_NO_IR
 #define NL80211_RRF_NO_IR		NL80211_RRF_NO_IR
+#define NL80211_RRF_NO_HT40		(NL80211_RRF_NO_HT40MINUS |\
+					 NL80211_RRF_NO_HT40PLUS)
 
 /* For backport compatibility with older userspace */
 #define NL80211_RRF_NO_IR_ALL		(NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 558b0e3..efd6d0d 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
 	return get_cfg80211_regdom();
 }
 
-unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
-				   const struct ieee80211_reg_rule *rule)
+static unsigned int
+reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd,
+				 const struct ieee80211_reg_rule *rule)
 {
 	const struct ieee80211_freq_range *freq_range = &rule->freq_range;
 	const struct ieee80211_freq_range *freq_range_tmp;
@@ -621,6 +622,21 @@ unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
 	return end_freq - start_freq;
 }
 
+unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
+				   const struct ieee80211_reg_rule *rule)
+{
+	unsigned int bw = reg_get_max_bandwidth_from_range(rd, rule);
+
+	if (rule->flags & NL80211_RRF_NO_160MHZ)
+		bw = min_t(unsigned int, bw, MHZ_TO_KHZ(80));
+	if (rule->flags & NL80211_RRF_NO_80MHZ)
+		bw = min_t(unsigned int, bw, MHZ_TO_KHZ(40));
+	if (rule->flags & NL80211_RRF_NO_HT40)
+		bw = min_t(unsigned int, bw, MHZ_TO_KHZ(20));
+
+	return bw;
+}
+
 /* Sanity check on a regulatory rule */
 static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
 {
@@ -906,6 +922,16 @@ static u32 map_regdom_flags(u32 rd_flags)
 		channel_flags |= IEEE80211_CHAN_NO_OFDM;
 	if (rd_flags & NL80211_RRF_NO_OUTDOOR)
 		channel_flags |= IEEE80211_CHAN_INDOOR_ONLY;
+	if (rd_flags & NL80211_RRF_GO_CONCURRENT)
+		channel_flags |= IEEE80211_CHAN_GO_CONCURRENT;
+	if (rd_flags & NL80211_RRF_NO_HT40MINUS)
+		channel_flags |= IEEE80211_CHAN_NO_HT40MINUS;
+	if (rd_flags & NL80211_RRF_NO_HT40PLUS)
+		channel_flags |= IEEE80211_CHAN_NO_HT40PLUS;
+	if (rd_flags & NL80211_RRF_NO_80MHZ)
+		channel_flags |= IEEE80211_CHAN_NO_80MHZ;
+	if (rd_flags & NL80211_RRF_NO_160MHZ)
+		channel_flags |= IEEE80211_CHAN_NO_160MHZ;
 	return channel_flags;
 }
 
-- 
1.9.1

[PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arik Nemtsov]

Define a new wiphy callback allowing drivers to provide regulatory
settings.

Only The first wiphy registered with this callback will be able to provide
regulatory domain info. If such a wiphy exists, it takes precedence over
other data sources.

Change-Id: If8f8faf1d127120ae464b45098c5edbc5aee3dc0
Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
Reviewed-on: https://gerrit.rds.intel.com/32858
Tested-by: IWL Jenkins
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h | 16 +++++++++++++
 net/wireless/reg.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5c17b1f..b8f0269 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2950,6 +2950,19 @@ struct wiphy_vendor_command {
  *	low rssi when a frame is heard on different channel, then it should set
  *	this variable to the maximal offset for which it can compensate.
  *	This value should be set in MHz.
+ * @get_regd: a driver callback to for publishing regulatory information.
+ *	By implementing this callback, a wiphy indicates it will provide
+ *	regulatory information. The callback must be set before the wiphy is
+ *	is registered. Only the first wiphy with this callback will be called
+ *	to provide a regdomain on country-code changes.
+ *	Returns A driver allocated regdomain structure. The alpha2 in the
+ *	returned regdomain can be different from the one given via the alpha2
+ *	argument, if the argument contains "99", meaning unknown.
+ *	If an ERR_PTR is returned the regulatory core will consult other
+ *	sources for the regdomain info (internal regdb and CRDA). Returning
+ *	NULL will cause the regdomain to remain the same. The callee will
+ *	return a struct allocated with kmalloc(). After the struct is returned,
+ *	the regulatory core is responsible for freeing it.
  */
 struct wiphy {
 	/* assign these fields before you register the wiphy */
@@ -3033,6 +3046,9 @@ struct wiphy {
 	void (*reg_notifier)(struct wiphy *wiphy,
 			     struct regulatory_request *request);
 
+	struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
+						 const char *alpha2);
+
 	/* fields below are read-only, assigned by cfg80211 */
 
 	const struct ieee80211_regdomain __rcu *regd;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index efd6d0d..e2f33d7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -78,6 +78,8 @@
  *	further processing is required, i.e., not need to update last_request
  *	etc. This should be used for user hints that do not provide an alpha2
  *	but some other type of regulatory hint, i.e., indoor operation.
+ * @REG_REQ_HANDLED: a request was handled synchronously. No need to set
+ *	timeouts and potentially revert to the default settings.
  */
 enum reg_request_treatment {
 	REG_REQ_OK,
@@ -85,6 +87,7 @@ enum reg_request_treatment {
 	REG_REQ_INTERSECT,
 	REG_REQ_ALREADY_SET,
 	REG_REQ_USER_HINT_HANDLED,
+	REG_REQ_HANDLED,
 };
 
 static struct regulatory_request core_request_world = {
@@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
  */
 static bool reg_is_indoor;
 
+/*
+ * Wiphy with a get_regd() callback that can provide regulatory information
+ * when the country code changes. Only the first wiphy registered with the
+ * get_regd callback will be called to provide a regdomain on country-code
+ * changes.
+ * (protected by RTNL)
+ */
+static struct wiphy *regd_info_wiphy;
+
 static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
 {
 	return rtnl_dereference(cfg80211_regdomain);
@@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
 	return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
 }
 
+static int call_wiphy_regd_info(const char *alpha2)
+{
+	struct ieee80211_regdomain *regd;
+
+	if (!regd_info_wiphy)
+		return -ENOENT;
+
+	/* can happen if the driver removes the callback at runtime */
+	if (WARN_ON(!regd_info_wiphy->get_regd))
+		return -EINVAL;
+
+	regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
+	if (IS_ERR(regd))
+		return -EIO;
+
+	if (regd)
+		set_regdom(regd);
+
+	return 0;
+}
+
 static enum reg_request_treatment
-reg_call_crda(struct regulatory_request *request)
+reg_get_regdom_data(struct regulatory_request *request)
 {
+	ASSERT_RTNL();
+
+	/*
+	 * A wiphy wishing to set the regdomain takes precedence. Note the
+	 * regdomain setting happens synchronously inside.
+	 */
+	if (!call_wiphy_regd_info(request->alpha2))
+		return REG_REQ_HANDLED;
+
 	if (call_crda(request->alpha2))
 		return REG_REQ_IGNORE;
 	return REG_REQ_OK;
@@ -1641,7 +1683,7 @@ reg_process_hint_core(struct regulatory_request *core_request)
 
 	reg_update_last_request(core_request);
 
-	return reg_call_crda(core_request);
+	return reg_get_regdom_data(core_request);
 }
 
 static enum reg_request_treatment
@@ -1715,7 +1757,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
 	user_alpha2[0] = user_request->alpha2[0];
 	user_alpha2[1] = user_request->alpha2[1];
 
-	return reg_call_crda(user_request);
+	return reg_get_regdom_data(user_request);
 }
 
 static enum reg_request_treatment
@@ -1764,6 +1806,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
 		break;
 	case REG_REQ_IGNORE:
 	case REG_REQ_USER_HINT_HANDLED:
+	case REG_REQ_HANDLED:
 		reg_free_request(driver_request);
 		return treatment;
 	case REG_REQ_INTERSECT:
@@ -1794,7 +1837,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
 		return treatment;
 	}
 
-	return reg_call_crda(driver_request);
+	return reg_get_regdom_data(driver_request);
 }
 
 static enum reg_request_treatment
@@ -1864,6 +1907,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 		break;
 	case REG_REQ_IGNORE:
 	case REG_REQ_USER_HINT_HANDLED:
+	case REG_REQ_HANDLED:
 		/* fall through */
 	case REG_REQ_ALREADY_SET:
 		reg_free_request(country_ie_request);
@@ -1883,7 +1927,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 
 	reg_update_last_request(country_ie_request);
 
-	return reg_call_crda(country_ie_request);
+	return reg_get_regdom_data(country_ie_request);
 }
 
 /* This processes *all* regulatory hints */
@@ -1903,7 +1947,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 		treatment = reg_process_hint_user(reg_request);
 		if (treatment == REG_REQ_IGNORE ||
 		    treatment == REG_REQ_ALREADY_SET ||
-		    treatment == REG_REQ_USER_HINT_HANDLED)
+		    treatment == REG_REQ_USER_HINT_HANDLED ||
+		    treatment == REG_REQ_HANDLED)
 			return;
 		queue_delayed_work(system_power_efficient_wq,
 				   &reg_timeout, msecs_to_jiffies(3142));
@@ -2684,6 +2729,9 @@ void wiphy_regulatory_register(struct wiphy *wiphy)
 {
 	struct regulatory_request *lr;
 
+	if (wiphy->get_regd && !regd_info_wiphy)
+		regd_info_wiphy = wiphy;
+
 	if (!reg_dev_ignore_cell_hint(wiphy))
 		reg_num_devs_support_basehint++;
 
@@ -2696,6 +2744,8 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
 	struct wiphy *request_wiphy = NULL;
 	struct regulatory_request *lr;
 
+	ASSERT_RTNL();
+
 	lr = get_last_request();
 
 	if (!reg_dev_ignore_cell_hint(wiphy))
@@ -2704,6 +2754,9 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
 	rcu_free_regdom(get_wiphy_regdom(wiphy));
 	RCU_INIT_POINTER(wiphy->regd, NULL);
 
+	if (wiphy == regd_info_wiphy)
+		regd_info_wiphy = NULL;
+
 	if (lr)
 		request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
 
-- 
1.9.1

[PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Arik Nemtsov]

If the regulatory request contains the special "99" unknown-country code,
allow a different alpha2 as response.

This special alpha2 is used when the real alpha2 is unknown and should
be provided by the driver via its get_regd() wiphy callback, as part of
the regdomain info.

Change-Id: I286e3aed828cabf22292b6fe320fdcfa61f0b951
Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
Reviewed-on: https://gerrit.rds.intel.com/32266
Tested-by: IWL Jenkins
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e2f33d7..c429ec5 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -418,7 +418,8 @@ static bool is_user_regdom_saved(void)
 		return false;
 
 	/* This would indicate a mistake on the design */
-	if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
+	if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2) &&
+		 !is_unknown_alpha2(user_alpha2),
 		 "Unexpected user alpha2: %c%c\n",
 		 user_alpha2[0], user_alpha2[1]))
 		return false;
@@ -595,7 +596,8 @@ bool reg_is_valid_request(const char *alpha2)
 	if (!lr || lr->processed)
 		return false;
 
-	return alpha2_equal(lr->alpha2, alpha2);
+	return alpha2_equal(lr->alpha2, alpha2) ||
+	       is_unknown_alpha2(lr->alpha2);
 }
 
 static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
-- 
1.9.1

[PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

Allow driver and user hints to set the "world" regulatory domain, and
also allow them to set the same reg-domain. This is useful when sending
an unknown-country ("99") request to a wiphy-specific regulatory source,
requesting it to provide the current alpha2. It may return an updated
regdomain for the "world" alpha2.

Note there's a check for regdom change at the request level, so the case
where the same alpha2 is set repeatedly is still optimized.

Change-Id: Iced65fd43bd10e40f1f446a9aff5d5ea380eecaa
Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
Reviewed-on: https://gerrit.rds.intel.com/32267
Tested-by: IWL Jenkins
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index c429ec5..d6b83f9 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2539,9 +2539,6 @@ static int reg_set_rd_user(const struct ieee80211_regdomain *rd,
 {
 	const struct ieee80211_regdomain *intersected_rd = NULL;
 
-	if (!regdom_changes(rd->alpha2))
-		return -EALREADY;
-
 	if (!is_valid_rd(rd)) {
 		pr_err("Invalid regulatory domain detected:\n");
 		print_regdomain_info(rd);
@@ -2572,12 +2569,6 @@ static int reg_set_rd_driver(const struct ieee80211_regdomain *rd,
 	const struct ieee80211_regdomain *tmp;
 	struct wiphy *request_wiphy;
 
-	if (is_world_regdom(rd->alpha2))
-		return -EINVAL;
-
-	if (!regdom_changes(rd->alpha2))
-		return -EALREADY;
-
 	if (!is_valid_rd(rd)) {
 		pr_err("Invalid regulatory domain detected:\n");
 		print_regdomain_info(rd);
-- 
1.9.1

[PATCH 5/5] cfg80211: leave invalid channels on regdomain change

[Arik Nemtsov]

When the regulatory settings change, some channels might become invalid.
Disconnect interfaces acting on these channels, after giving userspace
code a grace period to leave them.

Because of cfg80211 channel tracking limitations, only disconnect
STA/P2P_CL interfaces if their current center channel became disabled.

Change-Id: I308cc02c06a11cfc30b206efaeabe64d67186228
Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
Reviewed-on: https://gerrit.rds.intel.com/32422
Tested-by: IWL Jenkins
Reviewed-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index d6b83f9..1d6afe6 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -221,6 +221,9 @@ struct reg_beacon {
 	struct ieee80211_channel chan;
 };
 
+static void reg_check_chans_work(struct work_struct *work);
+static DECLARE_DELAYED_WORK(reg_check_chans, reg_check_chans_work);
+
 static void reg_todo(struct work_struct *work);
 static DECLARE_WORK(reg_work, reg_todo);
 
@@ -1520,6 +1523,80 @@ static void reg_call_notifier(struct wiphy *wiphy,
 		wiphy->reg_notifier(wiphy, request);
 }
 
+static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
+{
+	struct ieee80211_channel *ch;
+	bool ret = true;
+
+	wdev_lock(wdev);
+
+	if (!wdev->netdev || !netif_running(wdev->netdev))
+		goto out;
+
+	switch (wdev->iftype) {
+	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_P2P_GO:
+		if (!wdev->beacon_interval)
+			goto out;
+
+		ret = cfg80211_reg_can_beacon(wiphy,
+					      &wdev->chandef, wdev->iftype);
+		break;
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_P2P_CLIENT:
+		if (!wdev->current_bss ||
+		    !wdev->current_bss->pub.channel)
+			goto out;
+
+		/* TODO: more elaborate tracking for wide channels */
+		ch = wdev->current_bss->pub.channel;
+		ret = !(ch->flags & IEEE80211_CHAN_DISABLED);
+		break;
+	default:
+		/* others not implemented for now */
+		break;
+	}
+
+out:
+	wdev_unlock(wdev);
+	return ret;
+}
+
+static void reg_leave_invalid_chans(struct wiphy *wiphy)
+{
+	struct wireless_dev *wdev;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+	ASSERT_RTNL();
+
+	list_for_each_entry(wdev, &rdev->wdev_list, list)
+		if (!reg_wdev_chan_valid(wiphy, wdev))
+			cfg80211_leave(rdev, wdev);
+}
+
+static void reg_check_chans_work(struct work_struct *work)
+{
+	struct cfg80211_registered_device *rdev;
+
+	REG_DBG_PRINT("Verifying active interfaces after reg change\n");
+	rtnl_lock();
+
+	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
+		reg_leave_invalid_chans(&rdev->wiphy);
+
+	rtnl_unlock();
+}
+
+static void reg_check_channels(void)
+{
+	/*
+	 * Give usermode a chance to do something nicer (move to another
+	 * channel, orderly disconnection), before forcing a disconnection.
+	 */
+	mod_delayed_work(system_power_efficient_wq,
+			 &reg_check_chans, msecs_to_jiffies(60000));
+}
+
 static void wiphy_update_regulatory(struct wiphy *wiphy,
 				    enum nl80211_reg_initiator initiator)
 {
@@ -1559,6 +1636,8 @@ static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
 		wiphy = &rdev->wiphy;
 		wiphy_update_regulatory(wiphy, initiator);
 	}
+
+	reg_check_channels();
 }
 
 static void handle_channel_custom(struct wiphy *wiphy,
@@ -1972,8 +2051,10 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 
 	/* This is required so that the orig_* parameters are saved */
 	if (treatment == REG_REQ_ALREADY_SET && wiphy &&
-	    wiphy->regulatory_flags & REGULATORY_STRICT_REG)
+	    wiphy->regulatory_flags & REGULATORY_STRICT_REG) {
 		wiphy_update_regulatory(wiphy, reg_request->initiator);
+		reg_check_channels();
+	}
 
 	return;
 
@@ -2853,6 +2934,7 @@ void regulatory_exit(void)
 
 	cancel_work_sync(&reg_work);
 	cancel_delayed_work_sync(&reg_timeout);
+	cancel_delayed_work_sync(&reg_check_chans);
 
 	/* Lock to suppress warnings */
 	rtnl_lock();
-- 
1.9.1

Re: [PATCH 1/5] cfg80211: introduce regulatory flags controlling bw

[Arik Nemtsov]

On Wed, Jun 11, 2014 at 9:55 AM, Arik Nemtsov <arik@wizery.com> wrote:
> Allow setting bandwidth related regulatory flags. These flags are mapped
> to the corresponding channel flags in the specified range.
> Make sure the new flags are consulted when calculating the maximum
> bandwidth allowed by a regulatory-rule.
>
> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
> channel.
>
> Change-Id: Ic120a291ef7a56f2efa800e77e9689c6c421affa

Sorry about this..

Arik

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arend van Spriel]

On 11-06-14 08:55, Arik Nemtsov wrote:
> Define a new wiphy callback allowing drivers to provide regulatory
> settings.
>
> Only The first wiphy registered with this callback will be able to provide
> regulatory domain info. If such a wiphy exists, it takes precedence over
> other data sources.

I should probably dig through linux-wireless archive for background 
info, but how is this different from the wiphy_apply_custom_regulatory() 
call. Is this for devices that have a regulatory database of sorts in 
the device firmware?

Regards,
Arend

> Change-Id: If8f8faf1d127120ae464b45098c5edbc5aee3dc0
> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> Reviewed-on: https://gerrit.rds.intel.com/32858
> Tested-by: IWL Jenkins
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
> ---
>   include/net/cfg80211.h | 16 +++++++++++++
>   net/wireless/reg.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
>   2 files changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 5c17b1f..b8f0269 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -2950,6 +2950,19 @@ struct wiphy_vendor_command {
>    *	low rssi when a frame is heard on different channel, then it should set
>    *	this variable to the maximal offset for which it can compensate.
>    *	This value should be set in MHz.
> + * @get_regd: a driver callback to for publishing regulatory information.
> + *	By implementing this callback, a wiphy indicates it will provide
> + *	regulatory information. The callback must be set before the wiphy is
> + *	is registered. Only the first wiphy with this callback will be called
> + *	to provide a regdomain on country-code changes.
> + *	Returns A driver allocated regdomain structure. The alpha2 in the
> + *	returned regdomain can be different from the one given via the alpha2
> + *	argument, if the argument contains "99", meaning unknown.
> + *	If an ERR_PTR is returned the regulatory core will consult other
> + *	sources for the regdomain info (internal regdb and CRDA). Returning
> + *	NULL will cause the regdomain to remain the same. The callee will
> + *	return a struct allocated with kmalloc(). After the struct is returned,
> + *	the regulatory core is responsible for freeing it.
>    */
>   struct wiphy {
>   	/* assign these fields before you register the wiphy */
> @@ -3033,6 +3046,9 @@ struct wiphy {
>   	void (*reg_notifier)(struct wiphy *wiphy,
>   			     struct regulatory_request *request);
>
> +	struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
> +						 const char *alpha2);
> +
>   	/* fields below are read-only, assigned by cfg80211 */
>
>   	const struct ieee80211_regdomain __rcu *regd;
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index efd6d0d..e2f33d7 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -78,6 +78,8 @@
>    *	further processing is required, i.e., not need to update last_request
>    *	etc. This should be used for user hints that do not provide an alpha2
>    *	but some other type of regulatory hint, i.e., indoor operation.
> + * @REG_REQ_HANDLED: a request was handled synchronously. No need to set
> + *	timeouts and potentially revert to the default settings.
>    */
>   enum reg_request_treatment {
>   	REG_REQ_OK,
> @@ -85,6 +87,7 @@ enum reg_request_treatment {
>   	REG_REQ_INTERSECT,
>   	REG_REQ_ALREADY_SET,
>   	REG_REQ_USER_HINT_HANDLED,
> +	REG_REQ_HANDLED,
>   };
>
>   static struct regulatory_request core_request_world = {
> @@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
>    */
>   static bool reg_is_indoor;
>
> +/*
> + * Wiphy with a get_regd() callback that can provide regulatory information
> + * when the country code changes. Only the first wiphy registered with the
> + * get_regd callback will be called to provide a regdomain on country-code
> + * changes.
> + * (protected by RTNL)
> + */
> +static struct wiphy *regd_info_wiphy;
> +
>   static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
>   {
>   	return rtnl_dereference(cfg80211_regdomain);
> @@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
>   	return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
>   }
>
> +static int call_wiphy_regd_info(const char *alpha2)
> +{
> +	struct ieee80211_regdomain *regd;
> +
> +	if (!regd_info_wiphy)
> +		return -ENOENT;
> +
> +	/* can happen if the driver removes the callback at runtime */
> +	if (WARN_ON(!regd_info_wiphy->get_regd))
> +		return -EINVAL;
> +
> +	regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
> +	if (IS_ERR(regd))
> +		return -EIO;
> +
> +	if (regd)
> +		set_regdom(regd);
> +
> +	return 0;
> +}
> +
>   static enum reg_request_treatment
> -reg_call_crda(struct regulatory_request *request)
> +reg_get_regdom_data(struct regulatory_request *request)
>   {
> +	ASSERT_RTNL();
> +
> +	/*
> +	 * A wiphy wishing to set the regdomain takes precedence. Note the
> +	 * regdomain setting happens synchronously inside.
> +	 */
> +	if (!call_wiphy_regd_info(request->alpha2))
> +		return REG_REQ_HANDLED;
> +
>   	if (call_crda(request->alpha2))
>   		return REG_REQ_IGNORE;
>   	return REG_REQ_OK;
> @@ -1641,7 +1683,7 @@ reg_process_hint_core(struct regulatory_request *core_request)
>
>   	reg_update_last_request(core_request);
>
> -	return reg_call_crda(core_request);
> +	return reg_get_regdom_data(core_request);
>   }
>
>   static enum reg_request_treatment
> @@ -1715,7 +1757,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
>   	user_alpha2[0] = user_request->alpha2[0];
>   	user_alpha2[1] = user_request->alpha2[1];
>
> -	return reg_call_crda(user_request);
> +	return reg_get_regdom_data(user_request);
>   }
>
>   static enum reg_request_treatment
> @@ -1764,6 +1806,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
>   		break;
>   	case REG_REQ_IGNORE:
>   	case REG_REQ_USER_HINT_HANDLED:
> +	case REG_REQ_HANDLED:
>   		reg_free_request(driver_request);
>   		return treatment;
>   	case REG_REQ_INTERSECT:
> @@ -1794,7 +1837,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
>   		return treatment;
>   	}
>
> -	return reg_call_crda(driver_request);
> +	return reg_get_regdom_data(driver_request);
>   }
>
>   static enum reg_request_treatment
> @@ -1864,6 +1907,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
>   		break;
>   	case REG_REQ_IGNORE:
>   	case REG_REQ_USER_HINT_HANDLED:
> +	case REG_REQ_HANDLED:
>   		/* fall through */
>   	case REG_REQ_ALREADY_SET:
>   		reg_free_request(country_ie_request);
> @@ -1883,7 +1927,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
>
>   	reg_update_last_request(country_ie_request);
>
> -	return reg_call_crda(country_ie_request);
> +	return reg_get_regdom_data(country_ie_request);
>   }
>
>   /* This processes *all* regulatory hints */
> @@ -1903,7 +1947,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>   		treatment = reg_process_hint_user(reg_request);
>   		if (treatment == REG_REQ_IGNORE ||
>   		    treatment == REG_REQ_ALREADY_SET ||
> -		    treatment == REG_REQ_USER_HINT_HANDLED)
> +		    treatment == REG_REQ_USER_HINT_HANDLED ||
> +		    treatment == REG_REQ_HANDLED)
>   			return;
>   		queue_delayed_work(system_power_efficient_wq,
>   				   &reg_timeout, msecs_to_jiffies(3142));
> @@ -2684,6 +2729,9 @@ void wiphy_regulatory_register(struct wiphy *wiphy)
>   {
>   	struct regulatory_request *lr;
>
> +	if (wiphy->get_regd && !regd_info_wiphy)
> +		regd_info_wiphy = wiphy;
> +
>   	if (!reg_dev_ignore_cell_hint(wiphy))
>   		reg_num_devs_support_basehint++;
>
> @@ -2696,6 +2744,8 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
>   	struct wiphy *request_wiphy = NULL;
>   	struct regulatory_request *lr;
>
> +	ASSERT_RTNL();
> +
>   	lr = get_last_request();
>
>   	if (!reg_dev_ignore_cell_hint(wiphy))
> @@ -2704,6 +2754,9 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
>   	rcu_free_regdom(get_wiphy_regdom(wiphy));
>   	RCU_INIT_POINTER(wiphy->regd, NULL);
>
> +	if (wiphy == regd_info_wiphy)
> +		regd_info_wiphy = NULL;
> +
>   	if (lr)
>   		request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
>
>

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arik Nemtsov]

On Wed, Jun 11, 2014 at 10:33 AM, Arend van Spriel <arend@broadcom.com> wrote:
> On 11-06-14 08:55, Arik Nemtsov wrote:
>>
>> Define a new wiphy callback allowing drivers to provide regulatory
>> settings.
>>
>> Only The first wiphy registered with this callback will be able to provide
>> regulatory domain info. If such a wiphy exists, it takes precedence over
>> other data sources.
>
>
> I should probably dig through linux-wireless archive for background info,
> but how is this different from the wiphy_apply_custom_regulatory() call. Is
> this for devices that have a regulatory database of sorts in the device
> firmware?
>

Yea that's basically it. And we want the get the info dynamically from
FW upon country changes.

Arik

Re: [PATCH 1/5] cfg80211: introduce regulatory flags controlling bw

[Luis R. Rodriguez]

On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> Allow setting bandwidth related regulatory flags. These flags are mapped
> to the corresponding channel flags in the specified range.
> Make sure the new flags are consulted when calculating the maximum
> bandwidth allowed by a regulatory-rule.
>
> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
> channel.
>
> Change-Id: Ic120a291ef7a56f2efa800e77e9689c6c421affa
> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> Reviewed-on: https://gerrit.rds.intel.com/32263
> Tested-by: IWL Jenkins
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@suse.com>

  Luis

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Luis R. Rodriguez]

On Wed, Jun 11, 2014 at 1:45 AM, Arik Nemtsov <arik@wizery.com> wrote:
> On Wed, Jun 11, 2014 at 10:33 AM, Arend van Spriel <arend@broadcom.com> wrote:
>> On 11-06-14 08:55, Arik Nemtsov wrote:
>>>
>>> Define a new wiphy callback allowing drivers to provide regulatory
>>> settings.
>>>
>>> Only The first wiphy registered with this callback will be able to provide
>>> regulatory domain info. If such a wiphy exists, it takes precedence over
>>> other data sources.
>>
>>
>> I should probably dig through linux-wireless archive for background info,
>> but how is this different from the wiphy_apply_custom_regulatory() call. Is
>> this for devices that have a regulatory database of sorts in the device
>> firmware?
>>
> Yea that's basically it. And we want the get the info dynamically from
> FW upon country changes.

Currently a few drivers exist that have regulatory data in firmware
and what we end up doing is allowing that through the usage of the
custom flag and then having the reg_notififer() poke firmware and and
ensure its OK with all the changes. It however has no way to then
splat data from firmware onto the wiphy dynamically, and although this
could be done through the custom firmware thing that is only for
initialization and before registration. Having the API explicit should
also allow us to extract vendor's regulatory data and then do our own
serialized analysis of regulatory data to populate the public regdb.
Using the firmware regulatory data is an architecture that is plain
stupid and non-scalable so if people want to shoot themsevles on the
foot with that stupidity I think its fine to let it through so long as
we can extract their regulatory data, which this helps accomplish.

Let the best firmware design win.

  Luis

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Luis R. Rodriguez]

On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> If the regulatory request contains the special "99" unknown-country code,
> allow a different alpha2 as response.
>
> This special alpha2 is used when the real alpha2 is unknown and should
> be provided by the driver via its get_regd() wiphy callback, as part of
> the regdomain info.
>
> Change-Id: I286e3aed828cabf22292b6fe320fdcfa61f0b951
> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> Reviewed-on: https://gerrit.rds.intel.com/32266
> Tested-by: IWL Jenkins
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/wireless/reg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index e2f33d7..c429ec5 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -418,7 +418,8 @@ static bool is_user_regdom_saved(void)
>                 return false;
>
>         /* This would indicate a mistake on the design */
> -       if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
> +       if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2) &&
> +                !is_unknown_alpha2(user_alpha2),
>                  "Unexpected user alpha2: %c%c\n",
>                  user_alpha2[0], user_alpha2[1]))
>                 return false;

When would the user alpha2 be a custom value? This is *very* different
than allowing an API to set the regulatory domain to a specific alpha2
value, which is what I though this was all about.

> @@ -595,7 +596,8 @@ bool reg_is_valid_request(const char *alpha2)
>         if (!lr || lr->processed)
>                 return false;
>
> -       return alpha2_equal(lr->alpha2, alpha2);
> +       return alpha2_equal(lr->alpha2, alpha2) ||
> +              is_unknown_alpha2(lr->alpha2);
>  }

This patch seems to be allowing custom user alpha2s to be passed,
while the commit log is indicating we want to allow a change from a
custom value that is set to a regular alpha2.

  Luis

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Luis R. Rodriguez]

On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> Define a new wiphy callback allowing drivers to provide regulatory
> settings.
>
> Only The first wiphy registered with this callback will be able to provide
> regulatory domain info. If such a wiphy exists, it takes precedence over
> other data sources.
>
> Change-Id: If8f8faf1d127120ae464b45098c5edbc5aee3dc0
> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> Reviewed-on: https://gerrit.rds.intel.com/32858
> Tested-by: IWL Jenkins
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  include/net/cfg80211.h | 16 +++++++++++++
>  net/wireless/reg.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 5c17b1f..b8f0269 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -2950,6 +2950,19 @@ struct wiphy_vendor_command {
>   *     low rssi when a frame is heard on different channel, then it should set
>   *     this variable to the maximal offset for which it can compensate.
>   *     This value should be set in MHz.
> + * @get_regd: a driver callback to for publishing regulatory information.
> + *     By implementing this callback, a wiphy indicates it will provide
> + *     regulatory information. The callback must be set before the wiphy is
> + *     is registered. Only the first wiphy with this callback will be called
> + *     to provide a regdomain on country-code changes.

The way this is implemented is a bit more exclusive than this kdoc
explains -- the way its implemented below allows only a single wiphy
*ever* to be registered with this capability. This means that external
attachments such as USB 802.11 devices that have this API requirement
cannot ensure that they will get this callback issued. What do we want
to do about that?

Why are we not just considering multiple cards with these capabilities
as equal and simply deal with the intersections as with the other
cases? The restriction seems to limit our capabilities and likely will
require bug reporting / issues to be fleshed out later. I rather
address this now.

> + *     Returns A driver allocated regdomain structure.

"a" instead of capital A.

> +                                                                           The alpha2 in the
> + *     returned regdomain can be different from the one given via the alpha2
> + *     argument, if the argument contains "99", meaning unknown.

Lets be upfront -- how many regulatory domains will you guys return
right now in which the same alpha2 will be kept? I don't like this for
one bit at all, if you are going to return a 99 for a specific alpha2
it must mean firmware / driver has the ability to search for an alpha2
for a set of custom regulatory domains, which should also mean you
should be able to return a set of alpha2s that can be used for the
custom regulatory domain, so I'd like to see that added as a possible
return set. This should set expectations for users, build a better
understanding of how all this works, and most importantly enable easy
and shorter scraping of the firmware for our own research and
development and advancement of wireless-regdb as otherwise we'd have
to loop over every alpha2 and then deduce grouping. This is the only
way I see this being reasonable to accept upstream.

We also should be clear now in the documentation about the differences
and purpose behind this API and the custom regulatory which tons of
folks already use. In this case the requirement was dynamic changes
and to ensure these changes get back to userspace permissively as
otherwise they could not. The way I see it the custom stuff should be
used for custom world regulatory domains -- this new API is for
specific alpha2s. This should also mean then that this API should
*not* be used to query the firmware for the world regulatory domain.
It also means that the custom flag has to be looked at carefully as
well now since the custom flag may need to be lifted dynamically. You
may want to look at the Atheros ath module for the different use cases
as they ship tons of different types of devices: world roaming, and
also cards with a specific alpha2 set. There's a bit of boiler place
code there that I saw tons of drivers share / copy so perhaps some
boiler plate code might be in order to help here.

This new API makes only sense for dynamic changes and as such I do
expect this to be used for direct alpha2 mappings, not some region
stuff. Grouping regulatory domains to groups makes sense to save size
-- we've seen this before by other vendors but we should be easily
able to get the group alpha2 mapping as well. If Intel is *not* going
to contribute to wireless-regdb this at the very least should help us
move forward with the public regdb by providing back mapping of all
the alpha2s that a regulatory domain passed also applies to.

> + *     If an ERR_PTR is returned the regulatory core will consult other
> + *     sources for the regdomain info (internal regdb and CRDA).

That's nice.

> +                                                                                           Returning
> + *     NULL will cause the regdomain to remain the same.


What do you mean by this? Can you please elaborate exactly what this means?

> +                                                                                    The callee will
> + *     return a struct allocated with kmalloc(). After the struct is returned
> + *     the regulatory core is responsible for freeing it.
>   */
>  struct wiphy {
>         /* assign these fields before you register the wiphy */
> @@ -3033,6 +3046,9 @@ struct wiphy {
>         void (*reg_notifier)(struct wiphy *wiphy,
>                              struct regulatory_request *request);
>
> +       struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
> +                                                const char *alpha2);
> +
>         /* fields below are read-only, assigned by cfg80211 */
>
>         const struct ieee80211_regdomain __rcu *regd;
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index efd6d0d..e2f33d7 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -78,6 +78,8 @@
>   *     further processing is required, i.e., not need to update last_request
>   *     etc. This should be used for user hints that do not provide an alpha2
>   *     but some other type of regulatory hint, i.e., indoor operation.
> + * @REG_REQ_HANDLED: a request was handled synchronously. No need to set
> + *     timeouts and potentially revert to the default settings.
>   */
>  enum reg_request_treatment {
>         REG_REQ_OK,
> @@ -85,6 +87,7 @@ enum reg_request_treatment {
>         REG_REQ_INTERSECT,
>         REG_REQ_ALREADY_SET,
>         REG_REQ_USER_HINT_HANDLED,
> +       REG_REQ_HANDLED,
>  };
>
>  static struct regulatory_request core_request_world = {
> @@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
>   */
>  static bool reg_is_indoor;
>
> +/*
> + * Wiphy with a get_regd() callback that can provide regulatory information
> + * when the country code changes. Only the first wiphy registered with the
> + * get_regd callback will be called to provide a regdomain on country-code
> + * changes.
> + * (protected by RTNL)
> + */
> +static struct wiphy *regd_info_wiphy;

Note all my feedback on the kdoc comment about this. This obviously
doesn't scale well but more importantly given that devices bus
configuration can change (regardless of what systemd folks think) it
means that we can get different results on a system with 2 internal
cards. Systems with multiple built in 802.11 cards were something of a
theoretical myth back in the day when we started with 802.11 but not
anymore. For example I'm very aware of some Freebox 802.11 APs with
multiple built in 802.11 cards and from different vendors!  No only
can this lead to issues with cards on different buses and races
therein, but it could also produce different results on different
architectures.

> +
>  static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
>  {
>         return rtnl_dereference(cfg80211_regdomain);
> @@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
>         return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
>  }
>
> +static int call_wiphy_regd_info(const char *alpha2)
> +{
> +       struct ieee80211_regdomain *regd;
> +
> +       if (!regd_info_wiphy)
> +               return -ENOENT;
> +
> +       /* can happen if the driver removes the callback at runtime */
> +       if (WARN_ON(!regd_info_wiphy->get_regd))
> +               return -EINVAL;

I'd rather see a capability flag for this, that way we can also tell
userspace of this ridiculous incarnation of crap on firmware, and when
set we'd expect this to be set. If we want to address the dynamic
removal / addition of the callback that can be dealt with when the
time comes, but that doesn't seem to be required givenou'refe that you
added API to let the firmware let cfg80211 pass through and use the
wireless-regdb data.

> +
> +       regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
> +       if (IS_ERR(regd))
> +               return -EIO;
> +

You're not feeding the firmware information on availability of the
userspace regulatory domain, in the case of it was allowed, it seems
to me that firmware might make a better decision in the case that
userspace lacks information. Userspace can also be upgraded at runtime
dynamically so whether or not userspace has a regulatory domain can
change at run time. Please consider this.

> +       if (regd)
> +               set_regdom(regd);
> +
> +       return 0;
> +}
> +
>  static enum reg_request_treatment
> -reg_call_crda(struct regulatory_request *request)
> +vreg_get_regdom_data(struct regulatory_request *request)
>  {
> +       ASSERT_RTNL();
> +
> +       /*
> +        * A wiphy wishing to set the regdomain takes precedence. Note the
> +        * regdomain setting happens synchronously inside.
> +        */
> +       if (!call_wiphy_regd_info(request->alpha2))
> +               return REG_REQ_HANDLED;
> +
>         if (call_crda(request->alpha2))
>                 return REG_REQ_IGNORE;
>         return REG_REQ_OK;
> @@ -1641,7 +1683,7 @@ reg_process_hint_core(struct regulatory_request *core_request)
>
>         reg_update_last_request(core_request);
>
> -       return reg_call_crda(core_request);
> +       return reg_get_regdom_data(core_request);
>  }
>
>  static enum reg_request_treatment
> @@ -1715,7 +1757,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
>         user_alpha2[0] = user_request->alpha2[0];
>         user_alpha2[1] = user_request->alpha2[1];
>
> -       return reg_call_crda(user_request);
> +       return reg_get_regdom_data(user_request);
>  }
>
>  static enum reg_request_treatment
> @@ -1764,6 +1806,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
>                 break;
>         case REG_REQ_IGNORE:
>         case REG_REQ_USER_HINT_HANDLED:
> +       case REG_REQ_HANDLED:
>                 reg_free_request(driver_request);
>                 return treatment;
>         case REG_REQ_INTERSECT:
> @@ -1794,7 +1837,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
>                 return treatment;
>         }
>
> -       return reg_call_crda(driver_request);
> +       return reg_get_regdom_data(driver_request);
>  }
>
>  static enum reg_request_treatment
> @@ -1864,6 +1907,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
>                 break;
>         case REG_REQ_IGNORE:
>         case REG_REQ_USER_HINT_HANDLED:
> +       case REG_REQ_HANDLED:
>                 /* fall through */
>         case REG_REQ_ALREADY_SET:
>                 reg_free_request(country_ie_request);
> @@ -1883,7 +1927,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
>
>         reg_update_last_request(country_ie_request);
>
> -       return reg_call_crda(country_ie_request);
> +       return reg_get_regdom_data(country_ie_request);
>  }
>
>  /* This processes *all* regulatory hints */
> @@ -1903,7 +1947,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>                 treatment = reg_process_hint_user(reg_request);
>                 if (treatment == REG_REQ_IGNORE ||
>                     treatment == REG_REQ_ALREADY_SET ||
> -                   treatment == REG_REQ_USER_HINT_HANDLED)
> +                   treatment == REG_REQ_USER_HINT_HANDLED ||
> +                   treatment == REG_REQ_HANDLED)
>                         return;
>                 queue_delayed_work(system_power_efficient_wq,
>                                    &reg_timeout, msecs_to_jiffies(3142));
> @@ -2684,6 +2729,9 @@ void wiphy_regulatory_register(struct wiphy *wiphy)
>  {
>         struct regulatory_request *lr;
>
> +       if (wiphy->get_regd && !regd_info_wiphy)
> +               regd_info_wiphy = wiphy;
> +

This is rather limiting in architecture.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> Allow driver and user hints to set the "world" regulatory domain,

NACK - as expressed in the other patch, letting the drivers to use the
new API to set the world regulatory domain doesn't make sense as we
have custom apply stuff, and the world regulatory domain is not
something dynamic.

  Luis

Re: [PATCH 5/5] cfg80211: leave invalid channels on regdomain change

[Luis R. Rodriguez]

On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> When the regulatory settings change, some channels might become invalid.
> Disconnect interfaces acting on these channels, after giving userspace
> code a grace period to leave them.
>
> Because of cfg80211 channel tracking limitations, only disconnect
> STA/P2P_CL interfaces if their current center channel became disabled.
>
> Change-Id: I308cc02c06a11cfc30b206efaeabe64d67186228
> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> Reviewed-on: https://gerrit.rds.intel.com/32422
> Tested-by: IWL Jenkins
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>

I welcome this change, and thanks for the work! But I think we need to
think about the cases where this really would be required. This should
not be an issue drivers using the public wireless-regd completely
given that the default world regulatory domain is a mathematical
intersection, and as such its restrictive by nature and only
world-allowed channels with the the minimum max EIRP would be allowed.
For drivers using an initial custom world regdomain with custom apply
API a change in regulatory would be permissive and as such we can lift
some restrictions when we associate to an AP with a country IE
specified. For the dynamic firmware juju that Intel wants to introduce
this race is intrinsically unavoidable although the likelihood of it
remains unclear.

It should then be possible for some drivers to not have to use this
sanity check. For example drivers that use ath properly (ath5k, ath9k,
ath10k) could likely skip this stuff, unless I'm missing something.
For drivers without regulatory data, that should also be the case as
the public regdb would be used. It'd be interesting to see if that'
not the case though so an informative print for now would be useful
for the case where bailing out does happen, in fact perhaps it makes
sense to issue a multicast nl80211 event for this so that userspace
knows something has happened and why. Since we already have a reg
change multicast event this seems then to provide a "safe" proactive
measure for the loose cases, as perhaps an API can be added to let
userspace acknowledge a regulatory change event -- that way upon
receipt we should be able to cancel this work. Thoughts?v

> ---
>  net/wireless/reg.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index d6b83f9..1d6afe6 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -221,6 +221,9 @@ struct reg_beacon {
>         struct ieee80211_channel chan;
>  };
>
> +static void reg_check_chans_work(struct work_struct *work);
> +static DECLARE_DELAYED_WORK(reg_check_chans, reg_check_chans_work);
> +
>  static void reg_todo(struct work_struct *work);
>  static DECLARE_WORK(reg_work, reg_todo);
>
> @@ -1520,6 +1523,80 @@ static void reg_call_notifier(struct wiphy *wiphy,
>                 wiphy->reg_notifier(wiphy, request);
>  }
>
> +static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
> +{
> +       struct ieee80211_channel *ch;
> +       bool ret = true;
> +
> +       wdev_lock(wdev);
> +
> +       if (!wdev->netdev || !netif_running(wdev->netdev))
> +               goto out;
> +
> +       switch (wdev->iftype) {
> +       case NL80211_IFTYPE_AP:
> +       case NL80211_IFTYPE_P2P_GO:
> +               if (!wdev->beacon_interval)
> +                       goto out;
> +
> +               ret = cfg80211_reg_can_beacon(wiphy,
> +                                             &wdev->chandef, wdev->iftype);
> +               break;
> +       case NL80211_IFTYPE_STATION:
> +       case NL80211_IFTYPE_P2P_CLIENT:
> +               if (!wdev->current_bss ||
> +                   !wdev->current_bss->pub.channel)
> +                       goto out;
> +
> +               /* TODO: more elaborate tracking for wide channels */
> +               ch = wdev->current_bss->pub.channel;
> +               ret = !(ch->flags & IEEE80211_CHAN_DISABLED);

That does not seem enough, the max EIRP could have changed too for
example -- but the reg_notifier() should be used for ensuring changes
to power requirements are respected, at least the Atheros shared ath
module does this, that after all is one of the reasons for the
reg_notifier(). Documenting as part of this change would be
appreciated.

> +               break;
> +       default:
> +               /* others not implemented for now */
> +               break;
> +       }
> +
> +out:
> +       wdev_unlock(wdev);
> +       return ret;
> +}
> +
> +static void reg_leave_invalid_chans(struct wiphy *wiphy)
> +{
> +       struct wireless_dev *wdev;
> +       struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
> +
> +       ASSERT_RTNL();
> +
> +       list_for_each_entry(wdev, &rdev->wdev_list, list)
> +               if (!reg_wdev_chan_valid(wiphy, wdev))
> +                       cfg80211_leave(rdev, wdev);

An informative print or multicast event and reason would be nice here.

  Luis

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Arik Nemtsov]

On Thu, Jun 19, 2014 at 1:08 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> If the regulatory request contains the special "99" unknown-country code,
>> allow a different alpha2 as response.
>>
>> This special alpha2 is used when the real alpha2 is unknown and should
>> be provided by the driver via its get_regd() wiphy callback, as part of
>> the regdomain info.
>>
>> Change-Id: I286e3aed828cabf22292b6fe320fdcfa61f0b951
>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
>> Reviewed-on: https://gerrit.rds.intel.com/32266
>> Tested-by: IWL Jenkins
>> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
>> ---
>>  net/wireless/reg.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index e2f33d7..c429ec5 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -418,7 +418,8 @@ static bool is_user_regdom_saved(void)
>>                 return false;
>>
>>         /* This would indicate a mistake on the design */
>> -       if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
>> +       if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2) &&
>> +                !is_unknown_alpha2(user_alpha2),
>>                  "Unexpected user alpha2: %c%c\n",
>>                  user_alpha2[0], user_alpha2[1]))
>>                 return false;
>
> When would the user alpha2 be a custom value? This is *very* different
> than allowing an API to set the regulatory domain to a specific alpha2
> value, which is what I though this was all about.

Well there's a default country burned in the FW/NVRAM. In order to get
this country I send the "99" country code to FW, and it replies with
the correct country code. Since we need the regulatory settings
anyway, I didn't want to invent some new API to just get the current
country code and then request settings for it.
We could go down that route but IMHO that would complicate things
needlessly.. Perhaps you have a different suggestion?

>
>> @@ -595,7 +596,8 @@ bool reg_is_valid_request(const char *alpha2)
>>         if (!lr || lr->processed)
>>                 return false;
>>
>> -       return alpha2_equal(lr->alpha2, alpha2);
>> +       return alpha2_equal(lr->alpha2, alpha2) ||
>> +              is_unknown_alpha2(lr->alpha2);
>>  }
>
> This patch seems to be allowing custom user alpha2s to be passed,
> while the commit log is indicating we want to allow a change from a
> custom value that is set to a regular alpha2.

See above.

Regards,
Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> Allow driver and user hints to set the "world" regulatory domain,
>
> NACK - as expressed in the other patch, letting the drivers to use the
> new API to set the world regulatory domain doesn't make sense as we
> have custom apply stuff, and the world regulatory domain is not
> something dynamic.

Well we want to set the world regdomain from FW. This obviously
happens after wiphy registration, so I don't think the custom apply
can be used here? (since we generally want cfg80211 to own the
regdomain settings).

Arik

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Luis R. Rodriguez]

On Mon, Jun 23, 2014 at 08:12:18PM +0300, Arik Nemtsov wrote:
> On Thu, Jun 19, 2014 at 1:08 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> >> If the regulatory request contains the special "99" unknown-country code,
> >> allow a different alpha2 as response.
> >>
> >> This special alpha2 is used when the real alpha2 is unknown and should
> >> be provided by the driver via its get_regd() wiphy callback, as part of
> >> the regdomain info.
> >>
> >> Change-Id: I286e3aed828cabf22292b6fe320fdcfa61f0b951
> >> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> >> Reviewed-on: https://gerrit.rds.intel.com/32266
> >> Tested-by: IWL Jenkins
> >> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
> >> ---
> >>  net/wireless/reg.c | 6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> >> index e2f33d7..c429ec5 100644
> >> --- a/net/wireless/reg.c
> >> +++ b/net/wireless/reg.c
> >> @@ -418,7 +418,8 @@ static bool is_user_regdom_saved(void)
> >>                 return false;
> >>
> >>         /* This would indicate a mistake on the design */
> >> -       if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
> >> +       if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2) &&
> >> +                !is_unknown_alpha2(user_alpha2),
> >>                  "Unexpected user alpha2: %c%c\n",
> >>                  user_alpha2[0], user_alpha2[1]))
> >>                 return false;
> >
> > When would the user alpha2 be a custom value? This is *very* different
> > than allowing an API to set the regulatory domain to a specific alpha2
> > value, which is what I though this was all about.
> 
> Well there's a default country burned in the FW/NVRAM. In order to get
> this country I send the "99" country code to FW, and it replies with
> the correct country code. Since we need the regulatory settings
> anyway, I didn't want to invent some new API to just get the current
> country code and then request settings for it.
> We could go down that route but IMHO that would complicate things
> needlessly.. Perhaps you have a different suggestion?

I see, OK the regulatory code is already complex as it is, and I think
we need to be careful avoid being loose on the API and specially be careful
so things are not used in ways they are not documented.

Getting a information from drivers as to what country is already
programmed in is definitely useful information and the regulatory_hint()
API was designed for this purpose. It seems we want to evolve this API
to handle different sources, in this case the firmware.

If I understand the goal correctly you want to let the driver tell
cfg80211 of the alpha2 but instead of having cfg08211 query CRDA /
wireless-regdb / internal-db you want to let the driver excercise the
ability to first query the firmware for the regulatory domain data
structure, the firmware then has the right to ignore the request and
then perhaps allow cfg0211 to proceed to query CRDA / wireless-regdb. If
I understand this correctly then I think a capability flag can be pegged
onto a wiphy to describe this modification on behavior for
regulatory_hint(), that is to query firmware first for regulatory data.
You can do this by extending the struct regulatory_request with the
fact that the driver request type is a bit different just as we have
modifieres for user_reg_hint_type, you'd want a driver_reg_hint_type,
we could also then use this to inform userspace of the type of driver
hint passed if sucessful. We already have code to queue the request
and then you'd just have to extend the processing code and have
a special case there to handle calling back to the driver when needed.

By centralizing the request we could end up using the driver's own
regulatory domain on all registered wiphys that can lack regulatory
information, drivers that issue the same regulatory_hint() alpha2 on
the same wdev might want to -EALREADY as the source would be the same.

Depending on the return value, as you already programmed, cfg80211 could
follow on to do something (document clearly the return value and their
consequences would be important), either apply the returned struct or
follow on to call wireless-regdb / internel-db, or bail or whatever.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> >> Allow driver and user hints to set the "world" regulatory domain,
> >
> > NACK - as expressed in the other patch, letting the drivers to use the
> > new API to set the world regulatory domain doesn't make sense as we
> > have custom apply stuff, and the world regulatory domain is not
> > something dynamic.
> 
> Well we want to set the world regdomain from FW. This obviously
> happens after wiphy registration, so I don't think the custom apply
> can be used here? (since we generally want cfg80211 to own the
> regdomain settings).

Can the driver not obtain the world regulatory domain from firmware
prior to wiphy registration? Why not?

One thing to be careful on all this new API design is to ensure that
upon disconnect we want the driver to go back to the original state,
whatever that is.

  Luis

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Arik Nemtsov]

On Mon, Jun 23, 2014 at 9:23 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> > Well there's a default country burned in the FW/NVRAM. In order to get
> > this country I send the "99" country code to FW, and it replies with
> > the correct country code. Since we need the regulatory settings
> > anyway, I didn't want to invent some new API to just get the current
> > country code and then request settings for it.
> > We could go down that route but IMHO that would complicate things
> > needlessly.. Perhaps you have a different suggestion?
>
> I see, OK the regulatory code is already complex as it is, and I think
> we need to be careful avoid being loose on the API and specially be careful
> so things are not used in ways they are not documented.
>
> Getting a information from drivers as to what country is already
> programmed in is definitely useful information and the regulatory_hint()
> API was designed for this purpose. It seems we want to evolve this API
> to handle different sources, in this case the firmware.
>
> If I understand the goal correctly you want to let the driver tell
> cfg80211 of the alpha2 but instead of having cfg08211 query CRDA /
> wireless-regdb / internal-db you want to let the driver excercise the
> ability to first query the firmware for the regulatory domain data
> structure, the firmware then has the right to ignore the request and
> then perhaps allow cfg0211 to proceed to query CRDA / wireless-regdb. If

Exactly :)

> I understand this correctly then I think a capability flag can be pegged
> onto a wiphy to describe this modification on behavior for
> regulatory_hint(), that is to query firmware first for regulatory data.
> You can do this by extending the struct regulatory_request with the
> fact that the driver request type is a bit different just as we have
> modifieres for user_reg_hint_type, you'd want a driver_reg_hint_type,
> we could also then use this to inform userspace of the type of driver
> hint passed if sucessful. We already have code to queue the request
> and then you'd just have to extend the processing code and have
> a special case there to handle calling back to the driver when needed.

Sounds good.

>
> By centralizing the request we could end up using the driver's own
> regulatory domain on all registered wiphys that can lack regulatory
> information, drivers that issue the same regulatory_hint() alpha2 on
> the same wdev might want to -EALREADY as the source would be the same.
>
> Depending on the return value, as you already programmed, cfg80211 could
> follow on to do something (document clearly the return value and their
> consequences would be important), either apply the returned struct or
> follow on to call wireless-regdb / internel-db, or bail or whatever.

Ok.

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
>> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> >> Allow driver and user hints to set the "world" regulatory domain,
>> >
>> > NACK - as expressed in the other patch, letting the drivers to use the
>> > new API to set the world regulatory domain doesn't make sense as we
>> > have custom apply stuff, and the world regulatory domain is not
>> > something dynamic.
>>
>> Well we want to set the world regdomain from FW. This obviously
>> happens after wiphy registration, so I don't think the custom apply
>> can be used here? (since we generally want cfg80211 to own the
>> regdomain settings).
>
> Can the driver not obtain the world regulatory domain from firmware
> prior to wiphy registration? Why not?

Since the FW is not running yet :)

>
> One thing to be careful on all this new API design is to ensure that
> upon disconnect we want the driver to go back to the original state,
> whatever that is.

This particular part is unrelated to connection state AFAICT. It just
allows someone (driver, user) to set the "00" regdomain.

Arik

Re: [PATCH 5/5] cfg80211: leave invalid channels on regdomain change

[Arik Nemtsov]

On Thu, Jun 19, 2014 at 3:13 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> When the regulatory settings change, some channels might become invalid.
>> Disconnect interfaces acting on these channels, after giving userspace
>> code a grace period to leave them.
>>
>> Because of cfg80211 channel tracking limitations, only disconnect
>> STA/P2P_CL interfaces if their current center channel became disabled.
>>
>> Change-Id: I308cc02c06a11cfc30b206efaeabe64d67186228
>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
>> Reviewed-on: https://gerrit.rds.intel.com/32422
>> Tested-by: IWL Jenkins
>> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
>
> I welcome this change, and thanks for the work! But I think we need to
> think about the cases where this really would be required. This should
> not be an issue drivers using the public wireless-regd completely
> given that the default world regulatory domain is a mathematical
> intersection, and as such its restrictive by nature and only
> world-allowed channels with the the minimum max EIRP would be allowed.
> For drivers using an initial custom world regdomain with custom apply
> API a change in regulatory would be permissive and as such we can lift
> some restrictions when we associate to an AP with a country IE
> specified. For the dynamic firmware juju that Intel wants to introduce
> this race is intrinsically unavoidable although the likelihood of it
> remains unclear.

It can happen if we're a connected AP/STA and are crossing some state
borders. Though I agree it's a corner case, they consider it mandatory
to certification.

>
> It should then be possible for some drivers to not have to use this
> sanity check. For example drivers that use ath properly (ath5k, ath9k,
> ath10k) could likely skip this stuff, unless I'm missing something.
> For drivers without regulatory data, that should also be the case as
> the public regdb would be used.

I think this should be useful for all drivers, no matter which
regulatory framework they use. If they defined the flags/settings
correctly, then everything should always be ok in the checks right?

But I agree it can expose bugs in a pretty nasty way (disconnections),
so maybe I'll add a new regulatory flag to enable this behavior?

Btw, we're also planning on adding tracking channel bandwidth, which
can also be disallowed in some circumstances. But that will probably
be handled within mac80211, since cfg80211 doesn't really know track
this info.

> It'd be interesting to see if that'
> not the case though so an informative print for now would be useful
> for the case where bailing out does happen, in fact perhaps it makes
> sense to issue a multicast nl80211 event for this so that userspace
> knows something has happened and why. Since we already have a reg
> change multicast event this seems then to provide a "safe" proactive
> measure for the loose cases, as perhaps an API can be added to let
> userspace acknowledge a regulatory change event -- that way upon
> receipt we should be able to cancel this work. Thoughts?v

The 60 second grace period is not accidental, it's enough time for
userspace to "move" the offending interfaces. In fact that's what the
Intel wpa_s does in the GO scenario - move it.
Upon a regulatory change it re-queries channels, and if the channel if
forbidden, ...

Therefore I don't really think there's a need for a more complicated
"ack" policy. We can always add it if the need arises?

>
>> ---
>>  net/wireless/reg.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 83 insertions(+), 1 deletion(-)

>> +
>> +               /* TODO: more elaborate tracking for wide channels */
>> +               ch = wdev->current_bss->pub.channel;
>> +               ret = !(ch->flags & IEEE80211_CHAN_DISABLED);
>
> That does not seem enough, the max EIRP could have changed too for
> example -- but the reg_notifier() should be used for ensuring changes
> to power requirements are respected, at least the Atheros shared ath
> module does this, that after all is one of the reasons for the
> reg_notifier(). Documenting as part of this change would be
> appreciated.

Not sure what kind of documentation you mean (that only channels are
tracked?), and where to put it.

>
>> +               break;
>> +       default:
>> +               /* others not implemented for now */
>> +               break;
>> +       }
>> +
>> +out:
>> +       wdev_unlock(wdev);
>> +       return ret;
>> +}
>> +
>> +static void reg_leave_invalid_chans(struct wiphy *wiphy)
>> +{
>> +       struct wireless_dev *wdev;
>> +       struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
>> +
>> +       ASSERT_RTNL();
>> +
>> +       list_for_each_entry(wdev, &rdev->wdev_list, list)
>> +               if (!reg_wdev_chan_valid(wiphy, wdev))
>> +                       cfg80211_leave(rdev, wdev);
>
> An informative print or multicast event and reason would be nice here.

I'll add a print.

Arik

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Luis R. Rodriguez]

On Mon, Jun 23, 2014 at 11:32 AM, Arik Nemtsov <arik@wizery.com> wrote:
>> modifieres for user_reg_hint_type, you'd want a driver_reg_hint_type,

Minor note, since the driver reg hint type can be require the
regulatory data source to be internal to the driver (firmware or
driver, we won't know) and since the new API can allow the
driver/firmware to pass and let the request come from CRDA /
wireless-regdb / internal db the driver hint is different from the
final *set* regulatory domain. So for example although the goal was to
query firmware first if the driver passed and let the source of
regulatory data come from CRDA / wireless-regdb / internal regdb we'd
want to ensure userspace is informed of this. So I think we'd need a
regulatory data structure source type as well, right now it'd default
to wireless-regdb as the source (that would suffice for CRDA or
internal-db), and your changes would add an internal-driver source.

Since this is allowing drivers to be explicit about their regulatory
data it would be nice if as part of this we can then get 'iw reg get'
the ability to then spit out per wiphy regd. Since even the custom
regd requires passing a custom regd this could even enable parsing
that data as well. This should make trouble shooting for intersections
much easier on complex systems.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
> >> <mcgrof@do-not-panic.com> wrote:
> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> >> >> Allow driver and user hints to set the "world" regulatory domain,
> >> >
> >> > NACK - as expressed in the other patch, letting the drivers to use the
> >> > new API to set the world regulatory domain doesn't make sense as we
> >> > have custom apply stuff, and the world regulatory domain is not
> >> > something dynamic.
> >>
> >> Well we want to set the world regdomain from FW. This obviously
> >> happens after wiphy registration, so I don't think the custom apply
> >> can be used here? (since we generally want cfg80211 to own the
> >> regdomain settings).
> >
> > Can the driver not obtain the world regulatory domain from firmware
> > prior to wiphy registration? Why not?
> 
> Since the FW is not running yet :)

Is that a limitation that can be corrected on the driver ? Can the wiphy
registration be moved to after wiphy firmware is loaded ? If this is too
hard I see this as a good reason to extend the API or add a new similar
call that would allow for this case. The reason for the restriction on wiphy
registration was due to the fact that we didn't want to mess with _orig
parameters after wiphy registration, and we didn't want drivers poking
with those. The _orig parameters then can be set by cfg80211 through two
ways, one is the driver say reads EEPROM stuff and sets the channel
structs with the restrictions it had prior to wiphy registration or if
drivers could deduce a regulatory domain structure they could use the
wiphy_apply_custom_regulatory() which would do the same for them.

Note that what this does is let drivers fill a set of channel data
structures and then with these mechanisms set the max allowed superset.
Anything after this is a subset of the original set of allowed
parameters. Please review handle_channel().

The difficulty in allowing changing the _orig stuff after wiphy
registration is we'd then have to ensure that the current state
of the regulatory machine is replicated on the device driver. Just
consider doing this properly and I think we'll be good.

> > One thing to be careful on all this new API design is to ensure that
> > upon disconnect we want the driver to go back to the original state,
> > whatever that is.
> 
> This particular part is unrelated to connection state AFAICT. It just
> allows someone (driver, user) to set the "00" regdomain.

There are two things here, one is the custom world regulatory domain
that firmware is setting, the other is the new API to allow an alpha2
regulatory domain. The resetting of regulatory domains needs to ensure
that if the first regulatory domain was a world regulatory domain, and
then a driver alpha2 is set these regulatory domains will be applied in
the same order when it disconnects. If the APIs are used properly this
is guaranteed already for us and its one reason for seeing if we can
just use existing APIs. See restore_custom_reg_settings() as an example.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Mon, Jun 23, 2014 at 11:48 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
>> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
>> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
>> >> <mcgrof@do-not-panic.com> wrote:
>> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> >> >> Allow driver and user hints to set the "world" regulatory domain,
>> >> >
>> >> > NACK - as expressed in the other patch, letting the drivers to use the
>> >> > new API to set the world regulatory domain doesn't make sense as we
>> >> > have custom apply stuff, and the world regulatory domain is not
>> >> > something dynamic.
>> >>
>> >> Well we want to set the world regdomain from FW. This obviously
>> >> happens after wiphy registration, so I don't think the custom apply
>> >> can be used here? (since we generally want cfg80211 to own the
>> >> regdomain settings).
>> >
>> > Can the driver not obtain the world regulatory domain from firmware
>> > prior to wiphy registration? Why not?
>>
>> Since the FW is not running yet :)
>
> Is that a limitation that can be corrected on the driver ? Can the wiphy
> registration be moved to after wiphy firmware is loaded ? If this is too
> hard I see this as a good reason to extend the API or add a new similar
> call that would allow for this case. The reason for the restriction on wiphy
> registration was due to the fact that we didn't want to mess with _orig
> parameters after wiphy registration, and we didn't want drivers poking
> with those. The _orig parameters then can be set by cfg80211 through two
> ways, one is the driver say reads EEPROM stuff and sets the channel
> structs with the restrictions it had prior to wiphy registration or if
> drivers could deduce a regulatory domain structure they could use the
> wiphy_apply_custom_regulatory() which would do the same for them.
>
> Note that what this does is let drivers fill a set of channel data
> structures and then with these mechanisms set the max allowed superset.
> Anything after this is a subset of the original set of allowed
> parameters. Please review handle_channel().
>
> The difficulty in allowing changing the _orig stuff after wiphy
> registration is we'd then have to ensure that the current state
> of the regulatory machine is replicated on the device driver. Just
> consider doing this properly and I think we'll be good.

We can't access the FW before wiphy registration. In some scenarios
the module is loaded during system boot, before we can really access
the HW bus. We only start the FW on the first ifup.
To allow ifup, we need the wiphy registered. We have no "hook" where
we can register the wiphy after the wifi HW bus if fully available.
Nor do we want to find this hook, because of multiple platforms etc.

But I'm not sure why you're mentioning the workings of
handle_channel() for this patch. It doesn't allow changing the
original flags set at registration. It just allows drivers and the
user to assign the world regdomain, and also to change the native "00"
definitions with the FW regdomain. It shouldn't harm anything AFAICT.

>
>> > One thing to be careful on all this new API design is to ensure that
>> > upon disconnect we want the driver to go back to the original state,
>> > whatever that is.
>>
>> This particular part is unrelated to connection state AFAICT. It just
>> allows someone (driver, user) to set the "00" regdomain.
>
> There are two things here, one is the custom world regulatory domain
> that firmware is setting, the other is the new API to allow an alpha2
> regulatory domain. The resetting of regulatory domains needs to ensure
> that if the first regulatory domain was a world regulatory domain, and
> then a driver alpha2 is set these regulatory domains will be applied in
> the same order when it disconnects. If the APIs are used properly this
> is guaranteed already for us and its one reason for seeing if we can
> just use existing APIs. See restore_custom_reg_settings() as an example.

I believe there's no issue introduced by this code.

Arik

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Arik Nemtsov]

On Mon, Jun 23, 2014 at 9:57 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Mon, Jun 23, 2014 at 11:32 AM, Arik Nemtsov <arik@wizery.com> wrote:
>>> modifieres for user_reg_hint_type, you'd want a driver_reg_hint_type,
>
> Minor note, since the driver reg hint type can be require the
> regulatory data source to be internal to the driver (firmware or
> driver, we won't know) and since the new API can allow the
> driver/firmware to pass and let the request come from CRDA /
> wireless-regdb / internal db the driver hint is different from the
> final *set* regulatory domain. So for example although the goal was to
> query firmware first if the driver passed and let the source of
> regulatory data come from CRDA / wireless-regdb / internal regdb we'd
> want to ensure userspace is informed of this. So I think we'd need a
> regulatory data structure source type as well, right now it'd default
> to wireless-regdb as the source (that would suffice for CRDA or
> internal-db), and your changes would add an internal-driver source.

Sure I can add a source to the regulatory_request structure. It will
have something like CORE, CRDA, INTERNAL_REGDB, DRIVER.

>
> Since this is allowing drivers to be explicit about their regulatory
> data it would be nice if as part of this we can then get 'iw reg get'
> the ability to then spit out per wiphy regd. Since even the custom
> regd requires passing a custom regd this could even enable parsing
> that data as well. This should make trouble shooting for intersections
> much easier on complex systems.

This would have to wait a bit on my side.

Arik

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arik Nemtsov]

On Thu, Jun 19, 2014 at 2:32 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> Define a new wiphy callback allowing drivers to provide regulatory
>> settings.
>>
>> Only The first wiphy registered with this callback will be able to provide
>> regulatory domain info. If such a wiphy exists, it takes precedence over
>> other data sources.
>>
>> Change-Id: If8f8faf1d127120ae464b45098c5edbc5aee3dc0
>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
>> Reviewed-on: https://gerrit.rds.intel.com/32858
>> Tested-by: IWL Jenkins
>> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
>> ---
>>  include/net/cfg80211.h | 16 +++++++++++++
>>  net/wireless/reg.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
>>  2 files changed, 75 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index 5c17b1f..b8f0269 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -2950,6 +2950,19 @@ struct wiphy_vendor_command {
>>   *     low rssi when a frame is heard on different channel, then it should set
>>   *     this variable to the maximal offset for which it can compensate.
>>   *     This value should be set in MHz.
>> + * @get_regd: a driver callback to for publishing regulatory information.
>> + *     By implementing this callback, a wiphy indicates it will provide
>> + *     regulatory information. The callback must be set before the wiphy is
>> + *     is registered. Only the first wiphy with this callback will be called
>> + *     to provide a regdomain on country-code changes.
>
> The way this is implemented is a bit more exclusive than this kdoc
> explains -- the way its implemented below allows only a single wiphy
> *ever* to be registered with this capability. This means that external
> attachments such as USB 802.11 devices that have this API requirement
> cannot ensure that they will get this callback issued. What do we want
> to do about that?

Well Intel regulatory constraints require us to be the "dictator".
This feature is also primarily designed for systems which are not
extensible, so you can't really add another device.
I guess we'll have to solve this when the need arises.

>
> Why are we not just considering multiple cards with these capabilities
> as equal and simply deal with the intersections as with the other
> cases? The restriction seems to limit our capabilities and likely will
> require bug reporting / issues to be fleshed out later. I rather
> address this now.
>
>> + *     Returns A driver allocated regdomain structure.
>
> "a" instead of capital A.

Thanks.

>
>> +                                                                           The alpha2 in the
>> + *     returned regdomain can be different from the one given via the alpha2
>> + *     argument, if the argument contains "99", meaning unknown.
>
> Lets be upfront -- how many regulatory domains will you guys return
> right now in which the same alpha2 will be kept? I don't like this for
> one bit at all, if you are going to return a 99 for a specific alpha2
> it must mean firmware / driver has the ability to search for an alpha2
> for a set of custom regulatory domains, which should also mean you
> should be able to return a set of alpha2s that can be used for the
> custom regulatory domain, so I'd like to see that added as a possible
> return set. This should set expectations for users, build a better
> understanding of how all this works, and most importantly enable easy
> and shorter scraping of the firmware for our own research and
> development and advancement of wireless-regdb as otherwise we'd have
> to loop over every alpha2 and then deduce grouping. This is the only
> way I see this being reasonable to accept upstream.

I think you're misunderstanding things here. We only give back real
regulatory domains. The "99" is only used as a means the get the
currently NVRAM flashed regdomain from FW.
We discussed this on the other patch.. Perhaps I should tweak the
explanation a bit here.

>
> We also should be clear now in the documentation about the differences
> and purpose behind this API and the custom regulatory which tons of
> folks already use. In this case the requirement was dynamic changes
> and to ensure these changes get back to userspace permissively as
> otherwise they could not. The way I see it the custom stuff should be
> used for custom world regulatory domains -- this new API is for
> specific alpha2s. This should also mean then that this API should
> *not* be used to query the firmware for the world regulatory domain.

Why not? What's problematic here?
I would much prefer to keep a single API for everything.

> It also means that the custom flag has to be looked at carefully as
> well now since the custom flag may need to be lifted dynamically. You
> may want to look at the Atheros ath module for the different use cases
> as they ship tons of different types of devices: world roaming, and
> also cards with a specific alpha2 set. There's a bit of boiler place
> code there that I saw tons of drivers share / copy so perhaps some
> boiler plate code might be in order to help here.

It is already lifted dynamically if a driver regulatory hint arrives.

>
> This new API makes only sense for dynamic changes and as such I do
> expect this to be used for direct alpha2 mappings, not some region
> stuff. Grouping regulatory domains to groups makes sense to save size
> -- we've seen this before by other vendors but we should be easily
> able to get the group alpha2 mapping as well. If Intel is *not* going
> to contribute to wireless-regdb this at the very least should help us
> move forward with the public regdb by providing back mapping of all
> the alpha2s that a regulatory domain passed also applies to.

The API currently doesn't work this way. I'll ask the regulatory folks
for a table, but I can't promise anything.

>
>> + *     If an ERR_PTR is returned the regulatory core will consult other
>> + *     sources for the regdomain info (internal regdb and CRDA).
>
> That's nice.
>
>> +                                                                                           Returning
>> + *     NULL will cause the regdomain to remain the same.
>
>
> What do you mean by this? Can you please elaborate exactly what this means?

It means we will ignore the request. Not sure what's to elaborate.

>>  static struct regulatory_request core_request_world = {
>> @@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
>>   */
>>  static bool reg_is_indoor;
>>
>> +/*
>> + * Wiphy with a get_regd() callback that can provide regulatory information
>> + * when the country code changes. Only the first wiphy registered with the
>> + * get_regd callback will be called to provide a regdomain on country-code
>> + * changes.
>> + * (protected by RTNL)
>> + */
>> +static struct wiphy *regd_info_wiphy;
>
> Note all my feedback on the kdoc comment about this. This obviously
> doesn't scale well but more importantly given that devices bus
> configuration can change (regardless of what systemd folks think) it
> means that we can get different results on a system with 2 internal
> cards. Systems with multiple built in 802.11 cards were something of a
> theoretical myth back in the day when we started with 802.11 but not
> anymore. For example I'm very aware of some Freebox 802.11 APs with
> multiple built in 802.11 cards and from different vendors!  No only
> can this lead to issues with cards on different buses and races
> therein, but it could also produce different results on different
> architectures.

Agree it's a hairy issue, but I think we should get to this in a later
patch. It's not a simple issue or "fix".
Might need different policies, intersections, etc.

>
>> +
>>  static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
>>  {
>>         return rtnl_dereference(cfg80211_regdomain);
>> @@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
>>         return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
>>  }
>>
>> +static int call_wiphy_regd_info(const char *alpha2)
>> +{
>> +       struct ieee80211_regdomain *regd;
>> +
>> +       if (!regd_info_wiphy)
>> +               return -ENOENT;
>> +
>> +       /* can happen if the driver removes the callback at runtime */
>> +       if (WARN_ON(!regd_info_wiphy->get_regd))
>> +               return -EINVAL;
>
> I'd rather see a capability flag for this, that way we can also tell
> userspace of this ridiculous incarnation of crap on firmware, and when
> set we'd expect this to be set. If we want to address the dynamic
> removal / addition of the callback that can be dealt with when the
> time comes, but that doesn't seem to be required givenou'refe that you
> added API to let the firmware let cfg80211 pass through and use the
> wireless-regdb data.

Sure I'll add a capability flag.

>
>> +
>> +       regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
>> +       if (IS_ERR(regd))
>> +               return -EIO;
>> +
>
> You're not feeding the firmware information on availability of the
> userspace regulatory domain, in the case of it was allowed, it seems
> to me that firmware might make a better decision in the case that
> userspace lacks information. Userspace can also be upgraded at runtime
> dynamically so whether or not userspace has a regulatory domain can
> change at run time. Please consider this.

The Intel use case doesn't use userspace regulatory information at
all. This can be extended later I guess.

Arik

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Luis R. Rodriguez]

On Tue, Jun 24, 2014 at 09:37:29AM +0300, Arik Nemtsov wrote:
> On Thu, Jun 19, 2014 at 2:32 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> >> Define a new wiphy callback allowing drivers to provide regulatory
> >> settings.
> >>
> >> Only The first wiphy registered with this callback will be able to provide
> >> regulatory domain info. If such a wiphy exists, it takes precedence over
> >> other data sources.
> >>
> >> Change-Id: If8f8faf1d127120ae464b45098c5edbc5aee3dc0
> >> Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
> >> Reviewed-on: https://gerrit.rds.intel.com/32858
> >> Tested-by: IWL Jenkins
> >> Reviewed-by: Johannes Berg <johannes.berg@intel.com>
> >> ---
> >>  include/net/cfg80211.h | 16 +++++++++++++
> >>  net/wireless/reg.c     | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
> >>  2 files changed, 75 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> >> index 5c17b1f..b8f0269 100644
> >> --- a/include/net/cfg80211.h
> >> +++ b/include/net/cfg80211.h
> >> @@ -2950,6 +2950,19 @@ struct wiphy_vendor_command {
> >>   *     low rssi when a frame is heard on different channel, then it should set
> >>   *     this variable to the maximal offset for which it can compensate.
> >>   *     This value should be set in MHz.
> >> + * @get_regd: a driver callback to for publishing regulatory information.
> >> + *     By implementing this callback, a wiphy indicates it will provide
> >> + *     regulatory information. The callback must be set before the wiphy is
> >> + *     is registered. Only the first wiphy with this callback will be called
> >> + *     to provide a regdomain on country-code changes.
> >
> > The way this is implemented is a bit more exclusive than this kdoc
> > explains -- the way its implemented below allows only a single wiphy
> > *ever* to be registered with this capability. This means that external
> > attachments such as USB 802.11 devices that have this API requirement
> > cannot ensure that they will get this callback issued. What do we want
> > to do about that?
> 
> Well Intel regulatory constraints require us to be the "dictator".

That requirement is in no way in conflict with addressing support
for the get_regd() callback on multiple drivers. Let's separate
a requirement from not having done the required work for a full
patch.

> This feature is also primarily designed for systems which are not
> extensible, so you can't really add another device.
> I guess we'll have to solve this when the need arises.

The patch in question does not address denying wiphy registration
if two drivers have get_regd() implemented, that's essentially what
this *should* be trying to do but:

1) Its not documented above
2) The limitation of the patch in no way is part of matching the
   requirement you mention. That is, the requirement to have Intel's
   driver as "dictator" has nothing to do with allowing or not
   multiple similar drivers that can help with compliance by providing
   their own regulatory dynamically.
3) You are putting the requirement of implmenting support for
   multiple drivers with get_regd() on the next user of get_regd()
   which wants to integrate support for an intel card with theirs,
   that's unfair and far sighted for an implementation.

The requirement you have has nothing to do with the limitation
you have so this patch is unacceptable. I also provided recommendations
on how you can lift this limitation, so it shouldn't be hard.

> > Lets be upfront -- how many regulatory domains will you guys return
> > right now in which the same alpha2 will be kept? I don't like this for
> > one bit at all, if you are going to return a 99 for a specific alpha2
> > it must mean firmware / driver has the ability to search for an alpha2
> > for a set of custom regulatory domains, which should also mean you
> > should be able to return a set of alpha2s that can be used for the
> > custom regulatory domain, so I'd like to see that added as a possible
> > return set. This should set expectations for users, build a better
> > understanding of how all this works, and most importantly enable easy
> > and shorter scraping of the firmware for our own research and
> > development and advancement of wireless-regdb as otherwise we'd have
> > to loop over every alpha2 and then deduce grouping. This is the only
> > way I see this being reasonable to accept upstream.
> 
> I think you're misunderstanding things here. We only give back real
> regulatory domains. The "99" is only used as a means the get the
> currently NVRAM flashed regdomain from FW.

If its real regulatory domains then why is the alpha2 not set?

> We discussed this on the other patch.. Perhaps I should tweak the
> explanation a bit here.

Yes please.

> > We also should be clear now in the documentation about the differences
> > and purpose behind this API and the custom regulatory which tons of
> > folks already use. In this case the requirement was dynamic changes
> > and to ensure these changes get back to userspace permissively as
> > otherwise they could not. The way I see it the custom stuff should be
> > used for custom world regulatory domains -- this new API is for
> > specific alpha2s. This should also mean then that this API should
> > *not* be used to query the firmware for the world regulatory domain.
> 
> Why not? What's problematic here?
> I would much prefer to keep a single API for everything.

Because APIs already exist for some of what you want already. In
particular it seems to me you can implement the alpha2 hint with
the callback by using a capability flag and having the core deal
the callback and then applying the regulatory domain.
Please check but I think you might want alpha2 cases to have handle_channel()
apply the rules on the orig_* parameters which which happens when
REGULATORY_STRICT_REG is set, ie, you may want to require having
set the callback to have REGULATORY_STRICT_REG set. This allows
drivers to enforce the orig_ values reflecting the regulatory
domain data structure. regulatory_hint() also propagates to other drivers
as well and the code also considers the regulatory state for older
types of requests to decide what it should do for each driver and
each case given that we have very different drivers with different
regulatory requirements.

The wiphy_apply_custom_regulatory() is only for custom regulatory
domains, the difference is that wiphy_apply_custom_regulatory() doesn't
take into consideration the regulatory state machine and changes that
have happened over time, it is also specific to only one wiphy. Lastly
it doesn't update orig_* parameters.

For the non alpha2 cases it seems you want a post registration
wiphy_apply_custom_regulatory() type of call which would only apply
to that wiphy, never update orig_* parameters, and consider then also
the regulatory state machine. If you don't care about the regulatory
state machine that makes you special and you must deal with both.

We can't blend the two types of calls because they are very different
in terms of purpose and also the implications on other drivers.
Adding a new API to let custom world regulatory domains or specific
alpha2s would make the other two APIs kind of pointless, we want to
be specific.

> > It also means that the custom flag has to be looked at carefully as
> > well now since the custom flag may need to be lifted dynamically. You
> > may want to look at the Atheros ath module for the different use cases
> > as they ship tons of different types of devices: world roaming, and
> > also cards with a specific alpha2 set. There's a bit of boiler place
> > code there that I saw tons of drivers share / copy so perhaps some
> > boiler plate code might be in order to help here.
> 
> It is already lifted dynamically if a driver regulatory hint arrives.

That indeed that was my point and it seems this was reviewed and considered.
Great.

> > This new API makes only sense for dynamic changes and as such I do
> > expect this to be used for direct alpha2 mappings, not some region
> > stuff. Grouping regulatory domains to groups makes sense to save size
> > -- we've seen this before by other vendors but we should be easily
> > able to get the group alpha2 mapping as well. If Intel is *not* going
> > to contribute to wireless-regdb this at the very least should help us
> > move forward with the public regdb by providing back mapping of all
> > the alpha2s that a regulatory domain passed also applies to.
> 
> The API currently doesn't work this way. I'll ask the regulatory folks
> for a table, but I can't promise anything.

OK.

> >> + *     If an ERR_PTR is returned the regulatory core will consult other
> >> + *     sources for the regdomain info (internal regdb and CRDA).
> >
> > That's nice.
> >
> >> +                                                                                           Returning
> >> + *     NULL will cause the regdomain to remain the same.
> >
> >
> > What do you mean by this? Can you please elaborate exactly what this means?
> 
> It means we will ignore the request. Not sure what's to elaborate.

Why would you do that? Explain why a regulatory hint would be issued
and then the driver could decide to ignore it. Seems kind of pointless
if the driver was the one originally issuing the request!

> >>  static struct regulatory_request core_request_world = {
> >> @@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
> >>   */
> >>  static bool reg_is_indoor;
> >>
> >> +/*
> >> + * Wiphy with a get_regd() callback that can provide regulatory information
> >> + * when the country code changes. Only the first wiphy registered with the
> >> + * get_regd callback will be called to provide a regdomain on country-code
> >> + * changes.
> >> + * (protected by RTNL)
> >> + */
> >> +static struct wiphy *regd_info_wiphy;
> >
> > Note all my feedback on the kdoc comment about this. This obviously
> > doesn't scale well but more importantly given that devices bus
> > configuration can change (regardless of what systemd folks think) it
> > means that we can get different results on a system with 2 internal
> > cards. Systems with multiple built in 802.11 cards were something of a
> > theoretical myth back in the day when we started with 802.11 but not
> > anymore. For example I'm very aware of some Freebox 802.11 APs with
> > multiple built in 802.11 cards and from different vendors!  No only
> > can this lead to issues with cards on different buses and races
> > therein, but it could also produce different results on different
> > architectures.
> 
> Agree it's a hairy issue, but I think we should get to this in a later
> patch. It's not a simple issue or "fix".
> Might need different policies, intersections, etc.

Exactly, no address this, otheriwse having an intel driver present would
mean not being able to register other similar type of drivers loaded
on a system. This is unacceptable.

> >>  static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
> >>  {
> >>         return rtnl_dereference(cfg80211_regdomain);
> >> @@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
> >>         return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
> >>  }
> >>
> >> +static int call_wiphy_regd_info(const char *alpha2)
> >> +{
> >> +       struct ieee80211_regdomain *regd;
> >> +
> >> +       if (!regd_info_wiphy)
> >> +               return -ENOENT;
> >> +
> >> +       /* can happen if the driver removes the callback at runtime */
> >> +       if (WARN_ON(!regd_info_wiphy->get_regd))
> >> +               return -EINVAL;
> >
> > I'd rather see a capability flag for this, that way we can also tell
> > userspace of this ridiculous incarnation of crap on firmware, and when
> > set we'd expect this to be set. If we want to address the dynamic
> > removal / addition of the callback that can be dealt with when the
> > time comes, but that doesn't seem to be required givenou'refe that you
> > added API to let the firmware let cfg80211 pass through and use the
> > wireless-regdb data.
> 
> Sure I'll add a capability flag.

OK.

> >> +
> >> +       regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
> >> +       if (IS_ERR(regd))
> >> +               return -EIO;
> >> +
> >
> > You're not feeding the firmware information on availability of the
> > userspace regulatory domain, in the case of it was allowed, it seems
> > to me that firmware might make a better decision in the case that
> > userspace lacks information. Userspace can also be upgraded at runtime
> > dynamically so whether or not userspace has a regulatory domain can
> > change at run time. Please consider this.
> 
> The Intel use case doesn't use userspace regulatory information at
> all. This can be extended later I guess.

The "Intel use case" is different than "I'm adding a general API" case.
You can easily extend the API to pass the regulatory domain for
inspection even if you don't use it on the driver.

  Luis

Re: [PATCH 3/5] cfg80211: treat the special "unknown" alpha2 as valid

[Luis R. Rodriguez]

On Tue, Jun 24, 2014 at 09:09:41AM +0300, Arik Nemtsov wrote:
> On Mon, Jun 23, 2014 at 9:57 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Mon, Jun 23, 2014 at 11:32 AM, Arik Nemtsov <arik@wizery.com> wrote:
> >>> modifieres for user_reg_hint_type, you'd want a driver_reg_hint_type,
> >
> > Minor note, since the driver reg hint type can be require the
> > regulatory data source to be internal to the driver (firmware or
> > driver, we won't know) and since the new API can allow the
> > driver/firmware to pass and let the request come from CRDA /
> > wireless-regdb / internal db the driver hint is different from the
> > final *set* regulatory domain. So for example although the goal was to
> > query firmware first if the driver passed and let the source of
> > regulatory data come from CRDA / wireless-regdb / internal regdb we'd
> > want to ensure userspace is informed of this. So I think we'd need a
> > regulatory data structure source type as well, right now it'd default
> > to wireless-regdb as the source (that would suffice for CRDA or
> > internal-db), and your changes would add an internal-driver source.
> 
> Sure I can add a source to the regulatory_request structure. It will
> have something like CORE, CRDA, INTERNAL_REGDB, DRIVER.

OK.

> > Since this is allowing drivers to be explicit about their regulatory
> > data it would be nice if as part of this we can then get 'iw reg get'
> > the ability to then spit out per wiphy regd. Since even the custom
> > regd requires passing a custom regd this could even enable parsing
> > that data as well. This should make trouble shooting for intersections
> > much easier on complex systems.
> 
> This would have to wait a bit on my side.

OK.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Tue, Jun 24, 2014 at 08:55:28AM +0300, Arik Nemtsov wrote:
> On Mon, Jun 23, 2014 at 11:48 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
> >> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
> >> <mcgrof@do-not-panic.com> wrote:
> >> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
> >> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
> >> >> <mcgrof@do-not-panic.com> wrote:
> >> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> >> >> >> Allow driver and user hints to set the "world" regulatory domain,
> >> >> >
> >> >> > NACK - as expressed in the other patch, letting the drivers to use the
> >> >> > new API to set the world regulatory domain doesn't make sense as we
> >> >> > have custom apply stuff, and the world regulatory domain is not
> >> >> > something dynamic.
> >> >>
> >> >> Well we want to set the world regdomain from FW. This obviously
> >> >> happens after wiphy registration, so I don't think the custom apply
> >> >> can be used here? (since we generally want cfg80211 to own the
> >> >> regdomain settings).
> >> >
> >> > Can the driver not obtain the world regulatory domain from firmware
> >> > prior to wiphy registration? Why not?
> >>
> >> Since the FW is not running yet :)
> >
> > Is that a limitation that can be corrected on the driver ? Can the wiphy
> > registration be moved to after wiphy firmware is loaded ? If this is too
> > hard I see this as a good reason to extend the API or add a new similar
> > call that would allow for this case. The reason for the restriction on wiphy
> > registration was due to the fact that we didn't want to mess with _orig
> > parameters after wiphy registration, and we didn't want drivers poking
> > with those. The _orig parameters then can be set by cfg80211 through two
> > ways, one is the driver say reads EEPROM stuff and sets the channel
> > structs with the restrictions it had prior to wiphy registration or if
> > drivers could deduce a regulatory domain structure they could use the
> > wiphy_apply_custom_regulatory() which would do the same for them.
> >
> > Note that what this does is let drivers fill a set of channel data
> > structures and then with these mechanisms set the max allowed superset.
> > Anything after this is a subset of the original set of allowed
> > parameters. Please review handle_channel().
> >
> > The difficulty in allowing changing the _orig stuff after wiphy
> > registration is we'd then have to ensure that the current state
> > of the regulatory machine is replicated on the device driver. Just
> > consider doing this properly and I think we'll be good.
> 
> We can't access the FW before wiphy registration. In some scenarios
> the module is loaded during system boot, before we can really access
> the HW bus. We only start the FW on the first ifup.
> To allow ifup, we need the wiphy registered. We have no "hook" where
> we can register the wiphy after the wifi HW bus if fully available.

What hw bus? Buses can probe, even if its a fake bus type of thing
you can use platform_device_register_simple(), look at
drivers/net/ethernet/8390/ne.c as a complex example.

Also if a bus is not yet set up or if resouces are still being
brought up and the driver needs to be poked at a later time you can
verify what you need access for and return -EPROBE_DEFER upon probe
if things can't move forward which will force the driver to be
probed at a later time.

> Nor do we want to find this hook, because of multiple platforms etc.

I'm pretty sure you can find it, but I do also understand that
restructuring the driver is a bit of work so I'm fine with you saying
its a lot of work but saying its no possible seems not fully
supported yet.

> But I'm not sure why you're mentioning the workings of
> handle_channel() for this patch. It doesn't allow changing the
> original flags set at registration. It just allows drivers and the
> user to assign the world regdomain, and also to change the native "00"
> definitions with the FW regdomain. It shouldn't harm anything AFAICT.

You're missing the point for why I bring up handle_channel().
handle_channel() *can* update orig_* parameters for you for an alpha2
which you *shoud* consider. Additionally handle_channel() takes into
consideration the regulatory state machine and can apply a regulatory
domain to different wiphys by design to help with compliance. The
handle_channel_custom() is designed for custom regulatory domains
and do not propagate to other wiphys.

> >> > One thing to be careful on all this new API design is to ensure that
> >> > upon disconnect we want the driver to go back to the original state,
> >> > whatever that is.
> >>
> >> This particular part is unrelated to connection state AFAICT. It just
> >> allows someone (driver, user) to set the "00" regdomain.
> >
> > There are two things here, one is the custom world regulatory domain
> > that firmware is setting, the other is the new API to allow an alpha2
> > regulatory domain. The resetting of regulatory domains needs to ensure
> > that if the first regulatory domain was a world regulatory domain, and
> > then a driver alpha2 is set these regulatory domains will be applied in
> > the same order when it disconnects. If the APIs are used properly this
> > is guaranteed already for us and its one reason for seeing if we can
> > just use existing APIs. See restore_custom_reg_settings() as an example.
> 
> I believe there's no issue introduced by this code.

You are extending usage of custom regulatory data to be hooked onto the
orig_* parameters, which by design was made to help upkeep the minimum
allowed set, the custom regulatory domain allows for *world roaming*
which can mean reducing this set, but note it will never *enable* new
channels. The alpha2 hint *can* enable new channels by design by
updating orig_* parameters. These are two different type of regulatory
domain hints.

  Luis

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arik Nemtsov]

On Tue, Jul 1, 2014 at 1:03 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
>> > The way this is implemented is a bit more exclusive than this kdoc
>> > explains -- the way its implemented below allows only a single wiphy
>> > *ever* to be registered with this capability. This means that external
>> > attachments such as USB 802.11 devices that have this API requirement
>> > cannot ensure that they will get this callback issued. What do we want
>> > to do about that?
>>
>> Well Intel regulatory constraints require us to be the "dictator".
>
> That requirement is in no way in conflict with addressing support
> for the get_regd() callback on multiple drivers. Let's separate
> a requirement from not having done the required work for a full
> patch.

Fair enough.

>
>> This feature is also primarily designed for systems which are not
>> extensible, so you can't really add another device.
>> I guess we'll have to solve this when the need arises.
>
> The patch in question does not address denying wiphy registration
> if two drivers have get_regd() implemented, that's essentially what
> this *should* be trying to do but:
>
> 1) Its not documented above
> 2) The limitation of the patch in no way is part of matching the
>    requirement you mention. That is, the requirement to have Intel's
>    driver as "dictator" has nothing to do with allowing or not
>    multiple similar drivers that can help with compliance by providing
>    their own regulatory dynamically.
> 3) You are putting the requirement of implmenting support for
>    multiple drivers with get_regd() on the next user of get_regd()
>    which wants to integrate support for an intel card with theirs,
>    that's unfair and far sighted for an implementation.
>
> The requirement you have has nothing to do with the limitation
> you have so this patch is unacceptable. I also provided recommendations
> on how you can lift this limitation, so it shouldn't be hard.

I already prevent the registration of a second "get_regd" callback. I
just re-read your previous email and I'm not sure what's the
recommendation here. The 2-card scenario where one of the cards
doesn't use "get_regd" should be fine - it will just use the
regulatory settings from the "get_regd" one. Only the
2-cards-using-"get_regd" scenario is not supported, and AFAICT it
doesn't exist in practice, and also requires complicated treatment
(perhaps the two "dictators" differ in their definition of the
regdomains?)


>
>> > Lets be upfront -- how many regulatory domains will you guys return
>> > right now in which the same alpha2 will be kept? I don't like this for
>> > one bit at all, if you are going to return a 99 for a specific alpha2
>> > it must mean firmware / driver has the ability to search for an alpha2
>> > for a set of custom regulatory domains, which should also mean you
>> > should be able to return a set of alpha2s that can be used for the
>> > custom regulatory domain, so I'd like to see that added as a possible
>> > return set. This should set expectations for users, build a better
>> > understanding of how all this works, and most importantly enable easy
>> > and shorter scraping of the firmware for our own research and
>> > development and advancement of wireless-regdb as otherwise we'd have
>> > to loop over every alpha2 and then deduce grouping. This is the only
>> > way I see this being reasonable to accept upstream.
>>
>> I think you're misunderstanding things here. We only give back real
>> regulatory domains. The "99" is only used as a means the get the
>> currently NVRAM flashed regdomain from FW.
>
> If its real regulatory domains then why is the alpha2 not set?

We call get_regd with "99", expecting the driver/FW to reply with the
current alpha2 as burned in ROM/FW.

>
>> We discussed this on the other patch.. Perhaps I should tweak the
>> explanation a bit here.
>
> Yes please.

Sure. It will explain what I just wrote above.

>
>> > We also should be clear now in the documentation about the differences
>> > and purpose behind this API and the custom regulatory which tons of
>> > folks already use. In this case the requirement was dynamic changes
>> > and to ensure these changes get back to userspace permissively as
>> > otherwise they could not. The way I see it the custom stuff should be
>> > used for custom world regulatory domains -- this new API is for
>> > specific alpha2s. This should also mean then that this API should
>> > *not* be used to query the firmware for the world regulatory domain.
>>
>> Why not? What's problematic here?
>> I would much prefer to keep a single API for everything.
>
> Because APIs already exist for some of what you want already. In
> particular it seems to me you can implement the alpha2 hint with
> the callback by using a capability flag and having the core deal
> the callback and then applying the regulatory domain.
> Please check but I think you might want alpha2 cases to have handle_channel()
> apply the rules on the orig_* parameters which which happens when
> REGULATORY_STRICT_REG is set, ie, you may want to require having
> set the callback to have REGULATORY_STRICT_REG set. This allows
> drivers to enforce the orig_ values reflecting the regulatory
> domain data structure. regulatory_hint() also propagates to other drivers
> as well and the code also considers the regulatory state for older
> types of requests to decide what it should do for each driver and
> each case given that we have very different drivers with different
> regulatory requirements.
>
> The wiphy_apply_custom_regulatory() is only for custom regulatory
> domains, the difference is that wiphy_apply_custom_regulatory() doesn't
> take into consideration the regulatory state machine and changes that
> have happened over time, it is also specific to only one wiphy. Lastly
> it doesn't update orig_* parameters.
>
> For the non alpha2 cases it seems you want a post registration
> wiphy_apply_custom_regulatory() type of call which would only apply
> to that wiphy, never update orig_* parameters, and consider then also
> the regulatory state machine. If you don't care about the regulatory
> state machine that makes you special and you must deal with both.
>
> We can't blend the two types of calls because they are very different
> in terms of purpose and also the implications on other drivers.
> Adding a new API to let custom world regulatory domains or specific
> alpha2s would make the other two APIs kind of pointless, we want to
> be specific.

I don't want to touch orig_flags, since the FW burned regdomain can be
overridden. I do want to register the wiphy very early, before the FW
is loaded. Also I want to get the default alpha2 and associated
regdomain from the FW at runtime. Don't think any combination of the
current APIs allows that.
I can prepare a separate API call just for the purpose of getting the
initial alpha2+regdomain setting, but I have to say it seems pretty
pointless. The current flow where we send "99" and get the real alpha2
works well.


>> >> +                                                                                           Returning
>> >> + *     NULL will cause the regdomain to remain the same.
>> >
>> >
>> > What do you mean by this? Can you please elaborate exactly what this means?
>>
>> It means we will ignore the request. Not sure what's to elaborate.
>
> Why would you do that? Explain why a regulatory hint would be issued
> and then the driver could decide to ignore it. Seems kind of pointless
> if the driver was the one originally issuing the request!

The driver isn't the only one that can issue requests. Userspace can
issue them too (and in fact does). Basically the driver/FW can say
"leave the same regdomain, don't touch it".
It's a policy decision.

>
>> >>  static struct regulatory_request core_request_world = {
>> >> @@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
>> >>   */
>> >>  static bool reg_is_indoor;
>> >>
>> >> +/*
>> >> + * Wiphy with a get_regd() callback that can provide regulatory information
>> >> + * when the country code changes. Only the first wiphy registered with the
>> >> + * get_regd callback will be called to provide a regdomain on country-code
>> >> + * changes.
>> >> + * (protected by RTNL)
>> >> + */
>> >> +static struct wiphy *regd_info_wiphy;
>> >
>> > Note all my feedback on the kdoc comment about this. This obviously
>> > doesn't scale well but more importantly given that devices bus
>> > configuration can change (regardless of what systemd folks think) it
>> > means that we can get different results on a system with 2 internal
>> > cards. Systems with multiple built in 802.11 cards were something of a
>> > theoretical myth back in the day when we started with 802.11 but not
>> > anymore. For example I'm very aware of some Freebox 802.11 APs with
>> > multiple built in 802.11 cards and from different vendors!  No only
>> > can this lead to issues with cards on different buses and races
>> > therein, but it could also produce different results on different
>> > architectures.
>>
>> Agree it's a hairy issue, but I think we should get to this in a later
>> patch. It's not a simple issue or "fix".
>> Might need different policies, intersections, etc.
>
> Exactly, no address this, otheriwse having an intel driver present would
> mean not being able to register other similar type of drivers loaded
> on a system. This is unacceptable.

I agree it will prevent other "get_regd" cards from registering on the
same system, but I don't think the limitation really exists in
practice.
These are not Intel cards one can buy off the shelf. They are soldered
to platforms that are non-extensible. It also means I have no ability
to test this code.

>
>> >>  static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
>> >>  {
>> >>         return rtnl_dereference(cfg80211_regdomain);
>> >> @@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
>> >>         return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
>> >>  }
>> >>
>> >> +static int call_wiphy_regd_info(const char *alpha2)
>> >> +{
>> >> +       struct ieee80211_regdomain *regd;
>> >> +
>> >> +       if (!regd_info_wiphy)
>> >> +               return -ENOENT;
>> >> +
>> >> +       /* can happen if the driver removes the callback at runtime */
>> >> +       if (WARN_ON(!regd_info_wiphy->get_regd))
>> >> +               return -EINVAL;
>> >
>> > I'd rather see a capability flag for this, that way we can also tell
>> > userspace of this ridiculous incarnation of crap on firmware, and when
>> > set we'd expect this to be set. If we want to address the dynamic
>> > removal / addition of the callback that can be dealt with when the
>> > time comes, but that doesn't seem to be required givenou'refe that you
>> > added API to let the firmware let cfg80211 pass through and use the
>> > wireless-regdb data.
>>
>> Sure I'll add a capability flag.
>
> OK.
>
>> >> +
>> >> +       regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
>> >> +       if (IS_ERR(regd))
>> >> +               return -EIO;
>> >> +
>> >
>> > You're not feeding the firmware information on availability of the
>> > userspace regulatory domain, in the case of it was allowed, it seems
>> > to me that firmware might make a better decision in the case that
>> > userspace lacks information. Userspace can also be upgraded at runtime
>> > dynamically so whether or not userspace has a regulatory domain can
>> > change at run time. Please consider this.
>>
>> The Intel use case doesn't use userspace regulatory information at
>> all. This can be extended later I guess.
>
> The "Intel use case" is different than "I'm adding a general API" case.
> You can easily extend the API to pass the regulatory domain for
> inspection even if you don't use it on the driver.

Perhaps this shouldn't be part of this feature? It's of no use to the
driver or to the user of the system. If someone wants to use userspace
information, they should extend the APIs IMHO.

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Tue, Jul 1, 2014 at 1:21 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
>
> On Tue, Jun 24, 2014 at 08:55:28AM +0300, Arik Nemtsov wrote:
> > On Mon, Jun 23, 2014 at 11:48 PM, Luis R. Rodriguez
> > <mcgrof@do-not-panic.com> wrote:
> > > On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
> > >> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
> > >> <mcgrof@do-not-panic.com> wrote:
> > >> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
> > >> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
> > >> >> <mcgrof@do-not-panic.com> wrote:
> > >> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> > >> >> >> Allow driver and user hints to set the "world" regulatory domain,
> > >> >> >
> > >> >> > NACK - as expressed in the other patch, letting the drivers to use the
> > >> >> > new API to set the world regulatory domain doesn't make sense as we
> > >> >> > have custom apply stuff, and the world regulatory domain is not
> > >> >> > something dynamic.
> > >> >>
> > >> >> Well we want to set the world regdomain from FW. This obviously
> > >> >> happens after wiphy registration, so I don't think the custom apply
> > >> >> can be used here? (since we generally want cfg80211 to own the
> > >> >> regdomain settings).
> > >> >
> > >> > Can the driver not obtain the world regulatory domain from firmware
> > >> > prior to wiphy registration? Why not?
> > >>
> > >> Since the FW is not running yet :)
> > >
> > > Is that a limitation that can be corrected on the driver ? Can the wiphy
> > > registration be moved to after wiphy firmware is loaded ? If this is too
> > > hard I see this as a good reason to extend the API or add a new similar
> > > call that would allow for this case. The reason for the restriction on wiphy
> > > registration was due to the fact that we didn't want to mess with _orig
> > > parameters after wiphy registration, and we didn't want drivers poking
> > > with those. The _orig parameters then can be set by cfg80211 through two
> > > ways, one is the driver say reads EEPROM stuff and sets the channel
> > > structs with the restrictions it had prior to wiphy registration or if
> > > drivers could deduce a regulatory domain structure they could use the
> > > wiphy_apply_custom_regulatory() which would do the same for them.
> > >
> > > Note that what this does is let drivers fill a set of channel data
> > > structures and then with these mechanisms set the max allowed superset.
> > > Anything after this is a subset of the original set of allowed
> > > parameters. Please review handle_channel().
> > >
> > > The difficulty in allowing changing the _orig stuff after wiphy
> > > registration is we'd then have to ensure that the current state
> > > of the regulatory machine is replicated on the device driver. Just
> > > consider doing this properly and I think we'll be good.
> >
> > We can't access the FW before wiphy registration. In some scenarios
> > the module is loaded during system boot, before we can really access
> > the HW bus. We only start the FW on the first ifup.
> > To allow ifup, we need the wiphy registered. We have no "hook" where
> > we can register the wiphy after the wifi HW bus if fully available.
>
> What hw bus? Buses can probe, even if its a fake bus type of thing
> you can use platform_device_register_simple(), look at
> drivers/net/ethernet/8390/ne.c as a complex example.
>
> Also if a bus is not yet set up or if resouces are still being
> brought up and the driver needs to be poked at a later time you can
> verify what you need access for and return -EPROBE_DEFER upon probe
> if things can't move forward which will force the driver to be
> probed at a later time.
>
> > Nor do we want to find this hook, because of multiple platforms etc.
>
> I'm pretty sure you can find it, but I do also understand that
> restructuring the driver is a bit of work so I'm fine with you saying
> its a lot of work but saying its no possible seems not fully
> supported yet.

That would be a better choice of words - we don't want to restructure
the entire driver over this.

>
> > But I'm not sure why you're mentioning the workings of
> > handle_channel() for this patch. It doesn't allow changing the
> > original flags set at registration. It just allows drivers and the
> > user to assign the world regdomain, and also to change the native "00"
> > definitions with the FW regdomain. It shouldn't harm anything AFAICT.
>
> You're missing the point for why I bring up handle_channel().
> handle_channel() *can* update orig_* parameters for you for an alpha2
> which you *shoud* consider. Additionally handle_channel() takes into
> consideration the regulatory state machine and can apply a regulatory
> domain to different wiphys by design to help with compliance. The
> handle_channel_custom() is designed for custom regulatory domains
> and do not propagate to other wiphys.

AFAICT, orig_flags only change if:

if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
   request_wiphy && request_wiphy == wiphy &&
   request_wiphy->regulatory_flags & REGULATORY_STRICT_REG) {

We don't use REGULATORY_STRICT_REG, nor is the use-case "faking" our
alpha2. The real alpha2 is used to narrow down the channel defs via
the flags parameter. This patch just allows the FW to provide the
definitions for the "00" regdomain via the get_regd() function.


>
> > >> > One thing to be careful on all this new API design is to ensure that
> > >> > upon disconnect we want the driver to go back to the original state,
> > >> > whatever that is.
> > >>
> > >> This particular part is unrelated to connection state AFAICT. It just
> > >> allows someone (driver, user) to set the "00" regdomain.
> > >
> > > There are two things here, one is the custom world regulatory domain
> > > that firmware is setting, the other is the new API to allow an alpha2
> > > regulatory domain. The resetting of regulatory domains needs to ensure
> > > that if the first regulatory domain was a world regulatory domain, and
> > > then a driver alpha2 is set these regulatory domains will be applied in
> > > the same order when it disconnects. If the APIs are used properly this
> > > is guaranteed already for us and its one reason for seeing if we can
> > > just use existing APIs. See restore_custom_reg_settings() as an example.
> >
> > I believe there's no issue introduced by this code.
>
> You are extending usage of custom regulatory data to be hooked onto the
> orig_* parameters, which by design was made to help upkeep the minimum
> allowed set, the custom regulatory domain allows for *world roaming*
> which can mean reducing this set, but note it will never *enable* new
> channels. The alpha2 hint *can* enable new channels by design by
> updating orig_* parameters. These are two different type of regulatory
> domain hints.

See comment above.

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Tue, Jul 01, 2014 at 05:03:42PM +0300, Arik Nemtsov wrote:
> On Tue, Jul 1, 2014 at 1:21 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> >
> > On Tue, Jun 24, 2014 at 08:55:28AM +0300, Arik Nemtsov wrote:
> > > On Mon, Jun 23, 2014 at 11:48 PM, Luis R. Rodriguez
> > > <mcgrof@do-not-panic.com> wrote:
> > > > On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
> > > >> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
> > > >> <mcgrof@do-not-panic.com> wrote:
> > > >> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
> > > >> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
> > > >> >> <mcgrof@do-not-panic.com> wrote:
> > > >> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> > > >> >> >> Allow driver and user hints to set the "world" regulatory domain,
> > > >> >> >
> > > >> >> > NACK - as expressed in the other patch, letting the drivers to use the
> > > >> >> > new API to set the world regulatory domain doesn't make sense as we
> > > >> >> > have custom apply stuff, and the world regulatory domain is not
> > > >> >> > something dynamic.
> > > >> >>
> > > >> >> Well we want to set the world regdomain from FW. This obviously
> > > >> >> happens after wiphy registration, so I don't think the custom apply
> > > >> >> can be used here? (since we generally want cfg80211 to own the
> > > >> >> regdomain settings).
> > > >> >
> > > >> > Can the driver not obtain the world regulatory domain from firmware
> > > >> > prior to wiphy registration? Why not?
> > > >>
> > > >> Since the FW is not running yet :)
> > > >
> > > > Is that a limitation that can be corrected on the driver ? Can the wiphy
> > > > registration be moved to after wiphy firmware is loaded ? If this is too
> > > > hard I see this as a good reason to extend the API or add a new similar
> > > > call that would allow for this case. The reason for the restriction on wiphy
> > > > registration was due to the fact that we didn't want to mess with _orig
> > > > parameters after wiphy registration, and we didn't want drivers poking
> > > > with those. The _orig parameters then can be set by cfg80211 through two
> > > > ways, one is the driver say reads EEPROM stuff and sets the channel
> > > > structs with the restrictions it had prior to wiphy registration or if
> > > > drivers could deduce a regulatory domain structure they could use the
> > > > wiphy_apply_custom_regulatory() which would do the same for them.
> > > >
> > > > Note that what this does is let drivers fill a set of channel data
> > > > structures and then with these mechanisms set the max allowed superset.
> > > > Anything after this is a subset of the original set of allowed
> > > > parameters. Please review handle_channel().
> > > >
> > > > The difficulty in allowing changing the _orig stuff after wiphy
> > > > registration is we'd then have to ensure that the current state
> > > > of the regulatory machine is replicated on the device driver. Just
> > > > consider doing this properly and I think we'll be good.
> > >
> > > We can't access the FW before wiphy registration. In some scenarios
> > > the module is loaded during system boot, before we can really access
> > > the HW bus. We only start the FW on the first ifup.
> > > To allow ifup, we need the wiphy registered. We have no "hook" where
> > > we can register the wiphy after the wifi HW bus if fully available.
> >
> > What hw bus? Buses can probe, even if its a fake bus type of thing
> > you can use platform_device_register_simple(), look at
> > drivers/net/ethernet/8390/ne.c as a complex example.
> >
> > Also if a bus is not yet set up or if resouces are still being
> > brought up and the driver needs to be poked at a later time you can
> > verify what you need access for and return -EPROBE_DEFER upon probe
> > if things can't move forward which will force the driver to be
> > probed at a later time.
> >
> > > Nor do we want to find this hook, because of multiple platforms etc.
> >
> > I'm pretty sure you can find it, but I do also understand that
> > restructuring the driver is a bit of work so I'm fine with you saying
> > its a lot of work but saying its no possible seems not fully
> > supported yet.
> 
> That would be a better choice of words - we don't want to restructure
> the entire driver over this.

Its a trade off call in the end and so far I can't see a good reason to
enable dynamic custom world regulatory domains just because the "bus" is
not available, please do look into using -EPROBE_DEFER. This would
enable us to only consider the use case of the callback *only* to fetch
direct alpha2s regulatory domains from the driver/firmware dynamically
and that should be fairly easy to do. I rather incur a bit of penalty
over work on your driver than to allow an API for a corner case that has
not yet been quantitatively evaluated. Folks already complain about the
complexity on regulatory code, I need to be a hard ass and ensure more
simplicity one way or another. This is one way.

So again, sorry but NACK.

> > > But I'm not sure why you're mentioning the workings of
> > > handle_channel() for this patch. It doesn't allow changing the
> > > original flags set at registration. It just allows drivers and the
> > > user to assign the world regdomain, and also to change the native "00"
> > > definitions with the FW regdomain. It shouldn't harm anything AFAICT.
> >
> > You're missing the point for why I bring up handle_channel().
> > handle_channel() *can* update orig_* parameters for you for an alpha2
> > which you *shoud* consider. Additionally handle_channel() takes into
> > consideration the regulatory state machine and can apply a regulatory
> > domain to different wiphys by design to help with compliance. The
> > handle_channel_custom() is designed for custom regulatory domains
> > and do not propagate to other wiphys.
> 
> AFAICT, orig_flags only change if:
> 
> if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
>    request_wiphy && request_wiphy == wiphy &&
>    request_wiphy->regulatory_flags & REGULATORY_STRICT_REG) {

And also upon registration the values are kept as orig_, it lets the
driver register what the device is capable of and not allow anything
more.

  Luis

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Luis R. Rodriguez]

On Tue, Jul 01, 2014 at 04:07:03PM +0300, Arik Nemtsov wrote:
> On Tue, Jul 1, 2014 at 1:03 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> >> This feature is also primarily designed for systems which are not
> >> extensible, so you can't really add another device.
> >> I guess we'll have to solve this when the need arises.
> >
> > The patch in question does not address denying wiphy registration
> > if two drivers have get_regd() implemented, that's essentially what
> > this *should* be trying to do but:
> >
> > 1) Its not documented above
> > 2) The limitation of the patch in no way is part of matching the
> >    requirement you mention. That is, the requirement to have Intel's
> >    driver as "dictator" has nothing to do with allowing or not
> >    multiple similar drivers that can help with compliance by providing
> >    their own regulatory dynamically.
> > 3) You are putting the requirement of implmenting support for
> >    multiple drivers with get_regd() on the next user of get_regd()
> >    which wants to integrate support for an intel card with theirs,
> >    that's unfair and far sighted for an implementation.
> >
> > The requirement you have has nothing to do with the limitation
> > you have so this patch is unacceptable. I also provided recommendations
> > on how you can lift this limitation, so it shouldn't be hard.
> 
> I already prevent the registration of a second "get_regd" callback. I
> just re-read your previous email and I'm not sure what's the
> recommendation here. The 2-card scenario where one of the cards
> doesn't use "get_regd" should be fine - it will just use the
> regulatory settings from the "get_regd" one. Only the
> 2-cards-using-"get_regd" scenario is not supported, and AFAICT it
> doesn't exist in practice, and also requires complicated treatment
> (perhaps the two "dictators" differ in their definition of the
> regdomains?)

The recommendation is to add support for more than one driver
that has the get_regd() callback, if your driver does not want
to allow others to register more than 1 driver on a system with
the reg_regd() callback add a flag and then the blame will be
put on you to justify this. Then customers who buy your products
won't use your devices in undesired combinations and I expect
tons of bug reports might end up being filed if another vendor
ends up going down the get_regd() path.

> >> > Lets be upfront -- how many regulatory domains will you guys return
> >> > right now in which the same alpha2 will be kept? I don't like this for
> >> > one bit at all, if you are going to return a 99 for a specific alpha2
> >> > it must mean firmware / driver has the ability to search for an alpha2
> >> > for a set of custom regulatory domains, which should also mean you
> >> > should be able to return a set of alpha2s that can be used for the
> >> > custom regulatory domain, so I'd like to see that added as a possible
> >> > return set. This should set expectations for users, build a better
> >> > understanding of how all this works, and most importantly enable easy
> >> > and shorter scraping of the firmware for our own research and
> >> > development and advancement of wireless-regdb as otherwise we'd have
> >> > to loop over every alpha2 and then deduce grouping. This is the only
> >> > way I see this being reasonable to accept upstream.
> >>
> >> I think you're misunderstanding things here. We only give back real
> >> regulatory domains. The "99" is only used as a means the get the
> >> currently NVRAM flashed regdomain from FW.
> >
> > If its real regulatory domains then why is the alpha2 not set?
> 
> We call get_regd with "99", expecting the driver/FW to reply with the
> current alpha2 as burned in ROM/FW.

Ah! OK then yes I was misunderstanding the use case here. If you change
the driver to load the firmware before registration though you can
then instead use regulatory_hint() properly here.

> >> > We also should be clear now in the documentation about the differences
> >> > and purpose behind this API and the custom regulatory which tons of
> >> > folks already use. In this case the requirement was dynamic changes
> >> > and to ensure these changes get back to userspace permissively as
> >> > otherwise they could not. The way I see it the custom stuff should be
> >> > used for custom world regulatory domains -- this new API is for
> >> > specific alpha2s. This should also mean then that this API should
> >> > *not* be used to query the firmware for the world regulatory domain.
> >>
> >> Why not? What's problematic here?
> >> I would much prefer to keep a single API for everything.

You are adding new API for a driver work around. The regulatory code
is already complex as is, adding support for dynamic custom world
regulatory hints is something I'd like to avoid if we can.

> > Because APIs already exist for some of what you want already. In
> > particular it seems to me you can implement the alpha2 hint with
> > the callback by using a capability flag and having the core deal
> > the callback and then applying the regulatory domain.
> > Please check but I think you might want alpha2 cases to have handle_channel()
> > apply the rules on the orig_* parameters which which happens when
> > REGULATORY_STRICT_REG is set, ie, you may want to require having
> > set the callback to have REGULATORY_STRICT_REG set. This allows
> > drivers to enforce the orig_ values reflecting the regulatory
> > domain data structure. regulatory_hint() also propagates to other drivers
> > as well and the code also considers the regulatory state for older
> > types of requests to decide what it should do for each driver and
> > each case given that we have very different drivers with different
> > regulatory requirements.
> >
> > The wiphy_apply_custom_regulatory() is only for custom regulatory
> > domains, the difference is that wiphy_apply_custom_regulatory() doesn't
> > take into consideration the regulatory state machine and changes that
> > have happened over time, it is also specific to only one wiphy. Lastly
> > it doesn't update orig_* parameters.
> >
> > For the non alpha2 cases it seems you want a post registration
> > wiphy_apply_custom_regulatory() type of call which would only apply
> > to that wiphy, never update orig_* parameters, and consider then also
> > the regulatory state machine. If you don't care about the regulatory
> > state machine that makes you special and you must deal with both.
> >
> > We can't blend the two types of calls because they are very different
> > in terms of purpose and also the implications on other drivers.
> > Adding a new API to let custom world regulatory domains or specific
> > alpha2s would make the other two APIs kind of pointless, we want to
> > be specific.
> 
> I don't want to touch orig_flags, since the FW burned regdomain can be
> overridden. I do want to register the wiphy very early, before the FW
> is loaded. Also I want to get the default alpha2 and associated
> regdomain from the FW at runtime. Don't think any combination of the
> current APIs allows that.

You are right but you seem to be adding a lot of API / code for
something the driver can instead be changed with -EPROBE_DEFER and
then use proper APIs for. The only thing then you'd need to add is
new API to let the driver's regulatory domain be sent out from
firmware, as a source and that it seems you can easily add with
the capability flag and the get_regd() callback.

> I can prepare a separate API call just for the purpose of getting the
> initial alpha2+regdomain setting, but I have to say it seems pretty
> pointless. The current flow where we send "99" and get the real alpha2
> works well.

Working well is different then scaling for all 802.11 vendors on the
Linux kernel, part of my job is to push you to consider helping us
write APIs that help things to be cleaner. If some functionality is
missing as part of the core that doesn't let us do something we
extend it.

> >> >> +                                                                                           Returning
> >> >> + *     NULL will cause the regdomain to remain the same.
> >> >
> >> >
> >> > What do you mean by this? Can you please elaborate exactly what this means?
> >>
> >> It means we will ignore the request. Not sure what's to elaborate.
> >
> > Why would you do that? Explain why a regulatory hint would be issued
> > and then the driver could decide to ignore it. Seems kind of pointless
> > if the driver was the one originally issuing the request!
> 
> The driver isn't the only one that can issue requests. Userspace can
> issue them too (and in fact does). Basically the driver/FW can say
> "leave the same regdomain, don't touch it".
> It's a policy decision.

Sure, but why would the driver send a request to the core only to then
say things are OK and don't do anything. Seems rather pointless.

> >> >>  static struct regulatory_request core_request_world = {
> >> >> @@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
> >> >>   */
> >> >>  static bool reg_is_indoor;
> >> >>
> >> >> +/*
> >> >> + * Wiphy with a get_regd() callback that can provide regulatory information
> >> >> + * when the country code changes. Only the first wiphy registered with the
> >> >> + * get_regd callback will be called to provide a regdomain on country-code
> >> >> + * changes.
> >> >> + * (protected by RTNL)
> >> >> + */
> >> >> +static struct wiphy *regd_info_wiphy;
> >> >
> >> > Note all my feedback on the kdoc comment about this. This obviously
> >> > doesn't scale well but more importantly given that devices bus
> >> > configuration can change (regardless of what systemd folks think) it
> >> > means that we can get different results on a system with 2 internal
> >> > cards. Systems with multiple built in 802.11 cards were something of a
> >> > theoretical myth back in the day when we started with 802.11 but not
> >> > anymore. For example I'm very aware of some Freebox 802.11 APs with
> >> > multiple built in 802.11 cards and from different vendors!  No only
> >> > can this lead to issues with cards on different buses and races
> >> > therein, but it could also produce different results on different
> >> > architectures.
> >>
> >> Agree it's a hairy issue, but I think we should get to this in a later
> >> patch. It's not a simple issue or "fix".
> >> Might need different policies, intersections, etc.
> >
> > Exactly, no address this, otheriwse having an intel driver present would
> > mean not being able to register other similar type of drivers loaded
> > on a system. This is unacceptable.
> 
> I agree it will prevent other "get_regd" cards from registering on the
> same system, but I don't think the limitation really exists in
> practice.

It does't matter, you can't assume your restrictions are universal.

> These are not Intel cards one can buy off the shelf. They are soldered
> to platforms that are non-extensible. It also means I have no ability
> to test this code.

Make a capability thing or requirement flag somewhere and have it be
stuck to your driver, don't make it universal.

> >> >> +
> >> >> +       regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
> >> >> +       if (IS_ERR(regd))
> >> >> +               return -EIO;
> >> >> +
> >> >
> >> > You're not feeding the firmware information on availability of the
> >> > userspace regulatory domain, in the case of it was allowed, it seems
> >> > to me that firmware might make a better decision in the case that
> >> > userspace lacks information. Userspace can also be upgraded at runtime
> >> > dynamically so whether or not userspace has a regulatory domain can
> >> > change at run time. Please consider this.
> >>
> >> The Intel use case doesn't use userspace regulatory information at
> >> all. This can be extended later I guess.
> >
> > The "Intel use case" is different than "I'm adding a general API" case.
> > You can easily extend the API to pass the regulatory domain for
> > inspection even if you don't use it on the driver.
> 
> Perhaps this shouldn't be part of this feature? It's of no use to the
> driver or to the user of the system. If someone wants to use userspace
> information, they should extend the APIs IMHO.

Sure.. I can just ensure this likely will get extended soon.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Wed, Jul 2, 2014 at 5:04 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jul 01, 2014 at 05:03:42PM +0300, Arik Nemtsov wrote:
>> On Tue, Jul 1, 2014 at 1:21 AM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> >
>> > On Tue, Jun 24, 2014 at 08:55:28AM +0300, Arik Nemtsov wrote:
>> > > On Mon, Jun 23, 2014 at 11:48 PM, Luis R. Rodriguez
>> > > <mcgrof@do-not-panic.com> wrote:
>> > > > On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
>> > > >> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
>> > > >> <mcgrof@do-not-panic.com> wrote:
>> > > >> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
>> > > >> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
>> > > >> >> <mcgrof@do-not-panic.com> wrote:
>> > > >> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> > > >> >> >> Allow driver and user hints to set the "world" regulatory domain,
>> > > >> >> >
>> > > >> >> > NACK - as expressed in the other patch, letting the drivers to use the
>> > > >> >> > new API to set the world regulatory domain doesn't make sense as we
>> > > >> >> > have custom apply stuff, and the world regulatory domain is not
>> > > >> >> > something dynamic.
>> > > >> >>
>> > > >> >> Well we want to set the world regdomain from FW. This obviously
>> > > >> >> happens after wiphy registration, so I don't think the custom apply
>> > > >> >> can be used here? (since we generally want cfg80211 to own the
>> > > >> >> regdomain settings).
>> > > >> >
>> > > >> > Can the driver not obtain the world regulatory domain from firmware
>> > > >> > prior to wiphy registration? Why not?
>> > > >>
>> > > >> Since the FW is not running yet :)
>> > > >
>> > > > Is that a limitation that can be corrected on the driver ? Can the wiphy
>> > > > registration be moved to after wiphy firmware is loaded ? If this is too
>> > > > hard I see this as a good reason to extend the API or add a new similar
>> > > > call that would allow for this case. The reason for the restriction on wiphy
>> > > > registration was due to the fact that we didn't want to mess with _orig
>> > > > parameters after wiphy registration, and we didn't want drivers poking
>> > > > with those. The _orig parameters then can be set by cfg80211 through two
>> > > > ways, one is the driver say reads EEPROM stuff and sets the channel
>> > > > structs with the restrictions it had prior to wiphy registration or if
>> > > > drivers could deduce a regulatory domain structure they could use the
>> > > > wiphy_apply_custom_regulatory() which would do the same for them.
>> > > >
>> > > > Note that what this does is let drivers fill a set of channel data
>> > > > structures and then with these mechanisms set the max allowed superset.
>> > > > Anything after this is a subset of the original set of allowed
>> > > > parameters. Please review handle_channel().
>> > > >
>> > > > The difficulty in allowing changing the _orig stuff after wiphy
>> > > > registration is we'd then have to ensure that the current state
>> > > > of the regulatory machine is replicated on the device driver. Just
>> > > > consider doing this properly and I think we'll be good.
>> > >
>> > > We can't access the FW before wiphy registration. In some scenarios
>> > > the module is loaded during system boot, before we can really access
>> > > the HW bus. We only start the FW on the first ifup.
>> > > To allow ifup, we need the wiphy registered. We have no "hook" where
>> > > we can register the wiphy after the wifi HW bus if fully available.
>> >
>> > What hw bus? Buses can probe, even if its a fake bus type of thing
>> > you can use platform_device_register_simple(), look at
>> > drivers/net/ethernet/8390/ne.c as a complex example.
>> >
>> > Also if a bus is not yet set up or if resouces are still being
>> > brought up and the driver needs to be poked at a later time you can
>> > verify what you need access for and return -EPROBE_DEFER upon probe
>> > if things can't move forward which will force the driver to be
>> > probed at a later time.
>> >
>> > > Nor do we want to find this hook, because of multiple platforms etc.
>> >
>> > I'm pretty sure you can find it, but I do also understand that
>> > restructuring the driver is a bit of work so I'm fine with you saying
>> > its a lot of work but saying its no possible seems not fully
>> > supported yet.
>>
>> That would be a better choice of words - we don't want to restructure
>> the entire driver over this.
>
> Its a trade off call in the end and so far I can't see a good reason to
> enable dynamic custom world regulatory domains just because the "bus" is
> not available, please do look into using -EPROBE_DEFER. This would
> enable us to only consider the use case of the callback *only* to fetch
> direct alpha2s regulatory domains from the driver/firmware dynamically
> and that should be fairly easy to do. I rather incur a bit of penalty
> over work on your driver than to allow an API for a corner case that has
> not yet been quantitatively evaluated. Folks already complain about the
> complexity on regulatory code, I need to be a hard ass and ensure more
> simplicity one way or another. This is one way.
>
> So again, sorry but NACK.

I'm not sure why you're NACKing this patch for this reason - we're not
really only using this on boot. Basically this is a valid scenario:

1. User removes the SIM from his cellphone.
2. A user hint is sent with alpha2 "99", telling the driver this -
"please reset the regdomain to whatever is default, i have no idea
what this is"
3. The driver/FW replies with a valid alpha2 + regdom settings, where
alpha2 can also be "00".

It just so happens that the above scenario also happens on boot (we
have no idea what's the right alpha2).

Arik

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arik Nemtsov]

On Wed, Jul 2, 2014 at 5:23 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jul 01, 2014 at 04:07:03PM +0300, Arik Nemtsov wrote:
>> On Tue, Jul 1, 2014 at 1:03 AM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> >> This feature is also primarily designed for systems which are not
>> >> extensible, so you can't really add another device.
>> >> I guess we'll have to solve this when the need arises.
>> >
>> > The patch in question does not address denying wiphy registration
>> > if two drivers have get_regd() implemented, that's essentially what
>> > this *should* be trying to do but:
>> >
>> > 1) Its not documented above
>> > 2) The limitation of the patch in no way is part of matching the
>> >    requirement you mention. That is, the requirement to have Intel's
>> >    driver as "dictator" has nothing to do with allowing or not
>> >    multiple similar drivers that can help with compliance by providing
>> >    their own regulatory dynamically.
>> > 3) You are putting the requirement of implmenting support for
>> >    multiple drivers with get_regd() on the next user of get_regd()
>> >    which wants to integrate support for an intel card with theirs,
>> >    that's unfair and far sighted for an implementation.
>> >
>> > The requirement you have has nothing to do with the limitation
>> > you have so this patch is unacceptable. I also provided recommendations
>> > on how you can lift this limitation, so it shouldn't be hard.
>>
>> I already prevent the registration of a second "get_regd" callback. I
>> just re-read your previous email and I'm not sure what's the
>> recommendation here. The 2-card scenario where one of the cards
>> doesn't use "get_regd" should be fine - it will just use the
>> regulatory settings from the "get_regd" one. Only the
>> 2-cards-using-"get_regd" scenario is not supported, and AFAICT it
>> doesn't exist in practice, and also requires complicated treatment
>> (perhaps the two "dictators" differ in their definition of the
>> regdomains?)
>
> The recommendation is to add support for more than one driver
> that has the get_regd() callback, if your driver does not want
> to allow others to register more than 1 driver on a system with
> the reg_regd() callback add a flag and then the blame will be
> put on you to justify this. Then customers who buy your products
> won't use your devices in undesired combinations and I expect
> tons of bug reports might end up being filed if another vendor
> ends up going down the get_regd() path.

Not sure I'm getting you. You're suggesting a new regulatory flag that
ensures a single get_regd registration, and fails others?
So that even if someone extends get_regd() to multiple devices, we'll
always remain the single one? I'm ok with this.

About your other comments - let's continue the discussion at the
"cfg80211: accept world/same regdom from driver/user hints" patch. I
don't think the PROBE_DEFER thing really covers the use-case we've had
in mind.

Arik

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Luis R. Rodriguez]

On Thu, Jul 03, 2014 at 01:04:23PM +0300, Arik Nemtsov wrote:
> On Wed, Jul 2, 2014 at 5:23 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Tue, Jul 01, 2014 at 04:07:03PM +0300, Arik Nemtsov wrote:
> >> On Tue, Jul 1, 2014 at 1:03 AM, Luis R. Rodriguez
> >> <mcgrof@do-not-panic.com> wrote:
> >> >> This feature is also primarily designed for systems which are not
> >> >> extensible, so you can't really add another device.
> >> >> I guess we'll have to solve this when the need arises.
> >> >
> >> > The patch in question does not address denying wiphy registration
> >> > if two drivers have get_regd() implemented, that's essentially what
> >> > this *should* be trying to do but:
> >> >
> >> > 1) Its not documented above
> >> > 2) The limitation of the patch in no way is part of matching the
> >> >    requirement you mention. That is, the requirement to have Intel's
> >> >    driver as "dictator" has nothing to do with allowing or not
> >> >    multiple similar drivers that can help with compliance by providing
> >> >    their own regulatory dynamically.
> >> > 3) You are putting the requirement of implmenting support for
> >> >    multiple drivers with get_regd() on the next user of get_regd()
> >> >    which wants to integrate support for an intel card with theirs,
> >> >    that's unfair and far sighted for an implementation.
> >> >
> >> > The requirement you have has nothing to do with the limitation
> >> > you have so this patch is unacceptable. I also provided recommendations
> >> > on how you can lift this limitation, so it shouldn't be hard.
> >>
> >> I already prevent the registration of a second "get_regd" callback. I
> >> just re-read your previous email and I'm not sure what's the
> >> recommendation here. The 2-card scenario where one of the cards
> >> doesn't use "get_regd" should be fine - it will just use the
> >> regulatory settings from the "get_regd" one. Only the
> >> 2-cards-using-"get_regd" scenario is not supported, and AFAICT it
> >> doesn't exist in practice, and also requires complicated treatment
> >> (perhaps the two "dictators" differ in their definition of the
> >> regdomains?)
> >
> > The recommendation is to add support for more than one driver
> > that has the get_regd() callback, if your driver does not want
> > to allow others to register more than 1 driver on a system with
> > the reg_regd() callback add a flag and then the blame will be
> > put on you to justify this. Then customers who buy your products
> > won't use your devices in undesired combinations and I expect
> > tons of bug reports might end up being filed if another vendor
> > ends up going down the get_regd() path.
> 
> Not sure I'm getting you. You're suggesting a new regulatory flag that
> ensures a single get_regd registration, and fails others?

Yeah a wiphy flag that would inform regulatory code that this is the
interpretation a vendor has.

> So that even if someone extends get_regd() to multiple devices, we'll
> always remain the single one?

Right, if the flag can be used to update a refcount, if the refcount
for this flag is > 0 then that means at least one driver is present
already that uses get_regd() and it would disallow more any other
wiphys to register if it also had get_regd(). That would let you do
what you want -- but it means you still have to add the code to support
the cases that do not have this restriction to let vendors who don't
have such nutty requirements to exist and so that they can sell their
products with other vendors and integration with partners on systems
that would need multiple vendors with drivers that have get_regd().

> I'm ok with this.

Ok.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Thu, Jul 03, 2014 at 12:58:20PM +0300, Arik Nemtsov wrote:
> On Wed, Jul 2, 2014 at 5:04 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > On Tue, Jul 01, 2014 at 05:03:42PM +0300, Arik Nemtsov wrote:
> >> On Tue, Jul 1, 2014 at 1:21 AM, Luis R. Rodriguez
> >> <mcgrof@do-not-panic.com> wrote:
> >> >
> >> > On Tue, Jun 24, 2014 at 08:55:28AM +0300, Arik Nemtsov wrote:
> >> > > On Mon, Jun 23, 2014 at 11:48 PM, Luis R. Rodriguez
> >> > > <mcgrof@do-not-panic.com> wrote:
> >> > > > On Mon, Jun 23, 2014 at 09:34:13PM +0300, Arik Nemtsov wrote:
> >> > > >> On Mon, Jun 23, 2014 at 9:26 PM, Luis R. Rodriguez
> >> > > >> <mcgrof@do-not-panic.com> wrote:
> >> > > >> > On Mon, Jun 23, 2014 at 08:14:43PM +0300, Arik Nemtsov wrote:
> >> > > >> >> On Thu, Jun 19, 2014 at 2:34 AM, Luis R. Rodriguez
> >> > > >> >> <mcgrof@do-not-panic.com> wrote:
> >> > > >> >> > On Tue, Jun 10, 2014 at 11:55 PM, Arik Nemtsov <arik@wizery.com> wrote:
> >> > > >> >> >> Allow driver and user hints to set the "world" regulatory domain,
> >> > > >> >> >
> >> > > >> >> > NACK - as expressed in the other patch, letting the drivers to use the
> >> > > >> >> > new API to set the world regulatory domain doesn't make sense as we
> >> > > >> >> > have custom apply stuff, and the world regulatory domain is not
> >> > > >> >> > something dynamic.
> >> > > >> >>
> >> > > >> >> Well we want to set the world regdomain from FW. This obviously
> >> > > >> >> happens after wiphy registration, so I don't think the custom apply
> >> > > >> >> can be used here? (since we generally want cfg80211 to own the
> >> > > >> >> regdomain settings).
> >> > > >> >
> >> > > >> > Can the driver not obtain the world regulatory domain from firmware
> >> > > >> > prior to wiphy registration? Why not?
> >> > > >>
> >> > > >> Since the FW is not running yet :)
> >> > > >
> >> > > > Is that a limitation that can be corrected on the driver ? Can the wiphy
> >> > > > registration be moved to after wiphy firmware is loaded ? If this is too
> >> > > > hard I see this as a good reason to extend the API or add a new similar
> >> > > > call that would allow for this case. The reason for the restriction on wiphy
> >> > > > registration was due to the fact that we didn't want to mess with _orig
> >> > > > parameters after wiphy registration, and we didn't want drivers poking
> >> > > > with those. The _orig parameters then can be set by cfg80211 through two
> >> > > > ways, one is the driver say reads EEPROM stuff and sets the channel
> >> > > > structs with the restrictions it had prior to wiphy registration or if
> >> > > > drivers could deduce a regulatory domain structure they could use the
> >> > > > wiphy_apply_custom_regulatory() which would do the same for them.
> >> > > >
> >> > > > Note that what this does is let drivers fill a set of channel data
> >> > > > structures and then with these mechanisms set the max allowed superset.
> >> > > > Anything after this is a subset of the original set of allowed
> >> > > > parameters. Please review handle_channel().
> >> > > >
> >> > > > The difficulty in allowing changing the _orig stuff after wiphy
> >> > > > registration is we'd then have to ensure that the current state
> >> > > > of the regulatory machine is replicated on the device driver. Just
> >> > > > consider doing this properly and I think we'll be good.
> >> > >
> >> > > We can't access the FW before wiphy registration. In some scenarios
> >> > > the module is loaded during system boot, before we can really access
> >> > > the HW bus. We only start the FW on the first ifup.
> >> > > To allow ifup, we need the wiphy registered. We have no "hook" where
> >> > > we can register the wiphy after the wifi HW bus if fully available.
> >> >
> >> > What hw bus? Buses can probe, even if its a fake bus type of thing
> >> > you can use platform_device_register_simple(), look at
> >> > drivers/net/ethernet/8390/ne.c as a complex example.
> >> >
> >> > Also if a bus is not yet set up or if resouces are still being
> >> > brought up and the driver needs to be poked at a later time you can
> >> > verify what you need access for and return -EPROBE_DEFER upon probe
> >> > if things can't move forward which will force the driver to be
> >> > probed at a later time.
> >> >
> >> > > Nor do we want to find this hook, because of multiple platforms etc.
> >> >
> >> > I'm pretty sure you can find it, but I do also understand that
> >> > restructuring the driver is a bit of work so I'm fine with you saying
> >> > its a lot of work but saying its no possible seems not fully
> >> > supported yet.
> >>
> >> That would be a better choice of words - we don't want to restructure
> >> the entire driver over this.
> >
> > Its a trade off call in the end and so far I can't see a good reason to
> > enable dynamic custom world regulatory domains just because the "bus" is
> > not available, please do look into using -EPROBE_DEFER. This would
> > enable us to only consider the use case of the callback *only* to fetch
> > direct alpha2s regulatory domains from the driver/firmware dynamically
> > and that should be fairly easy to do. I rather incur a bit of penalty
> > over work on your driver than to allow an API for a corner case that has
> > not yet been quantitatively evaluated. Folks already complain about the
> > complexity on regulatory code, I need to be a hard ass and ensure more
> > simplicity one way or another. This is one way.
> >
> > So again, sorry but NACK.
> 
> I'm not sure why you're NACKing this patch for this reason - we're not
> really only using this on boot. Basically this is a valid scenario:

Lets start with the 0 case with what I am recommending:

0. The driver calls wiphy_apply_custom_regulatory(), this applies
settings to the channels's data structures before wiphy registration.
The driver then calls wiphy_register(). The regulatory core then uses
the set channel data structures and caches the values upon registration
on the orig_* parameters, the orig_* parameters are to be used only
internally by the core, its used here and as explained below also can
be requested to be updated when REGULATORY_STRICT_REG is used.

> 1. User removes the SIM from his cellphone.

This can implemented to trigger a reset to regulatory, for example we
already call a reset to regulatory when we disconnect from an AP or when
we suspend and resume, this is done since we cannot ensure the
information received earlier will be valid anymore, it also clears all
the heuristics on helping with world roaming like clearly NO-IR (iniatiting
radiation) flags for beacon hints, which would otherwise not enable a
device to intiate radiation if it was world roaming on some channels.
If done this way regulatory restore_custom_reg_settings() will update the
device based on on the orig_* parameters which are values used from the
original registration. Consider it as kind of like resetting the router when
you put the pin in it, things gets wiped out, we start from scratch again.

> 2. A user hint is sent with alpha2 "99", telling the driver this -
> "please reset the regdomain to whatever is default, i have no idea
> what this is"

The reason I am NACK'ing is that you should know the default before
registration, right now you had a bus issue which you can address
as well with -EPROBE_DEFER. I don't want to allow a dynamic custom
world regulatory domain settings as it doesn't make sense yet, there's
no reason for it and we already have APIs to handle this provided you
can fetch that regulatory domain before registration.

The callback for reg_get() to fetch regulatory domain data structures
from firmware is welcomed but only for real country ISO3166-alpha2 country
codes, and I can see that could likely be dynamic.

> 3. The driver/FW replies with a valid alpha2 + regdom settings, where
> alpha2 can also be "00".

Again this would not be needed. Now, if the driver / phone decides it
*does* want to use a new alpha2 (say from cell phone base station a cell
phone is connected to) the proper APIs can be used and passed, and in
fact if the device had used REGULATORY_STRICT_REG a regulatory_hint()
would override the *_orig parameters to ensure that upon reset that
default would be used as well. This would even allow you to use dynamic
override on the orig_* parameters, provided of course you trust the
source, and also that the regulatory hint is for a specific
ISO3166-alpha2.

The devices that would use the REGULATORY_STRICT_REG and
regulatory_hint() today after registration are for example APs. Most 802.11
devices on laptops however use a custom world regulatory domain, and then
techniques are used to help with world roaming such as becon hints, etc. For
some mobile devices however a wiphy_apply_custom_regulatory() can be used
which would set REGULATORY_CUSTOM_REG, and then if they wanted suppport
for cellular base station hints regulatory_hint() from userspace can be
used that will remove REGULATORY_CUSTOM_REG, and the new alpha2 is
trusted.

> It just so happens that the above scenario also happens on boot (we
> have no idea what's the right alpha2).

Upon boot you can should be able to do what you need to do to defer
probing until ready. Let me know if the core does not provide the means
to do this properly, I'd be interested to learn more about that as I'm
sure Greg might be too.

  Luis

Re: [PATCH 2/5] cfg80211: allow drivers to provide regulatory settings

[Arik Nemtsov]

On Fri, Jul 4, 2014 at 12:44 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Thu, Jul 03, 2014 at 01:04:23PM +0300, Arik Nemtsov wrote:
>> On Wed, Jul 2, 2014 at 5:23 AM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > On Tue, Jul 01, 2014 at 04:07:03PM +0300, Arik Nemtsov wrote:
>> >> On Tue, Jul 1, 2014 at 1:03 AM, Luis R. Rodriguez
>> >> <mcgrof@do-not-panic.com> wrote:
>> >> >> This feature is also primarily designed for systems which are not
>> >> >> extensible, so you can't really add another device.
>> >> >> I guess we'll have to solve this when the need arises.
>> >> >
>> >> > The patch in question does not address denying wiphy registration
>> >> > if two drivers have get_regd() implemented, that's essentially what
>> >> > this *should* be trying to do but:
>> >> >
>> >> > 1) Its not documented above
>> >> > 2) The limitation of the patch in no way is part of matching the
>> >> >    requirement you mention. That is, the requirement to have Intel's
>> >> >    driver as "dictator" has nothing to do with allowing or not
>> >> >    multiple similar drivers that can help with compliance by providing
>> >> >    their own regulatory dynamically.
>> >> > 3) You are putting the requirement of implmenting support for
>> >> >    multiple drivers with get_regd() on the next user of get_regd()
>> >> >    which wants to integrate support for an intel card with theirs,
>> >> >    that's unfair and far sighted for an implementation.
>> >> >
>> >> > The requirement you have has nothing to do with the limitation
>> >> > you have so this patch is unacceptable. I also provided recommendations
>> >> > on how you can lift this limitation, so it shouldn't be hard.
>> >>
>> >> I already prevent the registration of a second "get_regd" callback. I
>> >> just re-read your previous email and I'm not sure what's the
>> >> recommendation here. The 2-card scenario where one of the cards
>> >> doesn't use "get_regd" should be fine - it will just use the
>> >> regulatory settings from the "get_regd" one. Only the
>> >> 2-cards-using-"get_regd" scenario is not supported, and AFAICT it
>> >> doesn't exist in practice, and also requires complicated treatment
>> >> (perhaps the two "dictators" differ in their definition of the
>> >> regdomains?)
>> >
>> > The recommendation is to add support for more than one driver
>> > that has the get_regd() callback, if your driver does not want
>> > to allow others to register more than 1 driver on a system with
>> > the reg_regd() callback add a flag and then the blame will be
>> > put on you to justify this. Then customers who buy your products
>> > won't use your devices in undesired combinations and I expect
>> > tons of bug reports might end up being filed if another vendor
>> > ends up going down the get_regd() path.
>>
>> Not sure I'm getting you. You're suggesting a new regulatory flag that
>> ensures a single get_regd registration, and fails others?
>
> Yeah a wiphy flag that would inform regulatory code that this is the
> interpretation a vendor has.
>
>> So that even if someone extends get_regd() to multiple devices, we'll
>> always remain the single one?
>
> Right, if the flag can be used to update a refcount, if the refcount
> for this flag is > 0 then that means at least one driver is present
> already that uses get_regd() and it would disallow more any other
> wiphys to register if it also had get_regd(). That would let you do
> what you want -- but it means you still have to add the code to support
> the cases that do not have this restriction to let vendors who don't
> have such nutty requirements to exist and so that they can sell their
> products with other vendors and integration with partners on systems
> that would need multiple vendors with drivers that have get_regd().

Sure thing.

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

>
> Lets start with the 0 case with what I am recommending:
>
> 0. The driver calls wiphy_apply_custom_regulatory(), this applies
> settings to the channels's data structures before wiphy registration.
> The driver then calls wiphy_register(). The regulatory core then uses
> the set channel data structures and caches the values upon registration
> on the orig_* parameters, the orig_* parameters are to be used only
> internally by the core, its used here and as explained below also can
> be requested to be updated when REGULATORY_STRICT_REG is used.

This is not appropriate for the intel use-case. The orig_flags must be
all 0, since the initial regulatory domain doesn't mean much.
It can be changed at any time by usermode (= telephony indication). We
do not wish to intersect with orig_flags, which always happens AFAICT.

>
>> 1. User removes the SIM from his cellphone.
>
> This can implemented to trigger a reset to regulatory, for example we
> already call a reset to regulatory when we disconnect from an AP or when
> we suspend and resume, this is done since we cannot ensure the
> information received earlier will be valid anymore, it also clears all
> the heuristics on helping with world roaming like clearly NO-IR (iniatiting
> radiation) flags for beacon hints, which would otherwise not enable a
> device to intiate radiation if it was world roaming on some channels.
> If done this way regulatory restore_custom_reg_settings() will update the
> device based on on the orig_* parameters which are values used from the
> original registration. Consider it as kind of like resetting the router when
> you put the pin in it, things gets wiped out, we start from scratch again.

While the reset notion is fine, this fails for the normal use case. If
we set something other then 0 for orig_flags, handle_channel() will
always "or" with them when switching between regular alpha2 domains
(e.g. roaming).

>
>> 2. A user hint is sent with alpha2 "99", telling the driver this -
>> "please reset the regdomain to whatever is default, i have no idea
>> what this is"
>
> The reason I am NACK'ing is that you should know the default before
> registration, right now you had a bus issue which you can address
> as well with -EPROBE_DEFER. I don't want to allow a dynamic custom
> world regulatory domain settings as it doesn't make sense yet, there's
> no reason for it and we already have APIs to handle this provided you
> can fetch that regulatory domain before registration.
>
> The callback for reg_get() to fetch regulatory domain data structures
> from firmware is welcomed but only for real country ISO3166-alpha2 country
> codes, and I can see that could likely be dynamic.

I'll wait for you to address the issue above.

>
>> 3. The driver/FW replies with a valid alpha2 + regdom settings, where
>> alpha2 can also be "00".
>
> Again this would not be needed. Now, if the driver / phone decides it
> *does* want to use a new alpha2 (say from cell phone base station a cell
> phone is connected to) the proper APIs can be used and passed, and in
> fact if the device had used REGULATORY_STRICT_REG a regulatory_hint()
> would override the *_orig parameters to ensure that upon reset that
> default would be used as well. This would even allow you to use dynamic
> override on the orig_* parameters, provided of course you trust the
> source, and also that the regulatory hint is for a specific
> ISO3166-alpha2.

REGULATORY_STRICT_REG only sets orig_flags for driver hints, not user
hints. We'll have to identify the wiphy for user-hints etc. I'm also
very much reluctant to use the STRICT flag. Seems they were engineered
for specific scenarios/cards, and will generally work well for
world-roaming cards that want country IE updates. The CUSTOM
regulatory flag also has a specific use-case, which doesn't fit intel.

The simpler use-case of a "regular" card would work well here,
provided the regulatory settings come from FW.

>
> The devices that would use the REGULATORY_STRICT_REG and
> regulatory_hint() today after registration are for example APs. Most 802.11
> devices on laptops however use a custom world regulatory domain, and then
> techniques are used to help with world roaming such as becon hints, etc. For
> some mobile devices however a wiphy_apply_custom_regulatory() can be used
> which would set REGULATORY_CUSTOM_REG, and then if they wanted suppport
> for cellular base station hints regulatory_hint() from userspace can be
> used that will remove REGULATORY_CUSTOM_REG, and the new alpha2 is
> trusted.

But then the ability to restore the orig flag is lost.. since we can't
restore the CUSTOM flag. And again, removing the flag requires a
driver hint, not a user one.
Like I said, the scenarios are pretty specific.

>
>> It just so happens that the above scenario also happens on boot (we
>> have no idea what's the right alpha2).
>
> Upon boot you can should be able to do what you need to do to defer
> probing until ready. Let me know if the core does not provide the means
> to do this properly, I'd be interested to learn more about that as I'm
> sure Greg might be too.

I haven't explored the technical aspects here, but I'll give you the
gist of it. Right now we register the wiphy on boot, but load the fw
on the first "ifup". This ifup can come from some external component
(maybe Android framework?) that knows the entire system state (i.e.
FS, bus, rfkill, etc), and can ensure everything will work out OK. Now
there different HW platforms here, different software framework
(Android, ChromeOS, etc) and different bus types we support.

You're basically asking for us to find a "hook" in each configuration
in which we can load the FW during the wiphy registration, just before
ifup arrives (but not after - since ifup will fail without a wiphy).
The "hook" depends on the entire system state, not only kernel. It
will likely be hard or impossible to do right is some situations. It's
akin to shooting ourselves in the foot. :)

Therefore I would be glad if you reconsider a dynamic initial regulatory domain.

Regards,
Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Sun, Jul 6, 2014 at 10:22 PM, Arik Nemtsov <arik@wizery.com> wrote:
>>
>> Lets start with the 0 case with what I am recommending:
>>
>> 0. The driver calls wiphy_apply_custom_regulatory(), this applies
>> settings to the channels's data structures before wiphy registration.
>> The driver then calls wiphy_register(). The regulatory core then uses
>> the set channel data structures and caches the values upon registration
>> on the orig_* parameters, the orig_* parameters are to be used only
>> internally by the core, its used here and as explained below also can
>> be requested to be updated when REGULATORY_STRICT_REG is used.
>
> This is not appropriate for the intel use-case. The orig_flags must be
> all 0, since the initial regulatory domain doesn't mean much.
> It can be changed at any time by usermode (= telephony indication). We
> do not wish to intersect with orig_flags, which always happens AFAICT.
>
>>
>>> 1. User removes the SIM from his cellphone.
>>
>> This can implemented to trigger a reset to regulatory, for example we
>> already call a reset to regulatory when we disconnect from an AP or when
>> we suspend and resume, this is done since we cannot ensure the
>> information received earlier will be valid anymore, it also clears all
>> the heuristics on helping with world roaming like clearly NO-IR (iniatiting
>> radiation) flags for beacon hints, which would otherwise not enable a
>> device to intiate radiation if it was world roaming on some channels.
>> If done this way regulatory restore_custom_reg_settings() will update the
>> device based on on the orig_* parameters which are values used from the
>> original registration. Consider it as kind of like resetting the router when
>> you put the pin in it, things gets wiped out, we start from scratch again.
>
> While the reset notion is fine, this fails for the normal use case. If
> we set something other then 0 for orig_flags, handle_channel() will
> always "or" with them when switching between regular alpha2 domains
> (e.g. roaming).
>
>>
>>> 2. A user hint is sent with alpha2 "99", telling the driver this -
>>> "please reset the regdomain to whatever is default, i have no idea
>>> what this is"
>>
>> The reason I am NACK'ing is that you should know the default before
>> registration, right now you had a bus issue which you can address
>> as well with -EPROBE_DEFER. I don't want to allow a dynamic custom
>> world regulatory domain settings as it doesn't make sense yet, there's
>> no reason for it and we already have APIs to handle this provided you
>> can fetch that regulatory domain before registration.
>>
>> The callback for reg_get() to fetch regulatory domain data structures
>> from firmware is welcomed but only for real country ISO3166-alpha2 country
>> codes, and I can see that could likely be dynamic.
>
> I'll wait for you to address the issue above.
>
>>
>>> 3. The driver/FW replies with a valid alpha2 + regdom settings, where
>>> alpha2 can also be "00".
>>
>> Again this would not be needed. Now, if the driver / phone decides it
>> *does* want to use a new alpha2 (say from cell phone base station a cell
>> phone is connected to) the proper APIs can be used and passed, and in
>> fact if the device had used REGULATORY_STRICT_REG a regulatory_hint()
>> would override the *_orig parameters to ensure that upon reset that
>> default would be used as well. This would even allow you to use dynamic
>> override on the orig_* parameters, provided of course you trust the
>> source, and also that the regulatory hint is for a specific
>> ISO3166-alpha2.
>
> REGULATORY_STRICT_REG only sets orig_flags for driver hints, not user
> hints. We'll have to identify the wiphy for user-hints etc. I'm also
> very much reluctant to use the STRICT flag. Seems they were engineered
> for specific scenarios/cards, and will generally work well for
> world-roaming cards that want country IE updates. The CUSTOM
> regulatory flag also has a specific use-case, which doesn't fit intel.
>
> The simpler use-case of a "regular" card would work well here,
> provided the regulatory settings come from FW.
>
>>
>> The devices that would use the REGULATORY_STRICT_REG and
>> regulatory_hint() today after registration are for example APs. Most 802.11
>> devices on laptops however use a custom world regulatory domain, and then
>> techniques are used to help with world roaming such as becon hints, etc. For
>> some mobile devices however a wiphy_apply_custom_regulatory() can be used
>> which would set REGULATORY_CUSTOM_REG, and then if they wanted suppport
>> for cellular base station hints regulatory_hint() from userspace can be
>> used that will remove REGULATORY_CUSTOM_REG, and the new alpha2 is
>> trusted.
>
> But then the ability to restore the orig flag is lost.. since we can't
> restore the CUSTOM flag. And again, removing the flag requires a
> driver hint, not a user one.
> Like I said, the scenarios are pretty specific.
>
>>
>>> It just so happens that the above scenario also happens on boot (we
>>> have no idea what's the right alpha2).
>>
>> Upon boot you can should be able to do what you need to do to defer
>> probing until ready. Let me know if the core does not provide the means
>> to do this properly, I'd be interested to learn more about that as I'm
>> sure Greg might be too.
>
> I haven't explored the technical aspects here, but I'll give you the
> gist of it. Right now we register the wiphy on boot, but load the fw
> on the first "ifup". This ifup can come from some external component
> (maybe Android framework?) that knows the entire system state (i.e.
> FS, bus, rfkill, etc), and can ensure everything will work out OK. Now
> there different HW platforms here, different software framework
> (Android, ChromeOS, etc) and different bus types we support.
>
> You're basically asking for us to find a "hook" in each configuration
> in which we can load the FW during the wiphy registration, just before
> ifup arrives (but not after - since ifup will fail without a wiphy).
> The "hook" depends on the entire system state, not only kernel. It
> will likely be hard or impossible to do right is some situations. It's
> akin to shooting ourselves in the foot. :)
>
> Therefore I would be glad if you reconsider a dynamic initial regulatory domain.

Thinking about this a bit more, I guess we can find out the true
initial regdomain (e.g. US) on ifup (much after wiphy is registered),
and send it as as a real alpha2 in a driver regulatory hint. This will
keep orig_flags pristine and allows us to change regdomains at will.

But we still need the "99" from usermode for the "user removes the SIM
during operation" scenario. You're suggesting reg.c will get "99" and
somehow know that it needs to restore the alpha2 to US at this point.
Currently there's no functionality to do this. There's some limited
code to restore to the latest set user_alpha2, but that's not what we
want. We want the "US" that was set by the driver. I still believe the
simplest solution is to give it to the get_regd() callback, which will
then return "US". It prevents complexity and more special cases from
reg.c..

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Sun, Jul 06, 2014 at 10:22:00PM +0300, Arik Nemtsov wrote:
> >
> > Lets start with the 0 case with what I am recommending:
> >
> > 0. The driver calls wiphy_apply_custom_regulatory(), this applies
> > settings to the channels's data structures before wiphy registration.
> > The driver then calls wiphy_register(). The regulatory core then uses
> > the set channel data structures and caches the values upon registration
> > on the orig_* parameters, the orig_* parameters are to be used only
> > internally by the core, its used here and as explained below also can
> > be requested to be updated when REGULATORY_STRICT_REG is used.
> 
> This is not appropriate for the intel use-case. The orig_flags must be
> all 0, since the initial regulatory domain doesn't mean much.

Ok then the central cfg80211 world regulatory domain should be used
in between the time you can fetch the actual regulatory domain you need.
Right now cfg80211 does not let you ask the regulatory code to use the
world regdom to set the _orig parameters upon registration but its
not a bad idea to enable this as a feature. Quite a few drivers have
the practice now to use the apply custom with then own custom world
regdom, using the central one as an option is wise if you know later
some other third party entity will set the *real* regulatory domain.

> It can be changed at any time by usermode (= telephony indication). We
> do not wish to intersect with orig_flags, which always happens AFAICT.

No, that's my point, when you use REGULATORY_STRICT_REG and you issue
a regulatory_hint() with an alpha2 an intersection with _orig* does
not occur, we actually then trust the new regdom fully.

Have you looked into cell base station hints? That's exactly why this
code was added. The name "cell base station hints" is loose but
obviously will vary depending on the vendor / system integrator.
Have your regulatory folks read this FCC KDB 594280 [0] ? Check out
page 14.

[0] https://apps.fcc.gov/eas/comments/GetPublishedDocument.html?id=327&tn=528122

This doesn't mean that because FCC has made a clarification on MCC codes
cannot be used for host compliance that other regulatory bodes will not
allow for it, it also doesn't invalidate our code as well because other
system integrators can use other solutions. For example I'd envision a
really good solution to not only use MCC but a combination of set of
feeds that give userspace a certain amount of certainty on the alpha2
code. This is all documented here:

http://wireless.kernel.org/en/developers/Regulatory/processing_rules#Cellular_base_station_regulatory_hints

Pricicely because system integrators can vary on effort here its why we
enable the CFG80211_CERTIFICATION_ONUS Kconfig option.

> >> 1. User removes the SIM from his cellphone.
> >
> > This can implemented to trigger a reset to regulatory, for example we
> > already call a reset to regulatory when we disconnect from an AP or when
> > we suspend and resume, this is done since we cannot ensure the
> > information received earlier will be valid anymore, it also clears all
> > the heuristics on helping with world roaming like clearly NO-IR (iniatiting
> > radiation) flags for beacon hints, which would otherwise not enable a
> > device to intiate radiation if it was world roaming on some channels.
> > If done this way regulatory restore_custom_reg_settings() will update the
> > device based on on the orig_* parameters which are values used from the
> > original registration. Consider it as kind of like resetting the router when
> > you put the pin in it, things gets wiped out, we start from scratch again.
> 
> While the reset notion is fine, this fails for the normal use case. If
> we set something other then 0 for orig_flags, handle_channel() will
> always "or" with them when switching between regular alpha2 domains
> (e.g. roaming).

There's an exception to this, for example the shared ath module for
Atheros drivers lets drivers that have an alpha2 regulatory domain be
trusted fully on the _orig* parameters.

> >> 2. A user hint is sent with alpha2 "99", telling the driver this -
> >> "please reset the regdomain to whatever is default, i have no idea
> >> what this is"
> >
> > The reason I am NACK'ing is that you should know the default before
> > registration, right now you had a bus issue which you can address
> > as well with -EPROBE_DEFER. I don't want to allow a dynamic custom
> > world regulatory domain settings as it doesn't make sense yet, there's
> > no reason for it and we already have APIs to handle this provided you
> > can fetch that regulatory domain before registration.
> >
> > The callback for reg_get() to fetch regulatory domain data structures
> > from firmware is welcomed but only for real country ISO3166-alpha2 country
> > codes, and I can see that could likely be dynamic.
> 
> I'll wait for you to address the issue above.

OK.

> >> 3. The driver/FW replies with a valid alpha2 + regdom settings, where
> >> alpha2 can also be "00".
> >
> > Again this would not be needed. Now, if the driver / phone decides it
> > *does* want to use a new alpha2 (say from cell phone base station a cell
> > phone is connected to) the proper APIs can be used and passed, and in
> > fact if the device had used REGULATORY_STRICT_REG a regulatory_hint()
> > would override the *_orig parameters to ensure that upon reset that
> > default would be used as well. This would even allow you to use dynamic
> > override on the orig_* parameters, provided of course you trust the
> > source, and also that the regulatory hint is for a specific
> > ISO3166-alpha2.
> 
> REGULATORY_STRICT_REG only sets orig_flags for driver hints, not user
> hints.

Good point, I welcome this as a change, this can for example be a flag
passed as part of the wiphy's regulatory preferences. If the option
CFG80211_REG_CELLULAR_HINTS is enabled then the flag can be looked for
and if present your the core then use the user cell base station hint
to override the _orig* parameters.

> We'll have to identify the wiphy for user-hints etc. I'm also
> very much reluctant to use the STRICT flag. Seems they were engineered
> for specific scenarios/cards, and will generally work well for
> world-roaming cards  that want country IE updates. The CUSTOM
> regulatory flag also has a specific use-case, which doesn't fit intel.
> 
> The simpler use-case of a "regular" card would work well here,
> provided the regulatory settings come from FW.

Let's separate the use cases you wanted to add. One was a fw API which
I'm fine with, the other one was a custom world regdom after wiphy
registration which I am not comfortable with. The fw API is fine but
you seem now to also be explaining some uses cases on regulatory
feeds come from cellular data somehow. Is that the fw use case?
If so the API for CFG80211_REG_CELLULAR_HINTS can be used and extended
very easily for your use case. You can still have a fw API for
regulatory data being provided, but lets be clear that it is different
from the use case of letting the same API add a custom world regulatory
domain.

> > The devices that would use the REGULATORY_STRICT_REG and
> > regulatory_hint() today after registration are for example APs. Most 802.11
> > devices on laptops however use a custom world regulatory domain, and then
> > techniques are used to help with world roaming such as becon hints, etc. For
> > some mobile devices however a wiphy_apply_custom_regulatory() can be used
> > which would set REGULATORY_CUSTOM_REG, and then if they wanted suppport
> > for cellular base station hints regulatory_hint() from userspace can be
> > used that will remove REGULATORY_CUSTOM_REG, and the new alpha2 is
> > trusted.
> 
> But then the ability to restore the orig flag is lost.. since we can't
> restore the CUSTOM flag. And again, removing the flag requires a
> driver hint, not a user one.
> Like I said, the scenarios are pretty specific.

Evolutions are welcomed to the CFG80211_REG_CELLULAR_HINTS use case for
drivers that want to trust them.

> >> It just so happens that the above scenario also happens on boot (we
> >> have no idea what's the right alpha2).
> >
> > Upon boot you can should be able to do what you need to do to defer
> > probing until ready. Let me know if the core does not provide the means
> > to do this properly, I'd be interested to learn more about that as I'm
> > sure Greg might be too.
> 
> I haven't explored the technical aspects here, but I'll give you the
> gist of it. Right now we register the wiphy on boot, but load the fw
> on the first "ifup".

Why?

> This ifup can come from some external component
> (maybe Android framework?) that knows the entire system state (i.e.
> FS, bus, rfkill, etc), and can ensure everything will work out OK. Now
> there different HW platforms here, different software framework
> (Android, ChromeOS, etc) and different bus types we support.

OK, it still doesn't explain why you would load the firmware on the
first ifup rather than bootup.

> You're basically asking for us to find a "hook" in each configuration
> in which we can load the FW during the wiphy registration, just before
> ifup arrives (but not after - since ifup will fail without a wiphy).

No I'm asking you for clarification on why you can't load fw early
and why deferred probe does not suffice. You *found* your hook on
ifup, why can you do it on ifup and not a deffered probe? Seems a
bit odd.

The goal here is to see if there is something that we need more general
for not only you but for others on the driver core.

> The "hook" depends on the entire system state, not only kernel. It
> will likely be hard or impossible to do right is some situations. It's
> akin to shooting ourselves in the foot. :)

But "ifup" was the magic bullet for some reason, seems odd how that
worked out well but we can't generalize something here.

> Therefore I would be glad if you reconsider a dynamic initial
> regulatory domain.

The excercise of line of questioning has revealed already quite a bit of
details on existing APIs which you can or cannot use, and will help
simplify code. If APIs already exist for things I rather avoid them, the
goal here is to ensure that some other solution does not exist before
allowing something new in blindly.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Mon, Jul 07, 2014 at 12:13:20PM +0300, Arik Nemtsov wrote:
> On Sun, Jul 6, 2014 at 10:22 PM, Arik Nemtsov <arik@wizery.com> wrote:
> > Therefore I would be glad if you reconsider a dynamic initial regulatory domain.
> 
> Thinking about this a bit more, I guess we can find out the true
> initial regdomain (e.g. US) on ifup (much after wiphy is registered),
> and send it as as a real alpha2 in a driver regulatory hint. This will
> keep orig_flags pristine and allows us to change regdomains at will.

:)

> But we still need the "99" from usermode for the "user removes the SIM
> during operation" scenario. You're suggesting reg.c will get "99" and
> somehow know that it needs to restore the alpha2 to US at this point.

No I never made a recommendation to the linking.

> Currently there's no functionality to do this. There's some limited
> code to restore to the latest set user_alpha2, but that's not what we
> want.

I'll highlight you're mentioning the user_alpha2, which is used for
either users setting regulatory domains via 'iw reg set' or equivalent
and cellular base station hints, which use the same API but just qualify
the request to specifiy the source is coming from some form of cell base
station.

> We want the "US" that was set by the driver.

But here you say "driver" rather than userspace or cell base station
hints, so its unclear what you meant here. If the driver set it then
REGULATORY_STRICT_REG already supports letting the driver keep the
_orig* parameters.

> I still believe the
> simplest solution is to give it to the get_regd() callback, which will
> then return "US". It prevents complexity and more special cases from
> reg.c..

Let's separate cell base station hints from userspace from firmware
being loaded and assuming that magical firmware has now regulatory
data (which obviously I consider silly since we already provide the
same functionality with CRDA and let you override the db.txt). The
cellular base station hint infrastructure can certainly be expanded
to support overriding the *_orig parameters. The FW API looks less
attractive now as it seems more than anything the hint can actually
come from userspace and the base station hint mechanism for your
use case.

A FW API to let FW load regulatory data dynamically is still OK but
lets be clear that if you are using it for a userspace work around
for cell base astation hints it seems rather a work around for
existing APIs and you can likely implement what you want with less
code. If we don't need this FW API I rather not have it added.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Tue, Jul 8, 2014 at 5:57 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> Let's separate cell base station hints from userspace from firmware
> being loaded and assuming that magical firmware has now regulatory
> data (which obviously I consider silly since we already provide the
> same functionality with CRDA and let you override the db.txt). The
> cellular base station hint infrastructure can certainly be expanded
> to support overriding the *_orig parameters. The FW API looks less
> attractive now as it seems more than anything the hint can actually
> come from userspace and the base station hint mechanism for your
> use case.
>
> A FW API to let FW load regulatory data dynamically is still OK but
> lets be clear that if you are using it for a userspace work around
> for cell base astation hints it seems rather a work around for
> existing APIs and you can likely implement what you want with less
> code. If we don't need this FW API I rather not have it added.

I don't see a way around having the get_regd() API to get regulatory
data from FW. How else would we get regdomain settings?

A hint can come from userspace (as a cell-base hint) or from the
driver. In some platforms the Wifi HW is directly connected to the SIM
and the FW sends up events about country change. This eventually
causes a driver hint.

Sometimes the SIM is removed or there's no reception etc. In this case
we must get back to the "default" country, which is burned in
NVRAM/FW. As explained in my previous email, currently reg.c doesn't
have any suitable facility to save this "default" country-code. So
AFAICT, your suggestion of setting the default on boot and then
returning to it is out of the question. And anyway a "restore to
default" API doesn't exist. I'm suggesting the following solution for
this problem:
1. user/kernel calls regulatory_hint("99") to say we need to return to default
2. get_regd("99") is called and the driver/FW returns the default
country-code and it's regulatory data.

Of course we can also invent new APIs, something like:
1. on boot, the kernel calls set_default_country("XX")
2. user/kernel calls a new api restore_to_default_country(), and reg.c
uses the XX value and restores.

Personally I think the first offer is simpler and involves less
complexity on behalf of reg.c. That was my final point of the previous
email.

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

Dropping Greg as we're no longer focusing on questions on the driver
core. My reply below.

On Wed, Jul 09, 2014 at 05:46:05PM +0300, Arik Nemtsov wrote:
> On Tue, Jul 8, 2014 at 5:57 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > Let's separate cell base station hints from userspace from firmware
> > being loaded and assuming that magical firmware has now regulatory
> > data (which obviously I consider silly since we already provide the
> > same functionality with CRDA and let you override the db.txt). The
> > cellular base station hint infrastructure can certainly be expanded
> > to support overriding the *_orig parameters. The FW API looks less
> > attractive now as it seems more than anything the hint can actually
> > come from userspace and the base station hint mechanism for your
> > use case.
> >
> > A FW API to let FW load regulatory data dynamically is still OK but
> > lets be clear that if you are using it for a userspace work around
> > for cell base astation hints it seems rather a work around for
> > existing APIs and you can likely implement what you want with less
> > code. If we don't need this FW API I rather not have it added.
> 
> I don't see a way around having the get_regd() API to get regulatory
> data from FW. How else would we get regdomain settings?

Well it depends on the solution of course so let's review and be
crystal clear on what your *requirements* are and determine exactly
what you need.

So far it seems clear you have requirements for:

(0) asynchronous "cellular hint" from the *driver*, with the data
    source of the regulatory domain data structure being the driver

Right now we only provide asynchronous cell base station hints but from
userespace, and only for the alpha2.

> A hint can come from userspace (as a cell-base hint) or from the
> driver.

Right. Today we only allow cell base station hints from userspace
though.

> In some platforms the Wifi HW is directly connected to the SIM
> and the FW sends up events about country change. This eventually
> causes a driver hint.

OK, so userspace can't send the cell base station hint (or whatever
we want to call it to make this fit). In that case the firmware should
be able to trigger an event and send a regulatory hint but with a
regulatory domain data structure. As I see it instead of of having the
regulatory core *ask* the driver for the regdomain the driver should be
able to initiate the request preemptively, a new call seems best,
something like:

regulatory_hint_regd(struct wiphy,
		     const struct ieee80211_regdomain *regd,
		     enum nl80211_driver_reg_hint_type);

The nl80211_driver_reg_hint_type can be similar to nl80211_user_reg_hint_type
to qualify the source. The first source would be:

enum nl80211_driver_reg_hint_type {                                               
		NL80211_DRIVER_REG_HINT_CELL_BASE	= 0,
}; 

The struct regulatory_request can then be expanded with the
nl80211_driver_reg_hint_type and the handler code can do what is needed.
You can expand reg_request_cell_base() to include this type of request
but be sure that you review its uses, the only case I see that requires
treament of the cell base station hint differently is in
reg_ignore_cell_hint() where we check if the alpha2 is already set. In
the driver cell base station hint case where the regd structure comes
from the driver we'e need to ensure the source was the same, that is the
same wiphy.

When drivers are the sources of the regd we only want to trust that
regd struct for that wiphy, ideally however it is very useful information
for the entire kernel what alpha2 was passed, however using this alpha2
to for example call CRDA and use the wireless-regdb *only* for drivers
other than the caller can get rather sloppy and complicated really
quickly. For instance if CRDA is not installed we'd timeout on the request
and we don't want to reset the regulatory core as its not a fatal issue if
CRDA was not present in the case of an internal driver call.
Driver specific helpers added here should simply be documented and
implemented as driver specific then, they really can't *easily* contribute
much to the regulatory core unless we complicate things quite a bit. I don't
think its worth it. We should therefore treat these type of driver hints
that have the driver / firmware as source for the regulatory data as no
different than the custom reg hint calls, which are only internal to
the driver. The driver can still take advantage of other regulatory core
helpers like beacon hints, and other regulatory helpers if it so
wishes. Please make these new helpers EXPORT_SYMBOL_GPL() though.

This should simplify your implementation then. You can even piggy back on
top of the current cell base station hint code, it can be extended to include
support for driver cell base station hints and I think all you'd
need is to expand support to enable drivers to specify whether or not
they want driver cell base station hints to be able to be more authoritative
than what was specified on the _orig* parameters. To be clear you would
not be overriding the _orig* parameters but these new settings would be
a new super set for the driver. Right now we don't let stepping outside
what is on the _orig* parameters and it seems you do want to do that for
these hints, but you also *don't* want these saved and cached. Using a
flag for this preference seems reasonable. You'd still have cached any
previous settings on the _orig* parameters so a reset would restore
the device to previous state.

> Sometimes the SIM is removed or there's no reception etc. In this case
> we must get back to the "default" country, which is burned in
> NVRAM/FW.

You explaining *two* other requirements here:

(1) The driver provides an alpha2 regulatory domain which is burned onto
    the NVRAM or writtten on the firmware file. You could expand on the
    regulatory_hint_regd(), this would should probably go in first, so
    you'd have:

enum nl80211_driver_reg_hint_type {                                               
		NL80211_DRIVER_REG_HINT_INTERNAL	= 0,
		NL80211_DRIVER_REG_HINT_CELL_BASE	= 1,
}; 

As with (0) the source is internal and only applicable to the
driver. This should simplify the implementation. As with
REGULATORY_STRICT_REG, you'd want something similar to help you
override the *_orig parameters.

(2) regulatory_hint_cell_disconnect() - this would allow the regulatory core
    to reset the device to the "default" alpha2 country. If you
    implement the driver cell base station as explained on (0)
    and the driver reg hint as in (1) then the reset of the regulatory
    settings should already take care of this.

> As explained in my previous email, currently reg.c doesn't
> have any suitable facility to save this "default" country-code.

Yes it does, we have it for the REGULATORY_STRICT_REG and
regulatory_hint() case, you have a similar use case but the
source is simply internal to the driver. That's all.

> So AFAICT, your suggestion of setting the default on boot and then
> returning to it is out of the question.

No, its still valid, the use case and requirements however expand how
we should use cellular hints and internal driver hints.

> And anyway a "restore to default" API doesn't exist.

regulatory_hint_disconnect() exists but that's only used by the SME.
So indeed we a regulatory_hint_cell_disconnect() seems in order.

> I'm suggesting the following solution for
> this problem:
> 1. user/kernel calls regulatory_hint("99") to say we need to return to default

That is just an abuse over an existing API, I don't like it one bit, I'd
rather add a proper API and resuse code where possible.

> 2. get_regd("99") is called and the driver/FW returns the default
> country-code and it's regulatory data.

This is not needed if we cache the default alpha2 as we do with the
strict settings but for the internal regd hint.

> Of course we can also invent new APIs, something like:
> 1. on boot, the kernel calls set_default_country("XX")
> 2. user/kernel calls a new api restore_to_default_country(), and reg.c
> uses the XX value and restores.

This is not needed.

> Personally I think the first offer is simpler and involves less
> complexity on behalf of reg.c. That was my final point of the previous
> email.

This can be much simpler, I'll reorder your requirements:

(0) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_INTERNAL
    with a flag similar to REGULATORY_STRICT_REG
(1) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_CELL_BASE
(2) regulatory_hint_cell_disconnect()

Despite the fact that regulatory_hint_regd() calls are internal I'd
still like to see events issued so that userspace can keep track of
these. This also applies to regulatory_hint_cell_disconnect().

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Wed, Jul 23, 2014 at 3:41 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> Dropping Greg as we're no longer focusing on questions on the driver
> core. My reply below.
>
> On Wed, Jul 09, 2014 at 05:46:05PM +0300, Arik Nemtsov wrote:
>> On Tue, Jul 8, 2014 at 5:57 AM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > Let's separate cell base station hints from userspace from firmware
>> > being loaded and assuming that magical firmware has now regulatory
>> > data (which obviously I consider silly since we already provide the
>> > same functionality with CRDA and let you override the db.txt). The
>> > cellular base station hint infrastructure can certainly be expanded
>> > to support overriding the *_orig parameters. The FW API looks less
>> > attractive now as it seems more than anything the hint can actually
>> > come from userspace and the base station hint mechanism for your
>> > use case.
>> >
>> > A FW API to let FW load regulatory data dynamically is still OK but
>> > lets be clear that if you are using it for a userspace work around
>> > for cell base astation hints it seems rather a work around for
>> > existing APIs and you can likely implement what you want with less
>> > code. If we don't need this FW API I rather not have it added.
>>
>> I don't see a way around having the get_regd() API to get regulatory
>> data from FW. How else would we get regdomain settings?
>
> Well it depends on the solution of course so let's review and be
> crystal clear on what your *requirements* are and determine exactly
> what you need.
>
> So far it seems clear you have requirements for:
>
> (0) asynchronous "cellular hint" from the *driver*, with the data
>     source of the regulatory domain data structure being the driver
>
> Right now we only provide asynchronous cell base station hints but from
> userespace, and only for the alpha2.
>
>> A hint can come from userspace (as a cell-base hint) or from the
>> driver.
>
> Right. Today we only allow cell base station hints from userspace
> though.

Right.

>
>> In some platforms the Wifi HW is directly connected to the SIM
>> and the FW sends up events about country change. This eventually
>> causes a driver hint.
>
> OK, so userspace can't send the cell base station hint (or whatever
> we want to call it to make this fit). In that case the firmware should
> be able to trigger an event and send a regulatory hint but with a
> regulatory domain data structure. As I see it instead of of having the
> regulatory core *ask* the driver for the regdomain the driver should be
> able to initiate the request preemptively, a new call seems best,
> something like:
>
> regulatory_hint_regd(struct wiphy,
>                      const struct ieee80211_regdomain *regd,
>                      enum nl80211_driver_reg_hint_type);
>
> The nl80211_driver_reg_hint_type can be similar to nl80211_user_reg_hint_type
> to qualify the source. The first source would be:
>
> enum nl80211_driver_reg_hint_type {
>                 NL80211_DRIVER_REG_HINT_CELL_BASE       = 0,
> };

That could work nicely actually.

>
> The struct regulatory_request can then be expanded with the
> nl80211_driver_reg_hint_type and the handler code can do what is needed.
> You can expand reg_request_cell_base() to include this type of request
> but be sure that you review its uses, the only case I see that requires
> treament of the cell base station hint differently is in
> reg_ignore_cell_hint() where we check if the alpha2 is already set. In
> the driver cell base station hint case where the regd structure comes
> from the driver we'e need to ensure the source was the same, that is the
> same wiphy.

Well currently this code is only invoked for user hints, so not sure
we should touch it. For instance reg_request_cell_base() checks the
initiator is usermode, etc.
Also in the driver case, we probably don't want to ignore regular
updates after cell-base updates. So the logic is simpler there.

I also suggest adding another argument to regulatory_hint_regd() to
allow the driver to specify the intersection policy. It seems that
currently the code (__reg_process_hint_driver()) only allows to
intersect, which is not appropriate to us.
So the final form can look like:

regulatory_hint_regd(struct wiphy,
                              const struct ieee80211_regdomain *regd,
                              enum nl80211_driver_reg_hint_type,
                              enum nl80211_reg_intersect_policy policy);

enum nl80211_reg_intersect_policy {
         REG_INTERSECT_POLICY_DEFAULT,
         REG_INTERSECT_POLICY_OVERRIDE,
         REG_INTERSECT_POLICY_INTERSECT,
};

>
> When drivers are the sources of the regd we only want to trust that
> regd struct for that wiphy, ideally however it is very useful information
> for the entire kernel what alpha2 was passed, however using this alpha2
> to for example call CRDA and use the wireless-regdb *only* for drivers
> other than the caller can get rather sloppy and complicated really
> quickly. For instance if CRDA is not installed we'd timeout on the request
> and we don't want to reset the regulatory core as its not a fatal issue if
> CRDA was not present in the case of an internal driver call.
> Driver specific helpers added here should simply be documented and
> implemented as driver specific then, they really can't *easily* contribute
> much to the regulatory core unless we complicate things quite a bit. I don't
> think its worth it. We should therefore treat these type of driver hints
> that have the driver / firmware as source for the regulatory data as no
> different than the custom reg hint calls, which are only internal to
> the driver. The driver can still take advantage of other regulatory core
> helpers like beacon hints, and other regulatory helpers if it so
> wishes. Please make these new helpers EXPORT_SYMBOL_GPL() though.

Agree we should keep this simple. Why should these be exported as GPL?
Everything else is just EXPORT_SYMBOL() in reg.c.

>
> This should simplify your implementation then. You can even piggy back on
> top of the current cell base station hint code, it can be extended to include
> support for driver cell base station hints and I think all you'd
> need is to expand support to enable drivers to specify whether or not
> they want driver cell base station hints to be able to be more authoritative
> than what was specified on the _orig* parameters. To be clear you would
> not be overriding the _orig* parameters but these new settings would be
> a new super set for the driver. Right now we don't let stepping outside
> what is on the _orig* parameters and it seems you do want to do that for
> these hints, but you also *don't* want these saved and cached. Using a
> flag for this preference seems reasonable. You'd still have cached any
> previous settings on the _orig* parameters so a reset would restore
> the device to previous state.
>
>> Sometimes the SIM is removed or there's no reception etc. In this case
>> we must get back to the "default" country, which is burned in
>> NVRAM/FW.
>
> You explaining *two* other requirements here:
>
> (1) The driver provides an alpha2 regulatory domain which is burned onto
>     the NVRAM or writtten on the firmware file. You could expand on the
>     regulatory_hint_regd(), this would should probably go in first, so
>     you'd have:
>
> enum nl80211_driver_reg_hint_type {
>                 NL80211_DRIVER_REG_HINT_INTERNAL        = 0,
>                 NL80211_DRIVER_REG_HINT_CELL_BASE       = 1,
> };
>
> As with (0) the source is internal and only applicable to the
> driver. This should simplify the implementation. As with
> REGULATORY_STRICT_REG, you'd want something similar to help you
> override the *_orig parameters.

I don't think we ever want to override _orig as they restrict changes.
But the overall solution is ok.

>
> (2) regulatory_hint_cell_disconnect() - this would allow the regulatory core
>     to reset the device to the "default" alpha2 country. If you
>     implement the driver cell base station as explained on (0)
>     and the driver reg hint as in (1) then the reset of the regulatory
>     settings should already take care of this.
>
>> As explained in my previous email, currently reg.c doesn't
>> have any suitable facility to save this "default" country-code.
>
> Yes it does, we have it for the REGULATORY_STRICT_REG and
> regulatory_hint() case, you have a similar use case but the
> source is simply internal to the driver. That's all.

So the only use for nl80211_driver_reg_hint_type for now would be to
cache the default alpha2 value?
We'll need a new global variable, something like "driver_alpha2",
similar to the existing "user_alpha2". We can't just use the last
request, since it will change..

>
>> And anyway a "restore to default" API doesn't exist.
>
> regulatory_hint_disconnect() exists but that's only used by the SME.
> So indeed we a regulatory_hint_cell_disconnect() seems in order.

Ok that sounds good.

>
> This can be much simpler, I'll reorder your requirements:
>
> (0) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_INTERNAL
>     with a flag similar to REGULATORY_STRICT_REG
> (1) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_CELL_BASE
> (2) regulatory_hint_cell_disconnect()

All this is in addition to the new get_regd() callback for setting the
regdomain data when usermode requests a cell-base hint right?
If so, this should work well.

Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Wed, Jul 23, 2014 at 1:07 AM, Arik Nemtsov <arik@wizery.com> wrote:
> On Wed, Jul 23, 2014 at 3:41 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
>> The struct regulatory_request can then be expanded with the
>> nl80211_driver_reg_hint_type and the handler code can do what is needed.
>> You can expand reg_request_cell_base() to include this type of request
>> but be sure that you review its uses, the only case I see that requires
>> treament of the cell base station hint differently is in
>> reg_ignore_cell_hint() where we check if the alpha2 is already set. In
>> the driver cell base station hint case where the regd structure comes
>> from the driver we'e need to ensure the source was the same, that is the
>> same wiphy.
>
> Well currently this code is only invoked for user hints, so not sure
> we should touch it. For instance reg_request_cell_base() checks the
> initiator is usermode, etc.
> Also in the driver case, we probably don't want to ignore regular
> updates after cell-base updates. So the logic is simpler there.

True, these hints should be handled separately just as
wiphy_apply_custom_regulatory() is.

> I also suggest adding another argument to regulatory_hint_regd() to
> allow the driver to specify the intersection policy. It seems that
> currently the code (__reg_process_hint_driver()) only allows to
> intersect, which is not appropriate to us.
> So the final form can look like:
>
> regulatory_hint_regd(struct wiphy,
>                               const struct ieee80211_regdomain *regd,
>                               enum nl80211_driver_reg_hint_type,
>                               enum nl80211_reg_intersect_policy policy);
>
> enum nl80211_reg_intersect_policy {
>          REG_INTERSECT_POLICY_DEFAULT,
>          REG_INTERSECT_POLICY_OVERRIDE,
>          REG_INTERSECT_POLICY_INTERSECT,
> };

We don't want to use the driver regulatory data to help the regulatory
core at all because I noted it would get things messy fast. That would
mean you never have to deal with any intersection or the cfg80211
regulatory domain at all. So none of this would be needed. The hint
would just be for the driver. Its sad that driver regulatory data
can't contribute to the regulatory core but I think its best this way
to keep things clean and simple. If the driver does want to help the
best way is to contribute to wireless-regdb and move to that.

>> When drivers are the sources of the regd we only want to trust that
>> regd struct for that wiphy, ideally however it is very useful information
>> for the entire kernel what alpha2 was passed, however using this alpha2
>> to for example call CRDA and use the wireless-regdb *only* for drivers
>> other than the caller can get rather sloppy and complicated really
>> quickly. For instance if CRDA is not installed we'd timeout on the request
>> and we don't want to reset the regulatory core as its not a fatal issue if
>> CRDA was not present in the case of an internal driver call.
>> Driver specific helpers added here should simply be documented and
>> implemented as driver specific then, they really can't *easily* contribute
>> much to the regulatory core unless we complicate things quite a bit. I don't
>> think its worth it. We should therefore treat these type of driver hints
>> that have the driver / firmware as source for the regulatory data as no
>> different than the custom reg hint calls, which are only internal to
>> the driver. The driver can still take advantage of other regulatory core
>> helpers like beacon hints, and other regulatory helpers if it so
>> wishes. Please make these new helpers EXPORT_SYMBOL_GPL() though.
>
> Agree we should keep this simple. Why should these be exported as GPL?
> Everything else is just EXPORT_SYMBOL() in reg.c.

The regulatory code was changed to permissive license to help with the
BSD community. I've more than once now pitched to folks about sharing
and growing this code together, nothing has happened over years and in
fact got a positive nack on interest at least on FreeBSD, so although
the goal was good, upkeeping the permissive license doesn't do us any
good. These new symbols would be a good drawing line to move away from
that, and prevent proprietary drivers from making use of this.

>> This should simplify your implementation then. You can even piggy back on
>> top of the current cell base station hint code, it can be extended to include
>> support for driver cell base station hints and I think all you'd
>> need is to expand support to enable drivers to specify whether or not
>> they want driver cell base station hints to be able to be more authoritative
>> than what was specified on the _orig* parameters. To be clear you would
>> not be overriding the _orig* parameters but these new settings would be
>> a new super set for the driver. Right now we don't let stepping outside
>> what is on the _orig* parameters and it seems you do want to do that for
>> these hints, but you also *don't* want these saved and cached. Using a
>> flag for this preference seems reasonable. You'd still have cached any
>> previous settings on the _orig* parameters so a reset would restore
>> the device to previous state.
>>
>>> Sometimes the SIM is removed or there's no reception etc. In this case
>>> we must get back to the "default" country, which is burned in
>>> NVRAM/FW.
>>
>> You explaining *two* other requirements here:
>>
>> (1) The driver provides an alpha2 regulatory domain which is burned onto
>>     the NVRAM or writtten on the firmware file. You could expand on the
>>     regulatory_hint_regd(), this would should probably go in first, so
>>     you'd have:
>>
>> enum nl80211_driver_reg_hint_type {
>>                 NL80211_DRIVER_REG_HINT_INTERNAL        = 0,
>>                 NL80211_DRIVER_REG_HINT_CELL_BASE       = 1,
>> };
>>
>> As with (0) the source is internal and only applicable to the
>> driver. This should simplify the implementation. As with
>> REGULATORY_STRICT_REG, you'd want something similar to help you
>> override the *_orig parameters.
>
> I don't think we ever want to override _orig as they restrict changes.
> But the overall solution is ok.

OK, to be clear you would override _orig for the driver alpha2 hint,
and then the cell base station hint can either be the minimum set or
superset depending on a flag, but it won't touch the _orig parameters.
When a reset of the regulatory occurs the regulatory core would make
the driver go back to the _orig parameters, that work is already done.

>> (2) regulatory_hint_cell_disconnect() - this would allow the regulatory core
>>     to reset the device to the "default" alpha2 country. If you
>>     implement the driver cell base station as explained on (0)
>>     and the driver reg hint as in (1) then the reset of the regulatory
>>     settings should already take care of this.
>>
>>> As explained in my previous email, currently reg.c doesn't
>>> have any suitable facility to save this "default" country-code.
>>
>> Yes it does, we have it for the REGULATORY_STRICT_REG and
>> regulatory_hint() case, you have a similar use case but the
>> source is simply internal to the driver. That's all.
>
> So the only use for nl80211_driver_reg_hint_type for now would be to
> cache the default alpha2 value?

It would be good to expose the source to userspace as well both as an
event and when querying the wiphy.

> We'll need a new global variable, something like "driver_alpha2",
> similar to the existing "user_alpha2". We can't just use the last
> request, since it will change..

No, last_request stuff would never be touched by any of this work as
the regulatory data is purely internal and it'd be a mess to share
this with other drivers. I think its best to keep these hints
applicable only to the wiphy who issued them, even the driver cellular
hint with the regd. We should only share hints / reg info for drivers
using wireless-regdb. To help with debugging and as a compromise to
help with wireless-regdb evolution it would be good though to store
the passed regd for each type of driver hint so it can be made
available to userspace for easy inspection. I suspect you'll also want
a driver country IE hint too then, and I think this is fine. It is
better than the current situation where drivers / firmware would just
keep this regulatory data private.

>>> And anyway a "restore to default" API doesn't exist.
>>
>> regulatory_hint_disconnect() exists but that's only used by the SME.
>> So indeed we a regulatory_hint_cell_disconnect() seems in order.
>
> Ok that sounds good.
>
>>
>> This can be much simpler, I'll reorder your requirements:
>>
>> (0) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_INTERNAL
>>     with a flag similar to REGULATORY_STRICT_REG
>> (1) regulatory_hint_regd() for type NL80211_DRIVER_REG_HINT_CELL_BASE
>> (2) regulatory_hint_cell_disconnect()
>
> All this is in addition to the new get_regd() callback for setting the
> regdomain data when usermode requests a cell-base hint right?
> If so, this should work well.

No instead of the callback you'd just issue the calls directly for
each type of event, you'd pass the regd from the start. Note that the
hints are for specific alpha2 regdomains, not for custom world
regdomain as an API for that already exists. In fact perhaps you can
share that code with what you are doing, not sure, I think the only
difference is after registration we'd have to worry about locking.
Before registration wiphy_apply_custom_regulatory() can be used, after
that the other hints you're developing can be used.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Arik Nemtsov]

On Wed, Jul 23, 2014 at 9:06 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
>> I also suggest adding another argument to regulatory_hint_regd() to
>> allow the driver to specify the intersection policy. It seems that
>> currently the code (__reg_process_hint_driver()) only allows to
>> intersect, which is not appropriate to us.
>> So the final form can look like:
>>
>> regulatory_hint_regd(struct wiphy,
>>                               const struct ieee80211_regdomain *regd,
>>                               enum nl80211_driver_reg_hint_type,
>>                               enum nl80211_reg_intersect_policy policy);
>>
>> enum nl80211_reg_intersect_policy {
>>          REG_INTERSECT_POLICY_DEFAULT,
>>          REG_INTERSECT_POLICY_OVERRIDE,
>>          REG_INTERSECT_POLICY_INTERSECT,
>> };
>
> We don't want to use the driver regulatory data to help the regulatory
> core at all because I noted it would get things messy fast. That would
> mean you never have to deal with any intersection or the cfg80211
> regulatory domain at all. So none of this would be needed. The hint
> would just be for the driver. Its sad that driver regulatory data
> can't contribute to the regulatory core but I think its best this way
> to keep things clean and simple. If the driver does want to help the
> best way is to contribute to wireless-regdb and move to that.

I guess I didn't get that one.

Isn't that problematic for cell-base hints coming from usermode? I
mean, what wiphy are they from?
It's also problematic because currently usermode uses the
cfg80211_regdomain for its regulatory data (NL80211_CMD_GET_REG).

Granted, all of this can be changed, but I think it's easier to update
the global regdomain.

The cell-base hints from usermode with the regdomain data coming from
the FW was the original reason to introduce the get_regd() callback.
For driver originating hints I agree regulatory_hint_regd() is a
cleaner solution.

I feel the case of two different cards, one of which uses get_regd()
and the other wireless-regdb is a non-existent corner-case. And we're
throwing out the baby with the bathwater.
Why write extra code for a case that doesn't happen? How about adding
appropriate regulatory_flags to ensure this can't happen?

Overall it seems to be the API you've suggested in the previous email
with the addition of get_regd() for usermode hints can work well,
updating the global cfg80211_regdomain.

Regards,
Arik

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Wed, Jul 23, 2014 at 10:15:13PM +0300, Arik Nemtsov wrote:
> On Wed, Jul 23, 2014 at 9:06 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> >> I also suggest adding another argument to regulatory_hint_regd() to
> >> allow the driver to specify the intersection policy. It seems that
> >> currently the code (__reg_process_hint_driver()) only allows to
> >> intersect, which is not appropriate to us.
> >> So the final form can look like:
> >>
> >> regulatory_hint_regd(struct wiphy,
> >>                               const struct ieee80211_regdomain *regd,
> >>                               enum nl80211_driver_reg_hint_type,
> >>                               enum nl80211_reg_intersect_policy policy);
> >>
> >> enum nl80211_reg_intersect_policy {
> >>          REG_INTERSECT_POLICY_DEFAULT,
> >>          REG_INTERSECT_POLICY_OVERRIDE,
> >>          REG_INTERSECT_POLICY_INTERSECT,
> >> };
> >
> > We don't want to use the driver regulatory data to help the regulatory
> > core at all because I noted it would get things messy fast. That would
> > mean you never have to deal with any intersection or the cfg80211
> > regulatory domain at all. So none of this would be needed. The hint
> > would just be for the driver. Its sad that driver regulatory data
> > can't contribute to the regulatory core but I think its best this way
> > to keep things clean and simple. If the driver does want to help the
> > best way is to contribute to wireless-regdb and move to that.
> 
> I guess I didn't get that one.
> 
> Isn't that problematic for cell-base hints coming from usermode? I
> mean, what wiphy are they from?

We'd keep the userspace cell base station hint separate from driver
cell base station hint that provides its own regdomain which you are
implementing. The userspace cell base station hint has no wiphy associated
as its coming from userspace, its a general hint and there's a flag that lets
drivers specify they do support this feature (NL80211_FEATURE_CELL_BASE_REG_HINTS)
and if present and if the kernel was also compiled to support this feature
then the core uses the hint for the wiphy.

> It's also problematic because currently usermode uses the
> cfg80211_regdomain for its regulatory data (NL80211_CMD_GET_REG).

What's problematic? What you state is a fact, not an issue.

> Granted, all of this can be changed, but I think it's easier to update
> the global regdomain.

Nothing would be changed, you're adding new feature, how we add that can
vary. Now if you already implemented a solution and supporting it without it
being upstream yet -- that's definitely could change now depending on
what path we take.

I think you're suggesting to add support so that a driver can trigger
the cellular base station hint. That's a one step enhancement that I do
welcome regardless of the outcome of anything. Then there's another enhancement
you seem to want to add, you want to allow drivers to override the regulatory
domain with what they have internally in firmware for the alpha2 upon
application of the regulatory data with the get_regd() callback.
I thought about this and while this is possible this can get complicated
pretty quickly.  For instance consider CRDA not being present, we would
not trigger a setting of regulatory settings as we only do that once we
get back the regulatory domain from userespace. For drivers that have
their own regulatory data that would pose a problem unless CRDA was
present. That can be resolved by extending the routine called upon hitting the
timeout, just go ahead and send the callback to the wiphys that had the
internal regd data... but I suspect this is going to be just one corner case.
This would be complicated the code / paths even more.

Both approaches are certainly possible but I'm trying to ensure that we keep
things as clean and concise as possible. Let's review this below a bit
more.

> The cell-base hints from usermode with the regdomain data coming from
> the FW was the original reason to introduce the get_regd() callback.
> For driver originating hints I agree regulatory_hint_regd() is a
> cleaner solution.

OK. That would only be useful for when drivers have regulatory data
then.

> I feel the case of two different cards, one of which uses get_regd()
> and the other wireless-regdb is a non-existent corner-case.

If you tell that to customers or potential customers I'm going to assume
they'd react pretty negatively. I've seen the oddest combination, not sure
why you are ruling this out.

> And we're
> throwing out the baby with the bathwater.
> Why write extra code for a case that doesn't happen?

I've been told this before for regulatory code and to this day
I've been right, all corner cases I've envisioned have had to be
dealt with one way or another. My role is to help look out for these
cases to allow Linux to be flexible, not restrictive.

> How about adding appropriate regulatory_flags to ensure this can't happen?

That'd be being restrictive and lazy, for instance it'd let a
manufacturer build an image for phones that would not let users
plug in another type of 802.11 device. I think this flag is OK
however if once a device is found that uses wireless-regdb we'd
simply disallow the 802.11 devices that have the internal reg data
solution.

> Overall it seems to be the API you've suggested in the previous email
> with the addition of get_regd() for usermode hints can work well,
> updating the global cfg80211_regdomain.

OK I think we're on the same page, I just want to avoid complicating the
paths. If the above compromise seems reasonable I think that would help
keep things simple.

  Luis

Re: [PATCH 4/5] cfg80211: accept world/same regdom from driver/user hints

[Luis R. Rodriguez]

On Thu, Jul 24, 2014 at 12:43 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
>> How about adding appropriate regulatory_flags to ensure this can't happen?
>
> That'd be being restrictive and lazy, for instance it'd let a
> manufacturer build an image for phones that would not let users
> plug in another type of 802.11 device. I think this flag is OK
> however if once a device is found that uses wireless-regdb we'd
> simply disallow the 802.11 devices that have the internal reg data
> solution.

I'm growing mor eand more inclined to simply reject all this as
nonsense. I obviously can't and its why I'm seriously trying to find
ways to make this work in a sane architectural way that can scale, but
I hope you can see that this is really only bringing complexities. Can
you please, please try once more, and with all you got to push against
this internal firmware junk. Who the fuck could have decided this was
a good idea?

  Luis