| Message ID | 1427609931-6113-1-git-send-email-ilan.peer@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2015-03-29 at 09:18 +0300, Ilan Peer wrote: > @@ -3113,6 +3128,7 @@ int __init regulatory_init(void) > spin_lock_init(®_pending_beacons_lock); > spin_lock_init(®_indoor_lock); > > + reg_crda_timeouts = 0; Also, this is not necessary as BSS in the kernel is 0-initialized. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Johannes Berg [mailto:johannes@sipsolutions.net] > Sent: Monday, March 30, 2015 12:11 > To: Peer, Ilan > Cc: linux-wireless@vger.kernel.org; mcgrof@suse.com > 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 :) Will fix. > > > @@ -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?? > The original issue introduced in the previous patch, was that in case that crda is not responsive, on each timeout, a new flow to crda was issued. So with this change, upon reaching the limit, crda will not be called and no timeout would be scheduled, so this would break the loop. > Or in fact, duplicate messages - 'calling crda' followed by 'exceeded'? > I'll clear this one. Regards, Ilan.
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(®_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(®_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(®_pending_beacons_lock); spin_lock_init(®_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);
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 <ilan.peer@intel.com> --- net/wireless/nl80211.c | 2 +- net/wireless/reg.c | 20 ++++++++++++++++++-- net/wireless/reg.h | 9 ++++++++- 3 files changed, 27 insertions(+), 4 deletions(-)