Johannes Berg wrote:
Hi,
The following patches prepare mac80211 to support multi-channel capable
hardware. The patchset prepares to channel per-vif split.
Work still needs to be done:
* powersave per-vif
* queue locking per-vif
* offchannel rework (hw_config, work_work)
* and a bit more
Questions:
* monitor interfaces:
Currently ieee80211_set_channel gets netdev==NULL when iface is
a monitor. Is there a particular reason behind it?
* ieee80211_hw_config:
Should we extend it to take ieee80211_sub_if_data or should we
use ieee80211_bss_info_change_notify? If so, is ieee80211_hw_config
eventually to be removed?
What do you think of this approach?
So I took a closer look at this now.
I think we should probably apply the first two patches now, and the
third with changes. After that, I'm not so sure.
You mean the following, right?
mac80211: prepare ieee80211_mandatory_rates to per-vif
mac80211: prepare ieee80211_sta_get_rates to per-vif
[tofix] mac80211: prepare ieee80211_frame_duration to per-vif
Should I resend them all, or just the 3rd one once I fix it?
Overall, I think the approach isn't sufficient. At the very least, the
(unstated!) assumption that one channel per interface can always be done
is bad. Modifying this with the structure you gave it though seems to be
problematic.
We should also handle the queue mapping first I think. This isn't a
feature you can implement overnight :-)
I've been thinking more on queue handling in multi-channel. Here are my
thoughts.
If we assume that off-channel should be a global thing then we should
rethink the whole queue thing first.
What if we create a new structure that would be a channel
stream/reservation? Driver could then report multiple such objects. Each
such would have a tx queue and it's own locking. Let's assume something
like:
struct ieee80211_channel_stream {
struct list_head vif; // list of vifs we're attached to
struct ieee80211_channel *ch;
struct sk_buff_head pending;
int stop_reasons[IEEE80211_MAX_QUEUES];
// .. other stuff
u8 *drv_priv[0] __attribute__((__aligned__((void *))));
};
struct ieee80211_vif {
struct ieee80211_channel_stream *channel_stream;
// .. rest of struct
};
* We could then reject set_channel() based on number of different
oper_channels and channel reservations
* We could handle off-channel through a channel reservation by either
using an unused one (which would result in hardware offloaded
off-channel) or we would lock&flush it, switch to off-channel, do
work, and get back
We should however consider that a device may not support certain
interface combinations. One thing that comes immediately to my mind is
the following: suppose we support STA+STA on different channels, AP+AP
on the same channel, but do not support AP+AP on different channels:
1. create wlan0, set channel=1, and start AP
2. create wlan1, set channel=1, and start AP
3. do set_channel(wlan1, 3) <-- oops
This would probably be fixed easily with:
1. forbid changing channels on a running interface (i'm aware that's
the same case for mac addr)
2. thus we would require: ifdown + setchannel + ifup
But that's just one case. Are there any others? This needs investigation.
Other issue I'm thinking would be: what if a device supports specialized
off-channel queue (is there one we know? iwlwifi?) ? We can't advertise
this as a fully-usable ieee80211_channel_stream (again, is there
anything that has a shortcoming like that?). Should we add capability
flags for each ieee80211_channel_stream? Or we could add new driver
callback, eg. "set_channel_try" and allow the driver to decide whether
it allows a certain hardware state or not.
What is still missing here is the CAB queue (and possible other that may
come up in the future) you mentioned in the other thread. I'm not sure
whether we may use queue_mapping for our own evil purposes here. If not,
then how about this:
struct ieee80211_queue_types { // or tx_queue_types?
IEEE80211_QUEUE_VO,
IEEE80211_QUEUE_VI,
IEEE80211_QUEUE_BE,
IEEE80211_QUEUE_BK,
IEEE80211_QUEUE_CAB,
IEEE80211_QUEUE_LAST
};
enum ieee80211_queue_types
ieee80211_ac_number_to_queue_type(enum ieee80211_ac_numbers ac);
struct ieee80211_channel_stream_queue {
enum ieee80211_queue_types type;
u8 drv_priv[0];
};
struct XYZ_channel_stream_queue { // example driver priv
int internal_queue_id;
// other stuff, like driver queues, stats, etc.
};
struct ieee80211_channel_stream {
struct ieee80211_channel_stream_queue *queues[IEEE80211_QUEUE_LAST];
// ..
};
struct ieee80211_vif_queue {
struct ieee80211_vif *vif;
struct ieee80211_channel_stream_queue *queue;
};
struct ieee80211_vif {
// this is going to be just a helper
// we can't refer to ieee80211_channel_stream_queue directly since
// it wouldn't know what vif it belongs to (it may belong to
// multiple vifs)
struct ieee80211_vif_queue queues[IEEE80211_QUEUE_LAST];
// ..
};
struct ieee80211_tx_info {
// instead of ieee80211_vif* we use ieee80211_vif_queue*
};
This looks kind of ugly, but oh well. This way the driver will be able
to setup mapping between mac80211 and internal device queues. It should
also be possible to map one hardware queue to multiple
ieee80211_channel_stream structures (queues is a list of pointers) so
multiple vifs on the same channel is okay.
What are your thoughts on this? Is this terribly wrong or is it the way
to go? Have I missed something?
-- 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
