On Tue, Mar 18, 2014 at 3:53 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>
> ---
[...]
i started playing with the patches and got some warnings.
they seem to come from:
> +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;
> }
>
> - old_ctx = container_of(conf, struct ieee80211_chanctx, conf);
> + 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);
> + }
>
> - if (sdata->vif.bss_conf.chandef.width != sdata->reserved_chandef.width)
> - tmp_changed |= BSS_CHANGED_BANDWIDTH;
> + list_del_rcu(&ctx->list);
> + ieee80211_del_chanctx(local, ctx);
>
> - sdata->vif.bss_conf.chandef = sdata->reserved_chandef;
> + err = ieee80211_add_chanctx(local, new_ctx);
> + if (err)
> + goto err_revert;
>
> - /* unref our reservation */
> - sdata->reserved_chanctx = NULL;
> - sdata->radar_required = sdata->reserved_radar_required;
> - list_del(&sdata->reserved_chanctx_list);
> + /* 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;
> + }
>
> - if (old_ctx == ctx) {
> - /* This is our own context, just change it */
> - ret = __ieee80211_vif_change_channel(sdata, old_ctx,
> - &tmp_changed);
> - if (ret)
> - goto out;
> - } else {
> - ret = ieee80211_assign_vif_chanctx(sdata, ctx);
> - if (ieee80211_chanctx_refcount(local, old_ctx) == 0)
> - ieee80211_free_chanctx(local, old_ctx);
> - if (ret) {
> - /* if assign fails refcount stays the same */
> - if (ieee80211_chanctx_refcount(local, ctx) == 0)
> - ieee80211_free_chanctx(local, ctx);
> - goto out;
> - }
> + list_for_each_entry(sdata, &ctx->reserved_vifs, reserved_chanctx_list) {
> + err = drv_assign_vif_chanctx(local, sdata, ctx);
this should be new_ctx (ctx was already removed from the driver, etc.)
> + if (err)
> + goto err_unassign;
> + }
>
> + if (sdata->vif.type == NL80211_IFTYPE_AP)
> __ieee80211_vif_copy_chanctx_to_vlans(sdata, false);
> - }
>
> - *changed = tmp_changed;
> + list_add_rcu(&new_ctx->list, &local->chanctx_list);
> + kfree_rcu(ctx, rcu_head);
>
> ieee80211_recalc_chanctx_chantype(local, ctx);
> ieee80211_recalc_smps_chanctx(local, ctx);
> ieee80211_recalc_radar_chanctx(local, ctx);
> ieee80211_recalc_chanctx_min_def(local, ctx);
and same here i guess.
all the recalcs have to be moved after setting bss_conf.chandef in the
following loop, as otherwise you it recalcs according to the old
configuration (which is incompatible with the new one).
after these changes the warnings were gone.
i haven't done throughout testing, though.
Eliad.
--
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
