On Wed, Apr 1, 2009 at 1:29 AM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> On Tue, 2009-03-31 at 21:28 -0400, Luis R. Rodriguez wrote:
>
>> + * @NL80211_CMD_REG_BEACON_HINT: indicates to userspace that an AP beacon
>> + * has been found while world roaming thus enabling active scan or
>> + * any mode of operation that initiates TX (beacons) on a channel
>> + * where we would not have been able to do either before. As an example
>> + * if you are world roaming (regulatory domain set to world or if your
>> + * driver is using a custom world roaming regulatory domain) and while
>> + * doing a passive scan on the 5 GHz band you find an AP there (if not
>> + * on a DFS channel) you will now be able to actively scan for that AP
>> + * or use AP mode on your card on that same channel. Note that this will
>> + * never be used for channels 1-11 on the 2 GHz band as they are always
>> + * enabled world wide. This beacon hint is only sent if your device had
>> + * either disabled active scanning or beaconing on a channel. We send to
>> + * userspace the wiphy on which we removed a restriction from
>> + * (%NL80211_ATTR_WIPHY) and the channel on which this occurred
>> + * before (%NL80211_ATTR_FREQ_BEFORE) and after (%NL80211_ATTR_FREQ_AFTER)
>> + * the beacon hint was processed.
>
>
> I wonder -- are there other reasons we may ever update the regdomain? I
> could imagine the CMD being REG_UPDATE with an ATTR_REASON or so? Or
> just using REG_CHANGE with NL80211_REGDOM_TYPE_WORLD _and_ the new
> channel change attributes?
I don't follow what you were saying.
FYI -- we only call this when world roaming.
>> +static int nl80211_put_beacon_hint_channel(void *hdr,
>> + struct sk_buff *msg,
>> + enum nl80211_attrs attribute,
>> + struct ieee80211_channel *chan)
>> +{
>> + struct nlattr *nl_freq;
>> +
>> + nl_freq = nla_nest_start(msg, attribute);
>> + if (!nl_freq)
>> + goto nla_put_failure;
>> +
>> + NLA_PUT_U32(msg, NL80211_FREQUENCY_ATTR_FREQ,
>> + chan->center_freq);
>> +
>> + if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN)
>> + NLA_PUT_FLAG(msg, NL80211_FREQUENCY_ATTR_PASSIVE_SCAN);
>> + if (chan->flags & IEEE80211_CHAN_NO_IBSS)
>> + NLA_PUT_FLAG(msg, NL80211_FREQUENCY_ATTR_NO_IBSS);
>> +
>> + nla_nest_end(msg, nl_freq);
>> +
>> + return 0;
>
> I think you should send all possible channel attributes here -- please
> refactor the code and extract the bit that sends a channel from
> nl80211_send_wiphy(), this bit:
> NLA_PUT_U32(msg, NL80211_FREQUENCY_ATTR_FREQ,...)
> ..
> NLA_PUT_U32(msg, NL80211_FREQUENCY_ATTR_MAX_TX_POWER,...)
>
> put it into a new function that takes a msg and channel pointer and call
> it twice.
Heh.. sure.
> Does the relaxing we do on receiving beacons only apply to a single
> wiphy though?
We do it per wiphy if the wiphy is world roaming.
>> + /*
>> + * Since we are applying the beacon hint to a wiphy we know its
>> + * wiphy_idx is valid
>> + */
>> + NLA_PUT_U32(msg, NL80211_ATTR_WIPHY, get_wiphy_idx(wiphy));
>
> You're sending out the wiphy here.
Hm? We're sending its wiphy_idx here.
> And I just realised there's a bug in the existing code:
>
>> @@ -1070,18 +1062,26 @@ static void handle_reg_beacon(struct wiphy *wiphy,
>> if (likely(chan->center_freq != reg_beacon->chan.center_freq))
>> return;
>>
>> + if (chan->beacon_found)
>> + return;
>> +
>> + chan->beacon_found = true;
>> +
>> + chan_before.center_freq = chan->center_freq;
>> + chan_before.flags = chan->flags;
>> +
>> if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN) {
>> chan->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
>> - REG_DEBUG_BEACON_FLAG("active scanning");
>> + channel_changed = true;
>> }
>>
>> if (chan->flags & IEEE80211_CHAN_NO_IBSS) {
>> chan->flags &= ~IEEE80211_CHAN_NO_IBSS;
>> - REG_DEBUG_BEACON_FLAG("beaconing");
>> + channel_changed = true;
>> }
>
> This is incorrect, it has to be like this:
>
> if ((chan->flags & FLAG) && !(chan->orig_flags & FLAG)) {
> chan->flags &= ~FLAG;
> channel_changed = true;
> }
>
> to take into account the flags the _driver_ had already set on the
> channel. This is important for Intel hardware -- if you lift the IBSS
> restriction on a channel that iwlwifi thinks should not use IBSS then
> the firmware will hate you if you try to use it anyway. Now, I think
> iwlwifi isn't world-roaming in that sense, but it should still be fixed!
Ew, sure it can go in as a separate patch.
> Does this mean though you're sending one event per wiphy? Just
> wondering, it's probably acceptable.
Indeed as this flags won't change for for all wiphys, only for wiphys
for which the regulatory beacon hint had an effect on a change of a
channel. This will also be sent for each channel changed on the wiphy.
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
