Re: [PATCHv2 4/6] mac80211: add support for CSA in IBSS mode

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2013-08-09 at 16:35 +0200, Simon Wunderlich wrote:
> Ths

This ;-)


> +	case NL80211_IFTYPE_ADHOC:
> +		if (!sdata->vif.bss_conf.ibss_joined)
> +			return -EINVAL;
> +
> +		if (params->chandef.width != sdata->u.ibss.chandef.width)
> +			return -EINVAL;
> +
> +		switch (params->chandef.width) {
> +		case NL80211_CHAN_WIDTH_40:
> +			if (cfg80211_get_chandef_type(&params->chandef) !=
> +			    cfg80211_get_chandef_type(&sdata->u.ibss.chandef))
> +				return -EINVAL;

Is that really correct? It seems that you should be able to switch from
HT40- to HT40+ and vice versa when switching the channel?

And why disallow switching bandwidth (was above this code)? That doesn't
seem right either?

> +/* must hold sdata lock */

pretty useless comment if you have the assert in the function :)

> +	rcu_read_lock();
> +	ies = rcu_dereference(cbss->ies);
> +	tsf = ies->tsf;
> +	rcu_read_unlock();
> +	cfg80211_put_bss(sdata->local->hw.wiphy, cbss);
> +
> +	old_presp = rcu_dereference_protected(ifibss->presp,
> +					  lockdep_is_held(&sdata->wdev.mtx));
> +
> +	presp = ieee80211_ibss_build_presp(sdata,
> +					   sdata->vif.bss_conf.beacon_int,
> +					   sdata->vif.bss_conf.basic_rates,
> +					   capability, tsf, &ifibss->chandef,
> +					   NULL, csa_settings);

This is pretty odd - why does the TSF have to go here? It needs to be
set by the device when transmitting anyway, no?

> +static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata)
> +{

Is this some refactoring that should be separate? I don't see how it's
really related to CSA? Maybe I'm missing something?

johannes