Return-path: Received: from 30.mail-out.ovh.net ([213.186.62.213]:55703 "HELO 30.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752650Ab0APJz0 (ORCPT ); Sat, 16 Jan 2010 04:55:26 -0500 Message-ID: <4B518D09.5080907@free.fr> Date: Sat, 16 Jan 2010 10:55:21 +0100 From: Benoit PAPILLAULT MIME-Version: 1.0 To: "Luis R. Rodriguez" CC: linville@tuxdriver.com, linux-wireless@vger.kernel.org, Jouni Malinen , Johannes Berg Subject: Re: [PATCH] cfg80211: make regulatory_hint_11d() band specific References: <1263517700-21846-1-git-send-email-lrodriguez@atheros.com> In-Reply-To: <1263517700-21846-1-git-send-email-lrodriguez@atheros.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: Luis R. Rodriguez a ?crit : > In practice APs do not send country IE channel triplets for channels > the AP is not operating on and if they were to do so they would have > to use the regulatory extension which we currently do not process. > No AP has been seen in practice that does this though so just drop > those country IEs. > > Additionally it has been noted the first series of country IE > channels triplets are specific to the band the AP sends. Propagate > the band on which the country IE was found on reject the country > IE then if the triplets are ever oustide of the band. > > Although we now won't process country IE information with multiple > band information we leave the intersection work as is as it is > technically possible for someone to want to eventually process these > type of country IEs with regulatory extensions. > > Cc: Jouni Malinen > Cc: Johannes Berg > Signed-off-by: Luis R. Rodriguez > --- > > This goes out tested now. > > net/wireless/reg.c | 74 ++++++++++++++++++++++++++++++++++++++-------------- > net/wireless/reg.h | 11 +++++++ > net/wireless/sme.c | 1 + > 3 files changed, 66 insertions(+), 20 deletions(-) > > diff --git a/net/wireless/reg.c b/net/wireless/reg.c > index 5b03ca7..7242121 100644 > --- a/net/wireless/reg.c > +++ b/net/wireless/reg.c > @@ -485,6 +485,34 @@ static bool freq_in_rule_band(const struct ieee80211_freq_range *freq_range, > } > > /* > + * This is a work around for sanity checking ieee80211_channel_to_frequency()'s > + * work. ieee80211_channel_to_frequency() can for example currently provide a > + * 2 GHz channel when in fact a 5 GHz channel was desired. An example would be > + * an AP providing channel 8 on a country IE triplet when it sent this on the > + * 5 GHz band, that channel is designed to be channel 8 on 5 GHz, not a 2 GHz > + * channel. > + * > + * This can be removed once ieee80211_channel_to_frequency() takes in a band. > + */ > +static bool chan_in_band(int chan, enum ieee80211_band band) > +{ > + int center_freq = ieee80211_channel_to_frequency(chan); > + > + switch (band) { > + case IEEE80211_BAND_2GHZ: > + if (center_freq <= 2484) > + return true; > + return false; > + case IEEE80211_BAND_5GHZ: > + if (center_freq >= 5005) > + return true; > + return false; > + default: > + return false; > + } > +} > + > +/* > * Some APs may send a country IE triplet for each channel they > * support and while this is completely overkill and silly we still > * need to support it. We avoid making a single rule for each channel > @@ -532,7 +560,8 @@ static bool freq_in_rule_band(const struct ieee80211_freq_range *freq_range, > * Returns 0 if the IE has been found to be invalid in the middle > * somewhere. > */ > -static int max_subband_chan(int orig_cur_chan, > +static int max_subband_chan(enum ieee80211_band band, > + int orig_cur_chan, > int orig_end_channel, > s8 orig_max_power, > u8 **country_ie, > @@ -541,7 +570,6 @@ static int max_subband_chan(int orig_cur_chan, > u8 *triplets_start = *country_ie; > u8 len_at_triplet = *country_ie_len; > int end_subband_chan = orig_end_channel; > - enum ieee80211_band band; > > /* > * We'll deal with padding for the caller unless > @@ -557,17 +585,14 @@ static int max_subband_chan(int orig_cur_chan, > *country_ie += 3; > *country_ie_len -= 3; > > - if (orig_cur_chan <= 14) > - band = IEEE80211_BAND_2GHZ; > - else > - band = IEEE80211_BAND_5GHZ; > + if (!chan_in_band(orig_cur_chan, band)) > + return 0; > > while (*country_ie_len >= 3) { > int end_channel = 0; > struct ieee80211_country_ie_triplet *triplet = > (struct ieee80211_country_ie_triplet *) *country_ie; > int cur_channel = 0, next_expected_chan; > - enum ieee80211_band next_band = IEEE80211_BAND_2GHZ; > > /* means last triplet is completely unrelated to this one */ > if (triplet->ext.reg_extension_id >= > @@ -592,6 +617,9 @@ static int max_subband_chan(int orig_cur_chan, > if (triplet->chans.first_channel <= end_subband_chan) > return 0; > > + if (!chan_in_band(triplet->chans.first_channel, band)) > + return 0; > + > /* 2 GHz */ > if (triplet->chans.first_channel <= 14) { > end_channel = triplet->chans.first_channel + > @@ -600,14 +628,10 @@ static int max_subband_chan(int orig_cur_chan, > else { > end_channel = triplet->chans.first_channel + > (4 * (triplet->chans.num_channels - 1)); > - next_band = IEEE80211_BAND_5GHZ; > } > > - if (band != next_band) { > - *country_ie -= 3; > - *country_ie_len += 3; > - break; > - } > + if (!chan_in_band(end_channel, band)) > + return 0; > > if (orig_max_power != triplet->chans.max_power) { > *country_ie -= 3; > @@ -666,6 +690,7 @@ static int max_subband_chan(int orig_cur_chan, > * with our userspace regulatory agent to get lower bounds. > */ > static struct ieee80211_regdomain *country_ie_2_rd( > + enum ieee80211_band band, > u8 *country_ie, > u8 country_ie_len, > u32 *checksum) > @@ -743,8 +768,11 @@ static struct ieee80211_regdomain *country_ie_2_rd( > if (triplet->chans.num_channels == 0) > return NULL; > > + if (!chan_in_band(triplet->chans.first_channel, band)) > + return NULL; > + > /* 2 GHz */ > - if (triplet->chans.first_channel <= 14) > + if (band == IEEE80211_BAND_2GHZ) > end_channel = triplet->chans.first_channel + > triplet->chans.num_channels - 1; > else > @@ -767,7 +795,8 @@ static struct ieee80211_regdomain *country_ie_2_rd( > * or for whatever reason sends triplets with multiple channels > * separated when in fact they should be together. > */ > - end_channel = max_subband_chan(cur_channel, > + end_channel = max_subband_chan(band, > + cur_channel, > end_channel, > triplet->chans.max_power, > &country_ie, > @@ -775,6 +804,9 @@ static struct ieee80211_regdomain *country_ie_2_rd( > if (!end_channel) > return NULL; > > + if (!chan_in_band(end_channel, band)) > + return NULL; > + > cur_sub_max_channel = end_channel; > > /* Basic sanity check */ > @@ -867,14 +899,15 @@ static struct ieee80211_regdomain *country_ie_2_rd( > reg_rule->flags = flags; > > /* 2 GHz */ > - if (triplet->chans.first_channel <= 14) > + if (band == IEEE80211_BAND_2GHZ) > end_channel = triplet->chans.first_channel + > triplet->chans.num_channels -1; > else > end_channel = triplet->chans.first_channel + > (4 * (triplet->chans.num_channels - 1)); > > - end_channel = max_subband_chan(triplet->chans.first_channel, > + end_channel = max_subband_chan(band, > + triplet->chans.first_channel, > end_channel, > triplet->chans.max_power, > &country_ie, > @@ -1981,8 +2014,9 @@ static bool reg_same_country_ie_hint(struct wiphy *wiphy, > * therefore cannot iterate over the rdev list here. > */ > void regulatory_hint_11d(struct wiphy *wiphy, > - u8 *country_ie, > - u8 country_ie_len) > + enum ieee80211_band band, > + u8 *country_ie, > + u8 country_ie_len) > { > struct ieee80211_regdomain *rd = NULL; > char alpha2[2]; > @@ -2028,7 +2062,7 @@ void regulatory_hint_11d(struct wiphy *wiphy, > wiphy_idx_valid(last_request->wiphy_idx))) > goto out; > > - rd = country_ie_2_rd(country_ie, country_ie_len, &checksum); > + rd = country_ie_2_rd(band, country_ie, country_ie_len, &checksum); > if (!rd) { > REG_DBG_PRINT("cfg80211: Ignoring bogus country IE\n"); > goto out; > diff --git a/net/wireless/reg.h b/net/wireless/reg.h > index 3362c7c..3018508 100644 > --- a/net/wireless/reg.h > +++ b/net/wireless/reg.h > @@ -41,14 +41,25 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy, > * regulatory_hint_11d - hints a country IE as a regulatory domain > * @wiphy: the wireless device giving the hint (used only for reporting > * conflicts) > + * @band: the band on which the country IE was received on. This determines > + * the band we'll process the country IE channel triplets for. > * @country_ie: pointer to the country IE > * @country_ie_len: length of the country IE > * > * We will intersect the rd with the what CRDA tells us should apply > * for the alpha2 this country IE belongs to, this prevents APs from > * sending us incorrect or outdated information against a country. > + * > + * The AP is expected to provide Country IE channel triplets for the > + * band it is on. It is technically possible for APs to send channel > + * country IE triplets even for channels outside of the band they are > + * in but for that they would have to use the regulatory extension > + * in combination with a triplet but this behaviour is currently > + * not observed. For this reason if a triplet is seen with channel > + * information for a band the BSS is not present in it will be ignored. > */ > void regulatory_hint_11d(struct wiphy *wiphy, > + enum ieee80211_band band, > u8 *country_ie, > u8 country_ie_len); > > diff --git a/net/wireless/sme.c b/net/wireless/sme.c > index 2333d78..2ce5e16 100644 > --- a/net/wireless/sme.c > +++ b/net/wireless/sme.c > @@ -454,6 +454,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid, > * - and country_ie[1] which is the IE length > */ > regulatory_hint_11d(wdev->wiphy, > + bss->channel->band, > country_ie + 2, > country_ie[1]); > } > I think the channel conversion could be improved (we could directly compare the channel number with valid values as defined in IEEE802.11-2007 without converting them to a center frequency), but it could be done in a latter patch. So far : Acked-by: Benoit Papillault Regards, Benoit