On Tue, Apr 1, 2014 at 8:10 AM, Michal Kazior <michal.kazior@xxxxxxxxx> wrote:
> 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.
>
sure. i failed to explain - i'm referring only to the radar_required
swapping - it seems to happen only later.
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
