On 6 May 2014 16:42, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> On Wed, 2014-04-09 at 15:45 +0200, Michal Kazior wrote:
>> Channel switch finalization is now 2-step. First
>> step is when driver calls csa_finish(), the other
>> is when reservation is actually finalized (which
>> be defered for in-place reservation).
>
> "which be defered"? should that be "can be"?
Oh, you're right.
>> + if (sdata->reserved_chanctx) {
>> + /*
>> + * with multi-vif csa driver may call ieee80211_csa_finish()
>> + * many times while waiting for other interfaces to use their
>> + * reservations
>> + */
>> + if (sdata->reserved_ready)
>> + return 0;
>> +
>> + err = ieee80211_vif_use_reserved_context(sdata);
>> + if (err)
>> + return err;
>> +
>> + return 0;
>> }
>>
>> + if (!cfg80211_chandef_identical(&sdata->vif.bss_conf.chandef,
>> + &sdata->csa_chandef))
>> + return -EINVAL;
>> +
>> sdata->vif.csa_active = false;
>
> Should csa_active really stay true in the reserved_chanctx case?
> Wouldn't that leave it beaconing, with potentially suddenly negative
> (due to underflow) CSA counter, or something?
Hmm.. I think drivers should check ieee80211_csa_is_complete() before
calling to ieee80211_beacon_get(). At least that's what ath9k and
ath10k effectively do now. Hwsim might complain though but this should
be changed for multi-vif CSA. You may need to wait for other vifs to
finish CS (the incompat case) so you shouldn't call
ieee80211_beacon_get() after you get last CSA beacon on a given AP
vif.
Btw. shouldn't csa_active be protected/synchronized between CPUs
somehow? I think it's possible now to get inconsistencies and hit
WARN_ONs due to that.
Michał
--
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
