[v10,0/2] wifi: mwifiex: add code to support host mlme

Message

David Lin April 18, 2024, 6:06 a.m. UTC

With host mlme:
Tested-by: <[e-mail]> #Verdin AM62 IW416 SD
Without host mlme:
Tested-by: Francesco Dolcini <[e-mail]> # 88W8997-SD

This series add host based MLME support to the mwifiex driver, this
enables WPA3 support in both client and AP mode.
To enable WPA3, a firmware with corresponding V2 Key API support is
required.
The feature is currently only enabled on NXP IW416 (SD8978), and it
was internally validated by NXP QA team. Other NXP Wi-Fi chips
supported in current mwifiex are not affected by this change.

v10:
   - Use eth_broadcast_addr() to set the broadcast address.
   - Add comment for constant used for the length of FW special 4 address
     management header.
   - Check host_mlme_enabled to decide if creating host_mlme_workqueue
     or not.
   - Use cpu_to_le16 instead of casting via (__force __le16).
   - Change the abbreviation "disasso" to "disassoc" of the printout message.

v9:
   - Remove redundent code.
   - Remove unnecessary goto target.  

v8:
   - Separate 6/12 from patch v7.
     As it's a bug fix not part of host MLME feature.
   - Rearrnage MLME feature into 2 patches:
     a. Add host based MLME support for STA mode.
     b. Add host based MLME support for AP mode.

v7:
   - Fix regression: Downlink throughput degraded by 70% in AP mode.
   - Fix issue: On STAUT, kernel Oops occurs when there is no association
     response from AP.
   - Fix issue: On STAUT, if AP leaves abruptly and deauth is missing,
     STA can't connect to AP anymore.
   - Fix regression: STA can't connect to AP when host_mlme is disabled
     (impact all chips).
   - Address reviewer comments.

v6:
   - Correct mailing sequence.

v5:
   - Add host base MLME support to enable WPA3 functionalities for both
     STA and AP mode.
   - This feature (WPA3) required a firmware with corresponding Key API V2
     support.
   - QA validation and regression have been covered for IW416.
   - This feature (WPA3) is currently enabled and verified only for IW416.
   - Changelogs since patch V4:
     a. Add WPA3 support for AP mode.
     b. Bug fix: In WPA3 STA mode, deice gets disconnected from AP
        when group rekey occurs.
     c. Bug fix: STAUT doesn't send WMM IE in association request when
        associate to a WMM-AP.

v4:
   - Refine code segment per review comment.
   - Add API to check firmware encryption key command version when
     host_mlme is enabled.

v3:
   - Cleanup commit message.

v2:
   - Fix checkpatch error (pwe[1] -> pwe[0]).
   - Move module parameter 'host_mlme' to mwifiex_sdio_device structure.
     Default only enable for IW416.
   - Disable advertising NL80211_FEATURE_SAE if host_mlme is not enabled.

David Lin (2):
  wifi: mwifiex: add host mlme for client mode
  wifi: mwifiex: add host mlme for AP mode

 .../net/wireless/marvell/mwifiex/cfg80211.c   | 393 +++++++++++++++++-
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |  27 ++
 drivers/net/wireless/marvell/mwifiex/decl.h   |  23 +
 drivers/net/wireless/marvell/mwifiex/fw.h     |  54 +++
 drivers/net/wireless/marvell/mwifiex/init.c   |   6 +
 drivers/net/wireless/marvell/mwifiex/ioctl.h  |   5 +
 drivers/net/wireless/marvell/mwifiex/join.c   |  66 ++-
 drivers/net/wireless/marvell/mwifiex/main.c   |  62 +++
 drivers/net/wireless/marvell/mwifiex/main.h   |  16 +
 drivers/net/wireless/marvell/mwifiex/scan.c   |   6 +
 drivers/net/wireless/marvell/mwifiex/sdio.c   |  13 +
 drivers/net/wireless/marvell/mwifiex/sdio.h   |   2 +
 .../wireless/marvell/mwifiex/sta_cmdresp.c    |   2 +
 .../net/wireless/marvell/mwifiex/sta_event.c  |  36 +-
 .../net/wireless/marvell/mwifiex/sta_ioctl.c  |   2 +-
 drivers/net/wireless/marvell/mwifiex/sta_tx.c |   9 +-
 .../net/wireless/marvell/mwifiex/uap_cmd.c    | 171 ++++++++
 drivers/net/wireless/marvell/mwifiex/util.c   | 104 +++++
 18 files changed, 980 insertions(+), 17 deletions(-)


base-commit: e6a9208730a9b693e938e497cd43c494e4b95d7b

Comments

Marcel Holtmann April 18, 2024, 6:47 a.m. UTC | #1

Hi David,

> With host mlme:
> Tested-by: <[e-mail]> #Verdin AM62 IW416 SD
> Without host mlme:
> Tested-by: Francesco Dolcini <[e-mail]> # 88W8997-SD
> 
> This series add host based MLME support to the mwifiex driver, this
> enables WPA3 support in both client and AP mode.
> To enable WPA3, a firmware with corresponding V2 Key API support is
> required.
> The feature is currently only enabled on NXP IW416 (SD8978), and it
> was internally validated by NXP QA team. Other NXP Wi-Fi chips
> supported in current mwifiex are not affected by this change.

I am a bit confused here. If this is just for WPA3 support, then wasn’t
this suppose to be solved with external_auth support?

Regards

Marcel

David Lin April 18, 2024, 9:08 a.m. UTC | #2

> From: Marcel Holtmann <[e-mail]>
> Sent: Thursday, April 18, 2024 2:48 PM
> To: David Lin <[e-mail]>
> Cc: [e-mail]; LKML <[e-mail]>;
> [e-mail]; Kalle Valo <[e-mail]>;
> [e-mail]; Pete Hsieh <[e-mail]>; rafael.beims
> <[e-mail]>; Francesco Dolcini
> <[e-mail]>
> Subject: [EXT] Re: [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> Hi David,
> 
> > With host mlme:
> > Tested-by: <[e-mail]> #Verdin AM62 IW416 SD Without
> > host mlme:
> > Tested-by: Francesco Dolcini <[e-mail]> #
> > 88W8997-SD
> >
> > This series add host based MLME support to the mwifiex driver, this
> > enables WPA3 support in both client and AP mode.
> > To enable WPA3, a firmware with corresponding V2 Key API support is
> > required.
> > The feature is currently only enabled on NXP IW416 (SD8978), and it
> > was internally validated by NXP QA team. Other NXP Wi-Fi chips
> > supported in current mwifiex are not affected by this change.
> 
> I am a bit confused here. If this is just for WPA3 support, then wasn’t this
> suppose to be solved with external_auth support?
> 
> Regards
> 
> Marcel

FW can't support WPA3. In order to support WPA3, driver should leverage MLME of wpa_supplicant and hostapd.
This patch is used to let MLME is running by wpa_supplicant and hostapd instead of handling by FW.

David

Marcel Holtmann April 18, 2024, 9:15 a.m. UTC | #3

Hi David,

>>> With host mlme:
>>> Tested-by: <[e-mail]> #Verdin AM62 IW416 SD Without
>>> host mlme:
>>> Tested-by: Francesco Dolcini <[e-mail]> #
>>> 88W8997-SD
>>> 
>>> This series add host based MLME support to the mwifiex driver, this
>>> enables WPA3 support in both client and AP mode.
>>> To enable WPA3, a firmware with corresponding V2 Key API support is
>>> required.
>>> The feature is currently only enabled on NXP IW416 (SD8978), and it
>>> was internally validated by NXP QA team. Other NXP Wi-Fi chips
>>> supported in current mwifiex are not affected by this change.
>> 
>> I am a bit confused here. If this is just for WPA3 support, then wasn’t this
>> suppose to be solved with external_auth support?
>> 
>> Regards
>> 
>> Marcel
> 
> FW can't support WPA3. In order to support WPA3, driver should leverage MLME of wpa_supplicant and hostapd.
> This patch is used to let MLME is running by wpa_supplicant and hostapd instead of handling by FW.

as I said, isn’t external_auth exactly meant for this?

Regards

Marcel

David Lin April 19, 2024, 4 a.m. UTC | #4

> From: Marcel Holtmann <[e-mail]>
> Sent: Thursday, April 18, 2024 5:15 PM
> To: David Lin <[e-mail]>
> Cc: [e-mail]; LKML <[e-mail]>;
> [e-mail]; Kalle Valo <[e-mail]>;
> [e-mail]; Pete Hsieh <[e-mail]>; rafael.beims
> <[e-mail]>; Francesco Dolcini
> <[e-mail]>
> Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> 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 David,
> 
> >>> With host mlme:
> >>> Tested-by: <[e-mail]> #Verdin AM62 IW416 SD
> Without
> >>> host mlme:
> >>> Tested-by: Francesco Dolcini <[e-mail]> #
> >>> 88W8997-SD
> >>>
> >>> This series add host based MLME support to the mwifiex driver, this
> >>> enables WPA3 support in both client and AP mode.
> >>> To enable WPA3, a firmware with corresponding V2 Key API support is
> >>> required.
> >>> The feature is currently only enabled on NXP IW416 (SD8978), and it
> >>> was internally validated by NXP QA team. Other NXP Wi-Fi chips
> >>> supported in current mwifiex are not affected by this change.
> >>
> >> I am a bit confused here. If this is just for WPA3 support, then
> >> wasn’t this suppose to be solved with external_auth support?
> >>
> >> Regards
> >>
> >> Marcel
> >
> > FW can't support WPA3. In order to support WPA3, driver should leverage
> MLME of wpa_supplicant and hostapd.
> > This patch is used to let MLME is running by wpa_supplicant and hostapd
> instead of handling by FW.
> 
> as I said, isn’t external_auth exactly meant for this?
> 
> Regards
> 
> Marcel

Can you let me know what does "external_auth" mean?

David

Brian Norris April 19, 2024, 9:42 p.m. UTC | #5

On Fri, Apr 19, 2024 at 04:00:17AM +0000, David Lin wrote:
> Can you let me know what does "external_auth" mean?

Look around for NL80211_CMD_EXTERNAL_AUTH. It's in nl80211.h with plenty
of comments. (And also cfg80211_ops::cfg80211_ops,
cfg80211_external_auth_request, ...)

I've never looked at this interface before, personally. It seems to rely
on cfg80211_ops::mgmt_tx support too; that seems to exist in mwifiex,
although I have no clue the quality of support there.

Brian

David Lin April 22, 2024, 8:34 a.m. UTC | #6

> From: Brian Norris <[e-mail]>
> Sent: Saturday, April 20, 2024 5:42 AM
> To: David Lin <[e-mail]>
> Cc: Marcel Holtmann <[e-mail]>;
> [e-mail]; LKML <[e-mail]>; Kalle
> Valo <[e-mail]>; [e-mail]; Pete Hsieh
> <[e-mail]>; rafael.beims <[e-mail]>;
> Francesco Dolcini <[e-mail]>
> Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> 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
> 
> 
> On Fri, Apr 19, 2024 at 04:00:17AM +0000, David Lin wrote:
> > Can you let me know what does "external_auth" mean?
> 
> Look around for NL80211_CMD_EXTERNAL_AUTH. It's in nl80211.h with
> plenty of comments. (And also cfg80211_ops::cfg80211_ops,
> cfg80211_external_auth_request, ...)
> 
> I've never looked at this interface before, personally. It seems to rely on
> cfg80211_ops::mgmt_tx support too; that seems to exist in mwifiex,
> although I have no clue the quality of support there.
> 
> Brian

Thanks for your information. If "external_auth" means only offloading SAE
authentication to SME of wpa_supplicant and hostapd (driver should hook
cfg80211_ops. external_auth() to achieve this kind of offloading).
Then it is not the same as the job done by this patch.
This patch fully leverages SME of wpa_supplicant and hostapd.

David

David Lin April 23, 2024, 2:29 a.m. UTC | #7

Hi Brian and Kalle,

	Johannes agreed that cfg80211 is the correct way for the development of mwifiex
	(mac80211 can't offload association process to driver/FW).
	This patch is used to fully leverage SME of wpa_supplicant and hostapd which can complete the missing WPA3 feature of mwifiex.
	The patch series had been reviewed and discussed. It looks like there is no more comments for patch v10.
	I wonder can patch v10 be accepted by you?

David

Marcel Holtmann April 24, 2024, 8:11 a.m. UTC | #8

Hi David,

> Johannes agreed that cfg80211 is the correct way for the development of mwifiex
> (mac80211 can't offload association process to driver/FW).

that was never my question here.

> This patch is used to fully leverage SME of wpa_supplicant and hostapd which can complete the missing WPA3 feature of mwifiex.
> The patch series had been reviewed and discussed. It looks like there is no more comments for patch v10.
> I wonder can patch v10 be accepted by you?

If your hardware is a FullMac hardware then what is the point in now separating
auth/assoc out. Is this done just for WPA3 or also for WPA2/WPA1. Are you no
longer a FullMac hardware?

You keep saying that you just want to support WPA3 and if previously the HW
worked as FullMac hardware, then external_auth should be the way to go for
having SAE handled by wpa_supplicant (or iwd for that matter).

Now if you are fully embracing to auth/assoc and we can remove the support
for the connect ops, then lets do it. However I don’t see anything properly
described in the commit message. You keep saying WPA3 support and nothing
else explain what the new Key V2 API of the firmware would do.

Regards

Marcel

David Lin April 25, 2024, 2:09 a.m. UTC | #9

> From: David Lin <[e-mail]>
> Sent: Monday, April 22, 2024 4:35 PM
> To: Brian Norris <[e-mail]>
> Cc: Marcel Holtmann <[e-mail]>; [e-mail];
> LKML <[e-mail]>; Kalle Valo <[e-mail]>;
> [e-mail]; Pete Hsieh <[e-mail]>; rafael.beims
> <[e-mail]>; Francesco Dolcini
> <[e-mail]>
> Subject: RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme 
> 
> > From: Brian Norris <[e-mail]>
> > Sent: Saturday, April 20, 2024 5:42 AM
> > To: David Lin <[e-mail]>
> > Cc: Marcel Holtmann <[e-mail]>;
> > [e-mail]; LKML <[e-mail]>;
> > Kalle Valo <[e-mail]>; [e-mail]; Pete Hsieh
> > <[e-mail]>; rafael.beims <[e-mail]>;
> > Francesco Dolcini <[e-mail]>
> > Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support
> > host mlme
> >
> > 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
> >
> >
> > On Fri, Apr 19, 2024 at 04:00:17AM +0000, David Lin wrote:
> > > Can you let me know what does "external_auth" mean?
> >
> > Look around for NL80211_CMD_EXTERNAL_AUTH. It's in nl80211.h with
> > plenty of comments. (And also cfg80211_ops::cfg80211_ops,
> > cfg80211_external_auth_request, ...)
> >
> > I've never looked at this interface before, personally. It seems to
> > rely on cfg80211_ops::mgmt_tx support too; that seems to exist in
> > mwifiex, although I have no clue the quality of support there.
> >
> > Brian
> 
> Thanks for your information. If "external_auth" means only offloading SAE
> authentication to SME of wpa_supplicant and hostapd (driver should hook
> cfg80211_ops. external_auth() to achieve this kind of offloading).
> Then it is not the same as the job done by this patch.
> This patch fully leverages SME of wpa_supplicant and hostapd.
> 
> David

Should I give more detail information about this question? BTW, if there are other blocked items to let this patch be accepted,
please let me know. Thanks for your help.

David

David Lin April 25, 2024, 2:40 a.m. UTC | #10

> From: Marcel Holtmann <[e-mail]>
> Sent: Wednesday, April 24, 2024 4:11 PM
> To: David Lin <[e-mail]>
> Cc: Brian Norris <[e-mail]>; [e-mail];
> LKML <[e-mail]>; Kalle Valo <[e-mail]>;
> [e-mail]; Pete Hsieh <[e-mail]>; rafael.beims
> <[e-mail]>; Francesco Dolcini
> <[e-mail]>
> Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> Hi David,
> 
> > Johannes agreed that cfg80211 is the correct way for the development
> > of mwifiex
> > (mac80211 can't offload association process to driver/FW).
> 
> that was never my question here.
>

This is previous topic discussed with Johannes to confirm cfg80211 is correct decision for NXP FW.
 
> > This patch is used to fully leverage SME of wpa_supplicant and hostapd
> which can complete the missing WPA3 feature of mwifiex.
> > The patch series had been reviewed and discussed. It looks like there is no
> more comments for patch v10.
> > I wonder can patch v10 be accepted by you?
> 
> If your hardware is a FullMac hardware then what is the point in now
> separating auth/assoc out. Is this done just for WPA3 or also for WPA2/WPA1.

Yes. Our FW can's support WPA3, so this patch is used to hook separating auth/assoc to
leverage SME of wpa_supplicant and hostapd. WPA2 is also handled by SME of
wpa_supplicant and hostapd.

> Are you no longer a FullMac hardware?

You can check previous discussion with Johannes, FW still needs to involve association
process, so mac80211 is not suitable for NXP FW.
> 
> You keep saying that you just want to support WPA3 and if previously the HW
> worked as FullMac hardware, then external_auth should be the way to go for
> having SAE handled by wpa_supplicant (or iwd for that matter).

Although external_auth is one way to support SAE, but we think hook separating auth/assoc will
be the better way to resolve this issue. In this way, offloading SME to wpa_supplicant/hostpad will
let any future changes be easy to support (we only need to check if there is anything that we should
do when converting association request to the association command supported by FW).
> 
> Now if you are fully embracing to auth/assoc and we can remove the support
> for the connect ops, then lets do it. However I don’t see anything properly
> described in the commit message. You keep saying WPA3 support and nothing
> else explain what the new Key V2 API of the firmware would do.

We give a flag to let user to decide to use connect ops or separating auth/assoc. We will remove
connect ops for our new nxpwifi driver. New key V2 API supports more key solutions.

> 
> Regards
> 
> Marcel

Thanks,
David

Brian Norris April 26, 2024, 1 a.m. UTC | #11

Hi David,

On Mon, Apr 22, 2024 at 7:29 PM David Lin <[e-mail]> wrote:
>         I wonder can patch v10 be accepted by you?

I took another step back to see what Marcel had to say about
external_auth, as I was not familiar with it, and it didn't come up in
discussion with Johannes earlier. If we have agreement external_auth
is inappropriate, then I'll revisit v10. That may take some time
though, as I'll be preoccupied next week.

Brian

Brian Norris April 26, 2024, 1:08 a.m. UTC | #12

Hi David,

On Wed, Apr 24, 2024 at 7:40 PM David Lin <[e-mail]> wrote:
> > From: Marcel Holtmann <[e-mail]>
> >
> > Hi David,
> >
> > > Johannes agreed that cfg80211 is the correct way for the development
> > > of mwifiex
> > > (mac80211 can't offload association process to driver/FW).
> >
> > that was never my question here.
> >
>
> This is previous topic discussed with Johannes to confirm cfg80211 is correct decision for NXP FW.

To be clear: external_auth() is a cfg80211 feature. So this part of
your response still isn't very relevant.

> > > This patch is used to fully leverage SME of wpa_supplicant and hostapd
> > which can complete the missing WPA3 feature of mwifiex.
> > > The patch series had been reviewed and discussed. It looks like there is no
> > more comments for patch v10.
> > > I wonder can patch v10 be accepted by you?
> >
> > If your hardware is a FullMac hardware then what is the point in now
> > separating auth/assoc out. Is this done just for WPA3 or also for WPA2/WPA1.
>
> Yes. Our FW can's support WPA3, so this patch is used to hook separating auth/assoc to
> leverage SME of wpa_supplicant and hostapd. WPA2 is also handled by SME of
> wpa_supplicant and hostapd.
>
> > Are you no longer a FullMac hardware?
>
> You can check previous discussion with Johannes, FW still needs to involve association
> process, so mac80211 is not suitable for NXP FW.

For the record, that's the v9 thread, around here:

https://lore.kernel.org/all/[e-mail]/

> > You keep saying that you just want to support WPA3 and if previously the HW
> > worked as FullMac hardware, then external_auth should be the way to go for
> > having SAE handled by wpa_supplicant (or iwd for that matter).
>
> Although external_auth is one way to support SAE, but we think hook separating auth/assoc will
> be the better way to resolve this issue. In this way, offloading SME to wpa_supplicant/hostpad will
> let any future changes be easy to support (we only need to check if there is anything that we should
> do when converting association request to the association command supported by FW).

Perhaps I'm missing something (very likely), but I don't immediately
see much difference (with respect to your FW API, and future
extensibility) between external_auth() and your current solution (of
intercepting auth()/assoc() and constructing your own AUTH frames). It
mostly just means the AUTH mgmt frames will be coming in via
NL80211_CMD_FRAME instead of being manually constructed within your
.auth() hook. The external_auth() approach actually looks *more*
natural than your current solution.

How exactly does your solution make "future changes [easier] to
support [than with external_auth]"? Do you not trust that
wpa_supplicant will provide exactly the right NL80211_CMD_FRAME
content you're looking for, and you need to tweak it to make your
firmware happy? You're talking in extreme generality, which doesn't
make it easy for me (and presumably Marcel) to understand why you're
choosing one solution and rejecting another preexisting one.

> > Now if you are fully embracing to auth/assoc and we can remove the support
> > for the connect ops, then lets do it. However I don’t see anything properly
> > described in the commit message. You keep saying WPA3 support and nothing
> > else explain what the new Key V2 API of the firmware would do.
>
> We give a flag to let user to decide to use connect ops or separating auth/assoc. We will remove
> connect ops for our new nxpwifi driver. New key V2 API supports more key solutions.

To add to this: AFAIK, you don't even have "v2 API" firmware published
for all chips that mwifiex currently supports, so even if we wanted to
completely convert this driver, we can't do that today.


Brian

David Lin April 26, 2024, 3:04 a.m. UTC | #13

> From: Brian Norris <[e-mail]>
> Sent: Friday, April 26, 2024 9:00 AM
> To: David Lin <[e-mail]>
> Cc: Marcel Holtmann <[e-mail]>; [e-mail];
> LKML <[e-mail]>; Kalle Valo <[e-mail]>;
> [e-mail]; Pete Hsieh <[e-mail]>; rafael.beims
> <[e-mail]>; Francesco Dolcini
> <[e-mail]>
> Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> 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 David,
> 
> On Mon, Apr 22, 2024 at 7:29 PM David Lin <[e-mail]> wrote:
> >         I wonder can patch v10 be accepted by you?
> 
> I took another step back to see what Marcel had to say about external_auth, as
> I was not familiar with it, and it didn't come up in discussion with Johannes
> earlier. If we have agreement external_auth is inappropriate, then I'll revisit
> v10. That may take some time though, as I'll be preoccupied next week.
> 
> Brian

Thanks and take your time.

David

David Lin May 2, 2024, 8:34 a.m. UTC | #14

> From: Brian Norris <[e-mail]>
> Sent: Friday, April 26, 2024 9:09 AM
> To: David Lin <[e-mail]>
> Cc: Marcel Holtmann <[e-mail]>;
> [e-mail]; LKML <[e-mail]>; Kalle
> Valo <[e-mail]>; [e-mail]; Pete Hsieh
> <[e-mail]>; rafael.beims <[e-mail]>;
> Francesco Dolcini <[e-mail]>
> Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
>
> Hi David,
>
> On Wed, Apr 24, 2024 at 7:40 PM David Lin <[e-mail]> wrote:
> > > From: Marcel Holtmann <[e-mail]>
> > >
> > > Hi David,
> > >
> > > > Johannes agreed that cfg80211 is the correct way for the
> > > > development of mwifiex
> > > > (mac80211 can't offload association process to driver/FW).
> > >
> > > that was never my question here.
> > >
> >
> > This is previous topic discussed with Johannes to confirm cfg80211 is
> correct decision for NXP FW.
>
> To be clear: external_auth() is a cfg80211 feature. So this part of your
> response still isn't very relevant.
>

Sorry I mean external_auth() is another way to let wpa_supplicant handle SAE,
but it doesn’t necessarily mean we have to use external_auth for SAE.
Our previous discussion has covered how current patch implemented using cfg80211 is valid.

> > > > This patch is used to fully leverage SME of wpa_supplicant and
> > > > hostapd
> > > which can complete the missing WPA3 feature of mwifiex.
> > > > The patch series had been reviewed and discussed. It looks like
> > > > there is no
> > > more comments for patch v10.
> > > > I wonder can patch v10 be accepted by you?
> > >
> > > If your hardware is a FullMac hardware then what is the point in now
> > > separating auth/assoc out. Is this done just for WPA3 or also for
> WPA2/WPA1.
> >
> > Yes. Our FW can's support WPA3, so this patch is used to hook
> > separating auth/assoc to leverage SME of wpa_supplicant and hostapd.
> > WPA2 is also handled by SME of wpa_supplicant and hostapd.
> >
> > > Are you no longer a FullMac hardware?
> >
> > You can check previous discussion with Johannes, FW still needs to
> > involve association process, so mac80211 is not suitable for NXP FW.
>
> For the record, that's the v9 thread, around here:
>
> https://lore.ke/
> rnel.org%2Fall%2F7a08dbcaded25ec0d32865647d571afbd66062fe.camel%40
> sipsolutions.net%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cd5f6edc4
> 9f8b45171e8508dc658d744f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C0%7C638496905433159887%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7
> C%7C&sdata=eRuUVAXH%2B%2B27LeOo%2BHmGz%2Byc1QbuWl8f%2BbtSkE
> 6SxUg%3D&reserved=0
>
> > > You keep saying that you just want to support WPA3 and if previously
> > > the HW worked as FullMac hardware, then external_auth should be the
> > > way to go for having SAE handled by wpa_supplicant (or iwd for that
> matter).
> >
> > Although external_auth is one way to support SAE, but we think hook
> > separating auth/assoc will be the better way to resolve this issue. In
> > this way, offloading SME to wpa_supplicant/hostpad will let any future
> > changes be easy to support (we only need to check if there is anything that
> we should do when converting association request to the association
> command supported by FW).
>
> Perhaps I'm missing something (very likely), but I don't immediately see
> much difference (with respect to your FW API, and future
> extensibility) between external_auth() and your current solution (of
> intercepting auth()/assoc() and constructing your own AUTH frames). It
> mostly just means the AUTH mgmt frames will be coming in via
> NL80211_CMD_FRAME instead of being manually constructed within your
> .auth() hook. The external_auth() approach actually looks *more* natural
> than your current solution.
>
> How exactly does your solution make "future changes [easier] to support
> [than with external_auth]"? Do you not trust that wpa_supplicant will
> provide exactly the right NL80211_CMD_FRAME content you're looking for,
> and you need to tweak it to make your firmware happy? You're talking in
> extreme generality, which doesn't make it easy for me (and presumably
> Marcel) to understand why you're choosing one solution and rejecting
> another preexisting one.

1. The process of external_auth should be as follows:
  a. cfg80211_ops.connect() is called to establish connection with remote AP.
  b. If authentication is not WLAN_AUTH_SAE, FW will process authentication/association
  and reply connection result to cfg80211.
  c. If authentication is WLAN_AUTH_SAE, FW should notify driver to call
  cfg80211_external_auth_request() to offload authentication to wpa_supplicant.
  d. FW will wait for authentication result passed by cfg80211_ops.external_auth() to
  decide if association should be processed with remote AP for the original connection
  request and reply connection result to cfg80211.
  NXP FW only supports association with or without authentication, it can't support external_auth.

2. Hook separating auth/assoc will offload SME to wpa_supplicant completely. Driver/FW don't need
  to involve the process of authentication just like external_auth did. There is no effort for driver/FW
  to support any future modifications of authentication process.

David

>
> > > Now if you are fully embracing to auth/assoc and we can remove the
> > > support for the connect ops, then lets do it. However I don’t see
> > > anything properly described in the commit message. You keep saying
> > > WPA3 support and nothing else explain what the new Key V2 API of the
> firmware would do.
> >
> > We give a flag to let user to decide to use connect ops or separating
> > auth/assoc. We will remove connect ops for our new nxpwifi driver. New
> key V2 API supports more key solutions.
>
> To add to this: AFAIK, you don't even have "v2 API" firmware published for
> all chips that mwifiex currently supports, so even if we wanted to completely
> convert this driver, we can't do that today.
>
>
> Brian

David Lin May 13, 2024, 1:27 a.m. UTC | #15

Hi Brian,

> From: David Lin <[e-mail]>
> Sent: Thursday, May 2, 2024 4:35 PM
> To: Brian Norris <[e-mail]>
> Cc: Marcel Holtmann <[e-mail]>;
> [e-mail]; LKML <[e-mail]>; Kalle
> Valo <[e-mail]>; [e-mail]; Pete Hsieh
> <[e-mail]>; rafael.beims <[e-mail]>;
> Francesco Dolcini <[e-mail]>
> Subject: RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> > From: Brian Norris <[e-mail]>
> > Sent: Friday, April 26, 2024 9:09 AM
> > To: David Lin <[e-mail]>
> > Cc: Marcel Holtmann <[e-mail]>;
> > [e-mail]; LKML <[e-mail]>;
> > Kalle Valo <[e-mail]>; [e-mail]; Pete Hsieh
> > <[e-mail]>; rafael.beims <[e-mail]>;
> > Francesco Dolcini <[e-mail]>
> > Subject: Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support
> > host mlme
> >
> > Hi David,
> >
> > On Wed, Apr 24, 2024 at 7:40 PM David Lin <[e-mail]> wrote:
> > > > From: Marcel Holtmann <[e-mail]>
> > > >
> > > > Hi David,
> > > >
> > Perhaps I'm missing something (very likely), but I don't immediately
> > see much difference (with respect to your FW API, and future
> > extensibility) between external_auth() and your current solution (of
> > intercepting auth()/assoc() and constructing your own AUTH frames). It
> > mostly just means the AUTH mgmt frames will be coming in via
> > NL80211_CMD_FRAME instead of being manually constructed within your
> > .auth() hook. The external_auth() approach actually looks *more*
> > natural than your current solution.
> >
> > How exactly does your solution make "future changes [easier] to
> > support [than with external_auth]"? Do you not trust that
> > wpa_supplicant will provide exactly the right NL80211_CMD_FRAME
> > content you're looking for, and you need to tweak it to make your
> > firmware happy? You're talking in extreme generality, which doesn't
> > make it easy for me (and presumably
> > Marcel) to understand why you're choosing one solution and rejecting
> > another preexisting one.
> 
> 1. The process of external_auth should be as follows:
>   a. cfg80211_ops.connect() is called to establish connection with remote
> AP.
>   b. If authentication is not WLAN_AUTH_SAE, FW will process
> authentication/association
>   and reply connection result to cfg80211.
>   c. If authentication is WLAN_AUTH_SAE, FW should notify driver to call
>   cfg80211_external_auth_request() to offload authentication to
> wpa_supplicant.
>   d. FW will wait for authentication result passed by
> cfg80211_ops.external_auth() to
>   decide if association should be processed with remote AP for the original
> connection
>   request and reply connection result to cfg80211.
>   NXP FW only supports association with or without authentication, it can't
> support external_auth.
> 
> 2. Hook separating auth/assoc will offload SME to wpa_supplicant
> completely. Driver/FW don't need
>   to involve the process of authentication just like external_auth did. There
> is no effort for driver/FW
>   to support any future modifications of authentication process.
> 
> David
> 

Can we confirm that hooking separating auth/assoc is more suitable than "external_auth" for mwifiex?

David

Brian Norris May 23, 2024, 12:50 a.m. UTC | #16

On Mon, May 13, 2024 at 01:27:13AM +0000, David Lin wrote:
> > From: David Lin <[e-mail]>
> > Sent: Thursday, May 2, 2024 4:35 PM
> > 
> > 1. The process of external_auth should be as follows:
> >   a. cfg80211_ops.connect() is called to establish connection with remote
> > AP.
> >   b. If authentication is not WLAN_AUTH_SAE, FW will process
> > authentication/association
> >   and reply connection result to cfg80211.
> >   c. If authentication is WLAN_AUTH_SAE, FW should notify driver to call
> >   cfg80211_external_auth_request() to offload authentication to
> > wpa_supplicant.

FWIW, I expect you could just as well teach the driver to detect this --
I don't think we'd strictly require that firmware notify the driver.
Essentially, you could teach the driver to notice any sort of CONNECT
request (e.g., keep a list of FW-supported WLAN_AUTH_* modes?) that the
firmware can't handle on its own, and begin the external_auth() process.

I'm not sure this is ideal, but it does sound doable even without FW
notification.

> >   d. FW will wait for authentication result passed by
> > cfg80211_ops.external_auth() to
> >   decide if association should be processed with remote AP for the original
> > connection
> >   request and reply connection result to cfg80211.
> >   NXP FW only supports association with or without authentication, it can't
> > support external_auth.

But could it support my above description? Basically, the driver decides
whether to submit the connection request directly to the firmware, or
go with external_auth() instead.

> > 2. Hook separating auth/assoc will offload SME to wpa_supplicant
> > completely. Driver/FW don't need
> >   to involve the process of authentication just like external_auth did. There
> > is no effort for driver/FW
> >   to support any future modifications of authentication process.
> 
> Can we confirm that hooking separating auth/assoc is more suitable than "external_auth" for mwifiex?

I have one clarification question above. And I haven't heard anything
more from Marcel, so assuming the above is clarified, I suppose we can
drop the external_auth question.

Brian