Johannes Berg wrote:
On Thu, 2012-06-28 at 09:54 +0200, Michal Kazior wrote:
Yes, this is more or less what I also had in mind. I was just thinking
about solving the issue of channel context and hw.conf.channel
consistency. If we switch a channel we either modify channel in channel
context directly (violating the immutability of channel contexts) or we
iterate and re-set the new channel on each interface (because
single-channel drivers may still have multiple interfaces and we
probably want to use sdata->vif.chanctx_conf->channel instead of
hw.conf.channel inside mac80211).
Now that I think about it I guess violating the immutability for the
single-channel case is okay. It would greatly simplify the code and we'd
just put a comment down in hw_config where the only violation would occur.
I'm not sure why we would violate it? The way I see it, you'd never
change the channel context channel since internally in mac80211 you'd
never want to see a different channel, just like today we use
local->oper_channel everywhere we'd then use sdata->vif.chanctx->channel
throughout, right?
I think the only thing we need to do is put something like this into
hw_config:
if (local->tmp_channel) {
local->hw.conf.channel = local->tmp_channel;
...
} else {
local->hw.conf.channel = chanctx->channel;
}
No?
Using sdata->vif.chanctx_conf->channel instead of local->oper_channel
doesn't make any sense to me.
Take ieee80211_tx() for example. It does:
tx.channel = local->hw.conf.channel;
We don't use oper_channel here, but hw.conf.channel. TX can happen on
different interfaces so for multi-channel operation it should be saying:
tx.channel = sdata->vif.chanctx_conf->channel;
In this case if we want to support the swscan/tmpchan through
hw_config() we need update the channel context's channel somehow.
I'm more thinking of hw.conf.channel becoming more of a backup value for
single-channel drivers while we internally focus on channel contexts.
Yes, makes sense. I forgot all about the TX code. I'm a little wary of
making the contexts mutable, even in this case, because a lot of code
uses local->oper_channel as well, and that is expected to really be the
operating channel all of the time, even if we're scanning at some point
in time.
Yeah. The other option (maintaining the immutability) is to iterate
through all interfaces and call ieee80211_vif_use_channel when switching
channel for single-channel operation. Or do you have something else in
mind maybe?
Luckily, tx.channel isn't actually used much, only for the band. So if
we tag the SKBs with the band earlier (info->band), maybe we don't need
to use hw.conf.channel as much there for tx.channel?
Other uses where we do need the current channel are
* ieee80211_build_probe_req
* ieee80211_add_srates_ie/ieee80211_add_ext_srates_ie
* __ieee80211_start_scan uses it but need not, could use oper_channel
instead and the code never executes for multi-channel
* ieee80211_set_tx_power() is interesting, may need to make it all
per-sdata now through nl80211 etc.
What will drivers that don't support per-sdata tx_power do? Do all
multi-vif (not multi-channel) drivers support per-interface tx power?
I guess we'd have to manage:
a) common tx power value in ieee80211_local
b) provide a function that calculates the common value
so drivers may use it (and avoid code duplication)
c) ..or else drivers would need to implement the calculation on
their own
* rate_idx_to_bitrate can use the sta's sdata's channel
* ieee80211_change_bss can use the sdata's channel
* debugfs stuff probably just moves to per-sdata files
* ibss code all uses sdata channel
* ieee80211_if_change_type ... probably just set basic_rates = 0
* mesh can use sdata channel
* mlme.c should use sdata channel, but there's the channel switch stuff
* rate.h should use sta->sdata channel
Much of this is actually means we have bugs today! Whenever we use
hw.conf.channel and should be using sdata channel soon, we should be
using local->oper_channel today!
Oh! Now I understand why you wanted to use channel contexts in place of
oper_channel. This makes sense.
Maybe it's worth fixing that first, and getting rid of *most* instances
of hw.conf.channel, so we have a clearer idea of which changes in what
ways?
Sounds like a good idea.
--
Pozdrawiam / Best regards, Michal Kazior.
--
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
