On 31 March 2014 18:15, Eliad Peller <eliad@xxxxxxxxxx> wrote:
> hi Michal,
>
> On Mon, Mar 31, 2014 at 1:39 PM, Michal Kazior <michal.kazior@xxxxxxxxx> wrote:
>> Multi-vif in-place reservations happen when
>> it's impossible to allocate more chanctx as per
>> driver combinations.
>>
>> Such reservations aren't finalized until last
>> reservation interface calls in to use the
>> reservation.
>>
>> This introduces a special hook
>> ieee80211_vif_chanctx_reservation_complete(). This
>> is currently an empty stub and will be filled in
>> by AP/STA CSA code. This is required to implement
>> 2-step CSA finalization.
>>
>> This also gets rid of driver requirement to be
>> able to re-program channel of a chanctx.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx>
>> ---
>>
> [...]
>
>> - /* TODO: need to recheck if the chandef is usable etc.? */
>> +static int
>> +ieee80211_vif_use_reserved_incompat(struct ieee80211_local *local,
>> + struct ieee80211_chanctx *ctx,
>> + const struct cfg80211_chan_def *chandef)
>> +{
> [...]
>
>> + new_ctx = ieee80211_alloc_chanctx(local, chandef, ctx->mode);
>> + if (!new_ctx) {
>> + err = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + list_for_each_entry(sdata, &ctx->reserved_vifs, reserved_chanctx_list) {
>> + drv_unassign_vif_chanctx(local, sdata, ctx);
>> + rcu_assign_pointer(sdata->vif.chanctx_conf, &new_ctx->conf);
>> + }
>> +
>> + list_del_rcu(&ctx->list);
>> + ieee80211_del_chanctx(local, ctx);
>> +
>> + err = ieee80211_add_chanctx(local, new_ctx);
>> + if (err)
>> + goto err_revert;
>> +
>> + /* don't simply overwrite radar_required in case of failure */
>> + list_for_each_entry(sdata, &ctx->reserved_vifs, reserved_chanctx_list) {
>> + bool tmp = sdata->radar_required;
>> + sdata->radar_required = sdata->reserved_radar_required;
>> + sdata->reserved_radar_required = tmp;
>> + }
>> +
>> + list_for_each_entry(sdata, &ctx->reserved_vifs, reserved_chanctx_list) {
>> + err = drv_assign_vif_chanctx(local, sdata, new_ctx);
>> + if (err)
>> + goto err_unassign;
>> + }
>> +
> better recalc radar before assigning the chanctx. otherwise, you end
> up with radar_enabled configuration for non-radar channels -
> ieee80211_alloc_chanctx sets radar_enabled if any
> sdata->radar_required is true (which is true before the channel
> switch).
Good point, thanks!
>
> [...]
>
>> +
>> +err_unassign:
>> + list_for_each_entry_continue_reverse(sdata, &ctx->reserved_vifs,
>> + reserved_chanctx_list)
>> + drv_unassign_vif_chanctx(local, sdata, ctx);
>> + ieee80211_del_chanctx(local, new_ctx);
>> +err_revert:
>> + kfree_rcu(new_ctx, rcu_head);
>> + WARN_ON(ieee80211_add_chanctx(local, ctx));
>> + list_add_rcu(&ctx->list, &local->chanctx_list);
>> + list_for_each_entry(sdata, &ctx->reserved_vifs, reserved_chanctx_list) {
>> + sdata->radar_required = sdata->reserved_radar_required;
>> + rcu_assign_pointer(sdata->vif.chanctx_conf, &ctx->conf);
>> + WARN_ON(drv_assign_vif_chanctx(local, sdata, ctx));
>> + }
> seems like the list_for_each_entry should actually be under err_unassign?
No. This is correct. The err_unassign is used to bail out if any
drv_assign_vif_chanctx fails mid-way. We want to unassign only those
vif-chanctx we only managed to assign.
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
