Received-SPF: pass (google.com: domain of linux-wireless+bounces-7972-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: David Lin <yu-hao.lin@nxp.com>
To: Brian Norris <briannorris@chromium.org>
CC: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvalo@kernel.org" <kvalo@kernel.org>, "francesco@dolcini.it"
	<francesco@dolcini.it>, Pete Hsieh <tsung-hsien.hsieh@nxp.com>, Francesco
 Dolcini <francesco.dolcini@toradex.com>
Subject: RE: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP
 mode
Thread-Topic: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP
 mode
Thread-Index: AQHakVah/NlfkBV8NUyUKYHJ747D+LGkNjoAgAAWMtA=
Date: Thu, 23 May 2024 02:20:24 +0000
Message-ID:
 <PA4PR04MB96385E87C3F000554F325697D1F42@PA4PR04MB9638.eurprd04.prod.outlook.com>
References: <20240418060626.431202-1-yu-hao.lin@nxp.com>
 <20240418060626.431202-3-yu-hao.lin@nxp.com> <Zk6U7CYW2bP-DVTM@google.com>
In-Reply-To: <Zk6U7CYW2bP-DVTM@google.com>
Accept-Language: zh-TW, en-US
Content-Language: zh-TW
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?+psAPzi/MpzGQhkr0wZ8uVQZu4uS/06n/W/8tqQehCMSUSgQlzDZ1Iz9TqYH?=
 =?us-ascii?Q?1qE4C+9eCYpb4/ITTreky+HrrMhdbMrVT7SqLnEa5iEvrICzWyP6BWVLY5Oa?=
 =?us-ascii?Q?G3nBWAPlLGvKjOLx52h/3tFJ6VexSnkpR+V9IuGGO9gG1EQWaM1o9IShc6dE?=
 =?us-ascii?Q?3ysyjvjlorvEcenPH/NOhTpcyFnh5BxIabx/BBtf3VUpL950txdTbscCKVbA?=
 =?us-ascii?Q?Os7Afo6SbCn8zXTyayrvhRgAoYnL2WHNTSPzsRlPU4oQDHDvQwn1rqx8HefO?=
 =?us-ascii?Q?bvGSwi1QzWk22ecdwss/Zlz8AYn5SicJ0opkWgAcmmN2SmZqrHzD9yQDV3bw?=
 =?us-ascii?Q?/jRIeVp+NpEczv6I84gnfSAgXnt4BLI9rfJd97Rl2gPzVh8r/g806ymptgjJ?=
 =?us-ascii?Q?1PxUhhUIf9gI9JY8+uF+Wu4XmHYJyOfqC0pIs0N+PKPJBLDJHTeJZQ9CMpAc?=
 =?us-ascii?Q?gxtaU5ZsO5N4htJKJnITZZl5DbQP3B025vWaI9ki0lZbP3KLnve9vy8bN/NP?=
 =?us-ascii?Q?2Dz99bZu/aSZkT8AHCJWpGMMcIJA1N+4iBYPhrR5/GRb1NSf+qX1WYQjLzy9?=
 =?us-ascii?Q?rMInv4OCqNhZSE1Gw6rJmIyJv46XmoccuZldmyzrf4/xbb0kq4PhU0UhXPlg?=
 =?us-ascii?Q?2SHyeZ3BxtksnLIHgQphM61ovyiX1Wa2A4cdlL32gVN0scCMCKbgbQVD15nf?=
 =?us-ascii?Q?rdcyoJj2xvtW2B6Hu0fI+GOuEN1Vv7YCFMf+uSBAmeeW5ADHpqUMdIpz/YJD?=
 =?us-ascii?Q?6zkfjI+/S4sMJ6V8w4d/fCKyBEYBIbDfx/Xo4dX0ehjM39NUKxHybkhDS1+c?=
 =?us-ascii?Q?T6qTumQjWUTQD7Rpx3WLFudMUqXSMHaNaotXMHgPH3EhKSxNJdj5aTecs97v?=
 =?us-ascii?Q?45nGtiuYbi0UH+thGdouQL3sAux8QZO+2XYIVOW0D2HydmPjDotKLOdvP5ht?=
 =?us-ascii?Q?IT8gyJ8a4ZDXk7zQzWuqXkTfvzqsJrxcZagDVhE+2arkyhtz81UnxjJFnlgW?=
 =?us-ascii?Q?/oV4yVnw6agIeiY+ZE4ZR8daOYDj+mXk+ttXgctnCLr7pEIdjQriFawhB+Bu?=
 =?us-ascii?Q?vI+N4R7716EQhdzmFkcjqopj8s+rAn/t13hYG2iZx/Y+XDTMk9sj4KByM0vA?=
 =?us-ascii?Q?LI4iu0gd1Bffne++VtCXEduMWhSsIBSe5P313IlsF6yhjOBVir6I6AuZnla8?=
 =?us-ascii?Q?6mByRLta0ERdxqVXLt49HcSe7hbqt+J6axVtZuiuzjeqhwE6DEx8GE04IxoI?=
 =?us-ascii?Q?PAJLdd6AWChPIjIvwjr2IKMX6TPhyDSG3p77KJZRLCENUtvJt/kkIQVQ0P0z?=
 =?us-ascii?Q?coI4FCnvYg5q/7ngOfGxfaXAGnPlujXN/QmEAgCgmlkGXjXZIjFOKirxk6Ac?=
 =?us-ascii?Q?T0Ivd6Nbjqfj5tM41wHp9n+tj59j3vlEgKGjBrs2ZUYoB9SzbFbGhIebmtNp?=
 =?us-ascii?Q?Ds1+ahtOZ9me56x6DnEDovMvuwub9BgbFJ7XbZVB8YGSKxCHSvLTx0hVZG83?=
 =?us-ascii?Q?ekICKgn9ciKrLcIbeITawjQg6QLhMzpTsN2FiXjBOO2t/hk19jIifkyQOWRP?=
 =?us-ascii?Q?GgX42I8vBDtc4fCwhi3KPuPdMRoTM5luGLHul/9J?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-wireless@vger.kernel.org
List-Id: <linux-wireless.vger.kernel.org>
List-Subscribe: <mailto:linux-wireless+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-wireless+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PA4PR04MB9638.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c37e3033-9d7e-43cd-589c-08dc7acee7af
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 May 2024 02:20:24.8625
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: k+RkWQnV+e2baKcGFDz7iIxFSZ7lc23o7RdSYtpfnkGwnDPj7Tblos0ufco36vBunCsnacL/m8akau82Q2RxtA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GVXPR04MB10659

> From: Brian Norris <briannorris@chromium.org>
> Sent: Thursday, May 23, 2024 8:59 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mo=
de
>=20
> 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
>=20
>=20
> Hi,
>=20
> Hopefully-last comment for patch 2:
>=20
> 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
>=20
> > @@ -1712,7 +1735,7 @@ static const u32 mwifiex_cipher_suites[] =3D {  }=
;
> >
> >  /* 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] =3D {
> >       [NL80211_IFTYPE_STATION] =3D {
> >               .tx =3D BIT(IEEE80211_STYPE_ACTION >> 4) |
> ...
> > +     if (adapter->host_mlme_enabled) {
> > +             mwifiex_mgmt_stypes[NL80211_IFTYPE_AP].tx =3D 0xffff;
> > +             mwifiex_mgmt_stypes[NL80211_IFTYPE_AP].rx =3D
> > +                     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);
> > +     }
>=20
> 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=3D=3Dtrue, and then later registers a devic=
e with
> host_mlme_enabled=3D=3Dfalse. The second adapter will get the wrong value=
s.
>=20
> 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.
>=20
> Aside from that:
>=20
> Acked-by: Brian Norris <briannorris@chromium.org>
>=20
> (Feel free to carry that to a v11, since my only remaining substantial co=
ncerns
> are with patch 1 I think.)
>=20
> Brian

I will modify mwifiex_mgmt_stypes for patch v11 and carry your "Acked-by" t=
ag.

David