Return-path: Received: from mail-qc0-f172.google.com ([209.85.216.172]:33373 "EHLO mail-qc0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750747AbaETIdq (ORCPT ); Tue, 20 May 2014 04:33:46 -0400 Received: by mail-qc0-f172.google.com with SMTP id l6so201074qcy.17 for ; Tue, 20 May 2014 01:33:45 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1399798250-20987-1-git-send-email-emmanuel.grumbach@intel.com> <1399798250-20987-3-git-send-email-emmanuel.grumbach@intel.com> From: Arik Nemtsov Date: Tue, 20 May 2014 11:33:30 +0300 Message-ID: (sfid-20140520_103351_559974_88BC3A65) Subject: Re: [PATCH 2/7] cfg80211: introduce regulatory flags controlling bw To: "Luis R. Rodriguez" Cc: Emmanuel Grumbach , Johannes Berg , linux-wireless , Arik Nemtsov Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Tue, May 20, 2014 at 11:26 AM, Luis R. Rodriguez wrote: > On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach > wrote: >> From: Arik Nemtsov >> >> Allow setting bandwidth related regulatory flags. These flags are mapped >> to the corresponding channel flags in the specified range. >> Make sure the new flags are consulted when calculating the maximum >> bandwidth allowed by a regulatory-rule. >> >> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a >> channel. >> >> Change-Id: I1b0506ab332cdd268cbf4b02e03574f5c30ba5c0 >> Signed-off-by: Arik Nemtsov >> Signed-off-by: Emmanuel Grumbach >> --- >> include/uapi/linux/nl80211.h | 12 ++++++++++++ >> net/wireless/reg.c | 30 ++++++++++++++++++++++++++++-- >> 2 files changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h >> index 406010d..f332231 100644 >> --- a/include/uapi/linux/nl80211.h >> +++ b/include/uapi/linux/nl80211.h >> @@ -2558,6 +2558,11 @@ enum nl80211_sched_scan_match_attr { >> * @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated >> * base on contiguous rules and wider channels will be allowed to cross >> * multiple contiguous/overlapping frequency ranges. >> + * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT >> + * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation >> + * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation >> + * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed >> + * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed >> */ >> enum nl80211_reg_rule_flags { >> NL80211_RRF_NO_OFDM = 1<<0, >> @@ -2570,11 +2575,18 @@ enum nl80211_reg_rule_flags { >> NL80211_RRF_NO_IR = 1<<7, >> __NL80211_RRF_NO_IBSS = 1<<8, >> NL80211_RRF_AUTO_BW = 1<<11, >> + NL80211_RRF_GO_CONCURRENT = 1<<12, >> + NL80211_RRF_NO_HT40MINUS = 1<<13, >> + NL80211_RRF_NO_HT40PLUS = 1<<14, >> + NL80211_RRF_NO_80MHZ = 1<<15, >> + NL80211_RRF_NO_160MHZ = 1<<16, >> }; > > Automatic calculation on max bandwidth scales better, I'd much prefer > these only be used and properly documented to only be used in cases > where we want to be explicit about this, I don't think this should be > the automatic behavior, or at least your patch in no way explains any > justification as to why the move is being done, so I cannot guess what > the logic was here. We're not going to use CRDA/wireless-regdb as the regulatory data source. These flags fit the reg-data storage format of the intel FW. It's just a different way of doing things. > >> #define NL80211_RRF_PASSIVE_SCAN NL80211_RRF_NO_IR >> #define NL80211_RRF_NO_IBSS NL80211_RRF_NO_IR >> #define NL80211_RRF_NO_IR NL80211_RRF_NO_IR >> +#define NL80211_RRF_NO_HT40 (NL80211_RRF_NO_HT40MINUS |\ >> + NL80211_RRF_NO_HT40PLUS) >> >> /* For backport compatibility with older userspace */ >> #define NL80211_RRF_NO_IR_ALL (NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS) >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c >> index 558b0e3..efd6d0d 100644 >> --- a/net/wireless/reg.c >> +++ b/net/wireless/reg.c >> @@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy) >> return get_cfg80211_regdom(); >> } >> >> -unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd, >> - const struct ieee80211_reg_rule *rule) >> +static unsigned int >> +reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd, >> + const struct ieee80211_reg_rule *rule) > > This change is renaming a routine we used heavily to something tucked > in the closet and then introducing the usage of the new flags, which > have no justifications nor proper regdb updates, this also doesn't > even account for the old regdb situations so I'll have to provide a > huge NACK for all these reasons. > > You want to consider wireless-regdb / CRDA when making these changes. I can rename things any way you'd like. Not sure how this can hurt old regdb situations though. The new flags are not used in any rules there. Arik