[v4,2/2] wifi: mac80211: refactor STA CSA parsing flows

[Michael-cy Lee (李峻宇)]

Hi Johannes,

Please see the inlines.

On Thu, 2024-02-08 at 14:35 +0100, Johannes Berg wrote:
>  	 
> So ... I foolishly applied my other changes first, so I had to rebase
> this - please do check.
> 
> Couple of questions/notes, if anything needs changing please send
> another patch.

The rebase seems good. We will conduct a full channel-switch test,
including hwsim tests in hostap. If there are any problems, we will
send another patch to fix them.

> 
> > +he._oper.he_oper_params = cpu_to_le32(u32_encode_bits(1,
> > +IEEE80211_HE_OPERATION_6GHZ_OP_INFO));
> 
> (I changed this to le32_encode_bits, FWIW)
> 
> > +/* if data is there validate the bandwidth & use it */
> > +if (new_chandef.chan) {
> > +if (conn_flags & IEEE80211_CONN_DISABLE_320MHZ &&
> > +    new_chandef.width == NL80211_CHAN_WIDTH_320)
> > +ieee80211_chandef_downgrade(&new_chandef);
> >  
> >  if (conn_flags & IEEE80211_CONN_DISABLE_80P80MHZ &&
> > -    new_vht_chandef.width == NL80211_CHAN_WIDTH_80P80)
> > -ieee80211_chandef_downgrade(&new_vht_chandef);
> > +    new_chandef.width == NL80211_CHAN_WIDTH_80P80)
> > +ieee80211_chandef_downgrade(&new_chandef);
> > +
> >  if (conn_flags & IEEE80211_CONN_DISABLE_160MHZ &&
> > -    new_vht_chandef.width == NL80211_CHAN_WIDTH_160)
> > -ieee80211_chandef_downgrade(&new_vht_chandef);
> > -}
> > +    new_chandef.width == NL80211_CHAN_WIDTH_160)
> > +ieee80211_chandef_downgrade(&new_chandef);
> >  
> 
> 
> Shouldn't that have (had!) an 80 MHz handling case? Or maybe a loop a
> la
> the one in ieee80211_config_bw():
> 
>         /*
>          * Downgrade the new channel if we associated with restricted
>          * bandwidth capabilities. For example, if we associated as a
>          * 20 MHz STA to a 40 MHz AP (due to regulatory, capabilities
>          * or config reasons) then switching to a 40 MHz channel now
>          * won't do us any good -- we couldn't use it with the AP.
>          */
>         while (link->u.mgd.conn.bw_limit <
>                         ieee80211_min_bw_limit_from_chandef(&chanreq.
> oper))
>                 ieee80211_chandef_downgrade(&chanreq.oper, NULL);
> 
> 
> Feels like this should be the same here.

Yes, a loop to validate the operating bandwidth is necessary. We'll
send another patch that makes this change.

> 
> Also note how this uses ieee80211_chandef_downgrade() - we really
> need
> to track the "chanreq.oper" vs. "chanreq.ap" in this code as well for
> puncturing - can I ask you to take a brief look at that? I'll anyway
> probably have to look at that soon though.

Of course.
In fact, we have plans to study and implement puncturing on our MT76
driver. We're currently working on the AP side, and we expect to start
the STA side maybe three months later.

> 
> johannes

Best, 
Michael