On Tue, Nov 26, 2013 at 04:43:22PM +0100, Simon Wunderlich wrote:
> Hey Karl,
> > Sorry, I was not very clear.
> > What I meant is that, my change for hwsim is not correct because it
> > assumed something like what's above that would prevent beacon_get() to
> > return a beacon when csa is under completion. Instead, it should check
> > for csa completion prior to getting the beacon.
> >
> > Checking your ath9k change for CSA, I am under the impression this is
> > possible ? : {{{
> > ath9k_channel_switch_beacon()
> > ath9k_beacon_tasklet()
> > ath9k_csa_is_finished() // ret false
> > ..
> > ath9k_beacon_tasklet()
> > ath9k_csa_is_finished() // sets sc->csa_vif to NULL, ret true
> > ieee80211_csa_finish() // schedules csa work
> >
> > ath9k_beacon_tasklet()
> > ath9k_csa_is_finished() // ret false because sc->csa_vif==NULL
> >
> > csa_finalize_work() // too late
> > }}}
>
> Yeah, you are right, if the worker is too late ath9k still generates a beacon
> and will hit a warning that the csa was already finished. Actually that helped
> quite a lot while developing the feature.
>
> In practice, I don't think it's that bad - the worker shouldn't take so long
> too respond, and also it does not hurt to generate/send the beacon one more
> time on the same channel with count = 0 (or 1). It's useful to get a warning
> in this case so that the root cause for the delay can be worked on.
>
I think it's always nice to add a clean-room use of a new stack feature.
I was more expecting a WARN_ON(!vif && vif->csa_active) in the driver
than hitting a "forbidden" path in the stack.
The v4 I sent for hwsim is immune to this race but I'd have expected the
CSA code to handle this (e.g. with a vif->csa_finishing, or by letting
ieee80211_beacon_get* return an ERR_PTR) since many drivers would? have
to do this ATM.
> What do you think? I can change that easily in ath9k otherwise. :)
>
I have no strong opinion on this, no pb with me keeping it as is, plus
CSA being a recent addition, I doubt new ideas won't chime in.
Karl
--
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
