[PATCH 1/4] doc: document [General].Country main.conf option

[PATCH 1/4] doc: document [General].Country main.conf option

[James Prestwood]

This lets the user set a country code explicitly.
---
 src/iwd.config.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 422d4c03..bafb6b85 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -206,6 +206,16 @@ The group ``[General]`` contains general settings.
        required are LoadCredentialEncrypted or SetCredentialEncrypted, and the
        secret identifier should be named whatever SystemdEncrypt is set to.
 
+   * - Country
+     - Value: Country Code (ISO Alpha-2)
+
+       Requests the country be set for the system. Note that setting this is
+       simply a **request** to set the country, and does not guarantee the
+       country will be set. For a self-managed wiphy it is never possible to set
+       the country from userspace. For other devices any regulatory domain
+       request is just a 'hint' and ultimately left up to the kernel to set the
+       country.
+
 Network
 -------
 
-- 
2.34.3

[PATCH 2/4] wiphy: remove nl80211 from wiphy object

[James Prestwood]

The nl80211 object is already a global and there is no need
to track this in wiphy itself.
---
 src/wiphy.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index 28d4b66e..f295d87d 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -118,7 +118,6 @@ struct wiphy {
 	uint8_t extended_capabilities[EXT_CAP_LEN + 2]; /* max bitmap size + IE header */
 	uint8_t *iftype_extended_capabilities[NUM_NL80211_IFTYPES];
 	uint8_t rm_enabled_capabilities[7]; /* 5 size max + header */
-	struct l_genl_family *nl80211;
 	char regdom_country[2];
 	/* Work queue for this radio */
 	struct l_queue *work;
@@ -375,7 +374,6 @@ static void wiphy_free(void *data)
 	l_free(wiphy->model_str);
 	l_free(wiphy->vendor_str);
 	l_free(wiphy->driver_str);
-	l_genl_family_free(wiphy->nl80211);
 	l_queue_destroy(wiphy->work, destroy_work);
 	l_free(wiphy);
 }
@@ -1756,11 +1754,9 @@ static void wiphy_register(struct wiphy *wiphy)
 struct wiphy *wiphy_create(uint32_t wiphy_id, const char *name)
 {
 	struct wiphy *wiphy;
-	struct l_genl *genl = iwd_get_genl();
 
 	wiphy = wiphy_new(wiphy_id);
 	l_strlcpy(wiphy->name, name, sizeof(wiphy->name));
-	wiphy->nl80211 = l_genl_family_new(genl, NL80211_GENL_NAME);
 	l_queue_push_head(wiphy_list, wiphy);
 
 	if (!wiphy_is_managed(name))
@@ -2099,8 +2095,8 @@ static void wiphy_get_reg_domain(struct wiphy *wiphy)
 	msg = l_genl_msg_new(NL80211_CMD_GET_REG);
 	l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY, 4, &wiphy->id);
 
-	wiphy->get_reg_id = l_genl_family_send(wiphy->nl80211, msg,
-						wiphy_get_reg_cb, wiphy, NULL);
+	wiphy->get_reg_id = l_genl_family_send(nl80211, msg, wiphy_get_reg_cb,
+						wiphy, NULL);
 	if (!wiphy->get_reg_id) {
 		l_error("Error sending NL80211_CMD_GET_REG for %s", wiphy->name);
 		l_genl_msg_unref(msg);
-- 
2.34.3

[PATCH 3/4] wiphy: only do global GET_REG once

[James Prestwood]

Getting the regulatory domain for self-managed wiphys requires
providing the wiphy ID, but this is not the case for any device
under the global regulatory domain. This is not tied to any
specific wiphy so there is no need to supply a wiphy ID or do
a dump for each wiphy.

Now GET_REG will be tracked globally if its a global call and
will only be called once.
---
 src/wiphy.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index f295d87d..af675694 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -64,6 +64,7 @@ static struct l_hwdb *hwdb;
 static char **whitelist_filter;
 static char **blacklist_filter;
 static int mac_randomize_bytes = 6;
+static unsigned int get_reg_id;
 static char regdom_country[2];
 static uint32_t work_ids;
 static unsigned int wiphy_dump_id;
@@ -2076,7 +2077,10 @@ static void wiphy_get_reg_cb(struct l_genl_msg *msg, void *user_data)
 	uint32_t tmp;
 	bool global;
 
-	wiphy->get_reg_id = 0;
+	if (wiphy)
+		wiphy->get_reg_id = 0;
+	else
+		get_reg_id = 0;
 
 	/*
 	 * NL80211_CMD_GET_REG contains an NL80211_ATTR_WIPHY iff the wiphy
@@ -2091,16 +2095,28 @@ static void wiphy_get_reg_cb(struct l_genl_msg *msg, void *user_data)
 static void wiphy_get_reg_domain(struct wiphy *wiphy)
 {
 	struct l_genl_msg *msg;
+	unsigned int id;
+
+	/* Already doing a global dump */
+	if (!wiphy && get_reg_id)
+		return;
 
 	msg = l_genl_msg_new(NL80211_CMD_GET_REG);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY, 4, &wiphy->id);
 
-	wiphy->get_reg_id = l_genl_family_send(nl80211, msg, wiphy_get_reg_cb,
-						wiphy, NULL);
-	if (!wiphy->get_reg_id) {
-		l_error("Error sending NL80211_CMD_GET_REG for %s", wiphy->name);
+	if (wiphy)
+		l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY, 4, &wiphy->id);
+
+	id = l_genl_family_send(nl80211, msg, wiphy_get_reg_cb, wiphy, NULL);
+	if (!id) {
+		l_error("Error sending NL80211_CMD_GET_REG for %s",
+				wiphy ? wiphy->name : "(global)");
 		l_genl_msg_unref(msg);
 	}
+
+	if (wiphy)
+		wiphy->get_reg_id = id;
+	else
+		get_reg_id = id;
 }
 
 void wiphy_create_complete(struct wiphy *wiphy)
@@ -2117,7 +2133,7 @@ void wiphy_create_complete(struct wiphy *wiphy)
 
 	wiphy_set_station_capability_bits(wiphy);
 	wiphy_setup_rm_enabled_capabilities(wiphy);
-	wiphy_get_reg_domain(wiphy);
+	wiphy_get_reg_domain(wiphy->self_managed ? wiphy : NULL);
 
 	wiphy_print_basic_info(wiphy);
 }
@@ -2519,6 +2535,11 @@ static void wiphy_exit(void)
 		wiphy_dump_id = 0;
 	}
 
+	if (get_reg_id) {
+		l_genl_family_cancel(nl80211, get_reg_id);
+		get_reg_id = 0;
+	}
+
 	l_queue_destroy(wiphy_list, wiphy_free);
 	wiphy_list = NULL;
 
-- 
2.34.3

[PATCH 4/4] wiphy: support country code override

[James Prestwood]

Depending on the device the regulatory domain may never get
set on its own. This is worked around in wpa_supplicant by
allowing the user to supply a country code. The kernel
_should_ set this on its own based on the docs, but ultimately
will fall back to user defined if the driver doesn't set it.

IWD now needs to work around this problem by allowing users
to supply the country code in main.conf.

A few notes here:
 - Setting the country may not effect the actual regulatory
   domain. This is because the kernel still remains in control.
   Any request is merely a hint to the kernel.
 - This only applies to devices under the global regulatory
   domain. Self-managed devices will not allow setting the
   regdom.
---
 src/wiphy.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index af675694..b87cfefd 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -65,7 +65,9 @@ static char **whitelist_filter;
 static char **blacklist_filter;
 static int mac_randomize_bytes = 6;
 static unsigned int get_reg_id;
+static unsigned int set_reg_id;
 static char regdom_country[2];
+static char regdom_override[2];
 static uint32_t work_ids;
 static unsigned int wiphy_dump_id;
 
@@ -2090,6 +2092,14 @@ static void wiphy_get_reg_cb(struct l_genl_msg *msg, void *user_data)
 				NL80211_ATTR_UNSPEC) < 0;
 
 	wiphy_update_reg_domain(wiphy, global, msg);
+
+	if (global && regdom_override[0] && regdom_override[1]) {
+		if (regdom_override[0] != regdom_country[0] ||
+				regdom_override[1] != regdom_country[1])
+			l_warn("Regdom %c%c does not match override %c%c",
+				regdom_country[0], regdom_country[1],
+				regdom_override[0], regdom_override[1]);
+	}
 }
 
 static void wiphy_get_reg_domain(struct wiphy *wiphy)
@@ -2119,6 +2129,69 @@ static void wiphy_get_reg_domain(struct wiphy *wiphy)
 		get_reg_id = id;
 }
 
+static void wiphy_set_reg_cb(struct l_genl_msg *msg, void *user_data)
+{
+	int err = l_genl_msg_get_error(msg);
+
+	set_reg_id = 0;
+
+	/* Just warn if the CC was invalid, can't do much else here... */
+	if (err < 0)
+		l_warn("Error setting regulatory domain (%d)", err);
+
+	/*
+	 * Always get the regdom to see if the override actually worked. The
+	 * kernel may choose to ignore REQ_SET_REG requests. If so this will be
+	 * logged. Since this only applies to the global regdom do a global
+	 * GET_REG with wiphy set to NULL
+	 */
+	wiphy_get_reg_domain(NULL);
+}
+
+static void wiphy_set_reg_domain(const char *cc)
+{
+	struct l_genl_msg *msg;
+
+	regdom_override[0] = cc[0];
+	regdom_override[1] = cc[1];
+
+	msg = l_genl_msg_new(NL80211_CMD_REQ_SET_REG);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_REG_ALPHA2, 2, cc);
+
+	set_reg_id = l_genl_family_send(nl80211, msg, wiphy_set_reg_cb,
+					NULL, NULL);
+	/* Could try getting the regdom, but that likely wouldn't work either */
+	if (L_WARN_ON(!set_reg_id))
+		return;
+
+	l_debug("Trying to set country to %s", cc);
+}
+
+static void wiphy_setup_reg_domain(struct wiphy *wiphy)
+{
+	const struct l_settings *config = iwd_get_config();
+	const char *cc = l_settings_get_value(config, "General", "Country");
+
+	/*
+	 * At least do basic validation. If this fails force a GET_REG but do
+	 * not try setting the domain.
+	 */
+	if (cc) {
+		if (strlen(cc) != 2 || !l_ascii_isalpha(cc[0]) ||
+					!l_ascii_isalpha(cc[1])) {
+			l_error("Invalid setting [General].Country=%s", cc);
+			cc = NULL;
+		}
+	}
+
+	if (!cc || wiphy->self_managed) {
+		wiphy_get_reg_domain(wiphy->self_managed ? wiphy : NULL);
+		return;
+	}
+
+	wiphy_set_reg_domain(cc);
+}
+
 void wiphy_create_complete(struct wiphy *wiphy)
 {
 	wiphy_register(wiphy);
@@ -2133,7 +2206,7 @@ void wiphy_create_complete(struct wiphy *wiphy)
 
 	wiphy_set_station_capability_bits(wiphy);
 	wiphy_setup_rm_enabled_capabilities(wiphy);
-	wiphy_get_reg_domain(wiphy->self_managed ? wiphy : NULL);
+	wiphy_setup_reg_domain(wiphy);
 
 	wiphy_print_basic_info(wiphy);
 }
@@ -2540,6 +2613,11 @@ static void wiphy_exit(void)
 		get_reg_id = 0;
 	}
 
+	if (set_reg_id) {
+		l_genl_family_cancel(nl80211, set_reg_id);
+		set_reg_id = 0;
+	}
+
 	l_queue_destroy(wiphy_list, wiphy_free);
 	wiphy_list = NULL;
 
-- 
2.34.3

Re: [PATCH 2/4] wiphy: remove nl80211 from wiphy object

[Denis Kenzior]

Hi James,

On 9/7/22 18:31, James Prestwood wrote:
> The nl80211 object is already a global and there is no need
> to track this in wiphy itself.

Wiphy is a bit of a weird case since you have a global and a wiphy-specific dump 
for regdom.  As well as the regdom query.  But wouldn't you actually go the 
other direction?

l_genl_family_new creates a lightweight handle to the GENL object.  Once you 
unref this handle, any requests started on this handle are automagically 
destroyed.  So you strictly speaking do not need to track the command ids 
anymore.  So for example you could get rid of 'get_reg_id' fairly easily. 
'dump_id' is a bit trickier since it is used for conflict detection, but maybe..?

> ---
>   src/wiphy.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 

Regards,
-Denis

Re: [PATCH 3/4] wiphy: only do global GET_REG once

[Denis Kenzior]

Hi James,

On 9/7/22 18:32, James Prestwood wrote:
> Getting the regulatory domain for self-managed wiphys requires
> providing the wiphy ID, but this is not the case for any device
> under the global regulatory domain. This is not tied to any
> specific wiphy so there is no need to supply a wiphy ID or do
> a dump for each wiphy.

Are you sure?  I have an ath9k which isn't self-managed, but doesn't fall under 
the global regulatory domain:

denkenz@iwd-test ~/iwd $ iw reg get
global
country US: DFS-FCC
	(902 - 904 @ 2), (N/A, 30), (N/A)
	(904 - 920 @ 16), (N/A, 30), (N/A)
	(920 - 928 @ 8), (N/A, 30), (N/A)
	(2400 - 2472 @ 40), (N/A, 30), (N/A)
	(5150 - 5250 @ 80), (N/A, 23), (N/A), AUTO-BW
	(5250 - 5350 @ 80), (N/A, 24), (0 ms), DFS, AUTO-BW
	(5470 - 5730 @ 160), (N/A, 24), (0 ms), DFS
	(5730 - 5850 @ 80), (N/A, 30), (N/A), AUTO-BW
	(5850 - 5895 @ 40), (N/A, 27), (N/A), NO-OUTDOOR, AUTO-BW, PASSIVE-SCAN
	(5925 - 7125 @ 320), (N/A, 12), (N/A), NO-OUTDOOR, PASSIVE-SCAN
	(57240 - 71000 @ 2160), (N/A, 40), (N/A)

phy#3
country 99: DFS-UNSET
	(2402 - 2472 @ 40), (N/A, 20), (N/A)
	(2457 - 2482 @ 40), (N/A, 20), (N/A), PASSIVE-SCAN
	(5140 - 5360 @ 80), (N/A, 30), (N/A), PASSIVE-SCAN
	(5460 - 5860 @ 80), (N/A, 30), (N/A), PASSIVE-SCAN

> 
> Now GET_REG will be tracked globally if its a global call and
> will only be called once.

GET_REG acts like a dump.  Is there really any harm in using the wiphy id?

> ---
>   src/wiphy.c | 35 ++++++++++++++++++++++++++++-------
>   1 file changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/src/wiphy.c b/src/wiphy.c
> index f295d87d..af675694 100644
> --- a/src/wiphy.c
> +++ b/src/wiphy.c
> @@ -64,6 +64,7 @@ static struct l_hwdb *hwdb;
>   static char **whitelist_filter;
>   static char **blacklist_filter;
>   static int mac_randomize_bytes = 6;
> +static unsigned int get_reg_id;
>   static char regdom_country[2];
>   static uint32_t work_ids;
>   static unsigned int wiphy_dump_id;
> @@ -2076,7 +2077,10 @@ static void wiphy_get_reg_cb(struct l_genl_msg *msg, void *user_data)
>   	uint32_t tmp;
>   	bool global;
>   
> -	wiphy->get_reg_id = 0;
> +	if (wiphy)
> +		wiphy->get_reg_id = 0;
> +	else
> +		get_reg_id = 0;
>   
>   	/*
>   	 * NL80211_CMD_GET_REG contains an NL80211_ATTR_WIPHY iff the wiphy
> @@ -2091,16 +2095,28 @@ static void wiphy_get_reg_cb(struct l_genl_msg *msg, void *user_data)
>   static void wiphy_get_reg_domain(struct wiphy *wiphy)
>   {
>   	struct l_genl_msg *msg;
> +	unsigned int id;
> +
> +	/* Already doing a global dump */
> +	if (!wiphy && get_reg_id)
> +		return;
>   
>   	msg = l_genl_msg_new(NL80211_CMD_GET_REG);
> -	l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY, 4, &wiphy->id);
>   
> -	wiphy->get_reg_id = l_genl_family_send(nl80211, msg, wiphy_get_reg_cb,
> -						wiphy, NULL);
> -	if (!wiphy->get_reg_id) {
> -		l_error("Error sending NL80211_CMD_GET_REG for %s", wiphy->name);
> +	if (wiphy)
> +		l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY, 4, &wiphy->id);
> +
> +	id = l_genl_family_send(nl80211, msg, wiphy_get_reg_cb, wiphy, NULL);
> +	if (!id) {
> +		l_error("Error sending NL80211_CMD_GET_REG for %s",
> +				wiphy ? wiphy->name : "(global)");
>   		l_genl_msg_unref(msg);
>   	}
> +
> +	if (wiphy)
> +		wiphy->get_reg_id = id;
> +	else
> +		get_reg_id = id;
>   }
>   
>   void wiphy_create_complete(struct wiphy *wiphy)
> @@ -2117,7 +2133,7 @@ void wiphy_create_complete(struct wiphy *wiphy)
>   
>   	wiphy_set_station_capability_bits(wiphy);
>   	wiphy_setup_rm_enabled_capabilities(wiphy);
> -	wiphy_get_reg_domain(wiphy);
> +	wiphy_get_reg_domain(wiphy->self_managed ? wiphy : NULL);
>   
>   	wiphy_print_basic_info(wiphy);
>   }
> @@ -2519,6 +2535,11 @@ static void wiphy_exit(void)
>   		wiphy_dump_id = 0;
>   	}
>   
> +	if (get_reg_id) {
> +		l_genl_family_cancel(nl80211, get_reg_id);
> +		get_reg_id = 0;
> +	}
> +
>   	l_queue_destroy(wiphy_list, wiphy_free);
>   	wiphy_list = NULL;
>   

Regards,
-Denis

Re: [PATCH 4/4] wiphy: support country code override

[Denis Kenzior]

Hi James,

On 9/7/22 18:32, James Prestwood wrote:
> Depending on the device the regulatory domain may never get
> set on its own. This is worked around in wpa_supplicant by
> allowing the user to supply a country code. The kernel
> _should_ set this on its own based on the docs, but ultimately
> will fall back to user defined if the driver doesn't set it.
> 
> IWD now needs to work around this problem by allowing users
> to supply the country code in main.conf.
> 
> A few notes here:
>   - Setting the country may not effect the actual regulatory
>     domain. This is because the kernel still remains in control.
>     Any request is merely a hint to the kernel.
>   - This only applies to devices under the global regulatory
>     domain. Self-managed devices will not allow setting the
>     regdom.
> ---
>   src/wiphy.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 79 insertions(+), 1 deletion(-)
> 

<snip>

> +static void wiphy_setup_reg_domain(struct wiphy *wiphy)
> +{
> +	const struct l_settings *config = iwd_get_config();
> +	const char *cc = l_settings_get_value(config, "General", "Country");
> +
> +	/*
> +	 * At least do basic validation. If this fails force a GET_REG but do
> +	 * not try setting the domain.
> +	 */
> +	if (cc) {
> +		if (strlen(cc) != 2 || !l_ascii_isalpha(cc[0]) ||
> +					!l_ascii_isalpha(cc[1])) {
> +			l_error("Invalid setting [General].Country=%s", cc);
> +			cc = NULL;
> +		}
> +	}
> +

So why is this override being done per-wiphy detected and not at global level?

As far as I understand you can only set the global regdom.  So this should be 
done in wiphy_init or better yet, inside manager.c ?

> +	if (!cc || wiphy->self_managed) {
> +		wiphy_get_reg_domain(wiphy->self_managed ? wiphy : NULL);
> +		return;
> +	}
> +
> +	wiphy_set_reg_domain(cc);
> +}
> +
>   void wiphy_create_complete(struct wiphy *wiphy)
>   {
>   	wiphy_register(wiphy);

Regards,
-Denis