[PATCH v2 0/3] wireless: fix 11d lockdep and move 11d hint to cfg80211

[PATCH v2 0/3] wireless: fix 11d lockdep and move 11d hint to cfg80211

[Luis R. Rodriguez]

This cures the lockdep warning, now with a mutex instead of a spin_lock
to allow sleeping during the driver reg notifier, and we finish the
move of the regulatory_hint_11d() to cfg80211. We also add some missing
lock calls and remove the cfg80211 mutex usage from the driver custom
regulatory settings wiphy_apply_custom_regulatory(). This caller is now
used internally within cfg80211 automatically for all cfg80211 drivers.

Luis R. Rodriguez (3):
  cfg80211: use goto out on 11d reg hint failure
  cfg80211: decouple regulatory variables from cfg80211_mutex
  cfg80211: enable country IE support to all cfg80211 drivers

 include/net/cfg80211.h |   14 ------------
 net/mac80211/mlme.c    |    6 +----
 net/wireless/core.c    |    4 +--
 net/wireless/reg.c     |   53 +++++++++++++++++++++++++++++++++++------------
 net/wireless/reg.h     |   15 +++++++++++++
 net/wireless/sme.c     |   16 ++++++++++++++
 6 files changed, 72 insertions(+), 36 deletions(-)

[PATCH v2 1/3] cfg80211: use goto out on 11d reg hint failure

[Luis R. Rodriguez]

This does not change functionality.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/wireless/reg.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index fb40428..2d1d183 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1774,10 +1774,8 @@ void regulatory_hint_11d(struct wiphy *wiphy,
 
 	mutex_lock(&cfg80211_mutex);
 
-	if (unlikely(!last_request)) {
-		mutex_unlock(&cfg80211_mutex);
-		return;
-	}
+	if (unlikely(!last_request))
+		goto out;
 
 	/* IE len must be evenly divisible by 2 */
 	if (country_ie_len & 0x01)
-- 
1.6.0.4

Re: [PATCH v2 1/3] cfg80211: use goto out on 11d reg hint failure

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

On Thu, 2009-07-30 at 10:19 -0700, Luis R. Rodriguez wrote:
> This does not change functionality.
> 
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
>  net/wireless/reg.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index fb40428..2d1d183 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1774,10 +1774,8 @@ void regulatory_hint_11d(struct wiphy *wiphy,
>  
>  	mutex_lock(&cfg80211_mutex);
>  
> -	if (unlikely(!last_request)) {
> -		mutex_unlock(&cfg80211_mutex);
> -		return;
> -	}
> +	if (unlikely(!last_request))
> +		goto out;
>  
>  	/* IE len must be evenly divisible by 2 */
>  	if (country_ie_len & 0x01)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH v2 2/3] cfg80211: decouple regulatory variables from cfg80211_mutex

[Luis R. Rodriguez]

We change regulatory code to be protected by its own regulatory
mutex and alleviate cfg80211_mutex to only be used to protect
cfg80211_rdev_list, the registered device list.

By doing this we will be able to work on regulatory core components
without having to have hog up the cfg80211_mutex. An example here is
we no longer need to use the cfg80211_mutex during driver specific
wiphy_apply_custom_regulatory(). We also no longer need it for the
the country IE regulatory hint; by doing so we end up curing this
new lockdep warning:

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.31-rc4-wl #12
-------------------------------------------------------
phy1/1709 is trying to acquire lock:
 (cfg80211_mutex){+.+.+.}, at: [<ffffffffa00af852>] regulatory_hint_11d+0x32/0x3f0 [cfg80211]

but task is already holding lock:
 (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0144228>] ieee80211_sta_work+0x108/0x10f0 [mac80211]

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #3 (&ifmgd->mtx){+.+.+.}:
       [<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
       [<ffffffff81085dd3>] lock_acquire+0xe3/0x120
       [<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
       [<ffffffffa0141bb8>] ieee80211_mgd_auth+0x108/0x1f0 [mac80211]
       [<ffffffffa0148563>] ieee80211_auth+0x13/0x20 [mac80211]
       [<ffffffffa00bc3a1>] __cfg80211_mlme_auth+0x1b1/0x2a0 [cfg80211]
       [<ffffffffa00bc516>] cfg80211_mlme_auth+0x86/0xc0 [cfg80211]
       [<ffffffffa00b368d>] nl80211_authenticate+0x21d/0x230 [cfg80211]
       [<ffffffff81416ba6>] genl_rcv_msg+0x1b6/0x1f0
       [<ffffffff81415c39>] netlink_rcv_skb+0x89/0xb0
       [<ffffffff814169d9>] genl_rcv+0x29/0x40
       [<ffffffff8141591d>] netlink_unicast+0x29d/0x2b0
       [<ffffffff81416514>] netlink_sendmsg+0x214/0x300
       [<ffffffff813e4407>] sock_sendmsg+0x107/0x130
       [<ffffffff813e45b9>] sys_sendmsg+0x189/0x320
       [<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
       [<ffffffffffffffff>] 0xffffffffffffffff

-> #2 (&wdev->mtx){+.+.+.}:
       [<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
       [<ffffffff81085dd3>] lock_acquire+0xe3/0x120
       [<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
       [<ffffffffa00ab304>] cfg80211_netdev_notifier_call+0x1a4/0x390 [cfg80211]
       [<ffffffff814f3dff>] notifier_call_chain+0x3f/0x80
       [<ffffffff81075a91>] raw_notifier_call_chain+0x11/0x20
       [<ffffffff813f665a>] dev_open+0x10a/0x120
       [<ffffffff813f59bd>] dev_change_flags+0x9d/0x1e0
       [<ffffffff8144eb6e>] devinet_ioctl+0x6fe/0x760
       [<ffffffff81450204>] inet_ioctl+0x94/0xc0
       [<ffffffff813e25fa>] sock_ioctl+0x6a/0x290
       [<ffffffff8111e911>] vfs_ioctl+0x31/0xa0
       [<ffffffff8111ea9a>] do_vfs_ioctl+0x8a/0x5c0
       [<ffffffff8111f069>] sys_ioctl+0x99/0xa0
       [<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
       [<ffffffffffffffff>] 0xffffffffffffffff

-> #1 (&rdev->mtx){+.+.+.}:
       [<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
       [<ffffffff81085dd3>] lock_acquire+0xe3/0x120
       [<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
       [<ffffffffa00ac4d0>] cfg80211_get_dev_from_ifindex+0x60/0x90 [cfg80211]
       [<ffffffffa00b21ff>] get_rdev_dev_by_info_ifindex+0x6f/0xa0 [cfg80211]
       [<ffffffffa00b51eb>] nl80211_set_interface+0x3b/0x260 [cfg80211]
       [<ffffffff81416ba6>] genl_rcv_msg+0x1b6/0x1f0
       [<ffffffff81415c39>] netlink_rcv_skb+0x89/0xb0
       [<ffffffff814169d9>] genl_rcv+0x29/0x40
       [<ffffffff8141591d>] netlink_unicast+0x29d/0x2b0
       [<ffffffff81416514>] netlink_sendmsg+0x214/0x300
       [<ffffffff813e4407>] sock_sendmsg+0x107/0x130
       [<ffffffff813e45b9>] sys_sendmsg+0x189/0x320
       [<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
       [<ffffffffffffffff>] 0xffffffffffffffff

other info that might help us debug this:

3 locks held by phy1/1709:
 #0:  ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff8106b45d>] worker_thread+0x19d/0x340
 #1:  (&ifmgd->work){+.+.+.}, at: [<ffffffff8106b45d>] worker_thread+0x19d/0x340
 #2:  (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0144228>] ieee80211_sta_work+0x108/0x10f0 [mac80211]

Reported-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/wireless/core.c |    4 +---
 net/wireless/reg.c  |   46 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 755cdf1..d8dbd7f 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -34,9 +34,7 @@ MODULE_DESCRIPTION("wireless configuration support");
 LIST_HEAD(cfg80211_rdev_list);
 
 /*
- * This is used to protect the cfg80211_rdev_list, cfg80211_regdomain,
- * country_ie_regdomain, the reg_beacon_list and the the last regulatory
- * request receipt (last_request).
+ * This is used to protect the cfg80211_rdev_list
  */
 DEFINE_MUTEX(cfg80211_mutex);
 
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 2d1d183..14e0c87 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -62,6 +62,16 @@ const struct ieee80211_regdomain *cfg80211_regdomain;
  */
 static const struct ieee80211_regdomain *country_ie_regdomain;
 
+/*
+ * Protects static reg.c components:
+ *     - cfg80211_world_regdom
+ *     - cfg80211_regdom
+ *     - country_ie_regdomain
+ *     - last_request
+ */
+DEFINE_MUTEX(reg_mutex);
+#define assert_reg_lock() WARN_ON(!mutex_is_locked(&reg_mutex))
+
 /* Used to queue up regulatory hints */
 static LIST_HEAD(reg_requests_list);
 static spinlock_t reg_requests_lock;
@@ -1293,7 +1303,7 @@ static void handle_channel_custom(struct wiphy *wiphy,
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *chan;
 
-	assert_cfg80211_lock();
+	assert_reg_lock();
 
 	sband = wiphy->bands[band];
 	BUG_ON(chan_idx >= sband->n_channels);
@@ -1342,14 +1352,14 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 	enum ieee80211_band band;
 	unsigned int bands_set = 0;
 
-	mutex_lock(&cfg80211_mutex);
+	mutex_lock(&reg_mutex);
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
 		if (!wiphy->bands[band])
 			continue;
 		handle_band_custom(wiphy, band, regd);
 		bands_set++;
 	}
-	mutex_unlock(&cfg80211_mutex);
+	mutex_unlock(&reg_mutex);
 
 	/*
 	 * no point in calling this if it won't have any effect
@@ -1495,7 +1505,7 @@ static int ignore_request(struct wiphy *wiphy,
  * Returns zero if all went fine, %-EALREADY if a regulatory domain had
  * already been set or other standard error codes.
  *
- * Caller must hold &cfg80211_mutex
+ * Caller must hold &cfg80211_mutex and &reg_mutex
  */
 static int __regulatory_hint(struct wiphy *wiphy,
 			     struct regulatory_request *pending_request)
@@ -1570,6 +1580,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 	BUG_ON(!reg_request->alpha2);
 
 	mutex_lock(&cfg80211_mutex);
+	mutex_lock(&reg_mutex);
 
 	if (wiphy_idx_valid(reg_request->wiphy_idx))
 		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
@@ -1585,6 +1596,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 	if (r == -EALREADY && wiphy && wiphy->strict_regulatory)
 		wiphy_update_regulatory(wiphy, reg_request->initiator);
 out:
+	mutex_unlock(&reg_mutex);
 	mutex_unlock(&cfg80211_mutex);
 }
 
@@ -1613,6 +1625,10 @@ static void reg_process_pending_beacon_hints(void)
 	struct cfg80211_registered_device *rdev;
 	struct reg_beacon *pending_beacon, *tmp;
 
+	/*
+	 * No need to hold the reg_mutex here as we just touch wiphys
+	 * and do not read or access regulatory variables.
+	 */
 	mutex_lock(&cfg80211_mutex);
 
 	/* This goes through the _pending_ beacon list */
@@ -1734,12 +1750,13 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2)
 }
 EXPORT_SYMBOL(regulatory_hint);
 
+/* Caller must hold reg_mutex */
 static bool reg_same_country_ie_hint(struct wiphy *wiphy,
 			u32 country_ie_checksum)
 {
 	struct wiphy *request_wiphy;
 
-	assert_cfg80211_lock();
+	assert_reg_lock();
 
 	if (unlikely(last_request->initiator !=
 	    NL80211_REGDOM_SET_BY_COUNTRY_IE))
@@ -1772,7 +1789,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
 	enum environment_cap env = ENVIRON_ANY;
 	struct regulatory_request *request;
 
-	mutex_lock(&cfg80211_mutex);
+	mutex_lock(&reg_mutex);
 
 	if (unlikely(!last_request))
 		goto out;
@@ -1883,7 +1900,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
 	request->country_ie_checksum = checksum;
 	request->country_ie_env = env;
 
-	mutex_unlock(&cfg80211_mutex);
+	mutex_unlock(&reg_mutex);
 
 	queue_regulatory_request(request);
 
@@ -1892,7 +1909,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
 free_rd_out:
 	kfree(rd);
 out:
-	mutex_unlock(&cfg80211_mutex);
+	mutex_unlock(&reg_mutex);
 }
 EXPORT_SYMBOL(regulatory_hint_11d);
 
@@ -2225,10 +2242,13 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 
 	assert_cfg80211_lock();
 
+	mutex_lock(&reg_mutex);
+
 	/* Note that this doesn't update the wiphys, this is done below */
 	r = __set_regdom(rd);
 	if (r) {
 		kfree(rd);
+		mutex_unlock(&reg_mutex);
 		return r;
 	}
 
@@ -2243,6 +2263,8 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 
 	nl80211_send_reg_change_event(last_request);
 
+	mutex_unlock(&reg_mutex);
+
 	return r;
 }
 
@@ -2253,16 +2275,20 @@ void reg_device_remove(struct wiphy *wiphy)
 
 	assert_cfg80211_lock();
 
+	mutex_lock(&reg_mutex);
+
 	kfree(wiphy->regd);
 
 	if (last_request)
 		request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
 
 	if (!request_wiphy || request_wiphy != wiphy)
-		return;
+		goto out;
 
 	last_request->wiphy_idx = WIPHY_IDX_STALE;
 	last_request->country_ie_env = ENVIRON_ANY;
+out:
+	mutex_unlock(&reg_mutex);
 }
 
 int regulatory_init(void)
@@ -2323,6 +2349,7 @@ void regulatory_exit(void)
 	cancel_work_sync(&reg_work);
 
 	mutex_lock(&cfg80211_mutex);
+	mutex_lock(&reg_mutex);
 
 	reset_regdomains();
 
@@ -2361,5 +2388,6 @@ void regulatory_exit(void)
 	}
 	spin_unlock(&reg_requests_lock);
 
+	mutex_unlock(&reg_mutex);
 	mutex_unlock(&cfg80211_mutex);
 }
-- 
1.6.0.4

Re: [PATCH v2 2/3] cfg80211: decouple regulatory variables from cfg80211_mutex

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 11171 bytes --]

On Thu, 2009-07-30 at 10:19 -0700, Luis R. Rodriguez wrote:
> We change regulatory code to be protected by its own regulatory
> mutex and alleviate cfg80211_mutex to only be used to protect
> cfg80211_rdev_list, the registered device list.
> 
> By doing this we will be able to work on regulatory core components
> without having to have hog up the cfg80211_mutex. An example here is
> we no longer need to use the cfg80211_mutex during driver specific
> wiphy_apply_custom_regulatory(). We also no longer need it for the
> the country IE regulatory hint; by doing so we end up curing this
> new lockdep warning:

Nice.

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.31-rc4-wl #12
> -------------------------------------------------------
> phy1/1709 is trying to acquire lock:
>  (cfg80211_mutex){+.+.+.}, at: [<ffffffffa00af852>] regulatory_hint_11d+0x32/0x3f0 [cfg80211]
> 
> but task is already holding lock:
>  (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0144228>] ieee80211_sta_work+0x108/0x10f0 [mac80211]
> 
> which lock already depends on the new lock.
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #3 (&ifmgd->mtx){+.+.+.}:
>        [<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
>        [<ffffffff81085dd3>] lock_acquire+0xe3/0x120
>        [<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
>        [<ffffffffa0141bb8>] ieee80211_mgd_auth+0x108/0x1f0 [mac80211]
>        [<ffffffffa0148563>] ieee80211_auth+0x13/0x20 [mac80211]
>        [<ffffffffa00bc3a1>] __cfg80211_mlme_auth+0x1b1/0x2a0 [cfg80211]
>        [<ffffffffa00bc516>] cfg80211_mlme_auth+0x86/0xc0 [cfg80211]
>        [<ffffffffa00b368d>] nl80211_authenticate+0x21d/0x230 [cfg80211]
>        [<ffffffff81416ba6>] genl_rcv_msg+0x1b6/0x1f0
>        [<ffffffff81415c39>] netlink_rcv_skb+0x89/0xb0
>        [<ffffffff814169d9>] genl_rcv+0x29/0x40
>        [<ffffffff8141591d>] netlink_unicast+0x29d/0x2b0
>        [<ffffffff81416514>] netlink_sendmsg+0x214/0x300
>        [<ffffffff813e4407>] sock_sendmsg+0x107/0x130
>        [<ffffffff813e45b9>] sys_sendmsg+0x189/0x320
>        [<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> -> #2 (&wdev->mtx){+.+.+.}:
>        [<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
>        [<ffffffff81085dd3>] lock_acquire+0xe3/0x120
>        [<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
>        [<ffffffffa00ab304>] cfg80211_netdev_notifier_call+0x1a4/0x390 [cfg80211]
>        [<ffffffff814f3dff>] notifier_call_chain+0x3f/0x80
>        [<ffffffff81075a91>] raw_notifier_call_chain+0x11/0x20
>        [<ffffffff813f665a>] dev_open+0x10a/0x120
>        [<ffffffff813f59bd>] dev_change_flags+0x9d/0x1e0
>        [<ffffffff8144eb6e>] devinet_ioctl+0x6fe/0x760
>        [<ffffffff81450204>] inet_ioctl+0x94/0xc0
>        [<ffffffff813e25fa>] sock_ioctl+0x6a/0x290
>        [<ffffffff8111e911>] vfs_ioctl+0x31/0xa0
>        [<ffffffff8111ea9a>] do_vfs_ioctl+0x8a/0x5c0
>        [<ffffffff8111f069>] sys_ioctl+0x99/0xa0
>        [<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> -> #1 (&rdev->mtx){+.+.+.}:
>        [<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
>        [<ffffffff81085dd3>] lock_acquire+0xe3/0x120
>        [<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
>        [<ffffffffa00ac4d0>] cfg80211_get_dev_from_ifindex+0x60/0x90 [cfg80211]
>        [<ffffffffa00b21ff>] get_rdev_dev_by_info_ifindex+0x6f/0xa0 [cfg80211]
>        [<ffffffffa00b51eb>] nl80211_set_interface+0x3b/0x260 [cfg80211]
>        [<ffffffff81416ba6>] genl_rcv_msg+0x1b6/0x1f0
>        [<ffffffff81415c39>] netlink_rcv_skb+0x89/0xb0
>        [<ffffffff814169d9>] genl_rcv+0x29/0x40
>        [<ffffffff8141591d>] netlink_unicast+0x29d/0x2b0
>        [<ffffffff81416514>] netlink_sendmsg+0x214/0x300
>        [<ffffffff813e4407>] sock_sendmsg+0x107/0x130
>        [<ffffffff813e45b9>] sys_sendmsg+0x189/0x320
>        [<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
>        [<ffffffffffffffff>] 0xffffffffffffffff
> 
> other info that might help us debug this:
> 
> 3 locks held by phy1/1709:
>  #0:  ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff8106b45d>] worker_thread+0x19d/0x340
>  #1:  (&ifmgd->work){+.+.+.}, at: [<ffffffff8106b45d>] worker_thread+0x19d/0x340
>  #2:  (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0144228>] ieee80211_sta_work+0x108/0x10f0 [mac80211]
> 
> Reported-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
>  net/wireless/core.c |    4 +---
>  net/wireless/reg.c  |   46 +++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index 755cdf1..d8dbd7f 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -34,9 +34,7 @@ MODULE_DESCRIPTION("wireless configuration support");
>  LIST_HEAD(cfg80211_rdev_list);
>  
>  /*
> - * This is used to protect the cfg80211_rdev_list, cfg80211_regdomain,
> - * country_ie_regdomain, the reg_beacon_list and the the last regulatory
> - * request receipt (last_request).
> + * This is used to protect the cfg80211_rdev_list
>   */
>  DEFINE_MUTEX(cfg80211_mutex);
>  
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 2d1d183..14e0c87 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -62,6 +62,16 @@ const struct ieee80211_regdomain *cfg80211_regdomain;
>   */
>  static const struct ieee80211_regdomain *country_ie_regdomain;
>  
> +/*
> + * Protects static reg.c components:
> + *     - cfg80211_world_regdom
> + *     - cfg80211_regdom
> + *     - country_ie_regdomain
> + *     - last_request
> + */
> +DEFINE_MUTEX(reg_mutex);
> +#define assert_reg_lock() WARN_ON(!mutex_is_locked(&reg_mutex))
> +
>  /* Used to queue up regulatory hints */
>  static LIST_HEAD(reg_requests_list);
>  static spinlock_t reg_requests_lock;
> @@ -1293,7 +1303,7 @@ static void handle_channel_custom(struct wiphy *wiphy,
>  	struct ieee80211_supported_band *sband;
>  	struct ieee80211_channel *chan;
>  
> -	assert_cfg80211_lock();
> +	assert_reg_lock();
>  
>  	sband = wiphy->bands[band];
>  	BUG_ON(chan_idx >= sband->n_channels);
> @@ -1342,14 +1352,14 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
>  	enum ieee80211_band band;
>  	unsigned int bands_set = 0;
>  
> -	mutex_lock(&cfg80211_mutex);
> +	mutex_lock(&reg_mutex);
>  	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
>  		if (!wiphy->bands[band])
>  			continue;
>  		handle_band_custom(wiphy, band, regd);
>  		bands_set++;
>  	}
> -	mutex_unlock(&cfg80211_mutex);
> +	mutex_unlock(&reg_mutex);
>  
>  	/*
>  	 * no point in calling this if it won't have any effect
> @@ -1495,7 +1505,7 @@ static int ignore_request(struct wiphy *wiphy,
>   * Returns zero if all went fine, %-EALREADY if a regulatory domain had
>   * already been set or other standard error codes.
>   *
> - * Caller must hold &cfg80211_mutex
> + * Caller must hold &cfg80211_mutex and &reg_mutex
>   */
>  static int __regulatory_hint(struct wiphy *wiphy,
>  			     struct regulatory_request *pending_request)
> @@ -1570,6 +1580,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>  	BUG_ON(!reg_request->alpha2);
>  
>  	mutex_lock(&cfg80211_mutex);
> +	mutex_lock(&reg_mutex);
>  
>  	if (wiphy_idx_valid(reg_request->wiphy_idx))
>  		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
> @@ -1585,6 +1596,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>  	if (r == -EALREADY && wiphy && wiphy->strict_regulatory)
>  		wiphy_update_regulatory(wiphy, reg_request->initiator);
>  out:
> +	mutex_unlock(&reg_mutex);
>  	mutex_unlock(&cfg80211_mutex);
>  }
>  
> @@ -1613,6 +1625,10 @@ static void reg_process_pending_beacon_hints(void)
>  	struct cfg80211_registered_device *rdev;
>  	struct reg_beacon *pending_beacon, *tmp;
>  
> +	/*
> +	 * No need to hold the reg_mutex here as we just touch wiphys
> +	 * and do not read or access regulatory variables.
> +	 */
>  	mutex_lock(&cfg80211_mutex);
>  
>  	/* This goes through the _pending_ beacon list */
> @@ -1734,12 +1750,13 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2)
>  }
>  EXPORT_SYMBOL(regulatory_hint);
>  
> +/* Caller must hold reg_mutex */
>  static bool reg_same_country_ie_hint(struct wiphy *wiphy,
>  			u32 country_ie_checksum)
>  {
>  	struct wiphy *request_wiphy;
>  
> -	assert_cfg80211_lock();
> +	assert_reg_lock();
>  
>  	if (unlikely(last_request->initiator !=
>  	    NL80211_REGDOM_SET_BY_COUNTRY_IE))
> @@ -1772,7 +1789,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
>  	enum environment_cap env = ENVIRON_ANY;
>  	struct regulatory_request *request;
>  
> -	mutex_lock(&cfg80211_mutex);
> +	mutex_lock(&reg_mutex);
>  
>  	if (unlikely(!last_request))
>  		goto out;
> @@ -1883,7 +1900,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
>  	request->country_ie_checksum = checksum;
>  	request->country_ie_env = env;
>  
> -	mutex_unlock(&cfg80211_mutex);
> +	mutex_unlock(&reg_mutex);
>  
>  	queue_regulatory_request(request);
>  
> @@ -1892,7 +1909,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
>  free_rd_out:
>  	kfree(rd);
>  out:
> -	mutex_unlock(&cfg80211_mutex);
> +	mutex_unlock(&reg_mutex);
>  }
>  EXPORT_SYMBOL(regulatory_hint_11d);
>  
> @@ -2225,10 +2242,13 @@ int set_regdom(const struct ieee80211_regdomain *rd)
>  
>  	assert_cfg80211_lock();
>  
> +	mutex_lock(&reg_mutex);
> +
>  	/* Note that this doesn't update the wiphys, this is done below */
>  	r = __set_regdom(rd);
>  	if (r) {
>  		kfree(rd);
> +		mutex_unlock(&reg_mutex);
>  		return r;
>  	}
>  
> @@ -2243,6 +2263,8 @@ int set_regdom(const struct ieee80211_regdomain *rd)
>  
>  	nl80211_send_reg_change_event(last_request);
>  
> +	mutex_unlock(&reg_mutex);
> +
>  	return r;
>  }
>  
> @@ -2253,16 +2275,20 @@ void reg_device_remove(struct wiphy *wiphy)
>  
>  	assert_cfg80211_lock();
>  
> +	mutex_lock(&reg_mutex);
> +
>  	kfree(wiphy->regd);
>  
>  	if (last_request)
>  		request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
>  
>  	if (!request_wiphy || request_wiphy != wiphy)
> -		return;
> +		goto out;
>  
>  	last_request->wiphy_idx = WIPHY_IDX_STALE;
>  	last_request->country_ie_env = ENVIRON_ANY;
> +out:
> +	mutex_unlock(&reg_mutex);
>  }
>  
>  int regulatory_init(void)
> @@ -2323,6 +2349,7 @@ void regulatory_exit(void)
>  	cancel_work_sync(&reg_work);
>  
>  	mutex_lock(&cfg80211_mutex);
> +	mutex_lock(&reg_mutex);
>  
>  	reset_regdomains();
>  
> @@ -2361,5 +2388,6 @@ void regulatory_exit(void)
>  	}
>  	spin_unlock(&reg_requests_lock);
>  
> +	mutex_unlock(&reg_mutex);
>  	mutex_unlock(&cfg80211_mutex);
>  }


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH v2 3/3] cfg80211: enable country IE support to all cfg80211 drivers

[Luis R. Rodriguez]

Since the bss is always set now once we are connected, if the
bss has its own information element we refer to it and pass that
instead of relying on mac80211's parsing.

Now all cfg80211 drivers get country IE support, automatically.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 include/net/cfg80211.h |   14 --------------
 net/mac80211/mlme.c    |    6 +-----
 net/wireless/reg.c     |    1 -
 net/wireless/reg.h     |   15 +++++++++++++++
 net/wireless/sme.c     |   16 ++++++++++++++++
 5 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e1b9235..fa72997 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1514,20 +1514,6 @@ unsigned int cfg80211_classify8021d(struct sk_buff *skb);
 extern int regulatory_hint(struct wiphy *wiphy, const char *alpha2);
 
 /**
- * regulatory_hint_11d - hints a country IE as a regulatory domain
- * @wiphy: the wireless device giving the hint (used only for reporting
- *	conflicts)
- * @country_ie: pointer to the country IE
- * @country_ie_len: length of the country IE
- *
- * We will intersect the rd with the what CRDA tells us should apply
- * for the alpha2 this country IE belongs to, this prevents APs from
- * sending us incorrect or outdated information against a country.
- */
-extern void regulatory_hint_11d(struct wiphy *wiphy,
-				u8 *country_ie,
-				u8 country_ie_len);
-/**
  * wiphy_apply_custom_regulatory - apply a custom driver regulatory domain
  * @wiphy: the wireless device we want to process the regulatory domain on
  * @regd: the custom regulatory domain to use for this wiphy
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0779ba1..5ff1496 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1846,12 +1846,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 					       bssid, ap_ht_cap_flags);
 	}
 
+	/* Note: country IE parsing is done for us by cfg80211 */
 	if (elems.country_elem) {
-		/* Note we are only reviewing this on beacons
-		 * for the BSSID we are associated to */
-		regulatory_hint_11d(local->hw.wiphy,
-			elems.country_elem, elems.country_elem_len);
-
 		/* TODO: IBSS also needs this */
 		if (elems.pwr_constr_elem)
 			ieee80211_handle_pwr_constr(sdata,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 14e0c87..06abd7a 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1911,7 +1911,6 @@ free_rd_out:
 out:
 	mutex_unlock(&reg_mutex);
 }
-EXPORT_SYMBOL(regulatory_hint_11d);
 
 static bool freq_is_chan_12_13_14(u16 freq)
 {
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index e37829a..662a9da 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -36,4 +36,19 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
 					struct ieee80211_channel *beacon_chan,
 					gfp_t gfp);
 
+/**
+ * regulatory_hint_11d - hints a country IE as a regulatory domain
+ * @wiphy: the wireless device giving the hint (used only for reporting
+ *	conflicts)
+ * @country_ie: pointer to the country IE
+ * @country_ie_len: length of the country IE
+ *
+ * We will intersect the rd with the what CRDA tells us should apply
+ * for the alpha2 this country IE belongs to, this prevents APs from
+ * sending us incorrect or outdated information against a country.
+ */
+void regulatory_hint_11d(struct wiphy *wiphy,
+			 u8 *country_ie,
+			 u8 country_ie_len);
+
 #endif  /* __NET_WIRELESS_REG_H */
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 3728d2b..bc0fb7d 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -13,6 +13,7 @@
 #include <net/cfg80211.h>
 #include <net/rtnetlink.h>
 #include "nl80211.h"
+#include "reg.h"
 
 struct cfg80211_conn {
 	struct cfg80211_connect_params params;
@@ -320,6 +321,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 			       struct cfg80211_bss *bss)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	u8 *country_ie;
 #ifdef CONFIG_WIRELESS_EXT
 	union iwreq_data wrqu;
 #endif
@@ -401,6 +403,20 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 
 	wdev->sme_state = CFG80211_SME_CONNECTED;
 	cfg80211_upload_connect_keys(wdev);
+
+	country_ie = (u8 *) ieee80211_bss_get_ie(bss, WLAN_EID_COUNTRY);
+
+	if (!country_ie)
+		return;
+
+	/*
+	 * ieee80211_bss_get()_ie ensures we can access:
+	 * - country_ie + 2, the start of the country ie data, and
+	 * - and country_ie[1] which is the IE length
+	 */
+	regulatory_hint_11d(wdev->wiphy,
+			    country_ie + 2,
+			    country_ie[1]);
 }
 
 void cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
-- 
1.6.0.4

Re: [PATCH v2 3/3] cfg80211: enable country IE support to all cfg80211 drivers

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 5192 bytes --]

On Thu, 2009-07-30 at 10:19 -0700, Luis R. Rodriguez wrote:
> Since the bss is always set now once we are connected, if the
> bss has its own information element we refer to it and pass that
> instead of relying on mac80211's parsing.
> 
> Now all cfg80211 drivers get country IE support, automatically.

Cool, thanks.

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> 
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
>  include/net/cfg80211.h |   14 --------------
>  net/mac80211/mlme.c    |    6 +-----
>  net/wireless/reg.c     |    1 -
>  net/wireless/reg.h     |   15 +++++++++++++++
>  net/wireless/sme.c     |   16 ++++++++++++++++
>  5 files changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index e1b9235..fa72997 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1514,20 +1514,6 @@ unsigned int cfg80211_classify8021d(struct sk_buff *skb);
>  extern int regulatory_hint(struct wiphy *wiphy, const char *alpha2);
>  
>  /**
> - * regulatory_hint_11d - hints a country IE as a regulatory domain
> - * @wiphy: the wireless device giving the hint (used only for reporting
> - *	conflicts)
> - * @country_ie: pointer to the country IE
> - * @country_ie_len: length of the country IE
> - *
> - * We will intersect the rd with the what CRDA tells us should apply
> - * for the alpha2 this country IE belongs to, this prevents APs from
> - * sending us incorrect or outdated information against a country.
> - */
> -extern void regulatory_hint_11d(struct wiphy *wiphy,
> -				u8 *country_ie,
> -				u8 country_ie_len);
> -/**
>   * wiphy_apply_custom_regulatory - apply a custom driver regulatory domain
>   * @wiphy: the wireless device we want to process the regulatory domain on
>   * @regd: the custom regulatory domain to use for this wiphy
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 0779ba1..5ff1496 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -1846,12 +1846,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
>  					       bssid, ap_ht_cap_flags);
>  	}
>  
> +	/* Note: country IE parsing is done for us by cfg80211 */
>  	if (elems.country_elem) {
> -		/* Note we are only reviewing this on beacons
> -		 * for the BSSID we are associated to */
> -		regulatory_hint_11d(local->hw.wiphy,
> -			elems.country_elem, elems.country_elem_len);
> -
>  		/* TODO: IBSS also needs this */
>  		if (elems.pwr_constr_elem)
>  			ieee80211_handle_pwr_constr(sdata,
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 14e0c87..06abd7a 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1911,7 +1911,6 @@ free_rd_out:
>  out:
>  	mutex_unlock(&reg_mutex);
>  }
> -EXPORT_SYMBOL(regulatory_hint_11d);
>  
>  static bool freq_is_chan_12_13_14(u16 freq)
>  {
> diff --git a/net/wireless/reg.h b/net/wireless/reg.h
> index e37829a..662a9da 100644
> --- a/net/wireless/reg.h
> +++ b/net/wireless/reg.h
> @@ -36,4 +36,19 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
>  					struct ieee80211_channel *beacon_chan,
>  					gfp_t gfp);
>  
> +/**
> + * regulatory_hint_11d - hints a country IE as a regulatory domain
> + * @wiphy: the wireless device giving the hint (used only for reporting
> + *	conflicts)
> + * @country_ie: pointer to the country IE
> + * @country_ie_len: length of the country IE
> + *
> + * We will intersect the rd with the what CRDA tells us should apply
> + * for the alpha2 this country IE belongs to, this prevents APs from
> + * sending us incorrect or outdated information against a country.
> + */
> +void regulatory_hint_11d(struct wiphy *wiphy,
> +			 u8 *country_ie,
> +			 u8 country_ie_len);
> +
>  #endif  /* __NET_WIRELESS_REG_H */
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index 3728d2b..bc0fb7d 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -13,6 +13,7 @@
>  #include <net/cfg80211.h>
>  #include <net/rtnetlink.h>
>  #include "nl80211.h"
> +#include "reg.h"
>  
>  struct cfg80211_conn {
>  	struct cfg80211_connect_params params;
> @@ -320,6 +321,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
>  			       struct cfg80211_bss *bss)
>  {
>  	struct wireless_dev *wdev = dev->ieee80211_ptr;
> +	u8 *country_ie;
>  #ifdef CONFIG_WIRELESS_EXT
>  	union iwreq_data wrqu;
>  #endif
> @@ -401,6 +403,20 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
>  
>  	wdev->sme_state = CFG80211_SME_CONNECTED;
>  	cfg80211_upload_connect_keys(wdev);
> +
> +	country_ie = (u8 *) ieee80211_bss_get_ie(bss, WLAN_EID_COUNTRY);
> +
> +	if (!country_ie)
> +		return;
> +
> +	/*
> +	 * ieee80211_bss_get()_ie ensures we can access:
> +	 * - country_ie + 2, the start of the country ie data, and
> +	 * - and country_ie[1] which is the IE length
> +	 */
> +	regulatory_hint_11d(wdev->wiphy,
> +			    country_ie + 2,
> +			    country_ie[1]);
>  }
>  
>  void cfg80211_connect_result(struct net_device *dev, const u8 *bssid,


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 0/3] wireless: fix 11d lockdep and move 11d hint to cfg80211

[Luis R. Rodriguez]

On Thu, Jul 30, 2009 at 10:19 AM, Luis R.
Rodriguez<lrodriguez@atheros.com> wrote:
> This cures the lockdep warning, now with a mutex instead of a spin_lock
> to allow sleeping during the driver reg notifier, and we finish the
> move of the regulatory_hint_11d() to cfg80211. We also add some missing
> lock calls and remove the cfg80211 mutex usage from the driver custom
> regulatory settings wiphy_apply_custom_regulatory(). This caller is now
> used internally within cfg80211 automatically for all cfg80211 drivers.
>
> Luis R. Rodriguez (3):
>  cfg80211: use goto out on 11d reg hint failure
>  cfg80211: decouple regulatory variables from cfg80211_mutex
>  cfg80211: enable country IE support to all cfg80211 drivers

Just hit a snag, will try to fix and respin. We were still trying to
get the rdev for the last country ie request when we plug in a second
card. Will try to use the wiphy_idx only.

  Luis

[PATCH v2 0/3] wireless: fix 11d lockdep and move 11d hint to cfg80211

[Luis R. Rodriguez]

This cures the lockdep warning, now with a mutex instead of a spin_lock
to allow sleeping during the driver reg notifier, and we finish the
move of the regulatory_hint_11d() to cfg80211. We also add some missing
lock calls and remove the cfg80211 mutex usage from the driver custom
regulatory settings wiphy_apply_custom_regulatory(). This caller is now
used internally within cfg80211 automatically for all cfg80211 drivers.

Luis R. Rodriguez (3):
  cfg80211: use goto out on 11d reg hint failure
  cfg80211: decouple regulatory variables from cfg80211_mutex
  cfg80211: enable country IE support to all cfg80211 drivers

Sorry -- forgot to address John...

 include/net/cfg80211.h |   14 ------------
 net/mac80211/mlme.c    |    6 +----
 net/wireless/core.c    |    4 +--
 net/wireless/reg.c     |   53 +++++++++++++++++++++++++++++++++++------------
 net/wireless/reg.h     |   15 +++++++++++++
 net/wireless/sme.c     |   16 ++++++++++++++
 6 files changed, 72 insertions(+), 36 deletions(-)