[PATCH 0/3] cfg80211: process pending regulatory requests

[PATCH 0/3] cfg80211: process pending regulatory requests

[Luis R. Rodriguez]

Sander spotted that regulatory requests get stalled when
cfg80211 is built-in. This issue is hit since regulatory
requests only trigger a udev rule to run CRDA when
being processed and if cfg80211 is built-in the udev rule
that kicks CRDA could end up doing nothing as the filesystem
path that has CRDA may not be mounted yet. This series of
patches addresses this situation by allowing us to reprocess
the last pending regulatory request. We add support for that
by first taking into consideration the RCU case where the
request being processed is the last request, then by
trying to reprocess the last request if we find it hasn't
been processed yet when checking the queues, and lastly by
adding some opportunistic checks of the pending regulatory
work when bringing an interface up or down.

If folks want to consider this for stable the first two
seem least intrusive and address the issue but those two
patches still require a trigger to process the queue.
Typically the queues will be processed on a system after
bootup after the interface comes up and finds some beacon
hints on 5 gHz, or when a user asks to change regulatory
domains. The last patch tries to avoid requiring this
and I consider it more an enhancement.

Luis R. Rodriguez (3):
  cfg80211: allow reprocessing of pending requests
  cfg80211: fix processing world regdomain when non modular
  cfg80211: processing regulatory requests on netdev notifier

 net/wireless/core.c |  2 ++
 net/wireless/reg.c  | 21 ++++++++++++++-------
 net/wireless/reg.h  |  1 +
 3 files changed, 17 insertions(+), 7 deletions(-)

-- 
1.8.4.3

[PATCH 1/3] cfg80211: allow reprocessing of pending requests

[Luis R. Rodriguez]

This addresses the RCU component of allowing us to reprocess
the last regulatory request.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 9b897fc..5be2d7c 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -244,19 +244,21 @@ static char user_alpha2[2];
 module_param(ieee80211_regdom, charp, 0444);
 MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 
-static void reg_kfree_last_request(void)
+static void reg_kfree_last_request(struct regulatory_request *lr)
 {
-	struct regulatory_request *lr;
-
-	lr = get_last_request();
-
 	if (lr != &core_request_world && lr)
 		kfree_rcu(lr, rcu_head);
 }
 
 static void reg_update_last_request(struct regulatory_request *request)
 {
-	reg_kfree_last_request();
+	struct regulatory_request *lr;
+
+	lr = get_last_request();
+	if (lr == request)
+		return;
+
+	reg_kfree_last_request(lr);
 	rcu_assign_pointer(last_request, request);
 }
 
-- 
1.8.4.3

[PATCH 2/3] cfg80211: fix processing world regdomain when non modular

[Luis R. Rodriguez]

This allows processing of the last regulatory request when
we determine its still pending. This fixes the issue reported
by Sander of when cfg80211 is built-in and no further regulatory
domain processing can happen. This occurred because if the
filesystem was not mounted upon kicking off the udev rule
to run CRDA then the last request will always be left
unprocessed.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5be2d7c..ecf364e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1737,7 +1737,7 @@ static void reg_process_pending_hints(void)
 
 	/* When last_request->processed becomes true this will be rescheduled */
 	if (lr && !lr->processed) {
-		REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
+		reg_process_hint(lr);
 		return;
 	}
 
-- 
1.8.4.3

[PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Luis R. Rodriguez]

This adds a trigger to review any pending regulatory
requests whenever an 802.11 device interface is brought
down or up. We use this as an opportunistic trigger
for checking the regulatory work queues as otherwise
they they're only checked upon an initial regulatory
request or when beacon hints are found.

This opportunistic mechanism can be used to trigger
kicking the queues regulatory queues at any time from
userspace without having to change the regulatory state.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/core.c | 2 ++
 net/wireless/reg.c  | 5 +++++
 net/wireless/reg.h  | 1 +
 3 files changed, 8 insertions(+)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index d89dee2..bc0f518 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -812,6 +812,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		SET_NETDEV_DEVTYPE(dev, &wiphy_type);
 		break;
 	case NETDEV_REGISTER:
+		reg_process_pending_work();
 		/*
 		 * NB: cannot take rdev->mtx here because this may be
 		 * called within code protected by it when interfaces
@@ -871,6 +872,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		wake_up(&rdev->dev_wait);
 		break;
 	case NETDEV_UP:
+		reg_process_pending_work();
 		cfg80211_update_iface_num(rdev, wdev->iftype, 1);
 		wdev_lock(wdev);
 		switch (wdev->iftype) {
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ecf364e..1a049b3 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -196,6 +196,11 @@ static DECLARE_WORK(reg_work, reg_todo);
 static void reg_timeout_work(struct work_struct *work);
 static DECLARE_DELAYED_WORK(reg_timeout, reg_timeout_work);
 
+void reg_process_pending_work(void)
+{
+	schedule_work(&reg_work);
+}
+
 /* We keep a static world regulatory domain in case of the absence of CRDA */
 static const struct ieee80211_regdomain world_regdom = {
 	.n_reg_rules = 6,
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index 02bd8f4..063fb7a 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -36,6 +36,7 @@ void regulatory_exit(void);
 int set_regdom(const struct ieee80211_regdomain *rd);
 
 bool reg_last_request_cell_base(void);
+void reg_process_pending_work(void);
 
 /**
  * regulatory_hint_found_beacon - hints a beacon was found on a channel
-- 
1.8.4.3

Re: [PATCH 0/3] cfg80211: process pending regulatory requests

[Sander Eikelenboom]

Thursday, December 19, 2013, 9:53:16 PM, you wrote:

> Sander spotted that regulatory requests get stalled when
> cfg80211 is built-in. This issue is hit since regulatory
> requests only trigger a udev rule to run CRDA when
> being processed and if cfg80211 is built-in the udev rule
> that kicks CRDA could end up doing nothing as the filesystem
> path that has CRDA may not be mounted yet. This series of
> patches addresses this situation by allowing us to reprocess
> the last pending regulatory request. We add support for that
> by first taking into consideration the RCU case where the
> request being processed is the last request, then by
> trying to reprocess the last request if we find it hasn't
> been processed yet when checking the queues, and lastly by
> adding some opportunistic checks of the pending regulatory
> work when bringing an interface up or down.

> If folks want to consider this for stable the first two
> seem least intrusive and address the issue but those two
> patches still require a trigger to process the queue.
> Typically the queues will be processed on a system after
> bootup after the interface comes up and finds some beacon
> hints on 5 gHz, or when a user asks to change regulatory
> domains. The last patch tries to avoid requiring this
> and I consider it more an enhancement.

> Luis R. Rodriguez (3):
>   cfg80211: allow reprocessing of pending requests
>   cfg80211: fix processing world regdomain when non modular
>   cfg80211: processing regulatory requests on netdev notifier

>  net/wireless/core.c |  2 ++
>  net/wireless/reg.c  | 21 ++++++++++++++-------
>  net/wireless/reg.h  |  1 +
>  3 files changed, 17 insertions(+), 7 deletions(-)


Hi Luis,

I have tested the patches with 3.13-rc4 with wireless-next pulled on top
(patch 1 doesn't apply cleanly to 3.13-rc4 vanilla).

I can report that it works for me with both cases you describe:
- only patches 1 and 2 applied
- all patches applied

Thanks for fixing it !

--

Sander

Re: [PATCH 0/3] cfg80211: process pending regulatory requests

[Sander Eikelenboom]

Hi All,

"Give me a ping, Vasili. One ping only, please."

Or in other words .. what is the status of this patchset by Luis ?
It doesn't seem to be in wireless-next yet, nor does it seem to be backported for 3.13 and older ?

--

Sander


Thursday, December 19, 2013, 9:53:16 PM, you wrote:

> Sander spotted that regulatory requests get stalled when
> cfg80211 is built-in. This issue is hit since regulatory
> requests only trigger a udev rule to run CRDA when
> being processed and if cfg80211 is built-in the udev rule
> that kicks CRDA could end up doing nothing as the filesystem
> path that has CRDA may not be mounted yet. This series of
> patches addresses this situation by allowing us to reprocess
> the last pending regulatory request. We add support for that
> by first taking into consideration the RCU case where the
> request being processed is the last request, then by
> trying to reprocess the last request if we find it hasn't
> been processed yet when checking the queues, and lastly by
> adding some opportunistic checks of the pending regulatory
> work when bringing an interface up or down.

> If folks want to consider this for stable the first two
> seem least intrusive and address the issue but those two
> patches still require a trigger to process the queue.
> Typically the queues will be processed on a system after
> bootup after the interface comes up and finds some beacon
> hints on 5 gHz, or when a user asks to change regulatory
> domains. The last patch tries to avoid requiring this
> and I consider it more an enhancement.

> Luis R. Rodriguez (3):
>   cfg80211: allow reprocessing of pending requests
>   cfg80211: fix processing world regdomain when non modular
>   cfg80211: processing regulatory requests on netdev notifier

>  net/wireless/core.c |  2 ++
>  net/wireless/reg.c  | 21 ++++++++++++++-------
>  net/wireless/reg.h  |  1 +
>  3 files changed, 17 insertions(+), 7 deletions(-)

Re: [PATCH 0/3] cfg80211: process pending regulatory requests

[Johannes Berg]

On Mon, 2014-01-06 at 14:10 +0100, Sander Eikelenboom wrote:
> Hi All,
> 
> "Give me a ping, Vasili. One ping only, please."
> 
> Or in other words .. what is the status of this patchset by Luis ?

Well, I went on vacation the day Luis posted it, and now I'm back ...
I'll need to review it, etc. If anyone wants to port it to 3.13, be my
guest, but since this is not an immediate regression I doubt it makes
sense to try to bubble it into 3.13 from my tree at this point just
before the release.

johannes

Re: [PATCH 1/3] cfg80211: allow reprocessing of pending requests

[Johannes Berg]

On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
> This addresses the RCU component of allowing us to reprocess
> the last regulatory request.

That commit message makes very little sense to me.

johannes

Re: [PATCH 2/3] cfg80211: fix processing world regdomain when non modular

[Johannes Berg]

On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
> This allows processing of the last regulatory request when
> we determine its still pending. This fixes the issue reported
> by Sander of when cfg80211 is built-in and no further regulatory
> domain processing can happen. This occurred because if the
> filesystem was not mounted upon kicking off the udev rule
> to run CRDA then the last request will always be left
> unprocessed.

It'd be good to comment on what this actually changes and what happens,
in addition to discussing the user-visible effect?

johannes

Re: [PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Johannes Berg]

On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
> This adds a trigger to review any pending regulatory
> requests whenever an 802.11 device interface is brought
> down or up. We use this as an opportunistic trigger
> for checking the regulatory work queues as otherwise
> they they're only checked upon an initial regulatory
> request or when beacon hints are found.
> 
> This opportunistic mechanism can be used to trigger
> kicking the queues regulatory queues at any time from
> userspace without having to change the regulatory state.

I don't like this. Can't we just have a self-contained timeout on the
userspace request instead?

johannes

Re: [PATCH 1/3] cfg80211: allow reprocessing of pending requests

[Sander Eikelenboom]

Tuesday, January 7, 2014, 4:34:09 PM, you wrote:

> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>> This addresses the RCU component of allowing us to reprocess
>> the last regulatory request.

> That commit message makes very little sense to me.

> johannes

Hi Luis,

It seems i haven't seen any response from you regarding this
patch series ? (could have been i missed it, i'm not subscribed to linux-wireless)

 --
 Sander

Re: [PATCH 1/3] cfg80211: allow reprocessing of pending requests

[Luis R. Rodriguez]

On Thu, Jan 23, 2014 at 5:16 AM, Sander Eikelenboom
<linux@eikelenboom.it> wrote:
>
> Tuesday, January 7, 2014, 4:34:09 PM, you wrote:
>
>> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>>> This addresses the RCU component of allowing us to reprocess
>>> the last regulatory request.
>
>> That commit message makes very little sense to me.
>
>> johannes
>
> Hi Luis,
>
> It seems i haven't seen any response from you regarding this
> patch series ? (could have been i missed it, i'm not subscribed to linux-wireless)

I quit my job, went on vacation and during vacation lost my phone and
the ChromeBook PIxel [0] I bought and had installed my own OS on
decided to poop out thanks to the stupidity behind Google's policy on
a half assed developer mode on that thing. In summary I have been
ramping up a development environment, ramping up on a new job, have no
802.11 hardware to even test any alternative I come up with so -- I
think this will take a bit of time, but am starting to get at least a
development environment going again. Hope to look at this by next
week.

[0] https://plus.google.com/101487746899672598413/posts/9rS6qvGU8Q1

  Luis

Re: [PATCH 1/3] cfg80211: allow reprocessing of pending requests

[Sander Eikelenboom]

Saturday, January 25, 2014, 12:14:37 AM, you wrote:

> On Thu, Jan 23, 2014 at 5:16 AM, Sander Eikelenboom
> <linux@eikelenboom.it> wrote:
>>
>> Tuesday, January 7, 2014, 4:34:09 PM, you wrote:
>>
>>> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>>>> This addresses the RCU component of allowing us to reprocess
>>>> the last regulatory request.
>>
>>> That commit message makes very little sense to me.
>>
>>> johannes
>>
>> Hi Luis,
>>
>> It seems i haven't seen any response from you regarding this
>> patch series ? (could have been i missed it, i'm not subscribed to linux-wireless)

> I quit my job, went on vacation and during vacation lost my phone and
> the ChromeBook PIxel [0] I bought and had installed my own OS on
> decided to poop out thanks to the stupidity behind Google's policy on
> a half assed developer mode on that thing. In summary I have been
> ramping up a development environment, ramping up on a new job, have no
> 802.11 hardware to even test any alternative I come up with so -- I
> think this will take a bit of time, but am starting to get at least a
> development environment going again. Hope to look at this by next
> week.

NP, good luck with getting everything back together,
I was just wondering if it was still on someone's todo :-)


> [0] https://plus.google.com/101487746899672598413/posts/9rS6qvGU8Q1

>   Luis

Re: [PATCH 1/3] cfg80211: allow reprocessing of pending requests

[Luis R. Rodriguez]

On Tue, Jan 7, 2014 at 7:34 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>> This addresses the RCU component of allowing us to reprocess
>> the last regulatory request.
>
> That commit message makes very little sense to me.

Yeah sorry about that, it was a run-on sentence following the subject,
specifically it addressed the issue observed when trying to reprocess
the last regulatory request. I'll send something more coherent.

  Luis

Re: [PATCH 2/3] cfg80211: fix processing world regdomain when non modular

[Luis R. Rodriguez]

On Tue, Jan 7, 2014 at 7:35 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>> This allows processing of the last regulatory request when
>> we determine its still pending. This fixes the issue reported
>> by Sander of when cfg80211 is built-in and no further regulatory
>> domain processing can happen. This occurred because if the
>> filesystem was not mounted upon kicking off the udev rule
>> to run CRDA then the last request will always be left
>> unprocessed.
>
> It'd be good to comment on what this actually changes and what happens,
> in addition to discussing the user-visible effect?

Sure.

  Luis

Re: [PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Luis R. Rodriguez]

On Tue, Jan 7, 2014 at 7:35 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>> This adds a trigger to review any pending regulatory
>> requests whenever an 802.11 device interface is brought
>> down or up. We use this as an opportunistic trigger
>> for checking the regulatory work queues as otherwise
>> they they're only checked upon an initial regulatory
>> request or when beacon hints are found.
>>
>> This opportunistic mechanism can be used to trigger
>> kicking the queues regulatory queues at any time from
>> userspace without having to change the regulatory state.
>
> I don't like this. Can't we just have a self-contained timeout on the
> userspace request instead?

Sure I'll add this and send a v2 series.

  Luis

Re: [PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Luis R. Rodriguez]

On Tue, Feb 18, 2014 at 5:24 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Tue, Jan 7, 2014 at 7:35 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Thu, 2013-12-19 at 12:53 -0800, Luis R. Rodriguez wrote:
>>> This adds a trigger to review any pending regulatory
>>> requests whenever an 802.11 device interface is brought
>>> down or up. We use this as an opportunistic trigger
>>> for checking the regulatory work queues as otherwise
>>> they they're only checked upon an initial regulatory
>>> request or when beacon hints are found.
>>>
>>> This opportunistic mechanism can be used to trigger
>>> kicking the queues regulatory queues at any time from
>>> userspace without having to change the regulatory state.
>>
>> I don't like this. Can't we just have a self-contained timeout on the
>> userspace request instead?
>
> Sure I'll add this and send a v2 series.

Come to think of it -- I am not a fan of the timer. What time do we
set for it? What is reasonable? This is what made me instead want to
use the notifier hooks. You shouldn't be able to bring up an interface
without firmware so hence the hook there, the other one upon register
was just another opportunistic one.

Please consider this and if you still think a timer is best let me
know what the timer value should be :D

  Luis

Re: [PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Arik Nemtsov]

On Thu, Feb 27, 2014 at 7:20 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Thu, Feb 27, 2014 at 5:21 AM, Arik Nemtsov <arik@wizery.com> wrote:
>> Seems the actual patch is missing?
>
> Yeah sorry I sent out a v3.

Yea finally noticed. Thanks.

Arik

Re: [PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Luis R. Rodriguez]

On Thu, Feb 27, 2014 at 5:21 AM, Arik Nemtsov <arik@wizery.com> wrote:
> Seems the actual patch is missing?

Yeah sorry I sent out a v3.

 Luis

Re: [PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Arik Nemtsov]

On Wed, Feb 26, 2014 at 3:09 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> This adds a trigger to review any pending regulatory
> requests whenever an 802.11 device interface is brought
> down or up. We use this as an opportunistic trigger
> for checking the regulatory work queues as otherwise
> they they're only checked upon an initial regulatory
> request or when beacon hints are found.
>
> This opportunistic mechanism can be used to trigger
> kicking the queues regulatory queues at any time from
> userspace without having to change the regulatory state.
>
> A timer just waiting upon init is not that appropriate
> as when it should be triggered will depend on systems,
> a much better approach is to use and add opportunistic
> triggers. The interface coming up is typically a good
> indicator that filesystems have been mounted since
> firmware is required for some devices.
>
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
> ---
>  net/wireless/reg.h | 1 +
>  1 file changed, 1 insertion(+)

Seems the actual patch is missing?

Arik

[PATCH 3/3] cfg80211: processing regulatory requests on netdev notifier

[Luis R. Rodriguez]

This adds a trigger to review any pending regulatory
requests whenever an 802.11 device interface is brought
down or up. We use this as an opportunistic trigger
for checking the regulatory work queues as otherwise
they they're only checked upon an initial regulatory
request or when beacon hints are found.

This opportunistic mechanism can be used to trigger
kicking the queues regulatory queues at any time from
userspace without having to change the regulatory state.

A timer just waiting upon init is not that appropriate
as when it should be triggered will depend on systems,
a much better approach is to use and add opportunistic
triggers. The interface coming up is typically a good
indicator that filesystems have been mounted since
firmware is required for some devices.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index 37c180d..51912b3 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -37,6 +37,7 @@ unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
 				   const struct ieee80211_reg_rule *rule);
 
 bool reg_last_request_cell_base(void);
+void reg_process_pending_work(void);
 
 /**
  * regulatory_hint_found_beacon - hints a beacon was found on a channel
-- 
1.8.5.3