[PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Michael-cy Lee (李峻宇)]

Hi Johannes,

On Fri, 2023-11-24 at 20:31 +0100, Johannes Berg wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:
> > 
> > +new_chan_width =
> ieee80211_operating_class_to_chan_width(op_class);
> > +if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> > +      &center_freq1,
> > +      &center_freq2)) {
> 
> Unless I missed it, I see two places here calling it together, so
> seems
> reasonable to fill in a chandef here instead?
> 
> > +new_chan_width =
> ieee80211_operating_class_to_chan_width(op_class);
> > +if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> > +      &center_freq1,
> > +      &center_freq2)) {
> 
> 
> Here you have it too.
> 
> 
> > +new_chan_width = NL80211_CHAN_WIDTH_20;
> > +center_freq1 = chan->center_freq;
> 
> And actually you could just have a chandef created with
> 
>     cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_WIDTH_20)
> 
> which mirrors the failure case here, and just not update it when
> something like
> 
>     ieee80211_update_chandef_from_op_class(op_class, &chandef)
> 
> returned false (not that I necessarily think that name should be
> used.)
> 
> Or just pass the channel, and make it create one with WIDTH_20 in the
> failure case?
> 
>     ieee80211_create_chandef_from_opclass(chan, op_class, &chandef);
> 
> which is is maybe even nicer?

Good suggestion!
We'll decide which one is better.

> 
> I'm also not quite sure why you're converting to operation elements
> first?

The old flow also converted the Element to operation elements first,
then it used ieee80211_chandef_vht_oper() to build the new chandef from
operation elements.

We think it's necessary for the case that AP is trying to switch to a
160 MHz bandwidth, while the STA doesn't support the 160 MHz bandwidth.

Just like what had been done during the association,
ieee80211_chandef_vht_oper() checks the STA's capabilities and builds a
valid chandef for the STA. However, even if the STA doesn't support the
160 MHz bandwidth, ieee80211_chandef_vht_oper() doesn't mark the
conn_flags as IEEE80211_CONN_DISABLE_160MHZ, so the same check is
necessary when handling CSA.

Like we had discussed in previous mail, we expected the patch will be
simplified.
In summary, the steps for STA to handling CSA are, 
1. parse the new channel information from either operating class or
WBCS Element.
2. convert the channel information into corresponding operation Element
(HT/VHT in 5 GHz band and HE/EHT in 6 GHz band)
3. Build a valid chandef from the operation Element.

> 
> johannes

Best, 
Micahel
>