Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 4 Mar 2022 12:52:12 +0100
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To:     Jouni Malinen <j@w1.fi>
Cc:     Lorenzo Bianconi <lorenzo@kernel.org>, hostap@lists.infradead.org,
        linux-wireless@vger.kernel.org, nbd@nbd.name,
        ryder.lee@mediatek.com, evelyn.tsai@mediatek.com
Subject: Re: [PATCH 6/9] DFS: introduce hostapd_dfs_request_channel_switch
 routine
Message-ID: <YiH9bM9ay6/yXZiq@lore-desk>
References: <cover.1640014128.git.lorenzo@kernel.org>
 <c1bf7dc195281330e7108c533da5770adf479d84.1640014128.git.lorenzo@kernel.org>
 <20220303225439.GD33342@w1.fi>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="8rV6nLJGDvQGW+d0"
Content-Disposition: inline
In-Reply-To: <20220303225439.GD33342@w1.fi>
Precedence: bulk


--8rV6nLJGDvQGW+d0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

> On Mon, Dec 20, 2021 at 04:48:21PM +0100, Lorenzo Bianconi wrote:
> > This is a preliminary patch to add Channel Switch Announcement for
> > background radar detection.
>=20
> > diff --git a/src/ap/dfs.c b/src/ap/dfs.c
> > @@ -940,6 +940,85 @@ int hostapd_is_dfs_chan_available(struct hostapd_i=
face *iface)
> > =20
> > +static int hostapd_dfs_request_channel_switch(struct hostapd_iface *if=
ace,
>=20
> > +	struct hostapd_hw_modes *cmode =3D iface->current_mode;
> > +	int ieee80211_mode =3D IEEE80211_MODE_AP, err, i;
>=20
> That i needs to be unsigned int (like it was in the function from which
> it was moved to here) to avoid a compiler warning.

ack, I will fix it

>=20
> > +	u8 new_vht_oper_chwidth;
>=20
> > +	new_vht_oper_chwidth =3D hostapd_get_oper_chwidth(iface->conf);
> > +	hostapd_set_oper_chwidth(iface->conf,
> > +				 hostapd_get_oper_chwidth(iface->conf));
>=20
> This looks quite strange.. hostapd_dfs_start_channel_switch() used
> current_vht_oper_chwidth to store the current value from beginning of
> the function and restored it at this location. The new version in this
> helper function seems to something strange, or well, nothing. That does
> not feel correct.
>=20
> >  static int hostapd_dfs_start_channel_switch(struct hostapd_iface *ifac=
e)
> >  {
> > -	unsigned int i;
>=20
> That's where i was unsigned..
>=20
> > -	u8 current_vht_oper_chwidth =3D hostapd_get_oper_chwidth(iface->conf);
>=20
> And that's where the current value is picked at the beginning of
> hostapd_dfs_start_channel_switch().
>=20
> > -	new_vht_oper_chwidth =3D hostapd_get_oper_chwidth(iface->conf);
> > -	hostapd_set_oper_chwidth(iface->conf, current_vht_oper_chwidth);
>=20
> And this looks quite different compared to what was in the new helper
> function.

ack, I will fix it.

Regards,
Lorenzo

>=20
> --=20
> Jouni Malinen                                            PGP id EFC895FA
>=20

--8rV6nLJGDvQGW+d0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCYiH9bAAKCRA6cBh0uS2t
rLTdAQCBT/cUfb21tMJy6CNewKG4i81ro6tuFQJUVaWGDdhXYAEA7k8hGiojtj3o
6nCNz03wREKbwaUf9bkLHDsIDA5nxgg=
=1PQr
-----END PGP SIGNATURE-----

--8rV6nLJGDvQGW+d0--