All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
@ 2024-04-18  6:06 David Lin
  2024-04-18  6:06 ` [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode David Lin
                   ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: David Lin @ 2024-04-18  6:06 UTC (permalink / raw
  To: linux-wireless
  Cc: linux-kernel, briannorris, kvalo, francesco, tsung-hsien.hsieh,
	David Lin, rafael.beims, Francesco Dolcini

With host mlme:
Tested-by: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD
Without host mlme:
Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # 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
-- 
2.34.1


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-04-18  6:06 [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme David Lin
@ 2024-04-18  6:06 ` David Lin
  2024-04-18  6:37   ` Francesco Dolcini
  2024-05-23  0:53   ` Brian Norris
  2024-04-18  6:06 ` [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
  2024-04-18  6:47 ` [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme Marcel Holtmann
  2 siblings, 2 replies; 39+ messages in thread
From: David Lin @ 2024-04-18  6:06 UTC (permalink / raw
  To: linux-wireless
  Cc: linux-kernel, briannorris, kvalo, francesco, tsung-hsien.hsieh,
	David Lin, Francesco Dolcini

Add host based MLME to enable WPA3 functionalities in client mode.
This feature required a firmware with the corresponding V2 Key API
support. The feature (WPA3) is currently enabled and verified only
on IW416. Also, verified no regression with change when host MLME
is disabled.

Signed-off-by: David Lin <yu-hao.lin@nxp.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---

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.

v8:
   - first full and complete patch to support host based MLME for client
     mode.

---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 314 ++++++++++++++++++
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |  25 ++
 drivers/net/wireless/marvell/mwifiex/decl.h   |  23 ++
 drivers/net/wireless/marvell/mwifiex/fw.h     |  33 ++
 drivers/net/wireless/marvell/mwifiex/init.c   |   6 +
 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 +
 .../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 +-
 drivers/net/wireless/marvell/mwifiex/util.c   |  80 +++++
 15 files changed, 679 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b909a7665e9c..53eeda388802 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -268,6 +268,8 @@ mwifiex_cfg80211_update_mgmt_frame_registrations(struct wiphy *wiphy,
 
 	if (mask != priv->mgmt_frame_mask) {
 		priv->mgmt_frame_mask = mask;
+		if (priv->host_mlme_reg)
+			priv->mgmt_frame_mask |= HOST_MLME_MGMT_MASK;
 		mwifiex_send_cmd(priv, HostCmd_CMD_MGMT_FRAME_REG,
 				 HostCmd_ACT_GEN_SET, 0,
 				 &priv->mgmt_frame_mask, false);
@@ -848,6 +850,7 @@ static int mwifiex_deinit_priv_params(struct mwifiex_private *priv)
 	struct mwifiex_adapter *adapter = priv->adapter;
 	unsigned long flags;
 
+	priv->host_mlme_reg = false;
 	priv->mgmt_frame_mask = 0;
 	if (mwifiex_send_cmd(priv, HostCmd_CMD_MGMT_FRAME_REG,
 			     HostCmd_ACT_GEN_SET, 0,
@@ -3631,6 +3634,9 @@ static int mwifiex_set_rekey_data(struct wiphy *wiphy, struct net_device *dev,
 	if (!ISSUPP_FIRMWARE_SUPPLICANT(priv->adapter->fw_cap_info))
 		return -EOPNOTSUPP;
 
+	if (priv->adapter->host_mlme_enabled)
+		return 0;
+
 	return mwifiex_send_cmd(priv, HostCmd_CMD_GTK_REKEY_OFFLOAD_CFG,
 				HostCmd_ACT_GEN_SET, 0, data, true);
 }
@@ -4204,6 +4210,301 @@ mwifiex_cfg80211_change_station(struct wiphy *wiphy, struct net_device *dev,
 	return ret;
 }
 
+static int
+mwifiex_cfg80211_authenticate(struct wiphy *wiphy,
+			      struct net_device *dev,
+			      struct cfg80211_auth_request *req)
+{
+	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
+	struct mwifiex_adapter *adapter = priv->adapter;
+	struct sk_buff *skb;
+	u16 pkt_len, auth_alg;
+	int ret;
+	struct mwifiex_ieee80211_mgmt *mgmt;
+	struct mwifiex_txinfo *tx_info;
+	u32 tx_control = 0, pkt_type = PKT_TYPE_MGMT;
+	u8 trans = 1, status_code = 0;
+	u8 *varptr;
+
+	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
+		mwifiex_dbg(priv->adapter, ERROR, "Interface role is AP\n");
+		return -EFAULT;
+	}
+
+	if (priv->wdev.iftype != NL80211_IFTYPE_STATION) {
+		mwifiex_dbg(priv->adapter, ERROR,
+			    "Interface type is not correct (type %d)\n",
+			    priv->wdev.iftype);
+		return -EINVAL;
+	}
+
+	if (priv->auth_alg != WLAN_AUTH_SAE &&
+	    (priv->auth_flag & HOST_MLME_AUTH_PENDING)) {
+		mwifiex_dbg(priv->adapter, ERROR, "Pending auth on going\n");
+		return -EBUSY;
+	}
+
+	if (!priv->host_mlme_reg) {
+		priv->host_mlme_reg = true;
+		priv->mgmt_frame_mask |= HOST_MLME_MGMT_MASK;
+		mwifiex_send_cmd(priv, HostCmd_CMD_MGMT_FRAME_REG,
+				 HostCmd_ACT_GEN_SET, 0,
+				 &priv->mgmt_frame_mask, false);
+	}
+
+	switch (req->auth_type) {
+	case NL80211_AUTHTYPE_OPEN_SYSTEM:
+		auth_alg = WLAN_AUTH_OPEN;
+		break;
+	case NL80211_AUTHTYPE_SHARED_KEY:
+		auth_alg = WLAN_AUTH_SHARED_KEY;
+		break;
+	case NL80211_AUTHTYPE_FT:
+		auth_alg = WLAN_AUTH_FT;
+		break;
+	case NL80211_AUTHTYPE_NETWORK_EAP:
+		auth_alg = WLAN_AUTH_LEAP;
+		break;
+	case NL80211_AUTHTYPE_SAE:
+		auth_alg = WLAN_AUTH_SAE;
+		break;
+	default:
+		mwifiex_dbg(priv->adapter, ERROR,
+			    "unsupported auth type=%d\n", req->auth_type);
+		return -EOPNOTSUPP;
+	}
+
+	if (!priv->auth_flag) {
+		ret = mwifiex_remain_on_chan_cfg(priv, HostCmd_ACT_GEN_SET,
+						 req->bss->channel,
+						 AUTH_TX_DEFAULT_WAIT_TIME);
+
+		if (!ret) {
+			priv->roc_cfg.cookie = get_random_u32() | 1;
+			priv->roc_cfg.chan = *req->bss->channel;
+		} else {
+			return -EFAULT;
+		}
+	}
+
+	priv->sec_info.authentication_mode = auth_alg;
+
+	mwifiex_cancel_scan(adapter);
+
+	pkt_len = (u16)req->ie_len + req->auth_data_len +
+		MWIFIEX_MGMT_HEADER_LEN + MWIFIEX_AUTH_BODY_LEN;
+	if (req->auth_data_len >= 4)
+		pkt_len -= 4;
+
+	skb = dev_alloc_skb(MWIFIEX_MIN_DATA_HEADER_LEN +
+			    MWIFIEX_MGMT_FRAME_HEADER_SIZE +
+			    pkt_len + sizeof(pkt_len));
+	if (!skb) {
+		mwifiex_dbg(priv->adapter, ERROR,
+			    "allocate skb failed for management frame\n");
+		return -ENOMEM;
+	}
+
+	tx_info = MWIFIEX_SKB_TXCB(skb);
+	memset(tx_info, 0, sizeof(*tx_info));
+	tx_info->bss_num = priv->bss_num;
+	tx_info->bss_type = priv->bss_type;
+	tx_info->pkt_len = pkt_len;
+
+	skb_reserve(skb, MWIFIEX_MIN_DATA_HEADER_LEN +
+		    MWIFIEX_MGMT_FRAME_HEADER_SIZE + sizeof(pkt_len));
+	memcpy(skb_push(skb, sizeof(pkt_len)), &pkt_len, sizeof(pkt_len));
+	memcpy(skb_push(skb, sizeof(tx_control)),
+	       &tx_control, sizeof(tx_control));
+	memcpy(skb_push(skb, sizeof(pkt_type)), &pkt_type, sizeof(pkt_type));
+
+	mgmt = (struct mwifiex_ieee80211_mgmt *)skb_put(skb, pkt_len);
+	memset(mgmt, 0, pkt_len);
+	mgmt->frame_control =
+		cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_AUTH);
+	memcpy(mgmt->da, req->bss->bssid, ETH_ALEN);
+	memcpy(mgmt->sa, priv->curr_addr, ETH_ALEN);
+	memcpy(mgmt->bssid, req->bss->bssid, ETH_ALEN);
+	eth_broadcast_addr(mgmt->addr4);
+
+	if (req->auth_data_len >= 4) {
+		if (req->auth_type == NL80211_AUTHTYPE_SAE) {
+			__le16 *pos = (__le16 *)req->auth_data;
+
+			trans = le16_to_cpu(pos[0]);
+			status_code = le16_to_cpu(pos[1]);
+		}
+		memcpy((u8 *)(&mgmt->auth.variable), req->auth_data + 4,
+		       req->auth_data_len - 4);
+		varptr = (u8 *)&mgmt->auth.variable +
+			 (req->auth_data_len - 4);
+	}
+
+	mgmt->auth.auth_alg = cpu_to_le16(auth_alg);
+	mgmt->auth.auth_transaction = cpu_to_le16(trans);
+	mgmt->auth.status_code = cpu_to_le16(status_code);
+
+	if (req->ie && req->ie_len) {
+		if (!varptr)
+			varptr = (u8 *)&mgmt->auth.variable;
+		memcpy((u8 *)varptr, req->ie, req->ie_len);
+	}
+
+	priv->auth_flag = HOST_MLME_AUTH_PENDING;
+	priv->auth_alg = auth_alg;
+
+	skb->priority = WMM_HIGHEST_PRIORITY;
+	__net_timestamp(skb);
+
+	mwifiex_dbg(priv->adapter, MSG,
+		    "auth: send authentication to %pM\n", req->bss->bssid);
+
+	mwifiex_queue_tx_pkt(priv, skb);
+
+	return 0;
+}
+
+static int
+mwifiex_cfg80211_associate(struct wiphy *wiphy, struct net_device *dev,
+			   struct cfg80211_assoc_request *req)
+{
+	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
+	struct mwifiex_adapter *adapter = priv->adapter;
+	int ret;
+	struct cfg80211_ssid req_ssid;
+	const u8 *ssid_ie;
+
+	if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_STA) {
+		mwifiex_dbg(adapter, ERROR,
+			    "%s: reject infra assoc request in non-STA role\n",
+			    dev->name);
+		return -EINVAL;
+	}
+
+	if (test_bit(MWIFIEX_SURPRISE_REMOVED, &adapter->work_flags) ||
+	    test_bit(MWIFIEX_IS_CMD_TIMEDOUT, &adapter->work_flags)) {
+		mwifiex_dbg(adapter, ERROR,
+			    "%s: Ignore association.\t"
+			    "Card removed or FW in bad state\n",
+			    dev->name);
+		return -EFAULT;
+	}
+
+	if (priv->auth_alg == WLAN_AUTH_SAE)
+		priv->auth_flag = HOST_MLME_AUTH_DONE;
+
+	if (priv->auth_flag && !(priv->auth_flag & HOST_MLME_AUTH_DONE))
+		return -EBUSY;
+
+	if (priv->roc_cfg.cookie) {
+		ret = mwifiex_remain_on_chan_cfg(priv, HostCmd_ACT_GEN_REMOVE,
+						 &priv->roc_cfg.chan, 0);
+		if (!ret)
+			memset(&priv->roc_cfg, 0,
+			       sizeof(struct mwifiex_roc_cfg));
+		else
+			return -EFAULT;
+	}
+
+	if (!mwifiex_stop_bg_scan(priv))
+		cfg80211_sched_scan_stopped_locked(priv->wdev.wiphy, 0);
+
+	memset(&req_ssid, 0, sizeof(struct cfg80211_ssid));
+	rcu_read_lock();
+	ssid_ie = ieee80211_bss_get_ie(req->bss, WLAN_EID_SSID);
+
+	if (!ssid_ie)
+		goto ssid_err;
+
+	req_ssid.ssid_len = ssid_ie[1];
+	if (req_ssid.ssid_len > IEEE80211_MAX_SSID_LEN) {
+		mwifiex_dbg(adapter, ERROR, "invalid SSID - aborting\n");
+		goto ssid_err;
+	}
+
+	memcpy(req_ssid.ssid, ssid_ie + 2, req_ssid.ssid_len);
+	if (!req_ssid.ssid_len || req_ssid.ssid[0] < 0x20) {
+		mwifiex_dbg(adapter, ERROR, "invalid SSID - aborting\n");
+		goto ssid_err;
+	}
+	rcu_read_unlock();
+
+	/* As this is new association, clear locally stored
+	 * keys and security related flags
+	 */
+	priv->sec_info.wpa_enabled = false;
+	priv->sec_info.wpa2_enabled = false;
+	priv->wep_key_curr_index = 0;
+	priv->sec_info.encryption_mode = 0;
+	priv->sec_info.is_authtype_auto = 0;
+	if (mwifiex_set_encode(priv, NULL, NULL, 0, 0, NULL, 1)) {
+		mwifiex_dbg(priv->adapter, ERROR, "deleting the crypto keys\n");
+		return -EFAULT;
+	}
+
+	if (req->crypto.n_ciphers_pairwise)
+		priv->sec_info.encryption_mode =
+			req->crypto.ciphers_pairwise[0];
+
+	if (req->crypto.cipher_group)
+		priv->sec_info.encryption_mode = req->crypto.cipher_group;
+
+	if (req->ie)
+		mwifiex_set_gen_ie(priv, req->ie, req->ie_len);
+
+	memcpy(priv->cfg_bssid, req->bss->bssid, ETH_ALEN);
+
+	mwifiex_dbg(adapter, MSG,
+		    "assoc: send association to %pM\n", req->bss->bssid);
+
+	cfg80211_ref_bss(adapter->wiphy, req->bss);
+	ret = mwifiex_bss_start(priv, req->bss, &req_ssid);
+	if (ret) {
+		priv->auth_flag = 0;
+		priv->auth_alg = WLAN_AUTH_NONE;
+		eth_zero_addr(priv->cfg_bssid);
+	}
+
+	if (priv->assoc_rsp_size) {
+		priv->req_bss = req->bss;
+		adapter->assoc_resp_received = true;
+		queue_work(adapter->host_mlme_workqueue,
+			   &adapter->host_mlme_work);
+	}
+
+	cfg80211_put_bss(priv->adapter->wiphy, req->bss);
+
+	return 0;
+
+ssid_err:
+	rcu_read_unlock();
+	return -EFAULT;
+}
+
+static int
+mwifiex_cfg80211_deauthenticate(struct wiphy *wiphy,
+				struct net_device *dev,
+				struct cfg80211_deauth_request *req)
+{
+	return mwifiex_cfg80211_disconnect(wiphy, dev, req->reason_code);
+}
+
+static int
+mwifiex_cfg80211_disassociate(struct wiphy *wiphy,
+			      struct net_device *dev,
+			      struct cfg80211_disassoc_request *req)
+{
+	return mwifiex_cfg80211_disconnect(wiphy, dev, req->reason_code);
+}
+
+static int
+mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
+			      struct net_device *dev, const u8 *peer,
+			      u64 *cookie)
+{
+	return -EOPNOTSUPP;
+}
+
 /* station cfg80211 operations */
 static struct cfg80211_ops mwifiex_cfg80211_ops = {
 	.add_virtual_intf = mwifiex_add_virtual_intf,
@@ -4349,6 +4650,16 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
 			    "%s: creating new wiphy\n", __func__);
 		return -ENOMEM;
 	}
+	if (adapter->host_mlme_enabled) {
+		mwifiex_cfg80211_ops.auth = mwifiex_cfg80211_authenticate;
+		mwifiex_cfg80211_ops.assoc = mwifiex_cfg80211_associate;
+		mwifiex_cfg80211_ops.deauth = mwifiex_cfg80211_deauthenticate;
+		mwifiex_cfg80211_ops.disassoc = mwifiex_cfg80211_disassociate;
+		mwifiex_cfg80211_ops.disconnect = NULL;
+		mwifiex_cfg80211_ops.connect = NULL;
+		mwifiex_cfg80211_ops.probe_client =
+			mwifiex_cfg80211_probe_client;
+	}
 	wiphy->max_scan_ssids = MWIFIEX_MAX_SSID_LIST_LENGTH;
 	wiphy->max_scan_ie_len = MWIFIEX_MAX_VSIE_LEN;
 	wiphy->mgmt_stypes = mwifiex_mgmt_stypes;
@@ -4430,6 +4741,9 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
 			   NL80211_FEATURE_LOW_PRIORITY_SCAN |
 			   NL80211_FEATURE_NEED_OBSS_SCAN;
 
+	if (adapter->host_mlme_enabled)
+		wiphy->features |= NL80211_FEATURE_SAE;
+
 	if (ISSUPP_ADHOC_ENABLED(adapter->fw_cap_info))
 		wiphy->features |= NL80211_FEATURE_HT_IBSS;
 
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 9eff29a25544..da983e27023c 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -924,6 +924,24 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter)
 	return ret;
 }
 
+void mwifiex_process_assoc_resp(struct mwifiex_adapter *adapter)
+{
+	struct cfg80211_rx_assoc_resp_data assoc_resp = {
+		.uapsd_queues = -1,
+	};
+	struct mwifiex_private *priv =
+		mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA);
+
+	if (priv->assoc_rsp_size) {
+		assoc_resp.links[0].bss = priv->req_bss;
+		assoc_resp.buf = priv->assoc_rsp_buf;
+		assoc_resp.len = priv->assoc_rsp_size;
+		cfg80211_rx_assoc_resp(priv->netdev,
+				       &assoc_resp);
+		priv->assoc_rsp_size = 0;
+	}
+}
+
 /*
  * This function handles the timeout of command sending.
  *
@@ -1672,6 +1690,13 @@ int mwifiex_ret_get_hw_spec(struct mwifiex_private *priv,
 	if (adapter->fw_api_ver == MWIFIEX_FW_V15)
 		adapter->scan_chan_gap_enabled = true;
 
+	if (adapter->key_api_major_ver != KEY_API_VER_MAJOR_V2)
+		adapter->host_mlme_enabled = false;
+
+	mwifiex_dbg(adapter, MSG, "host_mlme: %s, key_api: %d\n",
+		    adapter->host_mlme_enabled ? "enable" : "disable",
+		    adapter->key_api_major_ver);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/decl.h b/drivers/net/wireless/marvell/mwifiex/decl.h
index 326ffb05d791..84603f1e7f6e 100644
--- a/drivers/net/wireless/marvell/mwifiex/decl.h
+++ b/drivers/net/wireless/marvell/mwifiex/decl.h
@@ -31,6 +31,29 @@
 						 *   + sizeof(tx_control)
 						 */
 
+#define FRMCTL_LEN                2
+#define DURATION_LEN              2
+#define SEQCTL_LEN                2
+/* special FW 4 address management header */
+#define MWIFIEX_MGMT_HEADER_LEN   (FRMCTL_LEN + DURATION_LEN + ETH_ALEN + \
+				   ETH_ALEN + ETH_ALEN + SEQCTL_LEN + ETH_ALEN)
+
+#define AUTH_ALG_LEN              2
+#define AUTH_TRANSACTION_LEN      2
+#define AUTH_STATUS_LEN           2
+#define MWIFIEX_AUTH_BODY_LEN     (AUTH_ALG_LEN + AUTH_TRANSACTION_LEN + \
+				   AUTH_STATUS_LEN)
+
+#define HOST_MLME_AUTH_PENDING    BIT(0)
+#define HOST_MLME_AUTH_DONE       BIT(1)
+
+#define HOST_MLME_MGMT_MASK       (BIT(IEEE80211_STYPE_AUTH >> 4) | \
+				   BIT(IEEE80211_STYPE_DEAUTH >> 4) | \
+				   BIT(IEEE80211_STYPE_DISASSOC >> 4))
+#define AUTH_TX_DEFAULT_WAIT_TIME 2400
+
+#define WLAN_AUTH_NONE            0xFFFF
+
 #define MWIFIEX_MAX_TX_BASTREAM_SUPPORTED	2
 #define MWIFIEX_MAX_RX_BASTREAM_SUPPORTED	16
 #define MWIFIEX_MAX_TDLS_PEER_SUPPORTED 8
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 3adc447b715f..0f89b86aa527 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -210,6 +210,8 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
 #define TLV_TYPE_RANDOM_MAC         (PROPRIETARY_TLV_BASE_ID + 236)
 #define TLV_TYPE_CHAN_ATTR_CFG      (PROPRIETARY_TLV_BASE_ID + 237)
 #define TLV_TYPE_MAX_CONN           (PROPRIETARY_TLV_BASE_ID + 279)
+#define TLV_TYPE_HOST_MLME          (PROPRIETARY_TLV_BASE_ID + 307)
+#define TLV_TYPE_SAE_PWE_MODE       (PROPRIETARY_TLV_BASE_ID + 339)
 
 #define MWIFIEX_TX_DATA_BUF_SIZE_2K        2048
 
@@ -744,6 +746,25 @@ struct uap_rxpd {
 	u8 flags;
 } __packed;
 
+struct mwifiex_auth {
+	__le16 auth_alg;
+	__le16 auth_transaction;
+	__le16 status_code;
+	/* possibly followed by Challenge text */
+	u8 variable[];
+} __packed;
+
+struct mwifiex_ieee80211_mgmt {
+	__le16 frame_control;
+	__le16 duration;
+	u8 da[ETH_ALEN];
+	u8 sa[ETH_ALEN];
+	u8 bssid[ETH_ALEN];
+	__le16 seq_ctrl;
+	u8 addr4[ETH_ALEN];
+	struct mwifiex_auth auth;
+} __packed;
+
 struct mwifiex_fw_chan_stats {
 	u8 chan_num;
 	u8 bandcfg;
@@ -803,6 +824,11 @@ struct mwifiex_ie_types_ssid_param_set {
 	u8 ssid[];
 } __packed;
 
+struct mwifiex_ie_types_host_mlme {
+	struct mwifiex_ie_types_header header;
+	u8 host_mlme;
+} __packed;
+
 struct mwifiex_ie_types_num_probes {
 	struct mwifiex_ie_types_header header;
 	__le16 num_probes;
@@ -906,6 +932,13 @@ struct mwifiex_ie_types_tdls_idle_timeout {
 	__le16 value;
 } __packed;
 
+#define MWIFIEX_AUTHTYPE_SAE 6
+
+struct mwifiex_ie_types_sae_pwe_mode {
+	struct mwifiex_ie_types_header header;
+	u8 pwe[];
+} __packed;
+
 struct mwifiex_ie_types_rsn_param_set {
 	struct mwifiex_ie_types_header header;
 	u8 rsn_ie[];
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index c9c58419c37b..a336d45b9677 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -81,6 +81,9 @@ int mwifiex_init_priv(struct mwifiex_private *priv)
 	priv->bcn_avg_factor = DEFAULT_BCN_AVG_FACTOR;
 	priv->data_avg_factor = DEFAULT_DATA_AVG_FACTOR;
 
+	priv->auth_flag = 0;
+	priv->auth_alg = WLAN_AUTH_NONE;
+
 	priv->sec_info.wep_enabled = 0;
 	priv->sec_info.authentication_mode = NL80211_AUTHTYPE_OPEN_SYSTEM;
 	priv->sec_info.encryption_mode = 0;
@@ -220,6 +223,9 @@ static void mwifiex_init_adapter(struct mwifiex_adapter *adapter)
 	adapter->cmd_resp_received = false;
 	adapter->event_received = false;
 	adapter->data_received = false;
+	adapter->assoc_resp_received = false;
+	adapter->priv_link_lost = NULL;
+	adapter->host_mlme_link_lost = false;
 
 	clear_bit(MWIFIEX_SURPRISE_REMOVED, &adapter->work_flags);
 
diff --git a/drivers/net/wireless/marvell/mwifiex/join.c b/drivers/net/wireless/marvell/mwifiex/join.c
index 9d98a1908dd6..249210fb2e75 100644
--- a/drivers/net/wireless/marvell/mwifiex/join.c
+++ b/drivers/net/wireless/marvell/mwifiex/join.c
@@ -382,7 +382,9 @@ int mwifiex_cmd_802_11_associate(struct mwifiex_private *priv,
 	struct mwifiex_ie_types_ss_param_set *ss_tlv;
 	struct mwifiex_ie_types_rates_param_set *rates_tlv;
 	struct mwifiex_ie_types_auth_type *auth_tlv;
+	struct mwifiex_ie_types_sae_pwe_mode *sae_pwe_tlv;
 	struct mwifiex_ie_types_chan_list_param_set *chan_tlv;
+	struct mwifiex_ie_types_host_mlme *host_mlme_tlv;
 	u8 rates[MWIFIEX_SUPPORTED_RATES];
 	u32 rates_size;
 	u16 tmp_cap;
@@ -460,6 +462,24 @@ int mwifiex_cmd_802_11_associate(struct mwifiex_private *priv,
 
 	pos += sizeof(auth_tlv->header) + le16_to_cpu(auth_tlv->header.len);
 
+	if (priv->sec_info.authentication_mode == WLAN_AUTH_SAE) {
+		auth_tlv->auth_type = cpu_to_le16(MWIFIEX_AUTHTYPE_SAE);
+		if (bss_desc->bcn_rsnx_ie &&
+		    bss_desc->bcn_rsnx_ie->ieee_hdr.len &&
+		    (bss_desc->bcn_rsnx_ie->data[0] &
+		    WLAN_RSNX_CAPA_SAE_H2E)) {
+			sae_pwe_tlv =
+				(struct mwifiex_ie_types_sae_pwe_mode *)pos;
+			sae_pwe_tlv->header.type =
+				cpu_to_le16(TLV_TYPE_SAE_PWE_MODE);
+			sae_pwe_tlv->header.len =
+				cpu_to_le16(sizeof(sae_pwe_tlv->pwe[0]));
+			sae_pwe_tlv->pwe[0] = bss_desc->bcn_rsnx_ie->data[0];
+			pos += sizeof(sae_pwe_tlv->header) +
+				sizeof(sae_pwe_tlv->pwe[0]);
+		}
+	}
+
 	if (IS_SUPPORT_MULTI_BANDS(priv->adapter) &&
 	    !(ISSUPP_11NENABLED(priv->adapter->fw_cap_info) &&
 	    (!bss_desc->disable_11n) &&
@@ -491,6 +511,16 @@ int mwifiex_cmd_802_11_associate(struct mwifiex_private *priv,
 			sizeof(struct mwifiex_chan_scan_param_set);
 	}
 
+	if (priv->adapter->host_mlme_enabled) {
+		host_mlme_tlv = (struct mwifiex_ie_types_host_mlme *)pos;
+		host_mlme_tlv->header.type = cpu_to_le16(TLV_TYPE_HOST_MLME);
+		host_mlme_tlv->header.len =
+			cpu_to_le16(sizeof(host_mlme_tlv->host_mlme));
+		host_mlme_tlv->host_mlme = 1;
+		pos += sizeof(host_mlme_tlv->header) +
+			sizeof(host_mlme_tlv->host_mlme);
+	}
+
 	if (!priv->wps.session_enable) {
 		if (priv->sec_info.wpa_enabled || priv->sec_info.wpa2_enabled)
 			rsn_ie_len = mwifiex_append_rsn_ie_wpa_wpa2(priv, &pos);
@@ -641,7 +671,21 @@ int mwifiex_ret_802_11_associate(struct mwifiex_private *priv,
 		goto done;
 	}
 
-	assoc_rsp = (struct ieee_types_assoc_rsp *) &resp->params;
+	if (adapter->host_mlme_enabled) {
+		struct ieee80211_mgmt *hdr;
+
+		hdr = (struct ieee80211_mgmt *)&resp->params;
+		if (!memcmp(hdr->bssid,
+			    priv->attempted_bss_desc->mac_address,
+			    ETH_ALEN))
+			assoc_rsp = (struct ieee_types_assoc_rsp *)
+				&hdr->u.assoc_resp;
+		else
+			assoc_rsp =
+				(struct ieee_types_assoc_rsp *)&resp->params;
+	} else {
+		assoc_rsp = (struct ieee_types_assoc_rsp *)&resp->params;
+	}
 
 	cap_info = le16_to_cpu(assoc_rsp->cap_info_bitmap);
 	status_code = le16_to_cpu(assoc_rsp->status_code);
@@ -680,6 +724,9 @@ int mwifiex_ret_802_11_associate(struct mwifiex_private *priv,
 				mwifiex_dbg(priv->adapter, ERROR,
 					    "ASSOC_RESP: UNSPECIFIED failure\n");
 			}
+
+			if (priv->adapter->host_mlme_enabled)
+				priv->assoc_rsp_size = 0;
 		} else {
 			ret = status_code;
 		}
@@ -778,7 +825,8 @@ int mwifiex_ret_802_11_associate(struct mwifiex_private *priv,
 
 	priv->adapter->dbg.num_cmd_assoc_success++;
 
-	mwifiex_dbg(priv->adapter, INFO, "info: ASSOC_RESP: associated\n");
+	mwifiex_dbg(priv->adapter, MSG, "assoc: associated with %pM\n",
+		    priv->attempted_bss_desc->mac_address);
 
 	/* Add the ra_list here for infra mode as there will be only 1 ra
 	   always */
@@ -1491,6 +1539,20 @@ int mwifiex_deauthenticate(struct mwifiex_private *priv, u8 *mac)
 	if (!priv->media_connected)
 		return 0;
 
+	if (priv->adapter->host_mlme_enabled) {
+		priv->auth_flag = 0;
+		priv->auth_alg = WLAN_AUTH_NONE;
+		priv->host_mlme_reg = false;
+		priv->mgmt_frame_mask = 0;
+		if (mwifiex_send_cmd(priv, HostCmd_CMD_MGMT_FRAME_REG,
+				     HostCmd_ACT_GEN_SET, 0,
+				     &priv->mgmt_frame_mask, false)) {
+			mwifiex_dbg(priv->adapter, ERROR,
+				    "could not unregister mgmt frame rx\n");
+			return -1;
+		}
+	}
+
 	switch (priv->bss_mode) {
 	case NL80211_IFTYPE_STATION:
 	case NL80211_IFTYPE_P2P_CLIENT:
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index d99127dc466e..88cd2d6a1dcb 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -530,6 +530,11 @@ static void mwifiex_terminate_workqueue(struct mwifiex_adapter *adapter)
 		destroy_workqueue(adapter->rx_workqueue);
 		adapter->rx_workqueue = NULL;
 	}
+
+	if (adapter->host_mlme_workqueue) {
+		destroy_workqueue(adapter->host_mlme_workqueue);
+		adapter->host_mlme_workqueue = NULL;
+	}
 }
 
 /*
@@ -802,6 +807,10 @@ mwifiex_bypass_tx_queue(struct mwifiex_private *priv,
 			    "bypass txqueue; eth type %#x, mgmt %d\n",
 			     ntohs(eth_hdr->h_proto),
 			     mwifiex_is_skb_mgmt_frame(skb));
+		if (eth_hdr->h_proto == htons(ETH_P_PAE))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "key: send EAPOL to %pM\n",
+				    eth_hdr->h_dest);
 		return true;
 	}
 
@@ -1384,6 +1393,35 @@ int is_command_pending(struct mwifiex_adapter *adapter)
 	return !is_cmd_pend_q_empty;
 }
 
+/* This is the host mlme work queue function.
+ * It handles the host mlme operations.
+ */
+static void mwifiex_host_mlme_work_queue(struct work_struct *work)
+{
+	struct mwifiex_adapter *adapter =
+		container_of(work, struct mwifiex_adapter, host_mlme_work);
+
+	if (test_bit(MWIFIEX_SURPRISE_REMOVED, &adapter->work_flags))
+		return;
+
+	/* Check for host mlme disconnection */
+	if (adapter->host_mlme_link_lost) {
+		if (adapter->priv_link_lost) {
+			mwifiex_reset_connect_state(adapter->priv_link_lost,
+						    WLAN_REASON_DEAUTH_LEAVING,
+						    true);
+			adapter->priv_link_lost = NULL;
+		}
+		adapter->host_mlme_link_lost = false;
+	}
+
+	/* Check for host mlme Assoc Resp */
+	if (adapter->assoc_resp_received) {
+		mwifiex_process_assoc_resp(adapter);
+		adapter->assoc_resp_received = false;
+	}
+}
+
 /*
  * This is the RX work queue function.
  *
@@ -1558,6 +1596,18 @@ mwifiex_reinit_sw(struct mwifiex_adapter *adapter)
 		INIT_WORK(&adapter->rx_work, mwifiex_rx_work_queue);
 	}
 
+	if (adapter->host_mlme_enabled) {
+		adapter->host_mlme_workqueue =
+			alloc_workqueue("MWIFIEX_HOST_MLME_WORK_QUEUE",
+					WQ_HIGHPRI |
+					WQ_MEM_RECLAIM |
+					WQ_UNBOUND, 0);
+		if (!adapter->host_mlme_workqueue)
+			goto err_kmalloc;
+		INIT_WORK(&adapter->host_mlme_work,
+			  mwifiex_host_mlme_work_queue);
+	}
+
 	/* Register the device. Fill up the private data structure with
 	 * relevant information from the card. Some code extracted from
 	 * mwifiex_register_dev()
@@ -1721,6 +1771,18 @@ mwifiex_add_card(void *card, struct completion *fw_done,
 		goto err_registerdev;
 	}
 
+	if (adapter->host_mlme_enabled) {
+		adapter->host_mlme_workqueue =
+			alloc_workqueue("MWIFIEX_HOST_MLME_WORK_QUEUE",
+					WQ_HIGHPRI |
+					WQ_MEM_RECLAIM |
+					WQ_UNBOUND, 0);
+		if (!adapter->host_mlme_workqueue)
+			goto err_kmalloc;
+		INIT_WORK(&adapter->host_mlme_work,
+			  mwifiex_host_mlme_work_queue);
+	}
+
 	if (mwifiex_init_hw_fw(adapter, true)) {
 		pr_err("%s: firmware init failed\n", __func__);
 		goto err_init_fw;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 175882485a19..a0fdabc43fff 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -424,6 +424,8 @@ struct mwifiex_bssdescriptor {
 	u16 wpa_offset;
 	struct ieee_types_generic *bcn_rsn_ie;
 	u16 rsn_offset;
+	struct ieee_types_generic *bcn_rsnx_ie;
+	u16 rsnx_offset;
 	struct ieee_types_generic *bcn_wapi_ie;
 	u16 wapi_offset;
 	u8 *beacon_buf;
@@ -525,6 +527,8 @@ struct mwifiex_private {
 	u8 bss_priority;
 	u8 bss_num;
 	u8 bss_started;
+	u8 auth_flag;
+	u16 auth_alg;
 	u8 frame_type;
 	u8 curr_addr[ETH_ALEN];
 	u8 media_connected;
@@ -608,6 +612,7 @@ struct mwifiex_private {
 #define MWIFIEX_ASSOC_RSP_BUF_SIZE  500
 	u8 assoc_rsp_buf[MWIFIEX_ASSOC_RSP_BUF_SIZE];
 	u32 assoc_rsp_size;
+	struct cfg80211_bss *req_bss;
 
 #define MWIFIEX_GENIE_BUF_SIZE      256
 	u8 gen_ie_buf[MWIFIEX_GENIE_BUF_SIZE];
@@ -647,6 +652,7 @@ struct mwifiex_private {
 	u16 gen_idx;
 	u8 ap_11n_enabled;
 	u8 ap_11ac_enabled;
+	bool host_mlme_reg;
 	u32 mgmt_frame_mask;
 	struct mwifiex_roc_cfg roc_cfg;
 	bool scan_aborting;
@@ -876,6 +882,8 @@ struct mwifiex_adapter {
 	struct work_struct main_work;
 	struct workqueue_struct *rx_workqueue;
 	struct work_struct rx_work;
+	struct workqueue_struct *host_mlme_workqueue;
+	struct work_struct host_mlme_work;
 	bool rx_work_enabled;
 	bool rx_processing;
 	bool delay_main_work;
@@ -907,6 +915,9 @@ struct mwifiex_adapter {
 	u8 cmd_resp_received;
 	u8 event_received;
 	u8 data_received;
+	u8 assoc_resp_received;
+	struct mwifiex_private *priv_link_lost;
+	u8 host_mlme_link_lost;
 	u16 seq_num;
 	struct cmd_ctrl_node *cmd_pool;
 	struct cmd_ctrl_node *curr_cmd;
@@ -996,6 +1007,7 @@ struct mwifiex_adapter {
 	bool is_up;
 
 	bool ext_scan;
+	bool host_mlme_enabled;
 	u8 fw_api_ver;
 	u8 key_api_major_ver, key_api_minor_ver;
 	u8 max_p2p_conn, max_sta_conn;
@@ -1061,6 +1073,9 @@ int mwifiex_recv_packet(struct mwifiex_private *priv, struct sk_buff *skb);
 int mwifiex_uap_recv_packet(struct mwifiex_private *priv,
 			    struct sk_buff *skb);
 
+void mwifiex_host_mlme_disconnect(struct mwifiex_private *priv,
+				  u16 reason_code, u8 *sa);
+
 int mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
 				struct sk_buff *skb);
 
@@ -1092,6 +1107,7 @@ void mwifiex_insert_cmd_to_pending_q(struct mwifiex_adapter *adapter,
 
 int mwifiex_exec_next_cmd(struct mwifiex_adapter *adapter);
 int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter);
+void mwifiex_process_assoc_resp(struct mwifiex_adapter *adapter);
 int mwifiex_handle_rx_packet(struct mwifiex_adapter *adapter,
 			     struct sk_buff *skb);
 int mwifiex_process_tx(struct mwifiex_private *priv, struct sk_buff *skb,
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 0326b121747c..e782d652cb93 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1371,6 +1371,12 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 			bss_entry->rsn_offset = (u16) (current_ptr -
 							bss_entry->beacon_buf);
 			break;
+		case WLAN_EID_RSNX:
+			bss_entry->bcn_rsnx_ie =
+				(struct ieee_types_generic *)current_ptr;
+			bss_entry->rsnx_offset =
+				(u16)(current_ptr - bss_entry->beacon_buf);
+			break;
 		case WLAN_EID_BSS_AC_ACCESS_DELAY:
 			bss_entry->bcn_wapi_ie =
 				(struct ieee_types_generic *) current_ptr;
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index f41048b5cd3c..f5c3267a23a2 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -332,6 +332,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8786 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = false,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8787 = {
@@ -348,6 +349,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8787 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8797 = {
@@ -364,6 +366,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8797 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8897 = {
@@ -380,6 +383,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8897 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8977 = {
@@ -397,6 +401,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8977 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8978 = {
@@ -414,6 +419,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8978 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = true,
+	.host_mlme = true,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
@@ -432,6 +438,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {
@@ -448,6 +455,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {
 	.can_auto_tdls = true,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8987 = {
@@ -465,6 +473,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8987 = {
 	.can_auto_tdls = true,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8801 = {
@@ -481,6 +490,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8801 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
+	.host_mlme = false,
 };
 
 static struct memory_type_mapping generic_mem_type_map[] = {
@@ -574,6 +584,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct sdio_device_id *id)
 		card->can_auto_tdls = data->can_auto_tdls;
 		card->can_ext_scan = data->can_ext_scan;
 		card->fw_ready_extra_delay = data->fw_ready_extra_delay;
+		card->host_mlme = data->host_mlme;
 		INIT_WORK(&card->work, mwifiex_sdio_work);
 	}
 
@@ -2512,6 +2523,8 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
 		adapter->num_mem_types = ARRAY_SIZE(mem_type_mapping_tbl);
 	}
 
+	adapter->host_mlme_enabled = card->host_mlme;
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
index cb63ad55d675..65d142286c46 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -256,6 +256,7 @@ struct sdio_mmc_card {
 	bool can_auto_tdls;
 	bool can_ext_scan;
 	bool fw_ready_extra_delay;
+	bool host_mlme;
 
 	struct mwifiex_sdio_mpa_tx mpa_tx;
 	struct mwifiex_sdio_mpa_rx mpa_rx;
@@ -280,6 +281,7 @@ struct mwifiex_sdio_device {
 	bool can_auto_tdls;
 	bool can_ext_scan;
 	bool fw_ready_extra_delay;
+	bool host_mlme;
 };
 
 /*
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c
index df9cdd10a494..b5f3821a6a8f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c
@@ -135,6 +135,9 @@ void mwifiex_reset_connect_state(struct mwifiex_private *priv, u16 reason_code,
 
 	priv->media_connected = false;
 
+	priv->auth_flag = 0;
+	priv->auth_alg = WLAN_AUTH_NONE;
+
 	priv->scan_block = false;
 	priv->port_open = false;
 
@@ -222,8 +225,12 @@ void mwifiex_reset_connect_state(struct mwifiex_private *priv, u16 reason_code,
 		    priv->cfg_bssid, reason_code);
 	if (priv->bss_mode == NL80211_IFTYPE_STATION ||
 	    priv->bss_mode == NL80211_IFTYPE_P2P_CLIENT) {
-		cfg80211_disconnected(priv->netdev, reason_code, NULL, 0,
-				      !from_ap, GFP_KERNEL);
+		if (adapter->host_mlme_enabled && adapter->host_mlme_link_lost)
+			mwifiex_host_mlme_disconnect(adapter->priv_link_lost,
+						     reason_code, NULL);
+		else
+			cfg80211_disconnected(priv->netdev, reason_code, NULL,
+					      0, !from_ap, GFP_KERNEL);
 	}
 	eth_zero_addr(priv->cfg_bssid);
 
@@ -746,7 +753,15 @@ int mwifiex_process_sta_event(struct mwifiex_private *priv)
 		if (priv->media_connected) {
 			reason_code =
 				get_unaligned_le16(adapter->event_body);
-			mwifiex_reset_connect_state(priv, reason_code, true);
+			if (adapter->host_mlme_enabled) {
+				adapter->priv_link_lost = priv;
+				adapter->host_mlme_link_lost = true;
+				queue_work(adapter->host_mlme_workqueue,
+					   &adapter->host_mlme_work);
+			} else {
+				mwifiex_reset_connect_state(priv, reason_code,
+							    true);
+			}
 		}
 		break;
 
@@ -999,10 +1014,17 @@ int mwifiex_process_sta_event(struct mwifiex_private *priv)
 	case EVENT_REMAIN_ON_CHAN_EXPIRED:
 		mwifiex_dbg(adapter, EVENT,
 			    "event: Remain on channel expired\n");
-		cfg80211_remain_on_channel_expired(&priv->wdev,
-						   priv->roc_cfg.cookie,
-						   &priv->roc_cfg.chan,
-						   GFP_ATOMIC);
+
+		if (adapter->host_mlme_enabled &&
+		    (priv->auth_flag & HOST_MLME_AUTH_PENDING)) {
+			priv->auth_flag = 0;
+			priv->auth_alg = WLAN_AUTH_NONE;
+		} else {
+			cfg80211_remain_on_channel_expired(&priv->wdev,
+							   priv->roc_cfg.cookie,
+							   &priv->roc_cfg.chan,
+							   GFP_ATOMIC);
+		}
 
 		memset(&priv->roc_cfg, 0x00, sizeof(struct mwifiex_roc_cfg));
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index 32a27fad7b79..a94659988a4c 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -339,7 +339,7 @@ int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss,
 			ret = mwifiex_associate(priv, bss_desc);
 		}
 
-		if (bss)
+		if (bss && !priv->adapter->host_mlme_enabled)
 			cfg80211_put_bss(priv->adapter->wiphy, bss);
 	} else {
 		/* Adhoc mode */
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_tx.c b/drivers/net/wireless/marvell/mwifiex/sta_tx.c
index 70c2790b8e35..9d0ef04ebe02 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_tx.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_tx.c
@@ -36,7 +36,7 @@ void mwifiex_process_sta_txpd(struct mwifiex_private *priv,
 	struct txpd *local_tx_pd;
 	struct mwifiex_txinfo *tx_info = MWIFIEX_SKB_TXCB(skb);
 	unsigned int pad;
-	u16 pkt_type, pkt_offset;
+	u16 pkt_type, pkt_length, pkt_offset;
 	int hroom = adapter->intf_hdr_len;
 
 	pkt_type = mwifiex_is_skb_mgmt_frame(skb) ? PKT_TYPE_MGMT : 0;
@@ -49,9 +49,10 @@ void mwifiex_process_sta_txpd(struct mwifiex_private *priv,
 	memset(local_tx_pd, 0, sizeof(struct txpd));
 	local_tx_pd->bss_num = priv->bss_num;
 	local_tx_pd->bss_type = priv->bss_type;
-	local_tx_pd->tx_pkt_length = cpu_to_le16((u16)(skb->len -
-						       (sizeof(struct txpd) +
-							pad)));
+	pkt_length = (u16)(skb->len - (sizeof(struct txpd) + pad));
+	if (pkt_type == PKT_TYPE_MGMT)
+		pkt_length -= MWIFIEX_MGMT_FRAME_HEADER_SIZE;
+	local_tx_pd->tx_pkt_length = cpu_to_le16(pkt_length);
 
 	local_tx_pd->priority = (u8) skb->priority;
 	local_tx_pd->pkt_delay_2ms =
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 745b1d925b21..3817c08a1507 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -370,6 +370,45 @@ mwifiex_parse_mgmt_packet(struct mwifiex_private *priv, u8 *payload, u16 len,
 
 	return 0;
 }
+
+/* This function sends deauth packet to the kernel. */
+void mwifiex_host_mlme_disconnect(struct mwifiex_private *priv,
+				  u16 reason_code, u8 *sa)
+{
+	u8 frame_buf[100];
+	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)frame_buf;
+
+	memset(frame_buf, 0, sizeof(frame_buf));
+	mgmt->frame_control = cpu_to_le16(IEEE80211_STYPE_DEAUTH);
+	mgmt->duration = 0;
+	mgmt->seq_ctrl = 0;
+	mgmt->u.deauth.reason_code = cpu_to_le16(reason_code);
+
+	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) {
+		eth_broadcast_addr(mgmt->da);
+		memcpy(mgmt->sa,
+		       priv->curr_bss_params.bss_descriptor.mac_address,
+		       ETH_ALEN);
+		memcpy(mgmt->bssid, priv->cfg_bssid, ETH_ALEN);
+		priv->auth_flag = 0;
+		priv->auth_alg = WLAN_AUTH_NONE;
+	} else {
+		memcpy(mgmt->da, priv->curr_addr, ETH_ALEN);
+		memcpy(mgmt->sa, sa, ETH_ALEN);
+		memcpy(mgmt->bssid, priv->curr_addr, ETH_ALEN);
+	}
+
+	if (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP) {
+		wiphy_lock(priv->wdev.wiphy);
+		cfg80211_rx_mlme_mgmt(priv->netdev, frame_buf, 26);
+		wiphy_unlock(priv->wdev.wiphy);
+	} else {
+		cfg80211_rx_mgmt(&priv->wdev,
+				 priv->bss_chandef.chan->center_freq,
+				 0, frame_buf, 26, 0);
+	}
+}
+
 /*
  * This function processes the received management packet and send it
  * to the kernel.
@@ -417,6 +456,47 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
 	pkt_len -= ETH_ALEN;
 	rx_pd->rx_pkt_length = cpu_to_le16(pkt_len);
 
+	if (priv->host_mlme_reg &&
+	    (GET_BSS_ROLE(priv) != MWIFIEX_BSS_ROLE_UAP) &&
+	    (ieee80211_is_auth(ieee_hdr->frame_control) ||
+	     ieee80211_is_deauth(ieee_hdr->frame_control) ||
+	     ieee80211_is_disassoc(ieee_hdr->frame_control))) {
+		if (ieee80211_is_auth(ieee_hdr->frame_control)) {
+			if (priv->auth_flag & HOST_MLME_AUTH_PENDING) {
+				if (priv->auth_alg != WLAN_AUTH_SAE) {
+					priv->auth_flag &=
+						~HOST_MLME_AUTH_PENDING;
+					priv->auth_flag |=
+						HOST_MLME_AUTH_DONE;
+				}
+			} else {
+				return 0;
+			}
+
+			mwifiex_dbg(priv->adapter, MSG,
+				    "auth: receive authentication from %pM\n",
+				    ieee_hdr->addr3);
+		} else {
+			if (!priv->wdev.connected)
+				return 0;
+
+			if (ieee80211_is_deauth(ieee_hdr->frame_control)) {
+				mwifiex_dbg(priv->adapter, MSG,
+					    "auth: receive deauth from %pM\n",
+					    ieee_hdr->addr3);
+				priv->auth_flag = 0;
+				priv->auth_alg = WLAN_AUTH_NONE;
+			} else {
+				mwifiex_dbg
+				(priv->adapter, MSG,
+				 "assoc: receive disassoc from %pM\n",
+				 ieee_hdr->addr3);
+			}
+		}
+
+		cfg80211_rx_mlme_mgmt(priv->netdev, skb->data, pkt_len);
+	}
+
 	cfg80211_rx_mgmt(&priv->wdev, priv->roc_cfg.chan.center_freq,
 			 CAL_RSSI(rx_pd->snr, rx_pd->nf), skb->data, pkt_len,
 			 0);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-04-18  6:06 [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme David Lin
  2024-04-18  6:06 ` [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode David Lin
@ 2024-04-18  6:06 ` David Lin
  2024-05-23  0:59   ` Brian Norris
  2024-06-12 13:11   ` Sascha Hauer
  2024-04-18  6:47 ` [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme Marcel Holtmann
  2 siblings, 2 replies; 39+ messages in thread
From: David Lin @ 2024-04-18  6:06 UTC (permalink / raw
  To: linux-wireless
  Cc: linux-kernel, briannorris, kvalo, francesco, tsung-hsien.hsieh,
	David Lin, Francesco Dolcini

Add host based MLME to enable WPA3 functionalities in AP mode.
This feature required a firmware with the corresponding V2 Key API
support. The feature (WPA3) is currently enabled and verified only
on IW416. Also, verified no regression with change when host MLME
is disabled.

Signed-off-by: David Lin <yu-hao.lin@nxp.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---

v10:
   - none

v9:
   - remove unnecessary goto target.

v8:
   - first full and complete patch to support host based MLME for AP
     mode.

---
 .../net/wireless/marvell/mwifiex/cfg80211.c   |  79 +++++++-
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |   2 +
 drivers/net/wireless/marvell/mwifiex/fw.h     |  21 +++
 drivers/net/wireless/marvell/mwifiex/ioctl.h  |   5 +
 .../wireless/marvell/mwifiex/sta_cmdresp.c    |   2 +
 .../net/wireless/marvell/mwifiex/uap_cmd.c    | 171 ++++++++++++++++++
 drivers/net/wireless/marvell/mwifiex/util.c   |  24 +++
 7 files changed, 301 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 53eeda388802..e122cc686dad 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -221,6 +221,26 @@ mwifiex_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		return 0;
 	}
 
+	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
+		if (ieee80211_is_auth(mgmt->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "auth: send auth to %pM\n", mgmt->da);
+		if (ieee80211_is_deauth(mgmt->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "auth: send deauth to %pM\n", mgmt->da);
+		if (ieee80211_is_disassoc(mgmt->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "assoc: send disassoc to %pM\n", mgmt->da);
+		if (ieee80211_is_assoc_resp(mgmt->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "assoc: send assoc resp to %pM\n",
+				    mgmt->da);
+		if (ieee80211_is_reassoc_resp(mgmt->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "assoc: send reassoc resp to %pM\n",
+				    mgmt->da);
+	}
+
 	pkt_len = len + ETH_ALEN;
 	skb = dev_alloc_skb(MWIFIEX_MIN_DATA_HEADER_LEN +
 			    MWIFIEX_MGMT_FRAME_HEADER_SIZE +
@@ -505,6 +525,9 @@ mwifiex_cfg80211_set_default_mgmt_key(struct wiphy *wiphy,
 
 	wiphy_dbg(wiphy, "set default mgmt key, key index=%d\n", key_index);
 
+	if (priv->adapter->host_mlme_enabled)
+		return 0;
+
 	memset(&encrypt_key, 0, sizeof(struct mwifiex_ds_encrypt_key));
 	encrypt_key.key_len = WLAN_KEY_LEN_CCMP;
 	encrypt_key.key_index = key_index;
@@ -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) |
@@ -3951,12 +3974,43 @@ mwifiex_cfg80211_tdls_cancel_chan_switch(struct wiphy *wiphy,
 	}
 }
 
+static int
+mwifiex_cfg80211_uap_add_station(struct mwifiex_private *priv, const u8 *mac,
+				 struct station_parameters *params)
+{
+	struct mwifiex_sta_info add_sta;
+	int ret;
+
+	memcpy(add_sta.peer_mac, mac, ETH_ALEN);
+	add_sta.params = params;
+
+	ret = mwifiex_send_cmd(priv, HostCmd_CMD_ADD_NEW_STATION,
+			       HostCmd_ACT_ADD_STA, 0, (void *)&add_sta, true);
+
+	if (!ret) {
+		struct station_info *sinfo;
+
+		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+		if (!sinfo)
+			return -ENOMEM;
+
+		cfg80211_new_sta(priv->netdev, mac, sinfo, GFP_KERNEL);
+		kfree(sinfo);
+	}
+
+	return ret;
+}
+
 static int
 mwifiex_cfg80211_add_station(struct wiphy *wiphy, struct net_device *dev,
 			     const u8 *mac, struct station_parameters *params)
 {
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 
+	if (priv->adapter->host_mlme_enabled &&
+	    (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP))
+		return mwifiex_cfg80211_uap_add_station(priv, mac, params);
+
 	if (!(params->sta_flags_set & BIT(NL80211_STA_FLAG_TDLS_PEER)))
 		return -EOPNOTSUPP;
 
@@ -4194,6 +4248,10 @@ mwifiex_cfg80211_change_station(struct wiphy *wiphy, struct net_device *dev,
 	int ret;
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 
+	if (priv->adapter->host_mlme_enabled &&
+	    (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP))
+		return 0;
+
 	/* we support change_station handler only for TDLS peers*/
 	if (!(params->sta_flags_set & BIT(NL80211_STA_FLAG_TDLS_PEER)))
 		return -EOPNOTSUPP;
@@ -4662,6 +4720,17 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
 	}
 	wiphy->max_scan_ssids = MWIFIEX_MAX_SSID_LIST_LENGTH;
 	wiphy->max_scan_ie_len = MWIFIEX_MAX_VSIE_LEN;
+	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);
+	}
 	wiphy->mgmt_stypes = mwifiex_mgmt_stypes;
 	wiphy->max_remain_on_channel_duration = 5000;
 	wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
@@ -4704,14 +4773,18 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
 
 	ether_addr_copy(wiphy->perm_addr, adapter->perm_addr);
 	wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;
-	wiphy->flags |= WIPHY_FLAG_HAVE_AP_SME |
-			WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD |
+	wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD |
 			WIPHY_FLAG_AP_UAPSD |
 			WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL |
 			WIPHY_FLAG_HAS_CHANNEL_SWITCH |
 			WIPHY_FLAG_NETNS_OK |
 			WIPHY_FLAG_PS_ON_BY_DEFAULT;
 
+	if (adapter->host_mlme_enabled)
+		wiphy->flags |= WIPHY_FLAG_REPORTS_OBSS;
+	else
+		wiphy->flags |= WIPHY_FLAG_HAVE_AP_SME;
+
 	if (ISSUPP_TDLS_ENABLED(adapter->fw_cap_info))
 		wiphy->flags |= WIPHY_FLAG_SUPPORTS_TDLS |
 				WIPHY_FLAG_TDLS_EXTERNAL_SETUP;
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index da983e27023c..ea6ebc9c23ef 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -635,6 +635,8 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
 		case HostCmd_CMD_UAP_STA_DEAUTH:
 		case HOST_CMD_APCMD_SYS_RESET:
 		case HOST_CMD_APCMD_STA_LIST:
+		case HostCmd_CMD_CHAN_REPORT_REQUEST:
+		case HostCmd_CMD_ADD_NEW_STATION:
 			ret = mwifiex_uap_prepare_cmd(priv, cmd_no, cmd_action,
 						      cmd_oid, data_buf,
 						      cmd_ptr);
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 0f89b86aa527..65799ae3bc72 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -211,6 +211,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
 #define TLV_TYPE_CHAN_ATTR_CFG      (PROPRIETARY_TLV_BASE_ID + 237)
 #define TLV_TYPE_MAX_CONN           (PROPRIETARY_TLV_BASE_ID + 279)
 #define TLV_TYPE_HOST_MLME          (PROPRIETARY_TLV_BASE_ID + 307)
+#define TLV_TYPE_UAP_STA_FLAGS      (PROPRIETARY_TLV_BASE_ID + 313)
 #define TLV_TYPE_SAE_PWE_MODE       (PROPRIETARY_TLV_BASE_ID + 339)
 
 #define MWIFIEX_TX_DATA_BUF_SIZE_2K        2048
@@ -407,6 +408,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
 #define HostCmd_CMD_STA_CONFIGURE		      0x023f
 #define HostCmd_CMD_CHAN_REGION_CFG		      0x0242
 #define HostCmd_CMD_PACKET_AGGR_CTRL		      0x0251
+#define HostCmd_CMD_ADD_NEW_STATION		      0x025f
 
 #define PROTOCOL_NO_SECURITY        0x01
 #define PROTOCOL_STATIC_WEP         0x02
@@ -417,6 +419,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
 #define KEY_MGMT_NONE               0x04
 #define KEY_MGMT_PSK                0x02
 #define KEY_MGMT_EAP                0x01
+#define KEY_MGMT_SAE                0x400
 #define CIPHER_TKIP                 0x04
 #define CIPHER_AES_CCMP             0x08
 #define VALID_CIPHER_BITMAP         0x0c
@@ -502,6 +505,9 @@ enum mwifiex_channel_flags {
 #define HostCmd_ACT_GET_TX              0x0008
 #define HostCmd_ACT_GET_BOTH            0x000c
 
+#define HostCmd_ACT_REMOVE_STA          0x0
+#define HostCmd_ACT_ADD_STA             0x1
+
 #define RF_ANTENNA_AUTO                 0xFFFF
 
 #define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) \
@@ -2331,6 +2337,20 @@ struct host_cmd_ds_sta_configure {
 	u8 tlv_buffer[];
 } __packed;
 
+struct mwifiex_ie_types_sta_flag {
+	struct mwifiex_ie_types_header header;
+	__le32 sta_flags;
+} __packed;
+
+struct host_cmd_ds_add_station {
+	__le16 action;
+	__le16 aid;
+	u8 peer_mac[ETH_ALEN];
+	__le32 listen_interval;
+	__le16 cap_info;
+	u8 tlv[];
+} __packed;
+
 struct host_cmd_ds_command {
 	__le16 command;
 	__le16 size;
@@ -2409,6 +2429,7 @@ struct host_cmd_ds_command {
 		struct host_cmd_ds_chan_region_cfg reg_cfg;
 		struct host_cmd_ds_pkt_aggr_ctrl pkt_aggr_ctrl;
 		struct host_cmd_ds_sta_configure sta_cfg;
+		struct host_cmd_ds_add_station sta_info;
 	} params;
 } __packed;
 
diff --git a/drivers/net/wireless/marvell/mwifiex/ioctl.h b/drivers/net/wireless/marvell/mwifiex/ioctl.h
index e8825f302de8..516159b721d3 100644
--- a/drivers/net/wireless/marvell/mwifiex/ioctl.h
+++ b/drivers/net/wireless/marvell/mwifiex/ioctl.h
@@ -158,6 +158,11 @@ struct mwifiex_bss_info {
 	u8 bssid[ETH_ALEN];
 };
 
+struct mwifiex_sta_info {
+	u8 peer_mac[ETH_ALEN];
+	struct station_parameters *params;
+};
+
 #define MAX_NUM_TID     8
 
 #define MAX_RX_WINSIZE  64
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 7b69d27e0c0e..9c53825f222d 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -1398,6 +1398,8 @@ int mwifiex_process_sta_cmdresp(struct mwifiex_private *priv, u16 cmdresp_no,
 		break;
 	case HostCmd_CMD_UAP_STA_DEAUTH:
 		break;
+	case HostCmd_CMD_ADD_NEW_STATION:
+		break;
 	case HOST_CMD_APCMD_SYS_RESET:
 		break;
 	case HostCmd_CMD_MEF_CFG:
diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
index 491e36611909..073c665183b3 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
@@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
 				bss_config->key_mgmt = KEY_MGMT_PSK;
 			}
 			break;
+		case WLAN_AKM_SUITE_SAE:
+			bss_config->protocol = PROTOCOL_WPA2;
+			bss_config->key_mgmt = KEY_MGMT_SAE;
+			break;
 		default:
 			break;
 		}
@@ -751,6 +755,28 @@ mwifiex_cmd_uap_sys_config(struct host_cmd_ds_command *cmd, u16 cmd_action,
 	return 0;
 }
 
+/* This function prepares AP start up command with or without host MLME
+ */
+static void mwifiex_cmd_uap_bss_start(struct mwifiex_private *priv,
+				     struct host_cmd_ds_command *cmd)
+{
+	struct mwifiex_ie_types_host_mlme *tlv;
+	int size;
+
+	cmd->command = cpu_to_le16(HostCmd_CMD_UAP_BSS_START);
+	size = S_DS_GEN;
+
+	if (priv->adapter->host_mlme_enabled) {
+		tlv = (struct mwifiex_ie_types_host_mlme *)((u8 *)cmd + size);
+		tlv->header.type = cpu_to_le16(TLV_TYPE_HOST_MLME);
+		tlv->header.len = cpu_to_le16(sizeof(tlv->host_mlme));
+		tlv->host_mlme = 1;
+		size += sizeof(struct mwifiex_ie_types_host_mlme);
+	}
+
+	cmd->size = cpu_to_le16(size);
+}
+
 /* This function prepares AP specific deauth command with mac supplied in
  * function parameter.
  */
@@ -768,6 +794,144 @@ static int mwifiex_cmd_uap_sta_deauth(struct mwifiex_private *priv,
 	return 0;
 }
 
+/* This function prepares AP specific add station command.
+ */
+static int mwifiex_cmd_uap_add_station(struct mwifiex_private *priv,
+				       struct host_cmd_ds_command *cmd,
+				       u16 cmd_action, void *data_buf)
+{
+	struct host_cmd_ds_add_station *new_sta = &cmd->params.sta_info;
+	struct mwifiex_sta_info *add_sta = (struct mwifiex_sta_info *)data_buf;
+	struct station_parameters *params = add_sta->params;
+	struct mwifiex_sta_node *sta_ptr;
+	u8 *pos;
+	u8 qos_capa;
+	u16 header_len = sizeof(struct mwifiex_ie_types_header);
+	u16 tlv_len;
+	int size;
+	struct mwifiex_ie_types_data *tlv;
+	struct mwifiex_ie_types_sta_flag *sta_flag;
+	int i;
+
+	cmd->command = cpu_to_le16(HostCmd_CMD_ADD_NEW_STATION);
+	new_sta->action = cpu_to_le16(cmd_action);
+	size = sizeof(struct host_cmd_ds_add_station) + S_DS_GEN;
+
+	if (cmd_action == HostCmd_ACT_ADD_STA)
+		sta_ptr = mwifiex_add_sta_entry(priv, add_sta->peer_mac);
+	else
+		sta_ptr = mwifiex_get_sta_entry(priv, add_sta->peer_mac);
+
+	if (!sta_ptr)
+		return -1;
+
+	memcpy(new_sta->peer_mac, add_sta->peer_mac, ETH_ALEN);
+
+	if (cmd_action == HostCmd_ACT_REMOVE_STA) {
+		cmd->size = cpu_to_le16(size);
+		return 0;
+	}
+
+	new_sta->aid = cpu_to_le16(params->aid);
+	new_sta->listen_interval = cpu_to_le32(params->listen_interval);
+	new_sta->cap_info = cpu_to_le16(params->capability);
+
+	pos = new_sta->tlv;
+
+	if (params->sta_flags_set & NL80211_STA_FLAG_WME)
+		sta_ptr->is_wmm_enabled = 1;
+	sta_flag = (struct mwifiex_ie_types_sta_flag *)pos;
+	sta_flag->header.type = cpu_to_le16(TLV_TYPE_UAP_STA_FLAGS);
+	sta_flag->header.len = cpu_to_le16(sizeof(__le32));
+	sta_flag->sta_flags = cpu_to_le32(params->sta_flags_set);
+	pos += sizeof(struct mwifiex_ie_types_sta_flag);
+	size += sizeof(struct mwifiex_ie_types_sta_flag);
+
+	if (params->ext_capab_len) {
+		tlv = (struct mwifiex_ie_types_data *)pos;
+		tlv->header.type = cpu_to_le16(WLAN_EID_EXT_CAPABILITY);
+		tlv_len = params->ext_capab_len;
+		tlv->header.len = cpu_to_le16(tlv_len);
+		memcpy(tlv->data, params->ext_capab, tlv_len);
+		pos += (header_len + tlv_len);
+		size += (header_len + tlv_len);
+	}
+
+	if (params->link_sta_params.supported_rates_len) {
+		tlv = (struct mwifiex_ie_types_data *)pos;
+		tlv->header.type = cpu_to_le16(WLAN_EID_SUPP_RATES);
+		tlv_len = params->link_sta_params.supported_rates_len;
+		tlv->header.len = cpu_to_le16(tlv_len);
+		memcpy(tlv->data,
+		       params->link_sta_params.supported_rates, tlv_len);
+		pos += (header_len + tlv_len);
+		size += (header_len + tlv_len);
+	}
+
+	if (params->uapsd_queues || params->max_sp) {
+		tlv = (struct mwifiex_ie_types_data *)pos;
+		tlv->header.type = cpu_to_le16(WLAN_EID_QOS_CAPA);
+		tlv_len = sizeof(qos_capa);
+		tlv->header.len = cpu_to_le16(tlv_len);
+		qos_capa = params->uapsd_queues | (params->max_sp << 5);
+		memcpy(tlv->data, &qos_capa, tlv_len);
+		pos += (header_len + tlv_len);
+		size += (header_len + tlv_len);
+		sta_ptr->is_wmm_enabled = 1;
+	}
+
+	if (params->link_sta_params.ht_capa) {
+		tlv = (struct mwifiex_ie_types_data *)pos;
+		tlv->header.type = cpu_to_le16(WLAN_EID_HT_CAPABILITY);
+		tlv_len = sizeof(struct ieee80211_ht_cap);
+		tlv->header.len = cpu_to_le16(tlv_len);
+		memcpy(tlv->data, params->link_sta_params.ht_capa, tlv_len);
+		pos += (header_len + tlv_len);
+		size += (header_len + tlv_len);
+		sta_ptr->is_11n_enabled = 1;
+		sta_ptr->max_amsdu =
+			le16_to_cpu(params->link_sta_params.ht_capa->cap_info) &
+			IEEE80211_HT_CAP_MAX_AMSDU ?
+			MWIFIEX_TX_DATA_BUF_SIZE_8K :
+			MWIFIEX_TX_DATA_BUF_SIZE_4K;
+	}
+
+	if (params->link_sta_params.vht_capa) {
+		tlv = (struct mwifiex_ie_types_data *)pos;
+		tlv->header.type = cpu_to_le16(WLAN_EID_VHT_CAPABILITY);
+		tlv_len = sizeof(struct ieee80211_vht_cap);
+		tlv->header.len = cpu_to_le16(tlv_len);
+		memcpy(tlv->data, params->link_sta_params.vht_capa, tlv_len);
+		pos += (header_len + tlv_len);
+		size += (header_len + tlv_len);
+		sta_ptr->is_11ac_enabled = 1;
+	}
+
+	if (params->link_sta_params.opmode_notif_used) {
+		tlv = (struct mwifiex_ie_types_data *)pos;
+		tlv->header.type = cpu_to_le16(WLAN_EID_OPMODE_NOTIF);
+		tlv_len = sizeof(u8);
+		tlv->header.len = cpu_to_le16(tlv_len);
+		memcpy(tlv->data, &params->link_sta_params.opmode_notif,
+		       tlv_len);
+		pos += (header_len + tlv_len);
+		size += (header_len + tlv_len);
+	}
+
+	for (i = 0; i < MAX_NUM_TID; i++) {
+		if (sta_ptr->is_11n_enabled)
+			sta_ptr->ampdu_sta[i] =
+				      priv->aggr_prio_tbl[i].ampdu_user;
+		else
+			sta_ptr->ampdu_sta[i] = BA_STREAM_NOT_ALLOWED;
+	}
+
+	memset(sta_ptr->rx_seq, 0xff, sizeof(sta_ptr->rx_seq));
+	cmd->size = cpu_to_le16(size);
+
+	return 0;
+}
+
 /* This function prepares the AP specific commands before sending them
  * to the firmware.
  * This is a generic function which calls specific command preparation
@@ -785,6 +949,8 @@ int mwifiex_uap_prepare_cmd(struct mwifiex_private *priv, u16 cmd_no,
 			return -1;
 		break;
 	case HostCmd_CMD_UAP_BSS_START:
+		mwifiex_cmd_uap_bss_start(priv, cmd);
+		break;
 	case HostCmd_CMD_UAP_BSS_STOP:
 	case HOST_CMD_APCMD_SYS_RESET:
 	case HOST_CMD_APCMD_STA_LIST:
@@ -800,6 +966,11 @@ int mwifiex_uap_prepare_cmd(struct mwifiex_private *priv, u16 cmd_no,
 							  data_buf))
 			return -1;
 		break;
+	case HostCmd_CMD_ADD_NEW_STATION:
+		if (mwifiex_cmd_uap_add_station(priv, cmd, cmd_action,
+						data_buf))
+			return -1;
+		break;
 	default:
 		mwifiex_dbg(priv->adapter, ERROR,
 			    "PREP_CMD: unknown cmd %#x\n", cmd_no);
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 3817c08a1507..42c04bf858da 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -497,6 +497,30 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
 		cfg80211_rx_mlme_mgmt(priv->netdev, skb->data, pkt_len);
 	}
 
+	if (priv->adapter->host_mlme_enabled &&
+	    (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP)) {
+		if (ieee80211_is_auth(ieee_hdr->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "auth: receive auth from %pM\n",
+				    ieee_hdr->addr2);
+		if (ieee80211_is_deauth(ieee_hdr->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "auth: receive deauth from %pM\n",
+				    ieee_hdr->addr2);
+		if (ieee80211_is_disassoc(ieee_hdr->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "assoc: receive disassoc from %pM\n",
+				    ieee_hdr->addr2);
+		if (ieee80211_is_assoc_req(ieee_hdr->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "assoc: receive assoc req from %pM\n",
+				    ieee_hdr->addr2);
+		if (ieee80211_is_reassoc_req(ieee_hdr->frame_control))
+			mwifiex_dbg(priv->adapter, MSG,
+				    "assoc: receive reassoc req from %pM\n",
+				    ieee_hdr->addr2);
+	}
+
 	cfg80211_rx_mgmt(&priv->wdev, priv->roc_cfg.chan.center_freq,
 			 CAL_RSSI(rx_pd->snr, rx_pd->nf), skb->data, pkt_len,
 			 0);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-04-18  6:06 ` [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode David Lin
@ 2024-04-18  6:37   ` Francesco Dolcini
  2024-04-18  8:24     ` [EXT] " David Lin
  2024-05-23  0:53   ` Brian Norris
  1 sibling, 1 reply; 39+ messages in thread
From: Francesco Dolcini @ 2024-04-18  6:37 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless, linux-kernel, briannorris, kvalo, francesco,
	tsung-hsien.hsieh, Francesco Dolcini

On Thu, Apr 18, 2024 at 02:06:25PM +0800, David Lin wrote:
> Add host based MLME to enable WPA3 functionalities in client mode.
> This feature required a firmware with the corresponding V2 Key API
> support. The feature (WPA3) is currently enabled and verified only
> on IW416. Also, verified no regression with change when host MLME
> is disabled.
> 
> Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>

...

> diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
> index 745b1d925b21..3817c08a1507 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -417,6 +456,47 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,

...

> +				mwifiex_dbg
> +				(priv->adapter, MSG,
> +				 "assoc: receive disassoc from %pM\n",
> +				 ieee_hdr->addr3);

The way you indented this does not seems kernel coding style compliant ...
however checkpatch does not complain ... so maybe I am just wrong.

In case you need to send a new version, please keep the open parenthesis together with the function name

				mwifiex_dbg(priv->adapter, MSG,
					    "assoc: receive disassoc from %pM\n",
					    ieee_hdr->addr3);

(yes, it's 81 column - and it's fine).

Again, IMHO, do not send a v11 just for this trivial change.

Francesco



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-18  6:06 [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme David Lin
  2024-04-18  6:06 ` [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode David Lin
  2024-04-18  6:06 ` [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
@ 2024-04-18  6:47 ` Marcel Holtmann
  2024-04-18  9:08   ` [EXT] " David Lin
  2 siblings, 1 reply; 39+ messages in thread
From: Marcel Holtmann @ 2024-04-18  6:47 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless, LKML, briannorris, Kalle Valo, francesco,
	tsung-hsien.hsieh, rafael.beims, Francesco Dolcini

Hi David,

> With host mlme:
> Tested-by: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD
> Without host mlme:
> Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # 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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-04-18  6:37   ` Francesco Dolcini
@ 2024-04-18  8:24     ` David Lin
  0 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-04-18  8:24 UTC (permalink / raw
  To: Francesco Dolcini
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	briannorris@chromium.org, kvalo@kernel.org, Pete Hsieh,
	Francesco Dolcini

> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Thursday, April 18, 2024 2:38 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.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 1/2] wifi: mwifiex: add host mlme for client
> 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
> 
> 
> On Thu, Apr 18, 2024 at 02:06:25PM +0800, David Lin wrote:
> > Add host based MLME to enable WPA3 functionalities in client mode.
> > This feature required a firmware with the corresponding V2 Key API
> > support. The feature (WPA3) is currently enabled and verified only on
> > IW416. Also, verified no regression with change when host MLME is
> > disabled.
> >
> > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> ...
> 
> > diff --git a/drivers/net/wireless/marvell/mwifiex/util.c
> > b/drivers/net/wireless/marvell/mwifiex/util.c
> > index 745b1d925b21..3817c08a1507 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/util.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> > @@ -417,6 +456,47 @@ mwifiex_process_mgmt_packet(struct
> > mwifiex_private *priv,
> 
> ...
> 
> > +                             mwifiex_dbg
> > +                             (priv->adapter, MSG,
> > +                              "assoc: receive disassoc from %pM\n",
> > +                              ieee_hdr->addr3);
> 
> The way you indented this does not seems kernel coding style compliant ...
> however checkpatch does not complain ... so maybe I am just wrong.
> 
> In case you need to send a new version, please keep the open parenthesis
> together with the function name
> 
>                                 mwifiex_dbg(priv->adapter, MSG,
>                                             "assoc: receive disassoc
> from %pM\n",
>                                             ieee_hdr->addr3);
> 
> (yes, it's 81 column - and it's fine).
> 
> Again, IMHO, do not send a v11 just for this trivial change.
> 
> Francesco
>

Because checkpatch.pl doesn't complain about this kind of coding, I use this way to let code less and equal 80.
I will correct it if new version of code is needed.

David
 


^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-18  6:47 ` [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme Marcel Holtmann
@ 2024-04-18  9:08   ` David Lin
  2024-04-18  9:15     ` [EXT] " Marcel Holtmann
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-04-18  9:08 UTC (permalink / raw
  To: Marcel Holtmann
  Cc: linux-wireless@vger.kernel.org, LKML, briannorris@chromium.org,
	Kalle Valo, francesco@dolcini.it, Pete Hsieh, rafael.beims,
	Francesco Dolcini

> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Thursday, April 18, 2024 2:48 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>;
> briannorris@chromium.org; Kalle Valo <kvalo@kernel.org>;
> francesco@dolcini.it; Pete Hsieh <tsung-hsien.hsieh@nxp.com>; rafael.beims
> <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: [EXT] Re: [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> Hi David,
> 
> > With host mlme:
> > Tested-by: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD Without
> > host mlme:
> > Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> #
> > 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 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-18  9:08   ` [EXT] " David Lin
@ 2024-04-18  9:15     ` Marcel Holtmann
  2024-04-19  4:00       ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Marcel Holtmann @ 2024-04-18  9:15 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless@vger.kernel.org, LKML, briannorris@chromium.org,
	Kalle Valo, francesco@dolcini.it, Pete Hsieh, rafael.beims,
	Francesco Dolcini

Hi David,

>>> With host mlme:
>>> Tested-by: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD Without
>>> host mlme:
>>> Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> #
>>> 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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-18  9:15     ` [EXT] " Marcel Holtmann
@ 2024-04-19  4:00       ` David Lin
  2024-04-19 21:42         ` Brian Norris
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-04-19  4:00 UTC (permalink / raw
  To: Marcel Holtmann
  Cc: linux-wireless@vger.kernel.org, LKML, briannorris@chromium.org,
	Kalle Valo, francesco@dolcini.it, Pete Hsieh, rafael.beims,
	Francesco Dolcini

> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Thursday, April 18, 2024 5:15 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>;
> briannorris@chromium.org; Kalle Valo <kvalo@kernel.org>;
> francesco@dolcini.it; Pete Hsieh <tsung-hsien.hsieh@nxp.com>; rafael.beims
> <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> 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: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD
> Without
> >>> host mlme:
> >>> Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> #
> >>> 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-19  4:00       ` David Lin
@ 2024-04-19 21:42         ` Brian Norris
  2024-04-22  8:34           ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-04-19 21:42 UTC (permalink / raw
  To: David Lin
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-19 21:42         ` Brian Norris
@ 2024-04-22  8:34           ` David Lin
  2024-04-23  2:29             ` David Lin
  2024-04-25  2:09             ` David Lin
  0 siblings, 2 replies; 39+ messages in thread
From: David Lin @ 2024-04-22  8:34 UTC (permalink / raw
  To: Brian Norris
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

> From: Brian Norris <briannorris@chromium.org>
> Sent: Saturday, April 20, 2024 5:42 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: Marcel Holtmann <marcel@holtmann.org>;
> linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>; Kalle
> Valo <kvalo@kernel.org>; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-22  8:34           ` David Lin
@ 2024-04-23  2:29             ` David Lin
  2024-04-24  8:11               ` Marcel Holtmann
  2024-04-26  1:00               ` Brian Norris
  2024-04-25  2:09             ` David Lin
  1 sibling, 2 replies; 39+ messages in thread
From: David Lin @ 2024-04-23  2:29 UTC (permalink / raw
  To: Brian Norris
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini,
	David Lin

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-23  2:29             ` David Lin
@ 2024-04-24  8:11               ` Marcel Holtmann
  2024-04-25  2:40                 ` David Lin
  2024-04-26  1:00               ` Brian Norris
  1 sibling, 1 reply; 39+ messages in thread
From: Marcel Holtmann @ 2024-04-24  8:11 UTC (permalink / raw
  To: David Lin
  Cc: Brian Norris, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-22  8:34           ` David Lin
  2024-04-23  2:29             ` David Lin
@ 2024-04-25  2:09             ` David Lin
  1 sibling, 0 replies; 39+ messages in thread
From: David Lin @ 2024-04-25  2:09 UTC (permalink / raw
  To: David Lin, Brian Norris
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

> From: David Lin <yu-hao.lin@nxp.com>
> Sent: Monday, April 22, 2024 4:35 PM
> To: Brian Norris <briannorris@chromium.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>; linux-wireless@vger.kernel.org;
> LKML <linux-kernel@vger.kernel.org>; Kalle Valo <kvalo@kernel.org>;
> francesco@dolcini.it; Pete Hsieh <tsung-hsien.hsieh@nxp.com>; rafael.beims
> <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme 
> 
> > From: Brian Norris <briannorris@chromium.org>
> > Sent: Saturday, April 20, 2024 5:42 AM
> > To: David Lin <yu-hao.lin@nxp.com>
> > Cc: Marcel Holtmann <marcel@holtmann.org>;
> > linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>;
> > Kalle Valo <kvalo@kernel.org>; francesco@dolcini.it; Pete Hsieh
> > <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> > Francesco Dolcini <francesco.dolcini@toradex.com>
> > 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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-24  8:11               ` Marcel Holtmann
@ 2024-04-25  2:40                 ` David Lin
  2024-04-26  1:08                   ` Brian Norris
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-04-25  2:40 UTC (permalink / raw
  To: Marcel Holtmann
  Cc: Brian Norris, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Wednesday, April 24, 2024 4:11 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: Brian Norris <briannorris@chromium.org>; linux-wireless@vger.kernel.org;
> LKML <linux-kernel@vger.kernel.org>; Kalle Valo <kvalo@kernel.org>;
> francesco@dolcini.it; Pete Hsieh <tsung-hsien.hsieh@nxp.com>; rafael.beims
> <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> 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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-23  2:29             ` David Lin
  2024-04-24  8:11               ` Marcel Holtmann
@ 2024-04-26  1:00               ` Brian Norris
  2024-04-26  3:04                 ` David Lin
  1 sibling, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-04-26  1:00 UTC (permalink / raw
  To: David Lin
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

Hi David,

On Mon, Apr 22, 2024 at 7:29 PM David Lin <yu-hao.lin@nxp.com> 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-25  2:40                 ` David Lin
@ 2024-04-26  1:08                   ` Brian Norris
  2024-05-02  8:34                     ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-04-26  1:08 UTC (permalink / raw
  To: David Lin
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

Hi David,

On Wed, Apr 24, 2024 at 7:40 PM David Lin <yu-hao.lin@nxp.com> wrote:
> > From: Marcel Holtmann <marcel@holtmann.org>
> >
> > 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/7a08dbcaded25ec0d32865647d571afbd66062fe.camel@sipsolutions.net/

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-26  1:00               ` Brian Norris
@ 2024-04-26  3:04                 ` David Lin
  0 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-04-26  3:04 UTC (permalink / raw
  To: Brian Norris
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

> From: Brian Norris <briannorris@chromium.org>
> Sent: Friday, April 26, 2024 9:00 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: Marcel Holtmann <marcel@holtmann.org>; linux-wireless@vger.kernel.org;
> LKML <linux-kernel@vger.kernel.org>; Kalle Valo <kvalo@kernel.org>;
> francesco@dolcini.it; Pete Hsieh <tsung-hsien.hsieh@nxp.com>; rafael.beims
> <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> 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 <yu-hao.lin@nxp.com> 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-04-26  1:08                   ` Brian Norris
@ 2024-05-02  8:34                     ` David Lin
  2024-05-13  1:27                       ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-05-02  8:34 UTC (permalink / raw
  To: Brian Norris
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

> From: Brian Norris <briannorris@chromium.org>
> Sent: Friday, April 26, 2024 9:09 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: Marcel Holtmann <marcel@holtmann.org>;
> linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>; Kalle
> Valo <kvalo@kernel.org>; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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 <yu-hao.lin@nxp.com> wrote:
> > > From: Marcel Holtmann <marcel@holtmann.org>
> > >
> > > 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-05-02  8:34                     ` David Lin
@ 2024-05-13  1:27                       ` David Lin
  2024-05-23  0:50                         ` Brian Norris
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-05-13  1:27 UTC (permalink / raw
  To: David Lin, Brian Norris
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

Hi Brian,

> From: David Lin <yu-hao.lin@nxp.com>
> Sent: Thursday, May 2, 2024 4:35 PM
> To: Brian Norris <briannorris@chromium.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>;
> linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>; Kalle
> Valo <kvalo@kernel.org>; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> Subject: RE: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host
> mlme
> 
> > From: Brian Norris <briannorris@chromium.org>
> > Sent: Friday, April 26, 2024 9:09 AM
> > To: David Lin <yu-hao.lin@nxp.com>
> > Cc: Marcel Holtmann <marcel@holtmann.org>;
> > linux-wireless@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>;
> > Kalle Valo <kvalo@kernel.org>; francesco@dolcini.it; Pete Hsieh
> > <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> > Francesco Dolcini <francesco.dolcini@toradex.com>
> > 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 <yu-hao.lin@nxp.com> wrote:
> > > > From: Marcel Holtmann <marcel@holtmann.org>
> > > >
> > > > 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme
  2024-05-13  1:27                       ` David Lin
@ 2024-05-23  0:50                         ` Brian Norris
  0 siblings, 0 replies; 39+ messages in thread
From: Brian Norris @ 2024-05-23  0:50 UTC (permalink / raw
  To: David Lin
  Cc: Marcel Holtmann, linux-wireless@vger.kernel.org, LKML, Kalle Valo,
	francesco@dolcini.it, Pete Hsieh, rafael.beims, Francesco Dolcini

On Mon, May 13, 2024 at 01:27:13AM +0000, David Lin wrote:
> > From: David Lin <yu-hao.lin@nxp.com>
> > 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-04-18  6:06 ` [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode David Lin
  2024-04-18  6:37   ` Francesco Dolcini
@ 2024-05-23  0:53   ` Brian Norris
  2024-05-24  9:46     ` [EXT] " David Lin
  1 sibling, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-05-23  0:53 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless, linux-kernel, kvalo, francesco, tsung-hsien.hsieh,
	Francesco Dolcini

On Thu, Apr 18, 2024 at 02:06:25PM +0800, David Lin wrote:
> +static int
> +mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
> +			      struct net_device *dev, const u8 *peer,
> +			      u64 *cookie)
> +{
> +	return -EOPNOTSUPP;
> +}
> +

> +		mwifiex_cfg80211_ops.probe_client =
> +			mwifiex_cfg80211_probe_client;

For the record, I feel like this question was not adequately handled
from v8. That thread is:

https://lore.kernel.org/all/CA+ASDXM1PEMRyxRpBryJ7G6e7yzG8Ku+g2_qpHN3g5djFpAWkw@mail.gmail.com/
Re: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client mode

Brian

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-04-18  6:06 ` [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
@ 2024-05-23  0:59   ` Brian Norris
  2024-05-23  2:20     ` [EXT] " David Lin
  2024-06-12 13:11   ` Sascha Hauer
  1 sibling, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-05-23  0:59 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless, linux-kernel, kvalo, francesco, tsung-hsien.hsieh,
	Francesco Dolcini

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@chromium.org>

(Feel free to carry that to a v11, since my only remaining substantial
concerns are with patch 1 I think.)

Brian

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-05-23  0:59   ` Brian Norris
@ 2024-05-23  2:20     ` David Lin
  0 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-05-23  2:20 UTC (permalink / raw
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-23  0:53   ` Brian Norris
@ 2024-05-24  9:46     ` David Lin
  2024-05-24 17:02       ` Brian Norris
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-05-24  9:46 UTC (permalink / raw
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

Hi Brian,

> From: Brian Norris <briannorris@chromium.org>
> Sent: Thursday, May 23, 2024 8:54 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 1/2] wifi: mwifiex: add host mlme for client
> mode
>
> On Thu, Apr 18, 2024 at 02:06:25PM +0800, David Lin wrote:
> > +static int
> > +mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
> > +                           struct net_device *dev, const u8 *peer,
> > +                           u64 *cookie) {
> > +     return -EOPNOTSUPP;
> > +}
> > +
>
> > +             mwifiex_cfg80211_ops.probe_client =
> > +                     mwifiex_cfg80211_probe_client;
>
> For the record, I feel like this question was not adequately handled from v8.
> That thread is:
>
> https://lore.kern/
> el.org%2Fall%2FCA%2BASDXM1PEMRyxRpBryJ7G6e7yzG8Ku%2Bg2_qpHN3g5d
> jFpAWkw%40mail.gmail.com%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%
> 7C0b65f7e4a5fc46c8bdbc08dc7ac2c9ff%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C638520224227876720%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C0%7C%7C%7C&sdata=MpqW1U4yTgDcM0g20DRSAxEnHkNNkd2hwsZrVAxg8p
> w%3D&reserved=0
> Re: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client mode
>
> Brian

The difference with and without hooking probe_client() is that "poll_command_supported" of hostapd will be set or not.
If "poll_command_supported" is not set (won't hook probe_client), it will let hostapd to set "use_monitor" and client can't
connect to AP.

Maybe I can put following comments:

Hook probe_client to avoid hostapd to set "poll_command_supported" as 0 and set "use_monitor" to 1.

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-24  9:46     ` [EXT] " David Lin
@ 2024-05-24 17:02       ` Brian Norris
  2024-05-24 22:00         ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-05-24 17:02 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

On Fri, May 24, 2024 at 2:46 AM David Lin <yu-hao.lin@nxp.com> wrote:
> > From: Brian Norris <briannorris@chromium.org>
> >
> > On Thu, Apr 18, 2024 at 02:06:25PM +0800, David Lin wrote:
> > > +static int
> > > +mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
> > > +                           struct net_device *dev, const u8 *peer,
> > > +                           u64 *cookie) {
> > > +     return -EOPNOTSUPP;
> > > +}
> > > +
> >
> > > +             mwifiex_cfg80211_ops.probe_client =
> > > +                     mwifiex_cfg80211_probe_client;
> >
> > For the record, I feel like this question was not adequately handled from v8.
> > That thread is:
> >
> > https://lore.kern/
> > el.org%2Fall%2FCA%2BASDXM1PEMRyxRpBryJ7G6e7yzG8Ku%2Bg2_qpHN3g5d
> > jFpAWkw%40mail.gmail.com%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%
> > 7C0b65f7e4a5fc46c8bdbc08dc7ac2c9ff%7C686ea1d3bc2b4c6fa92cd99c5c301
> > 635%7C0%7C0%7C638520224227876720%7CUnknown%7CTWFpbGZsb3d8eyJ
> > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > C0%7C%7C%7C&sdata=MpqW1U4yTgDcM0g20DRSAxEnHkNNkd2hwsZrVAxg8p
> > w%3D&reserved=0
> > Re: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client mode
>
> The difference with and without hooking probe_client() is that "poll_command_supported" of hostapd will be set or not.
> If "poll_command_supported" is not set (won't hook probe_client), it will let hostapd to set "use_monitor" and client can't
> connect to AP.

Yes, I already said that in the above reply.

If you read my v8 reply, my suggestion was that you need to fix
hostapd, rather than advertise lies in the kernel. You don't support
probe_client, so you shouldn't advertise it.

I think you should dig into the reasoning from this commit to figure
out what to do:
https://w1.fi/cgit/hostap/commit/?id=a11241fa114923b47892ad3279966839e9c2741d

Personally, I'm not sure what hostapd is doing with
NL80211_CMD_PROBE_CLIENT ... but you're the one submitting the code,
not me.

> Maybe I can put following comments:
>
> Hook probe_client to avoid hostapd to set "poll_command_supported" as 0 and set "use_monitor" to 1.

If we really can't fix hostapd, I'd avoid using such literal
descriptions of implementation details like variable names. Maybe
better:

"hostapd looks for NL80211_CMD_PROBE_CLIENT support; otherwise, it
requires monitor-mode support (which mwifiex doesn't support). Provide
fake probe_client support to work around this."

But again, please actually explore the reason hostapd is doing this
first, and see if you can fix it.

Brian

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-24 17:02       ` Brian Norris
@ 2024-05-24 22:00         ` David Lin
  2024-05-24 22:54           ` Brian Norris
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-05-24 22:00 UTC (permalink / raw
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

> From: Brian Norris <briannorris@chromium.org>
> Sent: Saturday, May 25, 2024 1:02 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: Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
> mode
> 
> On Fri, May 24, 2024 at 2:46 AM David Lin <yu-hao.lin@nxp.com> wrote:
> > > From: Brian Norris <briannorris@chromium.org>
> > >
> > > On Thu, Apr 18, 2024 at 02:06:25PM +0800, David Lin wrote:
> > > > +static int
> > > > +mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
> > > > +                           struct net_device *dev, const u8
> *peer,
> > > > +                           u64 *cookie) {
> > > > +     return -EOPNOTSUPP;
> > > > +}
> > > > +
> > >
> > > > +             mwifiex_cfg80211_ops.probe_client =
> > > > +                     mwifiex_cfg80211_probe_client;
> > >
> > > For the record, I feel like this question was not adequately handled from
> v8.
> > > That thread is:
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > >
> re.kern%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C2816a3135edd445
> d3a4
> > >
> 508dc7c13485e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6385
> 21669
> > >
> 455363246%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luMz
> > >
> IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=SsB%2FD1dUX
> %2FZD
> > > jU4MWursrFn3WRfSM6f3h%2Fnd0FZMYm0%3D&reserved=0
> > >
> el.org%2Fall%2FCA%2BASDXM1PEMRyxRpBryJ7G6e7yzG8Ku%2Bg2_qpHN3g5
> d
> > >
> jFpAWkw%40mail.gmail.com%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com
> %
> > >
> 7C0b65f7e4a5fc46c8bdbc08dc7ac2c9ff%7C686ea1d3bc2b4c6fa92cd99c5c301
> > >
> 635%7C0%7C0%7C638520224227876720%7CUnknown%7CTWFpbGZsb3d8eyJ
> > >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > >
> C0%7C%7C%7C&sdata=MpqW1U4yTgDcM0g20DRSAxEnHkNNkd2hwsZrVAxg8
> p
> > > w%3D&reserved=0
> > > Re: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client
> > > mode
> >
> > The difference with and without hooking probe_client() is that
> "poll_command_supported" of hostapd will be set or not.
> > If "poll_command_supported" is not set (won't hook probe_client), it
> > will let hostapd to set "use_monitor" and client can't connect to AP.
> 
> Yes, I already said that in the above reply.
> 
> If you read my v8 reply, my suggestion was that you need to fix hostapd,
> rather than advertise lies in the kernel. You don't support probe_client, so
> you shouldn't advertise it.
> 
> I think you should dig into the reasoning from this commit to figure out
> what to do:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw1.fi%
> 2Fcgit%2Fhostap%2Fcommit%2F%3Fid%3Da11241fa114923b47892ad3279966
> 839e9c2741d&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C2816a3135edd4
> 45d3a4508dc7c13485e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638521669455374712%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&
> sdata=QXFrzLUM2CgSgWR0COo1vrmi5VlhpCzGCmEbxlyySS0%3D&reserved=0
> 
> Personally, I'm not sure what hostapd is doing with
> NL80211_CMD_PROBE_CLIENT ... but you're the one submitting the code,
> not me.
> 
> > Maybe I can put following comments:
> >
> > Hook probe_client to avoid hostapd to set "poll_command_supported" as
> 0 and set "use_monitor" to 1.
> 
> If we really can't fix hostapd, I'd avoid using such literal descriptions of
> implementation details like variable names. Maybe
> better:
> 
> "hostapd looks for NL80211_CMD_PROBE_CLIENT support; otherwise, it
> requires monitor-mode support (which mwifiex doesn't support). Provide
> fake probe_client support to work around this."
> 
> But again, please actually explore the reason hostapd is doing this first, and
> see if you can fix it.
> 
> Brian

I think it needs time to support probe client. Can we put your suggested comments to the code
used to hook probe_client() and add

"TODO: support probe client" to mwifiex_cfg80211_probe_client().

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-24 22:00         ` David Lin
@ 2024-05-24 22:54           ` Brian Norris
  2024-05-25  0:50             ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-05-24 22:54 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@nxp.com> wrote:
> I think it needs time to support probe client. Can we put your suggested comments to the code
> used to hook probe_client() and add
>
> "TODO: support probe client" to mwifiex_cfg80211_probe_client().

Are you suggesting that you plan to actually implement proper
probe_client support? Did you already do what I suggested, and
understand why hostapd needs probe_client support? This seems to be a
common pattern -- that reviewers are asking for you to do your
research, and it takes several requests before you actually do it.

Now that I've tried to do that research for you ... it looks like
hostapd uses probe_client to augment TX MGMT acks, as a proxy for
station presence / inactivity. If a station is inactive and
non-responsive, we disconnect it eventually. So that looks to me like
probe_client support should actually be optional, if your driver
reports TX status? And in that case, I'd still recommend you try to
fix hostapd.

But if you're really planning to implement proper probe_client
support, then I suppose the TODO approach is also OK.

I'd also request that you please actually do your research when
reviewers ask questions. I'm frankly not sure why I'm spending my time
on the above research, when the onus should be on the submitter to
explain why they're doing what they're doing.

Brian

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-24 22:54           ` Brian Norris
@ 2024-05-25  0:50             ` David Lin
  2024-05-25 11:24               ` David Lin
                                 ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: David Lin @ 2024-05-25  0:50 UTC (permalink / raw
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

Hi Brian,

> From: Brian Norris <briannorris@chromium.org>
> Sent: Saturday, May 25, 2024 6:55 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: Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
> 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
> 
> 
> On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@nxp.com> wrote:
> > I think it needs time to support probe client. Can we put your
> > suggested comments to the code used to hook probe_client() and add
> >
> > "TODO: support probe client" to mwifiex_cfg80211_probe_client().
> 
> Are you suggesting that you plan to actually implement proper probe_client
> support? Did you already do what I suggested, and understand why hostapd
> needs probe_client support? This seems to be a common pattern -- that
> reviewers are asking for you to do your research, and it takes several
> requests before you actually do it.
> 
> Now that I've tried to do that research for you ... it looks like hostapd uses
> probe_client to augment TX MGMT acks, as a proxy for station presence /
> inactivity. If a station is inactive and non-responsive, we disconnect it
> eventually. So that looks to me like probe_client support should actually be
> optional, if your driver reports TX status? And in that case, I'd still
> recommend you try to fix hostapd.
> 
> But if you're really planning to implement proper probe_client support, then
> I suppose the TODO approach is also OK.
> 
> I'd also request that you please actually do your research when reviewers
> ask questions. I'm frankly not sure why I'm spending my time on the above
> research, when the onus should be on the submitter to explain why they're
> doing what they're doing.
> 
> Brian

Yes. I know when aging time of station is out, hostapd will use probe_client to check if station is still there before really disconnect it.

Without this feature, it won't really affect mayor function of hostapd.

That is the reason that I suggest that we put comments and TODO to the code.

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-25  0:50             ` David Lin
@ 2024-05-25 11:24               ` David Lin
  2024-06-14  2:18               ` David Lin
  2024-06-20 17:53               ` Brian Norris
  2 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-05-25 11:24 UTC (permalink / raw
  To: David Lin, Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

> From: David Lin <yu-hao.lin@nxp.com>
> Sent: Saturday, May 25, 2024 8:51 AM
> To: Brian Norris <briannorris@chromium.org>
> 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: RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
> mode
> 
> Hi Brian,
> 
> > From: Brian Norris <briannorris@chromium.org>
> > Sent: Saturday, May 25, 2024 6:55 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: Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme
> > for client 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
> >
> >
> > On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@nxp.com> wrote:
> > > I think it needs time to support probe client. Can we put your
> > > suggested comments to the code used to hook probe_client() and add
> > >
> > > "TODO: support probe client" to mwifiex_cfg80211_probe_client().
> >
> > Are you suggesting that you plan to actually implement proper
> > probe_client support? Did you already do what I suggested, and
> > understand why hostapd needs probe_client support? This seems to be a
> > common pattern -- that reviewers are asking for you to do your
> > research, and it takes several requests before you actually do it.
> >
> > Now that I've tried to do that research for you ... it looks like
> > hostapd uses probe_client to augment TX MGMT acks, as a proxy for
> > station presence / inactivity. If a station is inactive and
> > non-responsive, we disconnect it eventually. So that looks to me like
> > probe_client support should actually be optional, if your driver
> > reports TX status? And in that case, I'd still recommend you try to fix
> hostapd.
> >
> > But if you're really planning to implement proper probe_client
> > support, then I suppose the TODO approach is also OK.
> >
> > I'd also request that you please actually do your research when
> > reviewers ask questions. I'm frankly not sure why I'm spending my time
> > on the above research, when the onus should be on the submitter to
> > explain why they're doing what they're doing.
> >
> > Brian
> 
> Yes. I know when aging time of station is out, hostapd will use probe_client
> to check if station is still there before really disconnect it.
> 
> Without this feature, it won't really affect mayor function of hostapd.
> 
> That is the reason that I suggest that we put comments and TODO to the
> code.
> 
> David

If you agree that this extra check can be optional, maybe I can just put your suggested comments to the code.

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-04-18  6:06 ` [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
  2024-05-23  0:59   ` Brian Norris
@ 2024-06-12 13:11   ` Sascha Hauer
  2024-06-14  2:06     ` [EXT] " David Lin
  1 sibling, 1 reply; 39+ messages in thread
From: Sascha Hauer @ 2024-06-12 13:11 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless, linux-kernel, briannorris, kvalo, francesco,
	tsung-hsien.hsieh, Francesco Dolcini

Hi David,

On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> Add host based MLME to enable WPA3 functionalities in AP mode.
> This feature required a firmware with the corresponding V2 Key API
> support. The feature (WPA3) is currently enabled and verified only
> on IW416. Also, verified no regression with change when host MLME
> is disabled.
> 
> Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> 

> diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> index 491e36611909..073c665183b3 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> @@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
>  				bss_config->key_mgmt = KEY_MGMT_PSK;
>  			}
>  			break;
> +		case WLAN_AKM_SUITE_SAE:
> +			bss_config->protocol = PROTOCOL_WPA2;
> +			bss_config->key_mgmt = KEY_MGMT_SAE;
> +			break;

Shouldn't this be |= PROTOCOL_WPA2 and |= KEY_MGMT_SAE?
Clearing the other flags when SAE is enabled looks wrong to me.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-06-12 13:11   ` Sascha Hauer
@ 2024-06-14  2:06     ` David Lin
  2024-06-14  6:31       ` Sascha Hauer
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-06-14  2:06 UTC (permalink / raw
  To: Sascha Hauer
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	briannorris@chromium.org, kvalo@kernel.org, francesco@dolcini.it,
	Pete Hsieh, Francesco Dolcini

> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Wednesday, June 12, 2024 9:12 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.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 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 David,
> 
> On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> > Add host based MLME to enable WPA3 functionalities in AP mode.
> > This feature required a firmware with the corresponding V2 Key API
> > support. The feature (WPA3) is currently enabled and verified only on
> > IW416. Also, verified no regression with change when host MLME is
> > disabled.
> >
> > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >
> 
> > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > index 491e36611909..073c665183b3 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > @@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct
> mwifiex_private *priv,
> >                               bss_config->key_mgmt =
> KEY_MGMT_PSK;
> >                       }
> >                       break;
> > +             case WLAN_AKM_SUITE_SAE:
> > +                     bss_config->protocol = PROTOCOL_WPA2;
> > +                     bss_config->key_mgmt = KEY_MGMT_SAE;
> > +                     break;
> 
> Shouldn't this be |= PROTOCOL_WPA2 and |= KEY_MGMT_SAE?
> Clearing the other flags when SAE is enabled looks wrong to me.
> 
> Sascha
> 

These fields are used for the configuration of FW, this is the correct setting.

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-25  0:50             ` David Lin
  2024-05-25 11:24               ` David Lin
@ 2024-06-14  2:18               ` David Lin
  2024-06-20 17:53               ` Brian Norris
  2 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-06-14  2:18 UTC (permalink / raw
  To: David Lin, Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

Hi Brian,

> From: David Lin <yu-hao.lin@nxp.com>
> Sent: Saturday, May 25, 2024 8:51 AM
> To: Brian Norris <briannorris@chromium.org>
> 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: RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
> mode
> 
> Hi Brian,
> 
> > From: Brian Norris <briannorris@chromium.org>
> > Sent: Saturday, May 25, 2024 6:55 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: Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme
> > for client 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
> >
> >
> > On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@nxp.com> wrote:
> > > I think it needs time to support probe client. Can we put your
> > > suggested comments to the code used to hook probe_client() and add
> > >
> > > "TODO: support probe client" to mwifiex_cfg80211_probe_client().
> >
> > Are you suggesting that you plan to actually implement proper
> > probe_client support? Did you already do what I suggested, and
> > understand why hostapd needs probe_client support? This seems to be a
> > common pattern -- that reviewers are asking for you to do your
> > research, and it takes several requests before you actually do it.
> >
> > Now that I've tried to do that research for you ... it looks like
> > hostapd uses probe_client to augment TX MGMT acks, as a proxy for
> > station presence / inactivity. If a station is inactive and
> > non-responsive, we disconnect it eventually. So that looks to me like
> > probe_client support should actually be optional, if your driver
> > reports TX status? And in that case, I'd still recommend you try to fix hostapd.
> >
> > But if you're really planning to implement proper probe_client
> > support, then I suppose the TODO approach is also OK.
> >
> > I'd also request that you please actually do your research when
> > reviewers ask questions. I'm frankly not sure why I'm spending my time
> > on the above research, when the onus should be on the submitter to
> > explain why they're doing what they're doing.
> >
> > Brian
> 
> Yes. I know when aging time of station is out, hostapd will use probe_client to
> check if station is still there before really disconnect it.
> 
> Without this feature, it won't really affect mayor function of hostapd.
> 
> That is the reason that I suggest that we put comments and TODO to the code.
> 
> David

> If you agree that this extra check can be optional, maybe I can just put your suggested comments to the code.

> David

Please let me know what else should I do to let this patch be ACKed by you.

Thanks,
David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-06-14  2:06     ` [EXT] " David Lin
@ 2024-06-14  6:31       ` Sascha Hauer
  2024-06-17  2:15         ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Sascha Hauer @ 2024-06-14  6:31 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	briannorris@chromium.org, kvalo@kernel.org, francesco@dolcini.it,
	Pete Hsieh, Francesco Dolcini

On Fri, Jun 14, 2024 at 02:06:45AM +0000, David Lin wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Wednesday, June 12, 2024 9:12 PM
> > To: David Lin <yu-hao.lin@nxp.com>
> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > briannorris@chromium.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 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 David,
> > 
> > On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> > > Add host based MLME to enable WPA3 functionalities in AP mode.
> > > This feature required a firmware with the corresponding V2 Key API
> > > support. The feature (WPA3) is currently enabled and verified only on
> > > IW416. Also, verified no regression with change when host MLME is
> > > disabled.
> > >
> > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > ---
> > >
> > 
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > index 491e36611909..073c665183b3 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > @@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct
> > mwifiex_private *priv,
> > >                               bss_config->key_mgmt =
> > KEY_MGMT_PSK;
> > >                       }
> > >                       break;
> > > +             case WLAN_AKM_SUITE_SAE:
> > > +                     bss_config->protocol = PROTOCOL_WPA2;
> > > +                     bss_config->key_mgmt = KEY_MGMT_SAE;
> > > +                     break;
> > 
> > Shouldn't this be |= PROTOCOL_WPA2 and |= KEY_MGMT_SAE?
> > Clearing the other flags when SAE is enabled looks wrong to me.
> > 
> > Sascha
> > 
> 
> These fields are used for the configuration of FW, this is the correct setting.

This is done in a loop iterating over the different AKM suites, with your
patch this looks like this:

	for (i = 0; i < params->crypto.n_akm_suites; i++) {
		switch (params->crypto.akm_suites[i]) {
		case WLAN_AKM_SUITE_8021X:
			if (params->crypto.wpa_versions &
			    NL80211_WPA_VERSION_1) {
				bss_config->protocol = PROTOCOL_WPA;
				bss_config->key_mgmt = KEY_MGMT_EAP;
			}
			if (params->crypto.wpa_versions &
			    NL80211_WPA_VERSION_2) {
				bss_config->protocol |= PROTOCOL_WPA2;
				bss_config->key_mgmt = KEY_MGMT_EAP;
			}
			break;
		case WLAN_AKM_SUITE_PSK:
			if (params->crypto.wpa_versions &
			    NL80211_WPA_VERSION_1) {
				bss_config->protocol = PROTOCOL_WPA;
				bss_config->key_mgmt = KEY_MGMT_PSK;
			}
			if (params->crypto.wpa_versions &
			    NL80211_WPA_VERSION_2) {
				bss_config->protocol |= PROTOCOL_WPA2;
				bss_config->key_mgmt = KEY_MGMT_PSK;
			}
			break;
		case WLAN_AKM_SUITE_SAE:
			bss_config->protocol = PROTOCOL_WPA2;
			bss_config->key_mgmt = KEY_MGMT_SAE;
			break;

		default:
			break;
		}
	}

It looks wrong to overwrite bss_config->protocol and
bss_config->key_mgmt in each iteration of this loop. If that would be
correct, you wouldn't need a loop at all, but could instead configure
based on the last AKM suite entry.

In my understanding the bits in bss_config->key_mgmt should be ored
together depending on the possible AKM suites which is also what the
downstream driver does.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-06-14  6:31       ` Sascha Hauer
@ 2024-06-17  2:15         ` David Lin
  2024-06-17  6:29           ` Sascha Hauer
  0 siblings, 1 reply; 39+ messages in thread
From: David Lin @ 2024-06-17  2:15 UTC (permalink / raw
  To: Sascha Hauer
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	briannorris@chromium.org, kvalo@kernel.org, francesco@dolcini.it,
	Pete Hsieh, Francesco Dolcini

> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Friday, June 14, 2024 2:32 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; kvalo@kernel.org; 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
>
> 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, Jun 14, 2024 at 02:06:45AM +0000, David Lin wrote:
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > Sent: Wednesday, June 12, 2024 9:12 PM
> > > To: David Lin <yu-hao.lin@nxp.com>
> > > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > briannorris@chromium.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 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 David,
> > >
> > > On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> > > > Add host based MLME to enable WPA3 functionalities in AP mode.
> > > > This feature required a firmware with the corresponding V2 Key API
> > > > support. The feature (WPA3) is currently enabled and verified only
> > > > on IW416. Also, verified no regression with change when host MLME
> > > > is disabled.
> > > >
> > > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > >
> > >
> > > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > index 491e36611909..073c665183b3 100644
> > > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > @@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct
> > > mwifiex_private *priv,
> > > >                               bss_config->key_mgmt =
> > > KEY_MGMT_PSK;
> > > >                       }
> > > >                       break;
> > > > +             case WLAN_AKM_SUITE_SAE:
> > > > +                     bss_config->protocol = PROTOCOL_WPA2;
> > > > +                     bss_config->key_mgmt = KEY_MGMT_SAE;
> > > > +                     break;
> > >
> > > Shouldn't this be |= PROTOCOL_WPA2 and |= KEY_MGMT_SAE?
> > > Clearing the other flags when SAE is enabled looks wrong to me.
> > >
> > > Sascha
> > >
> >
> > These fields are used for the configuration of FW, this is the correct setting.
>
> This is done in a loop iterating over the different AKM suites, with your patch
> this looks like this:
>
>         for (i = 0; i < params->crypto.n_akm_suites; i++) {
>                 switch (params->crypto.akm_suites[i]) {
>                 case WLAN_AKM_SUITE_8021X:
>                         if (params->crypto.wpa_versions &
>                             NL80211_WPA_VERSION_1) {
>                                 bss_config->protocol =
> PROTOCOL_WPA;
>                                 bss_config->key_mgmt =
> KEY_MGMT_EAP;
>                         }
>                         if (params->crypto.wpa_versions &
>                             NL80211_WPA_VERSION_2) {
>                                 bss_config->protocol |=
> PROTOCOL_WPA2;
>                                 bss_config->key_mgmt =
> KEY_MGMT_EAP;
>                         }
>                         break;
>                 case WLAN_AKM_SUITE_PSK:
>                         if (params->crypto.wpa_versions &
>                             NL80211_WPA_VERSION_1) {
>                                 bss_config->protocol =
> PROTOCOL_WPA;
>                                 bss_config->key_mgmt =
> KEY_MGMT_PSK;
>                         }
>                         if (params->crypto.wpa_versions &
>                             NL80211_WPA_VERSION_2) {
>                                 bss_config->protocol |=
> PROTOCOL_WPA2;
>                                 bss_config->key_mgmt =
> KEY_MGMT_PSK;
>                         }
>                         break;
>                 case WLAN_AKM_SUITE_SAE:
>                         bss_config->protocol = PROTOCOL_WPA2;
>                         bss_config->key_mgmt = KEY_MGMT_SAE;
>                         break;
>
>                 default:
>                         break;
>                 }
>         }
>
> It looks wrong to overwrite bss_config->protocol and bss_config->key_mgmt in
> each iteration of this loop. If that would be correct, you wouldn't need a loop
> at all, but could instead configure based on the last AKM suite entry.
>
> In my understanding the bits in bss_config->key_mgmt should be ored together
> depending on the possible AKM suites which is also what the downstream
> driver does.
>
> Sascha
>
> --
> Pengutronix e.K.                           |
> |
> Steuerwalder Str. 21                       |
> http://www.pen/
> gutronix.de%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C7c90883e11414
> 5a307fa08dc8c3ba753%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638539435049441786%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&
> sdata=QSbX5IuEvP18tf0t6sMe8%2F4kuHo7IFEURWabdE4Cxcg%3D&reserved=0
> |
> 31137 Hildesheim, Germany                  | Phone:
> +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

For the configuration of FW, ored only happens for the same AKM suite.

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-06-17  2:15         ` David Lin
@ 2024-06-17  6:29           ` Sascha Hauer
  2024-06-17  7:44             ` David Lin
  0 siblings, 1 reply; 39+ messages in thread
From: Sascha Hauer @ 2024-06-17  6:29 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	briannorris@chromium.org, kvalo@kernel.org, francesco@dolcini.it,
	Pete Hsieh, Francesco Dolcini

On Mon, Jun 17, 2024 at 02:15:41AM +0000, David Lin wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Friday, June 14, 2024 2:32 PM
> > To: David Lin <yu-hao.lin@nxp.com>
> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > briannorris@chromium.org; kvalo@kernel.org; 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
> >
> > 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, Jun 14, 2024 at 02:06:45AM +0000, David Lin wrote:
> > > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Sent: Wednesday, June 12, 2024 9:12 PM
> > > > To: David Lin <yu-hao.lin@nxp.com>
> > > > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > briannorris@chromium.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 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 David,
> > > >
> > > > On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> > > > > Add host based MLME to enable WPA3 functionalities in AP mode.
> > > > > This feature required a firmware with the corresponding V2 Key API
> > > > > support. The feature (WPA3) is currently enabled and verified only
> > > > > on IW416. Also, verified no regression with change when host MLME
> > > > > is disabled.
> > > > >
> > > > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > > > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > > ---
> > > > >
> > > >
> > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > index 491e36611909..073c665183b3 100644
> > > > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > @@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct
> > > > mwifiex_private *priv,
> > > > >                               bss_config->key_mgmt =
> > > > KEY_MGMT_PSK;
> > > > >                       }
> > > > >                       break;
> > > > > +             case WLAN_AKM_SUITE_SAE:
> > > > > +                     bss_config->protocol = PROTOCOL_WPA2;
> > > > > +                     bss_config->key_mgmt = KEY_MGMT_SAE;
> > > > > +                     break;
> > > >
> > > > Shouldn't this be |= PROTOCOL_WPA2 and |= KEY_MGMT_SAE?
> > > > Clearing the other flags when SAE is enabled looks wrong to me.
> > > >
> > > > Sascha
> > > >
> > >
> > > These fields are used for the configuration of FW, this is the correct setting.
> >
> > This is done in a loop iterating over the different AKM suites, with your patch
> > this looks like this:
> >
> >         for (i = 0; i < params->crypto.n_akm_suites; i++) {
> >                 switch (params->crypto.akm_suites[i]) {
> >                 case WLAN_AKM_SUITE_8021X:
> >                         if (params->crypto.wpa_versions &
> >                             NL80211_WPA_VERSION_1) {
> >                                 bss_config->protocol =
> > PROTOCOL_WPA;
> >                                 bss_config->key_mgmt =
> > KEY_MGMT_EAP;
> >                         }
> >                         if (params->crypto.wpa_versions &
> >                             NL80211_WPA_VERSION_2) {
> >                                 bss_config->protocol |=
> > PROTOCOL_WPA2;
> >                                 bss_config->key_mgmt =
> > KEY_MGMT_EAP;
> >                         }
> >                         break;
> >                 case WLAN_AKM_SUITE_PSK:
> >                         if (params->crypto.wpa_versions &
> >                             NL80211_WPA_VERSION_1) {
> >                                 bss_config->protocol =
> > PROTOCOL_WPA;
> >                                 bss_config->key_mgmt =
> > KEY_MGMT_PSK;
> >                         }
> >                         if (params->crypto.wpa_versions &
> >                             NL80211_WPA_VERSION_2) {
> >                                 bss_config->protocol |=
> > PROTOCOL_WPA2;
> >                                 bss_config->key_mgmt =
> > KEY_MGMT_PSK;
> >                         }
> >                         break;
> >                 case WLAN_AKM_SUITE_SAE:
> >                         bss_config->protocol = PROTOCOL_WPA2;
> >                         bss_config->key_mgmt = KEY_MGMT_SAE;
> >                         break;
> >
> >                 default:
> >                         break;
> >                 }
> >         }
> >
> > It looks wrong to overwrite bss_config->protocol and bss_config->key_mgmt in
> > each iteration of this loop. If that would be correct, you wouldn't need a loop
> > at all, but could instead configure based on the last AKM suite entry.
> >
> > In my understanding the bits in bss_config->key_mgmt should be ored together
> > depending on the possible AKM suites which is also what the downstream
> > driver does.
> >
> 
> For the configuration of FW, ored only happens for the same AKM suite.

Sorry, I don't understand this. Could you elaborate what you mean here?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
  2024-06-17  6:29           ` Sascha Hauer
@ 2024-06-17  7:44             ` David Lin
  0 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-06-17  7:44 UTC (permalink / raw
  To: Sascha Hauer
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	briannorris@chromium.org, kvalo@kernel.org, francesco@dolcini.it,
	Pete Hsieh, Francesco Dolcini

> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Monday, June 17, 2024 2:30 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; kvalo@kernel.org; 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
>
> 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 Mon, Jun 17, 2024 at 02:15:41AM +0000, David Lin wrote:
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > Sent: Friday, June 14, 2024 2:32 PM
> > > To: David Lin <yu-hao.lin@nxp.com>
> > > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > briannorris@chromium.org; kvalo@kernel.org; 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
> > >
> > > 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, Jun 14, 2024 at 02:06:45AM +0000, David Lin wrote:
> > > > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > Sent: Wednesday, June 12, 2024 9:12 PM
> > > > > To: David Lin <yu-hao.lin@nxp.com>
> > > > > Cc: linux-wireless@vger.kernel.org;
> > > > > linux-kernel@vger.kernel.org; briannorris@chromium.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 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 David,
> > > > >
> > > > > On Thu, Apr 18, 2024 at 02:06:26PM +0800, David Lin wrote:
> > > > > > Add host based MLME to enable WPA3 functionalities in AP mode.
> > > > > > This feature required a firmware with the corresponding V2 Key
> > > > > > API support. The feature (WPA3) is currently enabled and
> > > > > > verified only on IW416. Also, verified no regression with
> > > > > > change when host MLME is disabled.
> > > > > >
> > > > > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > > > > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > > > ---
> > > > > >
> > > > >
> > > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > > b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > > index 491e36611909..073c665183b3 100644
> > > > > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > > > @@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct
> > > > > mwifiex_private *priv,
> > > > > >                               bss_config->key_mgmt =
> > > > > KEY_MGMT_PSK;
> > > > > >                       }
> > > > > >                       break;
> > > > > > +             case WLAN_AKM_SUITE_SAE:
> > > > > > +                     bss_config->protocol = PROTOCOL_WPA2;
> > > > > > +                     bss_config->key_mgmt =
> KEY_MGMT_SAE;
> > > > > > +                     break;
> > > > >
> > > > > Shouldn't this be |= PROTOCOL_WPA2 and |= KEY_MGMT_SAE?
> > > > > Clearing the other flags when SAE is enabled looks wrong to me.
> > > > >
> > > > > Sascha
> > > > >
> > > >
> > > > These fields are used for the configuration of FW, this is the correct
> setting.
> > >
> > > This is done in a loop iterating over the different AKM suites, with
> > > your patch this looks like this:
> > >
> > >         for (i = 0; i < params->crypto.n_akm_suites; i++) {
> > >                 switch (params->crypto.akm_suites[i]) {
> > >                 case WLAN_AKM_SUITE_8021X:
> > >                         if (params->crypto.wpa_versions &
> > >                             NL80211_WPA_VERSION_1) {
> > >                                 bss_config->protocol =
> PROTOCOL_WPA;
> > >                                 bss_config->key_mgmt =
> KEY_MGMT_EAP;
> > >                         }
> > >                         if (params->crypto.wpa_versions &
> > >                             NL80211_WPA_VERSION_2) {
> > >                                 bss_config->protocol |=
> > > PROTOCOL_WPA2;
> > >                                 bss_config->key_mgmt =
> KEY_MGMT_EAP;
> > >                         }
> > >                         break;
> > >                 case WLAN_AKM_SUITE_PSK:
> > >                         if (params->crypto.wpa_versions &
> > >                             NL80211_WPA_VERSION_1) {
> > >                                 bss_config->protocol =
> PROTOCOL_WPA;
> > >                                 bss_config->key_mgmt =
> KEY_MGMT_PSK;
> > >                         }
> > >                         if (params->crypto.wpa_versions &
> > >                             NL80211_WPA_VERSION_2) {
> > >                                 bss_config->protocol |=
> > > PROTOCOL_WPA2;
> > >                                 bss_config->key_mgmt =
> KEY_MGMT_PSK;
> > >                         }
> > >                         break;
> > >                 case WLAN_AKM_SUITE_SAE:
> > >                         bss_config->protocol = PROTOCOL_WPA2;
> > >                         bss_config->key_mgmt = KEY_MGMT_SAE;
> > >                         break;
> > >
> > >                 default:
> > >                         break;
> > >                 }
> > >         }
> > >
> > > It looks wrong to overwrite bss_config->protocol and
> > > bss_config->key_mgmt in each iteration of this loop. If that would
> > > be correct, you wouldn't need a loop at all, but could instead configure
> based on the last AKM suite entry.
> > >
> > > In my understanding the bits in bss_config->key_mgmt should be ored
> > > together depending on the possible AKM suites which is also what the
> > > downstream driver does.
> > >
> >
> > For the configuration of FW, ored only happens for the same AKM suite.
>
> Sorry, I don't understand this. Could you elaborate what you mean here?
>
> Sascha
>
> --
> Pengutronix e.K.                           |
> |
> Steuerwalder Str. 21                       |
> http://www.pen/
> gutronix.de%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cc852ce81dd734
> 2c25e0408dc8e96ebb2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638542026048434949%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&
> sdata=6Y%2FM9gFWYhf0UfClFTBwwsuMwrasEimaHKzJ2y2bHjQ%3D&reserved
> =0  |
> 31137 Hildesheim, Germany                  | Phone:
> +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

Firmware can only support one of WLAN_AKM_SUITE_8021X, WLAN_AKM_SUITE_PSK, or WLAN_AKM_SUITE_SAE.
For WLAN_AKM_SUITE_8021X and WLAN_AKM_SUITE_PSK, the protocol can be WPA, WPA2 or both.

David

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-05-25  0:50             ` David Lin
  2024-05-25 11:24               ` David Lin
  2024-06-14  2:18               ` David Lin
@ 2024-06-20 17:53               ` Brian Norris
  2024-06-21  4:36                 ` David Lin
  2 siblings, 1 reply; 39+ messages in thread
From: Brian Norris @ 2024-06-20 17:53 UTC (permalink / raw
  To: David Lin
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

Hi David,

On Sat, May 25, 2024 at 12:50:59AM +0000, David Lin wrote:
> 
> > From: Brian Norris <briannorris@chromium.org>
> > Sent: Saturday, May 25, 2024 6:55 AM
> > 
> > On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@nxp.com> wrote:
> > > I think it needs time to support probe client. Can we put your
> > > suggested comments to the code used to hook probe_client() and add
> > >
> > > "TODO: support probe client" to mwifiex_cfg80211_probe_client().
> > 
> > Are you suggesting that you plan to actually implement proper probe_client
> > support? Did you already do what I suggested, and understand why hostapd
> > needs probe_client support? This seems to be a common pattern -- that
> > reviewers are asking for you to do your research, and it takes several
> > requests before you actually do it.
> > 
> > Now that I've tried to do that research for you ... it looks like hostapd uses
> > probe_client to augment TX MGMT acks, as a proxy for station presence /
> > inactivity. If a station is inactive and non-responsive, we disconnect it
> > eventually. So that looks to me like probe_client support should actually be
> > optional, if your driver reports TX status? And in that case, I'd still
> > recommend you try to fix hostapd.
> > 
> > But if you're really planning to implement proper probe_client support, then
> > I suppose the TODO approach is also OK.
> > 
> > I'd also request that you please actually do your research when reviewers
> > ask questions. I'm frankly not sure why I'm spending my time on the above
> > research, when the onus should be on the submitter to explain why they're
> > doing what they're doing.
> 
> Yes. I know when aging time of station is out, hostapd will use probe_client to check if station is still there before really disconnect it.
> 
> Without this feature, it won't really affect mayor function of hostapd.

I'm glad *you* know all about the above behavior, but *I* didn't know
about it until I went and researched what this API does, and how hostapd
is using it. But that isn't my job -- it's your job, as the code
submitter, to explain your reasoning and reduce the amount of work that
readers/reviewers/maintainers have to do to understand your code and
agree that it is the right thing to do.

It's not clear to me that you've really learned the above lesson, and
it's really affecting the rate at which I review your code. This is by
far not the first time that you've placed the burden on the reader. And
if you're going to make the job difficult, then I'll prioritize enjoying
my free time, or stuff that actually pays me at $DAY_JOB, or ...

> That is the reason that I suggest that we put comments and TODO to the code.

OK, I suppose that works for me.

Brian

^ permalink raw reply	[flat|nested] 39+ messages in thread

* RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode
  2024-06-20 17:53               ` Brian Norris
@ 2024-06-21  4:36                 ` David Lin
  0 siblings, 0 replies; 39+ messages in thread
From: David Lin @ 2024-06-21  4:36 UTC (permalink / raw
  To: Brian Norris
  Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
	Francesco Dolcini

Hi Brian,

> From: Brian Norris <briannorris@chromium.org>
> Sent: Friday, June 21, 2024 1:53 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: Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
> 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 David,
> 
> On Sat, May 25, 2024 at 12:50:59AM +0000, David Lin wrote:
> >
> > > From: Brian Norris <briannorris@chromium.org>
> > > Sent: Saturday, May 25, 2024 6:55 AM
> > >
> > > On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@nxp.com> wrote:
> > > > I think it needs time to support probe client. Can we put your
> > > > suggested comments to the code used to hook probe_client() and add
> > > >
> > > > "TODO: support probe client" to mwifiex_cfg80211_probe_client().
> > >
> > > Are you suggesting that you plan to actually implement proper
> > > probe_client support? Did you already do what I suggested, and
> > > understand why hostapd needs probe_client support? This seems to be
> > > a common pattern -- that reviewers are asking for you to do your
> > > research, and it takes several requests before you actually do it.
> > >
> > > Now that I've tried to do that research for you ... it looks like
> > > hostapd uses probe_client to augment TX MGMT acks, as a proxy for
> > > station presence / inactivity. If a station is inactive and
> > > non-responsive, we disconnect it eventually. So that looks to me
> > > like probe_client support should actually be optional, if your
> > > driver reports TX status? And in that case, I'd still recommend you try to fix
> hostapd.
> > >
> > > But if you're really planning to implement proper probe_client
> > > support, then I suppose the TODO approach is also OK.
> > >
> > > I'd also request that you please actually do your research when
> > > reviewers ask questions. I'm frankly not sure why I'm spending my
> > > time on the above research, when the onus should be on the submitter
> > > to explain why they're doing what they're doing.
> >
> > Yes. I know when aging time of station is out, hostapd will use probe_client
> to check if station is still there before really disconnect it.
> >
> > Without this feature, it won't really affect mayor function of hostapd.
> 
> I'm glad *you* know all about the above behavior, but *I* didn't know about it
> until I went and researched what this API does, and how hostapd is using it. But
> that isn't my job -- it's your job, as the code submitter, to explain your
> reasoning and reduce the amount of work that readers/reviewers/maintainers
> have to do to understand your code and agree that it is the right thing to do.
> 
> It's not clear to me that you've really learned the above lesson, and it's really
> affecting the rate at which I review your code. This is by far not the first time
> that you've placed the burden on the reader. And if you're going to make the
> job difficult, then I'll prioritize enjoying my free time, or stuff that actually pays
> me at $DAY_JOB, or ...

I will keep this in mind.

> 
> > That is the reason that I suggest that we put comments and TODO to the
> code.
> 
> OK, I suppose that works for me.
> 
> Brian

I suggest that we just put your comments and prepare patch v11.

Thanks,
David

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2024-06-21  4:36 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-18  6:06 [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme David Lin
2024-04-18  6:06 ` [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode David Lin
2024-04-18  6:37   ` Francesco Dolcini
2024-04-18  8:24     ` [EXT] " David Lin
2024-05-23  0:53   ` Brian Norris
2024-05-24  9:46     ` [EXT] " David Lin
2024-05-24 17:02       ` Brian Norris
2024-05-24 22:00         ` David Lin
2024-05-24 22:54           ` Brian Norris
2024-05-25  0:50             ` David Lin
2024-05-25 11:24               ` David Lin
2024-06-14  2:18               ` David Lin
2024-06-20 17:53               ` Brian Norris
2024-06-21  4:36                 ` David Lin
2024-04-18  6:06 ` [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
2024-05-23  0:59   ` Brian Norris
2024-05-23  2:20     ` [EXT] " David Lin
2024-06-12 13:11   ` Sascha Hauer
2024-06-14  2:06     ` [EXT] " David Lin
2024-06-14  6:31       ` Sascha Hauer
2024-06-17  2:15         ` David Lin
2024-06-17  6:29           ` Sascha Hauer
2024-06-17  7:44             ` David Lin
2024-04-18  6:47 ` [PATCH v10 0/2] wifi: mwifiex: add code to support host mlme Marcel Holtmann
2024-04-18  9:08   ` [EXT] " David Lin
2024-04-18  9:15     ` [EXT] " Marcel Holtmann
2024-04-19  4:00       ` David Lin
2024-04-19 21:42         ` Brian Norris
2024-04-22  8:34           ` David Lin
2024-04-23  2:29             ` David Lin
2024-04-24  8:11               ` Marcel Holtmann
2024-04-25  2:40                 ` David Lin
2024-04-26  1:08                   ` Brian Norris
2024-05-02  8:34                     ` David Lin
2024-05-13  1:27                       ` David Lin
2024-05-23  0:50                         ` Brian Norris
2024-04-26  1:00               ` Brian Norris
2024-04-26  3:04                 ` David Lin
2024-04-25  2:09             ` David Lin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.