On 09/14/2012 02:11 AM, Luis R. Rodriguez wrote:
On Mon, Sep 10, 2012 at 3:06 AM, Mahesh Palivela <maheshp@xxxxxxxxxxx> wrote:
VHT (11ac) regulatory new design. VHT channel center freq, bandwidth and
primary channel are used to decide if that channel config is permitted in
current regulatory domain.
Please include me on cfg80211 regulatory changes in the future. My
review below. But in short I am not in agreement with this approach
for a few reasons explained below. I removed all hunks from my reply
for which I had no comments for.
Stanislaw felt flags based approach is not scalable, so is this new
approach.
+ *
+ * @chan_width1: channel bandwidth
Comment here is chan_width1 but you have only chan_width
Removed 1.
+ freq_range = ®_rule->freq_range;
+
+ if (freq_range->max_bandwidth_khz < bw_khz)
+ return false;
Do you really need the above two lines ? The reg_does_bw_fit() should
have figured this out no?
This is still required. what if the rule says max allowed BW is 40 MHz
and our desired is 80 MHz.
+
+ /* find left and right arms of center freq */
+ left_end_freq = center_freq - (bw_khz/2);
+ right_end_freq = center_freq + (bw_khz/2);
+
+ /* left_end_freq and right_end_freq are edge of left and right
+ * channels. Get center freq of left and right channels
+ * by adding 10MHz to left_end_freq and subtracting 10 MHZ from
+ * right_end_freq.
+ */
+ left_end_freq += MHZ_TO_KHZ(10);
+ right_end_freq -= MHZ_TO_KHZ(10);
Note: you are starting your left_end_freq here at the very first IEEE
channel from the left of the regulatory rule.
Not really. what if centre_freq is chan 46, BW is 80 MHz then chan 38
will be left_end. Plus 10 Mhz will give chan 40.
+ /* find out all possible secondary channels */
+ while (left_end_freq < right_end_freq) {
Should this be <= ? Otherwise the last 20 MHz IEEE channel would not
be checked, no ?
Agree. will change that..
+ chan = ieee80211_get_channel(wiphy, left_end_freq);
+ if (chan == NULL ||
+ chan->flags & IEEE80211_CHAN_DISABLED) {
+ return false;
+ }
+ left_end_freq -= MHZ_TO_KHZ(20);
From what I read here you are sweeping left to right for all 20 MHz
channels and you are allowing for the VHT channel of bw_khz bandwidth
only if all 20 MHz IEEE channels in between are allowed. Is that
correct?
Yes Luis.
+ }
+
+ return true;
+}
+
+bool reg_chan_use_permitted(struct wiphy *wiphy,
+ struct ieee80211_channel_config *chan_config,
+ const struct ieee80211_regdomain *regd)
+{
+ u32 desired_bw_khz = MHZ_TO_KHZ(20);
+ bool ret;
+
+ /* get chan BW from config */
+ switch (chan_config->chan_width) {
+ case IEEE80211_CHAN_WIDTH_20MHZ_NOHT:
+ case IEEE80211_CHAN_WIDTH_20MHZ:
+ desired_bw_khz = MHZ_TO_KHZ(20);
+ break;
+
+ case IEEE80211_CHAN_WIDTH_40MHZ:
+ desired_bw_khz = MHZ_TO_KHZ(40);
+ break;
+
+ case IEEE80211_CHAN_WIDTH_80MHZ:
+ case IEEE80211_CHAN_WIDTH_80P80MHZ:
+ desired_bw_khz = MHZ_TO_KHZ(80);
+ break;
+
+ case IEEE80211_CHAN_WIDTH_160MHZ:
+ desired_bw_khz = MHZ_TO_KHZ(160);
+ break;
+ default:
+ REG_DBG_PRINT("Invalid channel width %d\n",
+ chan_config->chan_width);
+ return false;
+ }
+
+ ret = reg_fits_reg_rule_sec_chans(wiphy,
+ chan_config->center_freq1,
+ desired_bw_khz,
+ regd);
+
+ if (ret == false)
+ return ret;
+
+ if (chan_config->chan_width == IEEE80211_CHAN_WIDTH_80P80MHZ) {
+ ret = reg_fits_reg_rule_sec_chans(wiphy,
+ chan_config->center_freq2,
+ desired_bw_khz,
+ regd);
+ }
And for the special case of 80 MHz + 80 MHz split channels (ie, not
contiguous) you then spin off the check for each segment of 80 MHz
channels.
But no code here uses it. Based on the previous threads on this VHT
regulatory topic I get the impression your goal is to end up using
this code to determine whether or not a non-HT, HT or VHT channel is
allowed for and based on feedback it seems that the objective is to
use this code eventually to do dynamic run time checks of whether or
not we're allowed to use this channel configuration setup. If so then
I have a few comments for that.
Yes. That's the objective. we are not calling these functions anywhere
yet. Once this proposal is accepted then I want to spread further. But I
am confused now....
Although VHT complicates the regulatory checks, to the extent you
check for valid 20 MHz IEEE channels in between a VHT wide channel,
the IEEE spec actually only allows in practice certain VHT
arrangements. I've asked for shiny diagrams that have this laid out
clearly and simple but unfortunately we do not have one, but such
simple static arrangements are apparently specified on the spec, its
not clear to me where exactly though... Technically then if we wanted
to keep a static cached early check of allowed channels maps for VHT
we should be able to have a bitmap representation of the allowed IEEE
VHT channel configurations and upon initialization / regulatory change
update this bitmap to determine if we're allowed to use the new
arrangement. The approach you use is also fine and while it does allow
for flexibility to do add more technologies one should consider the
penalty incurred at doing these computations at run time. The run time
impact is no issue if its done just once but consider changing
channels and how often we can do this. Consider a device now with one
radio but two virtual devices and each one doing their own set of
scans and two different types of HT / VHT configurations. This means
the code you just wrote will become a real hot path -- used for
anything that has to do with any channel change. The purpose of the
flags are to remove that run time penalty, so if we can take into
consideration more the nature of how we VHT channels are allowed and
how IEEE decided to simplify the arrangements for VHT then likely
keeping flags may make sense then. That is, not all VHT arrangements
are possible, only a subset, and it seems fairly trivial and
reasonable to me to do this upon regulatory change only once rather
than at every channel change.
And as for the question: "What about the future? Will we see 320 MHz
wide channels in 2020? :)"
I'm told through a shiny crystal ball: let's not expect 320 MHz channels.
So I'd rather keep this simple and also due to the fact that VHT
channels are static just try to use a bitmap for them and check for it
at regulatory change.
I agree Luis. Limiting the flags to a subset is fine with me. Hope no
more disagreements.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
