2015-03-29 06:16:59

by Peer, Ilan

[permalink] [raw]
Subject: [PATCH] cfg80211: Stop calling crda if it is not responsive

Patch eeca9fce1d71a4955855ceb0c3b13c1eb9db27c1 (cfg80211: Schedule
timeout for all CRDA call) introduced a regression, where in case
that crda is not installed (or not configured properly etc.), the
regulatory core will needlessly continue to call it.

Fix this by stop calling CRDA after too many continuous failures.

Signed-off-by: Ilan Peer <[email protected]>
---
net/wireless/nl80211.c | 2 +-
net/wireless/reg.c | 20 ++++++++++++++++++--
net/wireless/reg.h | 9 ++++++++-
3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b020853..3f4c768 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5655,7 +5655,7 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
}
}

- r = set_regdom(rd);
+ r = set_regdom(rd, REGD_SOURCE_CRDA);
/* set_regdom took ownership */
rd = NULL;

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 8c6cf52..3efc26e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -135,6 +135,11 @@ static spinlock_t reg_indoor_lock;
/* Used to track the userspace process controlling the indoor setting */
static u32 reg_is_indoor_portid;

+/* Max number of consecutive attempts to communicate with CRDA */
+#define REG_MAX_CRDA_TIMEOUTS 10
+
+static u32 reg_crda_timeouts;
+
static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
{
return rtnl_dereference(cfg80211_regdomain);
@@ -485,7 +490,7 @@ static void reg_regdb_search(struct work_struct *work)
mutex_unlock(&reg_regdb_search_mutex);

if (!IS_ERR_OR_NULL(regdom))
- set_regdom(regdom);
+ set_regdom(regdom, REGD_SOURCE_INTERNAL_DB);

rtnl_unlock();
}
@@ -544,6 +549,11 @@ static int call_crda(const char *alpha2)
/* query internal regulatory database (if it exists) */
reg_regdb_query(alpha2);

+ if (reg_crda_timeouts > REG_MAX_CRDA_TIMEOUTS) {
+ pr_info("Exceeded CRDA call max attempts. Not calling CRDA\n");
+ return -EINVAL;
+ }
+
return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, env);
}

@@ -2893,7 +2903,8 @@ static int reg_set_rd_country_ie(const struct ieee80211_regdomain *rd,
* multiple drivers can be ironed out later. Caller must've already
* kmalloc'd the rd structure.
*/
-int set_regdom(const struct ieee80211_regdomain *rd)
+int set_regdom(const struct ieee80211_regdomain *rd,
+ enum ieee80211_regd_source regd_src)
{
struct regulatory_request *lr;
bool user_reset = false;
@@ -2904,6 +2915,9 @@ int set_regdom(const struct ieee80211_regdomain *rd)
return -EINVAL;
}

+ if (regd_src == REGD_SOURCE_CRDA)
+ reg_crda_timeouts = 0;
+
lr = get_last_request();

/* Note that this doesn't update the wiphys, this is done below */
@@ -3063,6 +3077,7 @@ static void reg_timeout_work(struct work_struct *work)
{
REG_DBG_PRINT("Timeout while waiting for CRDA to reply, restoring regulatory settings\n");
rtnl_lock();
+ reg_crda_timeouts++;
restore_regulatory_settings(true);
rtnl_unlock();
}
@@ -3113,6 +3128,7 @@ int __init regulatory_init(void)
spin_lock_init(&reg_pending_beacons_lock);
spin_lock_init(&reg_indoor_lock);

+ reg_crda_timeouts = 0;
reg_regdb_size_check();

rcu_assign_pointer(cfg80211_regdomain, cfg80211_world_regdom);
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index a2c4e16..3f310a5 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -16,6 +16,11 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/

+enum ieee80211_regd_source {
+ REGD_SOURCE_INTERNAL_DB = 0,
+ REGD_SOURCE_CRDA,
+};
+
extern const struct ieee80211_regdomain __rcu *cfg80211_regdomain;

bool reg_is_valid_request(const char *alpha2);
@@ -46,7 +51,9 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy);
int __init regulatory_init(void);
void regulatory_exit(void);

-int set_regdom(const struct ieee80211_regdomain *rd);
+int set_regdom(const struct ieee80211_regdomain *rd,
+ enum ieee80211_regd_source regd_src);
+
unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
const struct ieee80211_reg_rule *rule);

--
1.9.1



2015-03-30 09:53:33

by Peer, Ilan

[permalink] [raw]
Subject: RE: [PATCH] cfg80211: Stop calling crda if it is not responsive

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWls
dG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0NCj4gU2VudDogTW9uZGF5LCBNYXJjaCAzMCwg
MjAxNSAxMjoxMQ0KPiBUbzogUGVlciwgSWxhbg0KPiBDYzogbGludXgtd2lyZWxlc3NAdmdlci5r
ZXJuZWwub3JnOyBtY2dyb2ZAc3VzZS5jb20NCj4gU3ViamVjdDogUmU6IFtQQVRDSF0gY2ZnODAy
MTE6IFN0b3AgY2FsbGluZyBjcmRhIGlmIGl0IGlzIG5vdCByZXNwb25zaXZlDQo+IA0KPiBPbiBT
dW4sIDIwMTUtMDMtMjkgYXQgMDk6MTggKzAzMDAsIElsYW4gUGVlciB3cm90ZToNCj4gPiBQYXRj
aCBlZWNhOWZjZTFkNzFhNDk1NTg1NWNlYjBjM2IxM2MxZWI5ZGIyN2MxIChjZmc4MDIxMToNCj4g
U2NoZWR1bGUNCj4gPiB0aW1lb3V0IGZvciBhbGwgQ1JEQSBjYWxsKSBpbnRyb2R1Y2VkIGEgcmVn
cmVzc2lvbiwgd2hlcmUgaW4gY2FzZSB0aGF0DQo+ID4gY3JkYSBpcyBub3QgaW5zdGFsbGVkIChv
ciBub3QgY29uZmlndXJlZCBwcm9wZXJseSBldGMuKSwgdGhlDQo+ID4gcmVndWxhdG9yeSBjb3Jl
IHdpbGwgbmVlZGxlc3NseSBjb250aW51ZSB0byBjYWxsIGl0Lg0KPiANCj4gQW5kIHByaW50IGEg
bWVzc2FnZSwgd2hpY2ggd2FzIHRoZSBjb21wbGFpbnQgOikNCg0KV2lsbCBmaXguDQoNCj4gDQo+
ID4gQEAgLTU0NCw2ICs1NDksMTEgQEAgc3RhdGljIGludCBjYWxsX2NyZGEoY29uc3QgY2hhciAq
YWxwaGEyKQ0KPiA+ICAJLyogcXVlcnkgaW50ZXJuYWwgcmVndWxhdG9yeSBkYXRhYmFzZSAoaWYg
aXQgZXhpc3RzKSAqLw0KPiA+ICAJcmVnX3JlZ2RiX3F1ZXJ5KGFscGhhMik7DQo+ID4NCj4gPiAr
CWlmIChyZWdfY3JkYV90aW1lb3V0cyA+IFJFR19NQVhfQ1JEQV9USU1FT1VUUykgew0KPiA+ICsJ
CXByX2luZm8oIkV4Y2VlZGVkIENSREEgY2FsbCBtYXggYXR0ZW1wdHMuIE5vdCBjYWxsaW5nDQo+
IENSREFcbiIpOw0KPiA+ICsJCXJldHVybiAtRUlOVkFMOw0KPiA+ICsJfQ0KPiANCj4gYW5kIHdv
bid0IHRoaXMganVzdCB0cmFkZSBvbmUgbWVzc2FnZSBmb3IgYW5vdGhlcj8/DQo+IA0KDQpUaGUg
b3JpZ2luYWwgaXNzdWUgaW50cm9kdWNlZCBpbiB0aGUgcHJldmlvdXMgcGF0Y2gsIHdhcyB0aGF0
IGluIGNhc2UgdGhhdCBjcmRhIGlzIG5vdCByZXNwb25zaXZlLCBvbiBlYWNoIHRpbWVvdXQsIGEg
bmV3IGZsb3cgdG8gY3JkYSB3YXMgaXNzdWVkLiBTbyB3aXRoIHRoaXMgY2hhbmdlLCB1cG9uIHJl
YWNoaW5nIHRoZSBsaW1pdCwgY3JkYSB3aWxsIG5vdCBiZSBjYWxsZWQgYW5kIG5vIHRpbWVvdXQg
d291bGQgYmUgc2NoZWR1bGVkLCBzbyB0aGlzIHdvdWxkIGJyZWFrIHRoZSBsb29wLg0KDQo+IE9y
IGluIGZhY3QsIGR1cGxpY2F0ZSBtZXNzYWdlcyAtICdjYWxsaW5nIGNyZGEnIGZvbGxvd2VkIGJ5
ICdleGNlZWRlZCc/DQo+IA0KDQpJJ2xsIGNsZWFyIHRoaXMgb25lLg0KDQpSZWdhcmRzLA0KDQpJ
bGFuLg0KDQo=

2015-03-30 09:15:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Stop calling crda if it is not responsive

On Sun, 2015-03-29 at 09:18 +0300, Ilan Peer wrote:

> @@ -3113,6 +3128,7 @@ int __init regulatory_init(void)
> spin_lock_init(&reg_pending_beacons_lock);
> spin_lock_init(&reg_indoor_lock);
>
> + reg_crda_timeouts = 0;

Also, this is not necessary as BSS in the kernel is 0-initialized.

johannes


2015-03-30 09:11:19

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Stop calling crda if it is not responsive

On Sun, 2015-03-29 at 09:18 +0300, Ilan Peer wrote:
> Patch eeca9fce1d71a4955855ceb0c3b13c1eb9db27c1 (cfg80211: Schedule
> timeout for all CRDA call) introduced a regression, where in case
> that crda is not installed (or not configured properly etc.), the
> regulatory core will needlessly continue to call it.

And print a message, which was the complaint :)

> @@ -544,6 +549,11 @@ static int call_crda(const char *alpha2)
> /* query internal regulatory database (if it exists) */
> reg_regdb_query(alpha2);
>
> + if (reg_crda_timeouts > REG_MAX_CRDA_TIMEOUTS) {
> + pr_info("Exceeded CRDA call max attempts. Not calling CRDA\n");
> + return -EINVAL;
> + }

and won't this just trade one message for another??

Or in fact, duplicate messages - 'calling crda' followed by 'exceeded'?

johannes