[PATCH 0/3] cfg80211: few regulatory beacon hint enhancements

[PATCH 0/3] cfg80211: few regulatory beacon hint enhancements

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

Johannes, these now go tested by Ben, please merge.

Luis R. Rodriguez (3):
  cfg80211: do not process beacon hints if one is already queued
  cfg80211: move reg_is_world_roaming()
  cfg80211: move world roaming check for beacon hints

 net/wireless/reg.c |   48 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 14 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] cfg80211: do not process beacon hints if one is already queued

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

Regulatory beacon hints are used to help with world roaming
and as it is right now we learn from a beacon hint processed
on one wiphy to all other wiphys. The processing of beacon
hints however is scheduled and if we have a lot of interfaces
we may hit the case that we'll queue a the same beacon hint
many times until its processed.

To avoid this do a lookup on the queued up beacon hints prior
to adding a new beacon hint. If the beacon hint is removed
from the pending reg beacon hint list then it would be processed
and we'd ensure all wiphys would have learned from it, if its
on the pending reg beacon list we'd now find it prior to it
being processed.

Tested-by: Ben Greear <greearb@candelatech.com>
Reported-by: Ben Greear <greearb@candelatech.com>
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 6e53089..a2d5a03 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2051,11 +2051,23 @@ static bool freq_is_chan_12_13_14(u16 freq)
 	return false;
 }
 
+static bool pending_reg_beacon(struct ieee80211_channel *beacon_chan)
+{
+	struct reg_beacon *pending_beacon;
+
+	list_for_each_entry(pending_beacon, &reg_pending_beacons, list)
+		if (beacon_chan->center_freq ==
+		    pending_beacon->chan.center_freq)
+			return true;
+	return false;
+}
+
 int regulatory_hint_found_beacon(struct wiphy *wiphy,
 				 struct ieee80211_channel *beacon_chan,
 				 gfp_t gfp)
 {
 	struct reg_beacon *reg_beacon;
+	bool processing;
 
 	if (likely((beacon_chan->beacon_found ||
 	    (beacon_chan->flags & IEEE80211_CHAN_RADAR) ||
@@ -2063,6 +2075,13 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
 	     !freq_is_chan_12_13_14(beacon_chan->center_freq)))))
 		return 0;
 
+	spin_lock_bh(&reg_pending_beacons_lock);
+	processing = pending_reg_beacon(beacon_chan);
+	spin_unlock_bh(&reg_pending_beacons_lock);
+
+	if (processing)
+		return 0;
+
 	reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
 	if (!reg_beacon)
 		return -ENOMEM;
-- 
1.7.10.4

[PATCH 2/3] cfg80211: move reg_is_world_roaming()

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

This will be used later by other code. This has no
functional change.

Tested-by: Ben Greear <greearb@candelatech.com>
Reported-by: Ben Greear <greearb@candelatech.com>
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index a2d5a03..f649b5e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1042,6 +1042,18 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 	return false;
 }
 
+static bool reg_is_world_roaming(struct wiphy *wiphy)
+{
+	if (is_world_regdom(cfg80211_regdomain->alpha2) ||
+	    (wiphy->regd && is_world_regdom(wiphy->regd->alpha2)))
+		return true;
+	if (last_request &&
+	    last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY)
+		return true;
+	return false;
+}
+
 static void handle_reg_beacon(struct wiphy *wiphy,
 			      unsigned int chan_idx,
 			      struct reg_beacon *reg_beacon)
@@ -1128,18 +1140,6 @@ static void wiphy_update_beacon_reg(struct wiphy *wiphy)
 	}
 }
 
-static bool reg_is_world_roaming(struct wiphy *wiphy)
-{
-	if (is_world_regdom(cfg80211_regdomain->alpha2) ||
-	    (wiphy->regd && is_world_regdom(wiphy->regd->alpha2)))
-		return true;
-	if (last_request &&
-	    last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY)
-		return true;
-	return false;
-}
-
 /* Reap the advantages of previously found beacons */
 static void reg_process_beacons(struct wiphy *wiphy)
 {
-- 
1.7.10.4

[PATCH 3/3] cfg80211: move world roaming check for beacon hints

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

We should not add new beacon hints even if the wiphy
is not world roaming. Without this we were always adding
a beacon hint if not world roaming for every non world
roaming wiphy interface.

Tested-by: Ben Greear <greearb@candelatech.com>
Reported-by: Ben Greear <greearb@candelatech.com>
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index f649b5e..bdc42d9 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1076,6 +1076,9 @@ static void handle_reg_beacon(struct wiphy *wiphy,
 
 	chan->beacon_found = true;
 
+	if (!reg_is_world_roaming(wiphy))
+		return;
+
 	if (wiphy->flags & WIPHY_FLAG_DISABLE_BEACON_HINTS)
 		return;
 
@@ -1149,8 +1152,6 @@ static void reg_process_beacons(struct wiphy *wiphy)
 	 */
 	if (!last_request)
 		return;
-	if (!reg_is_world_roaming(wiphy))
-		return;
 	wiphy_update_beacon_reg(wiphy);
 }
 
-- 
1.7.10.4

Re: [PATCH 0/3] cfg80211: few regulatory beacon hint enhancements

[Johannes Berg]

On Wed, 2012-12-19 at 10:53 -0800, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
> 
> Johannes, these now go tested by Ben, please merge.

Applied.

johannes

Re: [PATCH 3/3] cfg80211: move world roaming check for beacon hints

[Johannes Berg]

On Wed, 2012-12-19 at 10:53 -0800, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
> 
> We should not add new beacon hints even if the wiphy
> is not world roaming. Without this we were always adding
> a beacon hint if not world roaming for every non world
> roaming wiphy interface.

This introduced a locking bug, lockdep splat below. I'm going to drop
the series for my pull request, please respin.

johannes


[ 2442.916674] ===============================
[ 2442.917287] [ INFO: suspicious RCU usage. ]
[ 2442.919345] 3.7.0-rc1+ #868 Tainted: G        W  O
[ 2442.921729] -------------------------------
[ 2442.923741] /home/johannes/sys/wireless/net/wireless/reg.c:125 suspicious rcu_dereference_protected() usage!
[ 2442.928222] 
[ 2442.928222] other info that might help us debug this:
[ 2442.928222] 
[ 2442.930621] 
[ 2442.930621] rcu_scheduler_active = 1, debug_locks = 1
[ 2442.931408] 4 locks held by kworker/0:3/1773:
[ 2442.931948]  #0:  (events){.+.+.+}, at: [<ffffffff810615b7>] process_one_work+0x197/0x770
[ 2442.933022]  #1:  (reg_work){+.+.+.}, at: [<ffffffff810615b7>] process_one_work+0x197/0x770
[ 2442.934128]  #2:  (cfg80211_mutex){+.+.+.}, at: [<ffffffffa01c0706>] reg_todo+0x186/0x580 [cfg80211]
[ 2442.935310]  #3:  (&(&reg_pending_beacons_lock)->rlock){+.-...}, at: [<ffffffffa01c0712>] reg_todo+0x192/0x580 [cfg80211]
[ 2442.936690] 
[ 2442.936690] stack backtrace:
[ 2442.937217] Pid: 1773, comm: kworker/0:3 Tainted: G        W  O 3.7.0-rc1+ #868
[ 2442.938119] Call Trace:
[ 2442.938427]  [<ffffffff8109f8b2>] lockdep_rcu_suspicious+0xe2/0x130
[ 2442.939192]  [<ffffffffa01bf887>] get_cfg80211_regdom+0x57/0x60 [cfg80211]
[ 2442.940104]  [<ffffffffa01bffe1>] handle_reg_beacon.isra.14+0x61/0x110 [cfg80211]
[ 2442.941010]  [<ffffffff815081b1>] ? _raw_spin_lock_bh+0x71/0x80
[ 2442.941725]  [<ffffffffa01c07a8>] reg_todo+0x228/0x580 [cfg80211]
[ 2442.942476]  [<ffffffff81061627>] process_one_work+0x207/0x770
[ 2442.943172]  [<ffffffff810615b7>] ? process_one_work+0x197/0x770
[ 2442.943893]  [<ffffffff8109ebb2>] ? get_lock_stats+0x22/0x70
[ 2442.944574]  [<ffffffffa01c0580>] ? regdom_changes+0x60/0x60 [cfg80211]
[ 2442.945367]  [<ffffffff81062a2e>] worker_thread+0x14e/0x3e0
[ 2442.946059]  [<ffffffff810628e0>] ? manage_workers+0x280/0x280
[ 2442.946762]  [<ffffffff8106911b>] kthread+0xdb/0xe0
[ 2442.947353]  [<ffffffff81069040>] ? __init_kthread_worker+0x70/0x70
[ 2442.948130]  [<ffffffff815099ec>] ret_from_fork+0x7c/0xb0
[ 2442.948760]  [<ffffffff81069040>] ? __init_kthread_worker+0x70/0x70
[ 2442.949515] 
[ 2442.949706] ===============================
[ 2442.950290] [ INFO: suspicious RCU usage. ]
[ 2442.950800] 3.7.0-rc1+ #868 Tainted: G        W  O
[ 2442.951379] -------------------------------
[ 2442.951887] /home/johannes/sys/wireless/net/wireless/reg.c:131 suspicious rcu_dereference_protected() usage!
[ 2442.953051] 
[ 2442.953051] other info that might help us debug this:
[ 2442.953051] 
[ 2442.954021] 
[ 2442.954021] rcu_scheduler_active = 1, debug_locks = 1
[ 2442.954807] 4 locks held by kworker/0:3/1773:
[ 2442.955331]  #0:  (events){.+.+.+}, at: [<ffffffff810615b7>] process_one_work+0x197/0x770
[ 2442.956381]  #1:  (reg_work){+.+.+.}, at: [<ffffffff810615b7>] process_one_work+0x197/0x770
[ 2442.957604]  #2:  (cfg80211_mutex){+.+.+.}, at: [<ffffffffa01c0706>] reg_todo+0x186/0x580 [cfg80211]
[ 2442.958909]  #3:  (&(&reg_pending_beacons_lock)->rlock){+.-...}, at: [<ffffffffa01c0712>] reg_todo+0x192/0x580 [cfg80211]
[ 2442.960366] 
[ 2442.960366] stack backtrace:
[ 2442.960901] Pid: 1773, comm: kworker/0:3 Tainted: G        W  O 3.7.0-rc1+ #868
[ 2442.961780] Call Trace:
[ 2442.962102]  [<ffffffff8109f8b2>] lockdep_rcu_suspicious+0xe2/0x130
[ 2442.962869]  [<ffffffffa01bfcdf>] get_wiphy_regdom.isra.10+0x5f/0x70 [cfg80211]
[ 2442.963765]  [<ffffffffa01bfff0>] handle_reg_beacon.isra.14+0x70/0x110 [cfg80211]
[ 2442.964663]  [<ffffffff815081b1>] ? _raw_spin_lock_bh+0x71/0x80
[ 2442.965383]  [<ffffffffa01c07a8>] reg_todo+0x228/0x580 [cfg80211]
[ 2442.966142]  [<ffffffff81061627>] process_one_work+0x207/0x770
[ 2442.966856]  [<ffffffff810615b7>] ? process_one_work+0x197/0x770
[ 2442.967584]  [<ffffffff8109ebb2>] ? get_lock_stats+0x22/0x70
[ 2442.968272]  [<ffffffffa01c0580>] ? regdom_changes+0x60/0x60 [cfg80211]
[ 2442.969073]  [<ffffffff81062a2e>] worker_thread+0x14e/0x3e0
[ 2442.969749]  [<ffffffff810628e0>] ? manage_workers+0x280/0x280
[ 2442.970597]  [<ffffffff8106911b>] kthread+0xdb/0xe0
[ 2442.971303]  [<ffffffff81069040>] ? __init_kthread_worker+0x70/0x70
[ 2442.972055]  [<ffffffff815099ec>] ret_from_fork+0x7c/0xb0
[ 2442.972771]  [<ffffffff81069040>] ? __init_kthread_worker+0x70/0x70
[ 2442.973609] 
[ 2442.973800] ===============================
[ 2442.974322] [ INFO: suspicious RCU usage. ]
[ 2442.974829] 3.7.0-rc1+ #868 Tainted: G        W  O
[ 2442.975451] -------------------------------
[ 2442.975958] /home/johannes/sys/wireless/net/wireless/reg.c:144 suspicious rcu_dereference_protected() usage!
[ 2442.977170] 
[ 2442.977170] other info that might help us debug this:
[ 2442.977170] 
[ 2442.978201] 
[ 2442.978201] rcu_scheduler_active = 1, debug_locks = 1
[ 2442.978988] 4 locks held by kworker/0:3/1773:
[ 2442.979522]  #0:  (events){.+.+.+}, at: [<ffffffff810615b7>] process_one_work+0x197/0x770
[ 2442.980654]  #1:  (reg_work){+.+.+.}, at: [<ffffffff810615b7>] process_one_work+0x197/0x770
[ 2442.981726]  #2:  (cfg80211_mutex){+.+.+.}, at: [<ffffffffa01c0706>] reg_todo+0x186/0x580 [cfg80211]
[ 2442.983048]  #3:  (&(&reg_pending_beacons_lock)->rlock){+.-...}, at: [<ffffffffa01c0712>] reg_todo+0x192/0x580 [cfg80211]
[ 2442.984560] 
[ 2442.984560] stack backtrace:
[ 2442.985086] Pid: 1773, comm: kworker/0:3 Tainted: G        W  O 3.7.0-rc1+ #868
[ 2442.985956] Call Trace:
[ 2442.986265]  [<ffffffff8109f8b2>] lockdep_rcu_suspicious+0xe2/0x130
[ 2442.987014]  [<ffffffffa01bf827>] get_last_request+0x57/0x60 [cfg80211]
[ 2442.987801]  [<ffffffffa01bfff8>] handle_reg_beacon.isra.14+0x78/0x110 [cfg80211]
[ 2442.988694]  [<ffffffff815081b1>] ? _raw_spin_lock_bh+0x71/0x80
[ 2442.989404]  [<ffffffffa01c07a8>] reg_todo+0x228/0x580 [cfg80211]
[ 2442.990277]  [<ffffffff81061627>] process_one_work+0x207/0x770
[ 2442.991026]  [<ffffffff810615b7>] ? process_one_work+0x197/0x770
[ 2442.991754]  [<ffffffff8109ebb2>] ? get_lock_stats+0x22/0x70
[ 2442.992439]  [<ffffffffa01c0580>] ? regdom_changes+0x60/0x60 [cfg80211]
[ 2442.993265]  [<ffffffff81062a2e>] worker_thread+0x14e/0x3e0
[ 2442.993997]  [<ffffffff810628e0>] ? manage_workers+0x280/0x280
[ 2442.994684]  [<ffffffff8106911b>] kthread+0xdb/0xe0
[ 2442.995271]  [<ffffffff81069040>] ? __init_kthread_worker+0x70/0x70
[ 2442.996070]  [<ffffffff815099ec>] ret_from_fork+0x7c/0xb0
[ 2442.996762]  [<ffffffff81069040>] ? __init_kthread_worker+0x70/0x70

Re: [PATCH 3/3] cfg80211: move world roaming check for beacon hints

[Ben Greear]

On 12/27/2012 09:16 AM, Johannes Berg wrote:
> On Wed, 2012-12-19 at 10:53 -0800, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
>>
>> We should not add new beacon hints even if the wiphy
>> is not world roaming. Without this we were always adding
>> a beacon hint if not world roaming for every non world
>> roaming wiphy interface.
>
> This introduced a locking bug, lockdep splat below. I'm going to drop
> the series for my pull request, please respin.

Were new patches ever posted for this?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 3/3] cfg80211: move world roaming check for beacon hints

[Johannes Berg]

On Mon, 2013-01-21 at 09:39 -0800, Ben Greear wrote:
> On 12/27/2012 09:16 AM, Johannes Berg wrote:
> > On Wed, 2012-12-19 at 10:53 -0800, Luis R. Rodriguez wrote:
> >> From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>
> >>
> >> We should not add new beacon hints even if the wiphy
> >> is not world roaming. Without this we were always adding
> >> a beacon hint if not world roaming for every non world
> >> roaming wiphy interface.
> >
> > This introduced a locking bug, lockdep splat below. I'm going to drop
> > the series for my pull request, please respin.
> 
> Were new patches ever posted for this?

I fixed it and applied the patches.

johannes