Re: [PATCH] mac80211: Add 802.11h CSA support

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

On Wed, 2008-12-31 at 15:39 +0530, Sujith wrote:
> Move to the advertised channel on reception of
> a CSA element. This is needed for 802.11h compliance.
> 
> Signed-off-by: Sujith <Sujith.Manoharan@atheros.com>

One thing I've been wondering about though, should we have a hook into
the driver to actually stop transmit at a lower level, where possible?
For example Broadcom hardware we can actually set some value on the chip
and then let it reject all frames, drain its FIFOs, and then in the
driver we could schedule those frames again for afterwards.

One other small note:

> +	bss = ieee80211_rx_bss_get(sdata->local, ifsta->bssid,
> +				   sdata->local->hw.conf.channel->center_freq,
> +				   ifsta->ssid, ifsta->ssid_len);
> +	if (!bss)
> +		goto exit;
> +
> +	if (!ieee80211_hw_config(sdata->local, IEEE80211_CONF_CHANGE_CHANNEL))
> +		bss->freq = sdata->local->oper_channel->center_freq;

This looks a little odd. (read on)


> +void ieee80211_process_chanswitch(struct ieee80211_sub_if_data *sdata,
> +				  struct ieee80211_channel_sw_ie *sw_elem,
> +				  struct ieee80211_bss *bss)
> +{
> +	struct ieee80211_channel *new_ch;
> +	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
> +	int new_freq = ieee80211_channel_to_frequency(sw_elem->new_ch_num);
> +	int exp;
> +
> +	/* FIXME: Handle ADHOC later */
> +	if (sdata->vif.type != NL80211_IFTYPE_STATION)
> +		return;
> +
> +	if (ifsta->state != IEEE80211_STA_MLME_ASSOCIATED)
> +		return;
> +
> +	if (sdata->local->sw_scanning || sdata->local->hw_scanning)
> +		return;
> +
> +	/* Disregard subsequent beacons if we are already running a timer
> +	   processing a CSA */
> +
> +	if (ifsta->flags & IEEE80211_STA_CSA_RECEIVED)
> +		return;
> +
> +	new_ch = ieee80211_get_channel(sdata->local->hw.wiphy, new_freq);
> +	if (!new_ch || new_ch->flags & IEEE80211_CHAN_DISABLED)
> +		return;
> +
> +	sdata->local->oper_channel = new_ch;

Here you're setting oper_channel, but is that really the right thing to
do? That means any hw config call that comes in between this and later
will already switch to that channel, and hw config can happen for
various other reasons, for instance PS mode stuff. I think we may need
to have a separate variable?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]