Received-SPF: pass (google.com: domain of linux-wireless+bounces-3772-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <3a6f90ceb86013782329ba0054995aef225a53b1.camel@sipsolutions.net>
Subject: Re: [v4,2/2] wifi: mac80211: refactor STA CSA parsing flows
From: Johannes Berg <johannes@sipsolutions.net>
To: Michael-cy Lee =?UTF-8?Q?=28=E6=9D=8E=E5=B3=BB=E5=AE=87=29?=
	 <Michael-cy.Lee@mediatek.com>, "linux-wireless@vger.kernel.org"
	 <linux-wireless@vger.kernel.org>
Cc: "linux-mediatek@lists.infradead.org"
 <linux-mediatek@lists.infradead.org>,  "nbd@nbd.name" <nbd@nbd.name>,
 Evelyn Tsai =?UTF-8?Q?=28=E8=94=A1=E7=8F=8A=E9=88=BA=29?=
 <Evelyn.Tsai@mediatek.com>, "lorenzo@kernel.org" <lorenzo@kernel.org>,
 Money Wang =?UTF-8?Q?=28=E7=8E=8B=E4=BF=A1=E5=AE=89=29?=
 <Money.Wang@mediatek.com>
Date: Mon, 19 Feb 2024 11:06:06 +0100
In-Reply-To: <765fb43a372825f6063894809f204150d87d4d18.camel@mediatek.com>
References: <20231222010914.6521-1-michael-cy.lee@mediatek.com>
	 <20231222010914.6521-3-michael-cy.lee@mediatek.com>
	 <b74f21d22a41b2ca635e64f04c06d5db6d38d920.camel@sipsolutions.net>
	 <765fb43a372825f6063894809f204150d87d4d18.camel@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

Hi,

> 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.

OK, thanks for checking. The tests from hostap are passing, I make sure
of that anyway, but ... who knows what they miss :)

> > Shouldn't that have (had!) an 80 MHz handling case? Or maybe a loop a
> > la
> > the one in ieee80211_config_bw():
> >=20
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/*
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* Downgrade the n=
ew channel if we associated with restricted
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* bandwidth capab=
ilities. For example, if we associated as a
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* 20 MHz STA to a=
 40 MHz AP (due to regulatory, capabilities
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* or config reaso=
ns) then switching to a 40 MHz channel now
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* won't do us any=
 good -- we couldn't use it with the AP.
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0while (link->u.mgd.conn=
.bw_limit <
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
ieee80211_min_bw_limit_from_chandef(&chanreq.
> > oper))
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0ieee80211_chandef_downgrade(&chanreq.oper, NULL)=
;
> >=20
> >=20
> > Feels like this should be the same here.
>=20
> Yes, a loop to validate the operating bandwidth is necessary. We'll
> send another patch that makes this change.

Sounds good, thanks!

> > 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.
>=20
> 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.

OK, let's see. I probably have plans earlier than 3 months from now for
this, but not sure yet. Let me know when/if you start working on it so
we can sync up again then?

johannes