[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PA4PR04MB96385E87C3F000554F325697D1F42@PA4PR04MB9638.eurprd04.prod.outlook.com>
Date: Thu, 23 May 2024 02:20:24 +0000
From: David Lin <yu-hao.lin@....com>
To: Brian Norris <briannorris@...omium.org>
CC: "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kvalo@...nel.org" <kvalo@...nel.org>, "francesco@...cini.it"
<francesco@...cini.it>, Pete Hsieh <tsung-hsien.hsieh@....com>, Francesco
Dolcini <francesco.dolcini@...adex.com>
Subject: RE: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP
mode
> From: Brian Norris <briannorris@...omium.org>
> Sent: Thursday, May 23, 2024 8:59 AM
> To: David Lin <yu-hao.lin@....com>
> Cc: linux-wireless@...r.kernel.org; linux-kernel@...r.kernel.org;
> kvalo@...nel.org; francesco@...cini.it; Pete Hsieh
> <tsung-hsien.hsieh@....com>; Francesco Dolcini
> <francesco.dolcini@...adex.com>
> Subject: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hi,
>
> Hopefully-last comment for patch 2:
>
> On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>
> > @@ -1712,7 +1735,7 @@ static const u32 mwifiex_cipher_suites[] = { };
> >
> > /* Supported mgmt frame types to be advertised to cfg80211 */ -static
> > const struct ieee80211_txrx_stypes
> > +static struct ieee80211_txrx_stypes
> > mwifiex_mgmt_stypes[NUM_NL80211_IFTYPES] = {
> > [NL80211_IFTYPE_STATION] = {
> > .tx = BIT(IEEE80211_STYPE_ACTION >> 4) |
> ...
> > + if (adapter->host_mlme_enabled) {
> > + mwifiex_mgmt_stypes[NL80211_IFTYPE_AP].tx = 0xffff;
> > + mwifiex_mgmt_stypes[NL80211_IFTYPE_AP].rx =
> > + BIT(IEEE80211_STYPE_ASSOC_REQ >> 4) |
> > + BIT(IEEE80211_STYPE_REASSOC_REQ >> 4) |
> > + BIT(IEEE80211_STYPE_PROBE_REQ >> 4) |
> > + BIT(IEEE80211_STYPE_DISASSOC >> 4) |
> > + BIT(IEEE80211_STYPE_AUTH >> 4) |
> > + BIT(IEEE80211_STYPE_DEAUTH >> 4) |
> > + BIT(IEEE80211_STYPE_ACTION >> 4);
> > + }
>
> This doesn't look like a sound approach. I think you should keep
> 'mwifiex_mgmt_stypes' const, because if you're making modifications to it,
> then you may break multi-adapter situations. Consider someone loads this
> driver with host_mlme_enabled==true, and then later registers a device with
> host_mlme_enabled==false. The second adapter will get the wrong values.
>
> I think 'mwifiex_mgmt_stypes' is small enough you might as well just make a
> second copy with the MLME-enabled values, rather than fiddling with
> modifying a single copy.
>
> Aside from that:
>
> Acked-by: Brian Norris <briannorris@...omium.org>
>
> (Feel free to carry that to a v11, since my only remaining substantial concerns
> are with patch 1 I think.)
>
> Brian
I will modify mwifiex_mgmt_stypes for patch v11 and carry your "Acked-by" tag.
David
Powered by blists - more mailing lists