Re: [PATCH v6 4/6] mac80211: use chanctx reservation for AP CSA

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2014-05-23 at 08:49 +0200, Michal Kazior wrote:

> > But you'll WARN_ON() if they're actually not at the same time and you
> > grab a beacon (or for the template case, call csa_update) in the
> > meantime, right? I'd really like to have all those driver requirements
> > (e.g. to stop beaconing) better documented.
> 
> Good point. I suppose it should be stated in the docs that once you
> reach ieee80211_csa_is_complete() being true you must not call
> ieee80211_beacon_get() nor ieee80211_csa_update_counter(). ath9k and
> ath10k conform to this.

Right, it's still a bit strange that you just have to stop beaconing.
Dunno. I guess I don't care all that much since our driver won't support
multiple AP interfaces anyway :-)

> I wonder what driver should be supposed to look at before starting to
> beacon again? csa_active isn't well protected to be depended upon. If
> we should create a ieee80211_csa_is_active() that just checks if
> beacon->csa_counter_offset[0] != 0 (assuming my other csa counter
> patches are applied) then it's still racy:
>  a) rcu_dereference() across ieee80211_csa_is_active(), _is_complete()
> and _beacon_get() can yield different beacon pointers
>  b) cs_count <= 1 yields no beacon update (thus no counters/offsets,
> meaning both _csa_is_complete and _csa_is_active() are `false` thus
> suggesting driver can beacon as if nothing happened)
> 
> We could fix (b) by simply not treating cs_count <= 1 so special and
> update the beacon anyway. For (a) to work we'd need either make a
> single-call do-all function or introduce an additional call and a
> generic pointer/structure to be passed to other functions so that a
> beacon pointer is consistent across calls.

You could technically just return NULL in this period, but I don't know
how well drivers would cope with that (though technically they have to
cope with it due to memory allocation failures anyway)

johannes