Re: [PATCH v2 5/7] mac80211: Adjust chan_ctx when assigning reserved vif

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2015-04-21 at 11:53 +0300, andrei.otc@gmail.com wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> When a vif is assigned to a reserved channel context (during CSA, for example)

This .. doesn't make much sense. vifs aren't assigned to channel
contexts, it's the other way around. I guess I'll rewrite that to "When
a vif starts using a reserved  channel context (...)"

> the width of this chanctx should be adjusted to be the maximum between the
> reserved chandef and all the chandefs of other assigned vifs.

This is not what you do - you don't take anything in the chanctx into
account. ieee80211_chanctx_non_reserved_chandef() just recalculates the
required chanctx, and you're fixing the code to actually apply the
recalculated value.

> Not doing so would result in using chanctx with narrower width than actually
> required. Fix this by calling ieee80211_change_chanctx with the widest common
> chandef. This both changes the chanctx's width and recalcs min_def.

This seems possible, yeah.


>         chandef = ieee80211_chanctx_non_reserved_chandef(local, new_ctx,
>                                 &sdata->reserved_chandef);

>  	if (WARN_ON(!chandef))
>  		return -EINVAL;
>  
> +	ieee80211_change_chanctx(local, new_ctx, chandef);
> +
>  	vif_chsw[0].vif = &sdata->vif;
>  	vif_chsw[0].old_ctx = &old_ctx->conf;
>  	vif_chsw[0].new_ctx = &new_ctx->conf;
[...]
> 	ieee80211_vif_update_chandef(sdata, &sdata->reserved_chandef);

Hmm. (added more context)

The code here seems to be wrong though. It shouldn't overwrite
reserved_chandef, or it shouldn't call ieee80211_vif_update_chandef()
with it, as the vif's bss_conf.chandef should represent what *this* vif
wants/needs, while you're now putting there what the *combination*
requires in the chandef.

That's clearly wrong - please submit a new patch that fixes all the
issues in these functions.

johannes