2024-03-06 02:01:27

by David Lin

[permalink] [raw]
Subject: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

With host mlme:
Tested-by: <[email protected]> #Verdin AM62 IW416 SD
Without host mlme:
Tested-by: Francesco Dolcini <[email protected]> # 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.

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 | 394 +++++++++++++++++-
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 27 ++
drivers/net/wireless/marvell/mwifiex/decl.h | 22 +
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 | 54 +++
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, 972 insertions(+), 17 deletions(-)


base-commit: b621df176d4d6eacd8b057f7324229655f10e77a
--
2.34.1



2024-03-06 02:02:44

by David Lin

[permalink] [raw]
Subject: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode

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 <[email protected]>
Reviewed-by: Francesco Dolcini <[email protected]>
---

v9:
- remove redundent code.

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

---
.../net/wireless/marvell/mwifiex/cfg80211.c | 315 ++++++++++++++++++
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 25 ++
drivers/net/wireless/marvell/mwifiex/decl.h | 22 ++
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 | 54 +++
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, 671 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index b909a7665e9c..bcf4f87dcaab 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,302 @@ 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 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
+ 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);
+ memcpy(mgmt->addr4, addr, ETH_ALEN);
+
+ 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 +4651,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 +4742,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..796e5be515f3 100644
--- a/drivers/net/wireless/marvell/mwifiex/decl.h
+++ b/drivers/net/wireless/marvell/mwifiex/decl.h
@@ -31,6 +31,28 @@
* + sizeof(tx_control)
*/

+#define FRMCTL_LEN 2
+#define DURATION_LEN 2
+#define SEQCTL_LEN 2
+#define MWIFIEX_MGMT_HEADER_LEN (FRMCTL_LEN + FRMCTL_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..27dc86cb9e41 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,14 @@ mwifiex_reinit_sw(struct mwifiex_adapter *adapter)
INIT_WORK(&adapter->rx_work, mwifiex_rx_work_queue);
}

+ 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()
@@ -1714,6 +1760,14 @@ mwifiex_add_card(void *card, struct completion *fw_done,
INIT_WORK(&adapter->rx_work, mwifiex_rx_work_queue);
}

+ 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. */
if (adapter->if_ops.register_dev(adapter)) {
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 75f53c2f1e1f..e9f742dece4e 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..af82f0401c63 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -370,6 +370,46 @@ 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 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+ u8 frame_buf[100];
+ struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)frame_buf;
+
+ memset(frame_buf, 0, sizeof(frame_buf));
+ mgmt->frame_control = (__force __le16)IEEE80211_STYPE_DEAUTH;
+ mgmt->duration = 0;
+ mgmt->seq_ctrl = 0;
+ mgmt->u.deauth.reason_code = (__force __le16)reason_code;
+
+ if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) {
+ memcpy(mgmt->da, broadcast_addr, ETH_ALEN);
+ 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 +457,46 @@ 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 disasso 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


2024-03-15 09:49:44

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

Hello Brian (and Kalle),

On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> This series add host based MLME support to the mwifiex driver, this
> enables WPA3 support in both client and AP mode.

What's your plan for this series? I know you raised some concern when
this started months ago and I'd love to know if there is something that
would need to be addressed to move forward here.


Francesco

p.s. I'm aware we are in the middle of the Linux merge window and
nothing will happen till it closes.


2024-03-15 23:59:26

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

Hi Francesco,

On Fri, Mar 15, 2024 at 10:49:27AM +0100, Francesco Dolcini wrote:
> Hello Brian (and Kalle),
>
> On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> > This series add host based MLME support to the mwifiex driver, this
> > enables WPA3 support in both client and AP mode.
>
> What's your plan for this series? I know you raised some concern when
> this started months ago and I'd love to know if there is something that
> would need to be addressed to move forward here.

My plan was (especially given the "Odd Fixes" status in MAINTAINERS) to
wait until a 2nd party was happy with things here. From my cursory
following of things, that has now occurred -- thanks for all the review
Francesco.

My plan recently was to get back to reviewing the code again, and it's
been sitting in my inbox. Unfortunately, I haven't made time in these
last ~9 days so far.

I'm revisiting it now.

Regards,
Brian

2024-03-16 00:00:20

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode

Hi David,

Thanks for the persistence here (and thanks Francesco for all the review
help). I think things are mostly well structured here, but I'll also
admit it's a pretty large bit of work to review at once. So please bear
with me if it takes a bit of time to really get through it. I'll post a
few thoughts now, but it's possible I'll have more after another pass.

On Wed, Mar 06, 2024 at 10:00:52AM +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 <[email protected]>
> Reviewed-by: Francesco Dolcini <[email protected]>
> ---
>
> v9:
> - remove redundent code.
>
> v8:
> - first full and complete patch to support host based MLME for client
> mode.
>
> ---
> .../net/wireless/marvell/mwifiex/cfg80211.c | 315 ++++++++++++++++++
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 25 ++
> drivers/net/wireless/marvell/mwifiex/decl.h | 22 ++
> 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 | 54 +++
> 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, 671 insertions(+), 14 deletions(-)

(Per the above, I'd normally consider whether ~671 new lines is worth
splitting into multiple patches. But I don't see any great logical ways
to do that.)

> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index b909a7665e9c..bcf4f87dcaab 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c

> @@ -4204,6 +4210,302 @@ 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 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};


> + memcpy(mgmt->addr4, addr, ETH_ALEN);

eth_broadcast_addr(mgmt->addr4);

> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c

> @@ -1558,6 +1596,14 @@ mwifiex_reinit_sw(struct mwifiex_adapter *adapter)
> INIT_WORK(&adapter->rx_work, mwifiex_rx_work_queue);
> }
>
> + adapter->host_mlme_workqueue =
> + alloc_workqueue("MWIFIEX_HOST_MLME_WORK_QUEUE",
> + WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 0);

Hmm, why do you need a whole new workqueue? This driver is already full
of race conditions, while many race conditions are avoided simply
because most work is sequentialized onto the main work queue. If you
don't have a good reason here, I'd probably prefer you share the
existing queue.

Or otherwise, if this is *truly* independent and race-free, do you
actually need to create a new queue? We could just use schedule_work(),
which uses the system queue.

If you do really need it, is it possible to key off 'host_mlme_enabled'
or similar? There's no need to allocate the queue if we're not using it.

> + 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()


> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -370,6 +370,46 @@ 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 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> + u8 frame_buf[100];
> + struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)frame_buf;
> +
> + memset(frame_buf, 0, sizeof(frame_buf));
> + mgmt->frame_control = (__force __le16)IEEE80211_STYPE_DEAUTH;

Hmm, "__force" is a pretty good sign that you're doing something wrong.
Please think twice before using it.

I believe the right answer here is cpu_to_le16(). It's a no-op on little
endian architectures, but it'll make big-endian work.

> + mgmt->duration = 0;
> + mgmt->seq_ctrl = 0;
> + mgmt->u.deauth.reason_code = (__force __le16)reason_code;

Same here.

> +
> + if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) {
> + memcpy(mgmt->da, broadcast_addr, ETH_ALEN);

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;


> @@ -417,6 +457,46 @@ 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 disasso from %pM\n",

I get that sometimes abbreviations are nice, but perhaps at least use a
consistent one? I see "disassoc" elsewhere.

> + ieee_hdr->addr3);

Brian

2024-03-16 00:07:57

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> With host mlme:
> Tested-by: <[email protected]> #Verdin AM62 IW416 SD
> Without host mlme:
> Tested-by: Francesco Dolcini <[email protected]> # 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.

Thank you for all the evoluation in this series. This looks much better
than it did at the start, and I appreciate the additional explanation of
featureset (HW and FW versions). I'm not sure if this has been
asked/answered before, but are the MLME/WPA3 limitations exclusively
tied to the firmware support ("V2 Key API")? Or are there hardware
limitations on top (e.g., some firmware might get "V2 Key API" but still
be unsupported on a given chip family)? Could other chips chips
theoretically get this feature-set in the future?

In absence of a clear answer on this, it's definitely a good idea to do
things like you have in this series now -- that you have a short-list
(of 1) of HW where where it's validated, and additionally a FW
feature/revision check to ensure it's running appropriate firmware. But
I just wonder what the feasibility would be for adding to the shortlist
(or providing users/developers the option of doing so) in the future, if
people are so inclined.

To be clear, this is mostly an informational curiosity and
forward-looking question -- not a request to change the implementation
in this series.

Thanks,
Brian

2024-03-16 00:49:48

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

+ Johannes

On Fri, Mar 15, 2024 at 10:49:27AM +0100, Francesco Dolcini wrote:
> Hello Brian (and Kalle),
>
> On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> > This series add host based MLME support to the mwifiex driver, this
> > enables WPA3 support in both client and AP mode.
>
> What's your plan for this series? I know you raised some concern when
> this started months ago and I'd love to know if there is something that
> would need to be addressed to move forward here.

Now that I've looked a bit closer today: I'm realizing this may(?) be
one of the first "full MAC" drivers trying to implement its own MLME --
or at least, the auth/assoc bits. I wouldn't really consider myself an
expert on the core wireless APIs here, so this might be an area that
could warrant some extra look from Kalle and/or Johannes too.

Brian

2024-03-18 02:00:46

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode

> From: Brian Norris <[email protected]>
> Sent: Saturday, March 16, 2024 8:00 AM
> To: David Lin <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Pete Hsieh
> <[email protected]>; Francesco Dolcini
> <[email protected]>
> Subject: [EXT] Re: [PATCH v9 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,
>
> Thanks for the persistence here (and thanks Francesco for all the review help).
> I think things are mostly well structured here, but I'll also admit it's a pretty
> large bit of work to review at once. So please bear with me if it takes a bit of
> time to really get through it. I'll post a few thoughts now, but it's possible I'll
> have more after another pass.
>

Thanks for your help and take your time.

> On Wed, Mar 06, 2024 at 10:00:52AM +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 <[email protected]>
> > Reviewed-by: Francesco Dolcini <[email protected]>
> > ---
> >
> > v9:
> > - remove redundent code.
> >
> > v8:
> > - first full and complete patch to support host based MLME for client
> > mode.
> >
> > ---
> > .../net/wireless/marvell/mwifiex/cfg80211.c | 315
> ++++++++++++++++++
> > drivers/net/wireless/marvell/mwifiex/cmdevt.c | 25 ++
> > drivers/net/wireless/marvell/mwifiex/decl.h | 22 ++
> > 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 | 54 +++
> > 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, 671 insertions(+), 14 deletions(-)
>
> (Per the above, I'd normally consider whether ~671 new lines is worth splitting
> into multiple patches. But I don't see any great logical ways to do that.)
>

Francesco suggested to use two patches for this host mlme new feature from previous many patches. I knew it is a lot of changes, but I think it should be the best way to add host mlme with two patches (one for client and one for AP).

> > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > index b909a7665e9c..bcf4f87dcaab 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>
> > @@ -4204,6 +4210,302 @@ 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 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
>
>
> > + memcpy(mgmt->addr4, addr, ETH_ALEN);
>
> eth_broadcast_addr(mgmt->addr4);
>
> > --- a/drivers/net/wireless/marvell/mwifiex/main.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/main.c
>
> > @@ -1558,6 +1596,14 @@ mwifiex_reinit_sw(struct mwifiex_adapter
> *adapter)
> > INIT_WORK(&adapter->rx_work,
> mwifiex_rx_work_queue);
> > }
> >
> > + adapter->host_mlme_workqueue =
> > +
> alloc_workqueue("MWIFIEX_HOST_MLME_WORK_QUEUE",
> > + WQ_HIGHPRI | WQ_MEM_RECLAIM |
> > + WQ_UNBOUND, 0);
>
> Hmm, why do you need a whole new workqueue? This driver is already full of
> race conditions, while many race conditions are avoided simply because most
> work is sequentialized onto the main work queue. If you don't have a good
> reason here, I'd probably prefer you share the existing queue.
>
> Or otherwise, if this is *truly* independent and race-free, do you actually need
> to create a new queue? We could just use schedule_work(), which uses the
> system queue.
>
> If you do really need it, is it possible to key off 'host_mlme_enabled'
> or similar? There's no need to allocate the queue if we're not using it.
>

Will add the checking of 'host_mlme_enabled' to create this work queue if needed in patch v10.

> > + 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()
>
>
> > --- a/drivers/net/wireless/marvell/mwifiex/util.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> > @@ -370,6 +370,46 @@ 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 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
> > + u8 frame_buf[100];
> > + struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt
> > +*)frame_buf;
> > +
> > + memset(frame_buf, 0, sizeof(frame_buf));
> > + mgmt->frame_control = (__force __le16)IEEE80211_STYPE_DEAUTH;
>
> Hmm, "__force" is a pretty good sign that you're doing something wrong.
> Please think twice before using it.
>

Will modify it in patch v10.

> I believe the right answer here is cpu_to_le16(). It's a no-op on little endian
> architectures, but it'll make big-endian work.
>
> > + mgmt->duration = 0;
> > + mgmt->seq_ctrl = 0;
> > + mgmt->u.deauth.reason_code = (__force __le16)reason_code;
>
> Same here.
>

Will do in patch v10.

> > +
> > + if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_STA) {
> > + memcpy(mgmt->da, broadcast_addr, ETH_ALEN);
>
> eth_broadcast_addr(mgmt->da);
>

Will change it in patch v10.

> > + 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;
>
>
> > @@ -417,6 +457,46 @@ 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 disasso
> from
> > + %pM\n",
>
> I get that sometimes abbreviations are nice, but perhaps at least use a
> consistent one? I see "disassoc" elsewhere.
>

Will modify it in patch v10.

> > + ieee_hdr->addr3);
>
> Brian

2024-03-18 02:21:05

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Brian Norris <[email protected]>
> Sent: Saturday, March 16, 2024 8:08 AM
> To: David Lin <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Pete Hsieh
> <[email protected]>; rafael.beims <[email protected]>;
> Francesco Dolcini <[email protected]>
> Subject: [EXT] Re: [PATCH v9 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 Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> > With host mlme:
> > Tested-by: <[email protected]> #Verdin AM62 IW416 SD Without
> > host mlme:
> > Tested-by: Francesco Dolcini <[email protected]> #
> > 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.
>
> Thank you for all the evoluation in this series. This looks much better than it
> did at the start, and I appreciate the additional explanation of featureset (HW
> and FW versions). I'm not sure if this has been asked/answered before, but are
> the MLME/WPA3 limitations exclusively tied to the firmware support ("V2 Key
> API")? Or are there hardware limitations on top (e.g., some firmware might get
> "V2 Key API" but still be unsupported on a given chip family)? Could other
> chips chips theoretically get this feature-set in the future?
>
> In absence of a clear answer on this, it's definitely a good idea to do things like
> you have in this series now -- that you have a short-list (of 1) of HW where
> where it's validated, and additionally a FW feature/revision check to ensure it's
> running appropriate firmware. But I just wonder what the feasibility would be
> for adding to the shortlist (or providing users/developers the option of doing so)
> in the future, if people are so inclined.
>
> To be clear, this is mostly an informational curiosity and forward-looking
> question -- not a request to change the implementation in this series.
>
> Thanks,
> Brian

If firmware reported support of V2 Key API, then host mlme can be supported without issues. There is a flag 'host_mlme' in struct 'mwifiex_sdio_device' to indicate if host mlme should be supported. If this flag is set, driver will still check if firmware can support V2 Key API. If firmware can't support it, host mlme will be disabled.

Thanks,
David

2024-03-18 09:24:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

Francesco Dolcini <[email protected]> writes:

> Hello Brian (and Kalle),
>
> On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
>> This series add host based MLME support to the mwifiex driver, this
>> enables WPA3 support in both client and AP mode.
>
> What's your plan for this series? I know you raised some concern when
> this started months ago and I'd love to know if there is something that
> would need to be addressed to move forward here.

Based on the history of this patchset I am a bit concerned if these
patches break existing setups. I'm sure Brian will look at that in
detail but more test results from different setups we have the better.

> p.s. I'm aware we are in the middle of the Linux merge window and
> nothing will happen till it closes.

BTW, thanks to some for-next branch trickery, we keep wireless-next open
also during merge windows. This is to avoid unnecessarily stopping the
development.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2024-03-18 11:29:03

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Fri, Mar 15, 2024 at 04:59:19PM -0700, Brian Norris wrote:
> On Fri, Mar 15, 2024 at 10:49:27AM +0100, Francesco Dolcini wrote:
> > On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> > > This series add host based MLME support to the mwifiex driver, this
> > > enables WPA3 support in both client and AP mode.
> >
> > What's your plan for this series? I know you raised some concern when
> > this started months ago and I'd love to know if there is something that
> > would need to be addressed to move forward here.
>
> My plan was (especially given the "Odd Fixes" status in MAINTAINERS) to
> wait until a 2nd party was happy with things here. From my cursory
> following of things, that has now occurred -- thanks for all the review
> Francesco.

Brian/Kalle, would be ok for you to add myself as reviewer for mwifiex?
The patch flow on the driver is pretty limited, but I care about it
and I am happy to help out if needed (and I have some hardware available
for testing).

If you agree I'll send a patch to the MAINTAINERS file.

Francesco



2024-03-18 11:45:10

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

Hello David and Brian,

On Mon, Mar 18, 2024 at 02:20:56AM +0000, David Lin wrote:
> > From: Brian Norris <[email protected]>
> > running appropriate firmware. But I just wonder what the feasibility
> > would be for adding to the shortlist (or providing users/developers
> > the option of doing so) in the future, if people are so inclined.
>
> If firmware reported support of V2 Key API, then host mlme can be
> supported without issues. There is a flag 'host_mlme' in struct
> 'mwifiex_sdio_device' to indicate if host mlme should be supported. If
> this flag is set, driver will still check if firmware can support V2
> Key API. If firmware can't support it, host mlme will be disabled.

Once we are through this patch I plan to test hostmlme on
88W8997-SDIO-UART, that should have the required firmware available.

Francesco


2024-03-19 01:40:33

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Kalle Valo <[email protected]>
> Sent: Monday, March 18, 2024 5:25 PM
> To: Francesco Dolcini <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; David Lin <[email protected]>; Pete Hsieh
> <[email protected]>; rafael.beims <[email protected]>;
> Francesco Dolcini <[email protected]>
> Subject: [EXT] Re: [PATCH v9 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
>
>
> Francesco Dolcini <[email protected]> writes:
>
> > Hello Brian (and Kalle),
> >
> > On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> >> This series add host based MLME support to the mwifiex driver, this
> >> enables WPA3 support in both client and AP mode.
> >
> > What's your plan for this series? I know you raised some concern when
> > this started months ago and I'd love to know if there is something
> > that would need to be addressed to move forward here.
>
> Based on the history of this patchset I am a bit concerned if these patches
> break existing setups. I'm sure Brian will look at that in detail but more test
> results from different setups we have the better.
>

With host mlme: tested by NXP QA and Rafael.
Without host mlme: tested by Francesco and myself.

Thanks,
David

> > p.s. I'm aware we are in the middle of the Linux merge window and
> > nothing will happen till it closes.
>
> BTW, thanks to some for-next branch trickery, we keep wireless-next open also
> during merge windows. This is to avoid unnecessarily stopping the
> development.
>
> --
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.kernel.org%2Fproject%2Flinux-wireless%2Flist%2F&data=05%7C02%7Cyu-hao
> .lin%40nxp.com%7C8d38662f40b342dd6f9f08dc472d3cde%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C638463506823212295%7CUnknown%7CT
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6Mn0%3D%7C0%7C%7C%7C&sdata=fFO%2F1WjAubSnNNfMtXfRXXmAMP
> UEMjTHIIgjD4JDUnY%3D&reserved=0
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwireless.
> wiki.kernel.org%2Fen%2Fdevelopers%2Fdocumentation%2Fsubmittingpatches
> &data=05%7C02%7Cyu-hao.lin%40nxp.com%7C8d38662f40b342dd6f9f08dc47
> 2d3cde%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63846350682
> 3225278%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=uLvfm66Y
> G0OGFWNV2ZXngwVK%2FyuJlK5YO7wjbG8mUd0%3D&reserved=0

2024-03-19 10:34:06

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

Francesco Dolcini <[email protected]> writes:

> On Fri, Mar 15, 2024 at 04:59:19PM -0700, Brian Norris wrote:
>> On Fri, Mar 15, 2024 at 10:49:27AM +0100, Francesco Dolcini wrote:
>> > On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
>> > > This series add host based MLME support to the mwifiex driver, this
>> > > enables WPA3 support in both client and AP mode.
>> >
>> > What's your plan for this series? I know you raised some concern when
>> > this started months ago and I'd love to know if there is something that
>> > would need to be addressed to move forward here.
>>
>> My plan was (especially given the "Odd Fixes" status in MAINTAINERS) to
>> wait until a 2nd party was happy with things here. From my cursory
>> following of things, that has now occurred -- thanks for all the review
>> Francesco.
>
> Brian/Kalle, would be ok for you to add myself as reviewer for mwifiex?
> The patch flow on the driver is pretty limited, but I care about it
> and I am happy to help out if needed (and I have some hardware available
> for testing).
>
> If you agree I'll send a patch to the MAINTAINERS file.

At least from my point of view that sounds like a good idea, you have
provided good review comments for this driver. But let's wait what Brian
says.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2024-03-19 12:12:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Fri, 2024-03-15 at 17:49 -0700, Brian Norris wrote:
>
> Now that I've looked a bit closer today: I'm realizing this may(?) be
> one of the first "full MAC" drivers trying to implement its own MLME --
> or at least, the auth/assoc bits.

Hmm, yeah, why _is_ that? mwifiex was originally "sold" as a "full MAC"
driver, i.e. doing things in the firmware.

We've said that "soft MAC" drivers should be using mac80211, but this
thing can't seem to decide?

Also decl.h should probably _shrink_ rather than grow, a number of
things just replicate ieee80211.h (such as MWIFIEX_MGMT_HEADER_LEN
really is just sizeof(ieee80211_mgmt) or so? Not quite correctly.)

So yeah, agree with Brian, not only would this be the first, but it's
also something we don't really _want_. All other drivers that want stuff
like this are stuck in staging ...

So why is this needed for a supposedly "firmware does it all" driver,
and why can it not be integrated with mac80211 if it's no longer
"firmware does it all"?

johannes


2024-03-20 00:59:51

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Johannes Berg <[email protected]>
> Sent: Tuesday, March 19, 2024 8:13 PM
> To: Brian Norris <[email protected]>; Francesco Dolcini
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; David Lin <[email protected]>; Pete Hsieh
> <[email protected]>; rafael.beims <[email protected]>;
> Francesco Dolcini <[email protected]>
> Subject: [EXT] Re: [PATCH v9 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, 2024-03-15 at 17:49 -0700, Brian Norris wrote:
> >
> > Now that I've looked a bit closer today: I'm realizing this may(?) be
> > one of the first "full MAC" drivers trying to implement its own MLME
> > -- or at least, the auth/assoc bits.
>
> Hmm, yeah, why _is_ that? mwifiex was originally "sold" as a "full MAC"
> driver, i.e. doing things in the firmware.
>
> We've said that "soft MAC" drivers should be using mac80211, but this thing
> can't seem to decide?
>
> Also decl.h should probably _shrink_ rather than grow, a number of things just
> replicate ieee80211.h (such as MWIFIEX_MGMT_HEADER_LEN really is just
> sizeof(ieee80211_mgmt) or so? Not quite correctly.)
>

This can be done for feature patches.

> So yeah, agree with Brian, not only would this be the first, but it's also
> something we don't really _want_. All other drivers that want stuff like this are
> stuck in staging ...
>
> So why is this needed for a supposedly "firmware does it all" driver, and why
> can it not be integrated with mac80211 if it's no longer "firmware does it all"?
>
> Johannes

Our proprietary driver is cfg80211 driver, it is very hard to create a brand new mac80211 driver and still can port all tested stuffs from our proprietary driver.

Thanks,
David

2024-03-20 01:11:07

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: David Lin
> Sent: Wednesday, March 20, 2024 9:00 AM
> To: Johannes Berg <[email protected]>; Brian Norris
> <[email protected]>; Francesco Dolcini <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; Pete Hsieh <[email protected]>;
> rafael.beims <[email protected]>; Francesco Dolcini
> <[email protected]>
> Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> > From: Johannes Berg <[email protected]>
> > Sent: Tuesday, March 19, 2024 8:13 PM
> > To: Brian Norris <[email protected]>; Francesco Dolcini
> > <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; David Lin <[email protected]>; Pete
> > Hsieh <[email protected]>; rafael.beims
> > <[email protected]>; Francesco Dolcini
> > <[email protected]>
> > Subject: [EXT] Re: [PATCH v9 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, 2024-03-15 at 17:49 -0700, Brian Norris wrote:
> > >
> > > Now that I've looked a bit closer today: I'm realizing this may(?)
> > > be one of the first "full MAC" drivers trying to implement its own
> > > MLME
> > > -- or at least, the auth/assoc bits.
> >
> > Hmm, yeah, why _is_ that? mwifiex was originally "sold" as a "full MAC"
> > driver, i.e. doing things in the firmware.
> >
> > We've said that "soft MAC" drivers should be using mac80211, but this
> > thing can't seem to decide?
> >
> > Also decl.h should probably _shrink_ rather than grow, a number of
> > things just replicate ieee80211.h (such as MWIFIEX_MGMT_HEADER_LEN
> > really is just
> > sizeof(ieee80211_mgmt) or so? Not quite correctly.)
> >
>
> This can be done for feature patches.
>
> > So yeah, agree with Brian, not only would this be the first, but it's
> > also something we don't really _want_. All other drivers that want
> > stuff like this are stuck in staging ...
> >
> > So why is this needed for a supposedly "firmware does it all" driver,
> > and why can it not be integrated with mac80211 if it's no longer "firmware
> does it all"?
> >
> > Johannes
>
> Our proprietary driver is cfg80211 driver, it is very hard to create a brand new
> mac80211 driver and still can port all tested stuffs from our proprietary driver.
>
> Thanks,
> David

BTW, vendor should have the choice to use cfg80211 or mac80211 for their chips, right?

2024-03-20 09:12:55

by Johannes Berg

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Wed, 2024-03-20 at 01:10 +0000, David Lin wrote:
> > >
> > > Also decl.h should probably _shrink_ rather than grow, a number of
> > > things just replicate ieee80211.h (such as MWIFIEX_MGMT_HEADER_LEN
> > > really is just
> > > sizeof(ieee80211_mgmt) or so? Not quite correctly.)
> > >
> >
> > This can be done for feature patches.

But this is a feature patch :-)

> > > So yeah, agree with Brian, not only would this be the first, but it's
> > > also something we don't really _want_. All other drivers that want
> > > stuff like this are stuck in staging ...
> > >
> > > So why is this needed for a supposedly "firmware does it all" driver,
> > > and why can it not be integrated with mac80211 if it's no longer "firmware
> > does it all"?
> > >
> > > Johannes
> >
> > Our proprietary driver is cfg80211 driver, it is very hard to create a brand new
> > mac80211 driver and still can port all tested stuffs from our proprietary driver.

That basically doesn't matter for upstream at all.

>
> BTW, vendor should have the choice to use cfg80211 or mac80211 for their chips, right?

No, that's not how it works. The choice should be what makes sense
architecturally.

johannes

2024-03-20 21:07:29

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Tue, Mar 19, 2024 at 12:33:55PM +0200, Kalle Valo wrote:
> Francesco Dolcini <[email protected]> writes:
> > Brian/Kalle, would be ok for you to add myself as reviewer for mwifiex?
> > The patch flow on the driver is pretty limited, but I care about it
> > and I am happy to help out if needed (and I have some hardware available
> > for testing).
> >
> > If you agree I'll send a patch to the MAINTAINERS file.
>
> At least from my point of view that sounds like a good idea, you have
> provided good review comments for this driver. But let's wait what Brian
> says.

That's totally fine with me.

2024-03-20 21:13:30

by Brian Norris

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

Hi David,

On Mon, Mar 18, 2024 at 02:20:56AM +0000, David Lin wrote:
> > From: Brian Norris <[email protected]>

> > I'm not sure if this has been asked/answered before, but are
> > the MLME/WPA3 limitations exclusively tied to the firmware support ("V2 Key
> > API")? Or are there hardware limitations on top (e.g., some firmware might get
> > "V2 Key API" but still be unsupported on a given chip family)? Could other
> > chips chips theoretically get this feature-set in the future?
>
> If firmware reported support of V2 Key API, then host mlme can be
> supported without issues. There is a flag 'host_mlme' in struct
> 'mwifiex_sdio_device' to indicate if host mlme should be supported. If
> this flag is set, driver will still check if firmware can support V2
> Key API. If firmware can't support it, host mlme will be disabled.

Thanks! If I can distill the answer: it's just a software/firmware
limitation, and not a hardware limitation. The hardware limitation flag
in this series is added just out of caution.

Brian

2024-03-20 21:28:42

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Mon, Mar 18, 2024 at 11:24:34AM +0200, Kalle Valo wrote:
> Francesco Dolcini <[email protected]> writes:
>
> > Hello Brian (and Kalle),
> >
> > On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> >> This series add host based MLME support to the mwifiex driver, this
> >> enables WPA3 support in both client and AP mode.
> >
> > What's your plan for this series? I know you raised some concern when
> > this started months ago and I'd love to know if there is something that
> > would need to be addressed to move forward here.
>
> Based on the history of this patchset I am a bit concerned if these
> patches break existing setups. I'm sure Brian will look at that in
> detail but more test results from different setups we have the better.

It looks like the latest patches generally avoid touching behavior for
devices without this feature-set. And I've given it a bit of a whirl
myself, although I have a pretty blind eye to AP-mode as my systems tend
to be clients.

Yes, testing is always a concern for invasive changes, but I think we're
in OK shape at least w.r.t. regressing existing setups. Or, I won't
provide an Acked-by until I'm happy.

Brian

2024-03-20 21:50:38

by Brian Norris

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Wed, Mar 20, 2024 at 10:12:45AM +0100, Johannes Berg wrote:
> On Wed, 2024-03-20 at 01:10 +0000, David Lin wrote:
> > > >
> > > > Also decl.h should probably _shrink_ rather than grow, a number of
> > > > things just replicate ieee80211.h (such as MWIFIEX_MGMT_HEADER_LEN
> > > > really is just
> > > > sizeof(ieee80211_mgmt) or so? Not quite correctly.)
> > > >
> > >
> > > This can be done for feature patches.
>
> But this is a feature patch :-)

I'm going to hazard a guess David may have meant "future"?

But yeah, I get overwhelemed at how similar-but-not-quite-the-same
definitions in this driver sometimes. It definitely could use some
spring cleaning.

> > > > So yeah, agree with Brian, not only would this be the first, but it's
> > > > also something we don't really _want_. All other drivers that want
> > > > stuff like this are stuck in staging ...
> > > >
> > > > So why is this needed for a supposedly "firmware does it all" driver,
> > > > and why can it not be integrated with mac80211 if it's no longer "firmware
> > > does it all"?
> > > >
> > > > Johannes
> > >
> > > Our proprietary driver is cfg80211 driver, it is very hard to create a brand new
> > > mac80211 driver and still can port all tested stuffs from our proprietary driver.
>
> That basically doesn't matter for upstream at all.

+1

> > BTW, vendor should have the choice to use cfg80211 or mac80211 for their chips, right?
>
> No, that's not how it works. The choice should be what makes sense
> architecturally.

And to put some specifics on it, that's what's described here:

https://wireless.wiki.kernel.org/en/developers/documentation/mac80211
https://wireless.wiki.kernel.org/en/developers/documentation/cfg80211

(I don't consider myself an authority on this stuff, for the record.
But:)

I've often felt like the SoftMAC designation has a very fuzzy
definition. Or, that definition is very much subject to the whims of the
hardware/firmware vendor, and can change from day to day. For instance,
it feels like there are plenty of "fat firmware" features in mac80211
drivers today, where pretty much anything and everything *might* be
handled in some kind of firmware-offload feature, in addition or instead
of on the host CPU.

But a key point that *is* a pretty hard designation, from the mac80211
page:

"SoftMAC devices allow for a finer control of the hardware, allowing for
802.11 frame management to be done in software for them, for both
parsing and generation of 802.11 wireless frames"

AFAICT, mwifiex firmware still isn't allowing "parsing and generation of
802.11 wireless frames" in any general form -- everything I see is still
wrapped in custom firmware command protocols. I do see that the AUTH
frame looks like it's essentially duplicating the standard mgmt format,
and uses the driver's TX path for it, but there isn't a corresponding
ASSOC management frame that I can see...
...so I really can't tell how much control this firmware *does* give the
host regarding arbitrary 802.11 frame management.

But that's pretty much business as usual for anybody but the vendor in
priorietary firmware land; I can't answer pretty much any question,
other than what I can glean from a driver.

Brian

2024-03-21 02:12:13

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Brian Norris <[email protected]>
> Sent: Thursday, March 21, 2024 5:13 AM
> To: David Lin <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Pete Hsieh
> <[email protected]>; rafael.beims <[email protected]>;
> Francesco Dolcini <[email protected]>
> Subject: Re: [EXT] Re: [PATCH v9 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, Mar 18, 2024 at 02:20:56AM +0000, David Lin wrote:
> > > From: Brian Norris <[email protected]>
>
> > > I'm not sure if this has been asked/answered before, but are the
> > > MLME/WPA3 limitations exclusively tied to the firmware support ("V2
> > > Key API")? Or are there hardware limitations on top (e.g., some
> > > firmware might get
> > > "V2 Key API" but still be unsupported on a given chip family)? Could
> > > other chips chips theoretically get this feature-set in the future?
> >
> > If firmware reported support of V2 Key API, then host mlme can be
> > supported without issues. There is a flag 'host_mlme' in struct
> > 'mwifiex_sdio_device' to indicate if host mlme should be supported. If
> > this flag is set, driver will still check if firmware can support V2
> > Key API. If firmware can't support it, host mlme will be disabled.
>
> Thanks! If I can distill the answer: it's just a software/firmware limitation, and
> not a hardware limitation. The hardware limitation flag in this series is added
> just out of caution.
>
> Brian

Give user the choice to use host mlme or not.

2024-03-21 04:08:06

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Brian Norris <[email protected]>
> Sent: Thursday, March 21, 2024 5:50 AM
> To: Johannes Berg <[email protected]>
> Cc: David Lin <[email protected]>; Francesco Dolcini <[email protected]>;
> [email protected]; [email protected];
> [email protected]; Pete Hsieh <[email protected]>;
> rafael.beims <[email protected]>; Francesco Dolcini
> <[email protected]>
> Subject: Re: [EXT] Re: [PATCH v9 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 Wed, Mar 20, 2024 at 10:12:45AM +0100, Johannes Berg wrote:
> > On Wed, 2024-03-20 at 01:10 +0000, David Lin wrote:
> > > > >
> > > > > Also decl.h should probably _shrink_ rather than grow, a number
> > > > > of things just replicate ieee80211.h (such as
> > > > > MWIFIEX_MGMT_HEADER_LEN really is just
> > > > > sizeof(ieee80211_mgmt) or so? Not quite correctly.)
> > > > >
> > > >
> > > > This can be done for feature patches.
> >
> > But this is a feature patch :-)
>
> I'm going to hazard a guess David may have meant "future"?
>
> But yeah, I get overwhelemed at how similar-but-not-quite-the-same
> definitions in this driver sometimes. It definitely could use some spring
> cleaning.
>

Sorry. Typo. I will modify it in patch v10 instead of future patch.

> > > > > So yeah, agree with Brian, not only would this be the first, but
> > > > > it's also something we don't really _want_. All other drivers
> > > > > that want stuff like this are stuck in staging ...
> > > > >
> > > > > So why is this needed for a supposedly "firmware does it all"
> > > > > driver, and why can it not be integrated with mac80211 if it's
> > > > > no longer "firmware
> > > > does it all"?
> > > > >
> > > > > Johannes
> > > >
> > > > Our proprietary driver is cfg80211 driver, it is very hard to
> > > > create a brand new
> > > > mac80211 driver and still can port all tested stuffs from our proprietary
> driver.
> >
> > That basically doesn't matter for upstream at all.
>
> +1
>

Yes. Sorry. Please check my explanations below.

> > > BTW, vendor should have the choice to use cfg80211 or mac80211 for their
> chips, right?
> >
> > No, that's not how it works. The choice should be what makes sense
> > architecturally.
>
> And to put some specifics on it, that's what's described here:
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwireless.
> wiki.kernel.org%2Fen%2Fdevelopers%2Fdocumentation%2Fmac80211&data=0
> 5%7C02%7Cyu-hao.lin%40nxp.com%7C26588fdd1b6e4898479808dc4927c350
> %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63846568232661160
> 6%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=i3RTFqEZN3djd9L
> RsgxgdgXfiuCln%2BBy6%2B0akWYLvT8%3D&reserved=0
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwireless.
> wiki.kernel.org%2Fen%2Fdevelopers%2Fdocumentation%2Fcfg80211&data=05
> %7C02%7Cyu-hao.lin%40nxp.com%7C26588fdd1b6e4898479808dc4927c350%
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638465682326623950
> %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
> BTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=5erX1KaKcB5j6GzN
> u2bb0wt0j7DgUckO5hsazc1z%2FlM%3D&reserved=0
>
> (I don't consider myself an authority on this stuff, for the record.
> But:)
>
> I've often felt like the SoftMAC designation has a very fuzzy definition. Or, that
> definition is very much subject to the whims of the hardware/firmware vendor,
> and can change from day to day. For instance, it feels like there are plenty of
> "fat firmware" features in mac80211 drivers today, where pretty much
> anything and everything *might* be handled in some kind of firmware-offload
> feature, in addition or instead of on the host CPU.
>
> But a key point that *is* a pretty hard designation, from the mac80211
> page:
>
> "SoftMAC devices allow for a finer control of the hardware, allowing for
> 802.11 frame management to be done in software for them, for both parsing
> and generation of 802.11 wireless frames"
>
> AFAICT, mwifiex firmware still isn't allowing "parsing and generation of
> 802.11 wireless frames" in any general form -- everything I see is still wrapped
> in custom firmware command protocols. I do see that the AUTH frame looks
> like it's essentially duplicating the standard mgmt format, and uses the driver's
> TX path for it, but there isn't a corresponding ASSOC management frame that I
> can see...
> ...so I really can't tell how much control this firmware *does* give the host
> regarding arbitrary 802.11 frame management.
>
> But that's pretty much business as usual for anybody but the vendor in
> priorietary firmware land; I can't answer pretty much any question, other than
> what I can glean from a driver.
>
> Brian

Yes. This change is to offload wpa3 features to host. It's well tested and doesn't impact existing features.

David

2024-03-23 01:06:29

by Brian Norris

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Thu, Mar 21, 2024 at 04:07:58AM +0000, David Lin wrote:
> > From: Brian Norris <[email protected]>
> >
> > On Wed, Mar 20, 2024 at 10:12:45AM +0100, Johannes Berg wrote:
> > > On Wed, 2024-03-20 at 01:10 +0000, David Lin wrote:

> > > > BTW, vendor should have the choice to use cfg80211 or mac80211 for their
> > chips, right?
> > >
> > > No, that's not how it works. The choice should be what makes sense
> > > architecturally.
> >
> > And to put some specifics on it, that's what's described here:

[strip mangled URLs]

> > "SoftMAC devices allow for a finer control of the hardware, allowing for
> > 802.11 frame management to be done in software for them, for both parsing
> > and generation of 802.11 wireless frames"
> >
> > AFAICT, mwifiex firmware still isn't allowing "parsing and generation of
> > 802.11 wireless frames" in any general form -- everything I see is still wrapped
> > in custom firmware command protocols. I do see that the AUTH frame looks
> > like it's essentially duplicating the standard mgmt format, and uses the driver's
> > TX path for it, but there isn't a corresponding ASSOC management frame that I
> > can see...
> > ...so I really can't tell how much control this firmware *does* give the host
> > regarding arbitrary 802.11 frame management.
> >
> > But that's pretty much business as usual for anybody but the vendor in
> > priorietary firmware land; I can't answer pretty much any question, other than
> > what I can glean from a driver.
>
> Yes. This change is to offload wpa3 features to host. It's well tested
> and doesn't impact existing features.

We appreciate it's well tested, but testing is still orthogonal to the
architectural questions. Architectural questions are important because
they affect the future maintainability of the mainline Linux wireless
stack. If the assumption is that *either* a driver is a cfg80211 driver
(with firmware-MLME, etc.) or a mac80211 driver (with host MLME), then
your series is breaking those assumptions. It may be harder to add
future additions to the mac80211 stack [*], if we have to add new
concerns of a non-mac80211 implementation in the mix.

Is it not possible to implement these features via CONNECT? Does your
firmware not provide any kind of NL80211_EXT_FEATURE_SAE_OFFLOAD
support, or otherwise handle WPA3 MLME?

Or, *does* your firmware also provide low-level 802.11 framing support?
If so, then maybe Johannes is suggesting you'd need a (new)
mac80211-based driver to go down this path... although I'm sure that's a
lot of work on its own.

Anyway, I definitely want Johannes's thoughts, although some additional
info from David might help too.

Brian

[*] We definitely need Johannes to weigh in here.

2024-03-25 17:17:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Wed, 2024-03-20 at 14:50 -0700, Brian Norris wrote:
>
> AFAICT, mwifiex firmware still isn't allowing "parsing and generation of
> 802.11 wireless frames" in any general form -- everything I see is still
> wrapped in custom firmware command protocols. I do see that the AUTH
> frame looks like it's essentially duplicating the standard mgmt format,
> and uses the driver's TX path for it, but there isn't a corresponding
> ASSOC management frame that I can see...

Fair point, I didn't really look beyond "auth creates auth frames and
sends them normally like any other frame" ...

> ...so I really can't tell how much control this firmware *does* give the
> host regarding arbitrary 802.11 frame management.

Perhaps indeed FW does require the assoc to be done with that command.

> But that's pretty much business as usual for anybody but the vendor in
> priorietary firmware land; I can't answer pretty much any question,
> other than what I can glean from a driver.

:)

johannes

2024-03-29 09:58:41

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Johannes Berg <[email protected]>
> Sent: Monday, March 25, 2024 11:59 PM
> To: Brian Norris <[email protected]>
> Cc: David Lin <[email protected]>; Francesco Dolcini
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Pete Hsieh <[email protected]>;
> rafael.beims <[email protected]>; Francesco Dolcini
> <[email protected]>
> Subject: Re: [EXT] Re: [PATCH v9 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 Wed, 2024-03-20 at 14:50 -0700, Brian Norris wrote:
> >
> > AFAICT, mwifiex firmware still isn't allowing "parsing and generation
> > of
> > 802.11 wireless frames" in any general form -- everything I see is
> > still wrapped in custom firmware command protocols. I do see that the
> > AUTH frame looks like it's essentially duplicating the standard mgmt
> > format, and uses the driver's TX path for it, but there isn't a
> > corresponding ASSOC management frame that I can see...
>
> Fair point, I didn't really look beyond "auth creates auth frames and sends
> them normally like any other frame" ...
>
> > ...so I really can't tell how much control this firmware *does* give
> > the host regarding arbitrary 802.11 frame management.
>
> Perhaps indeed FW does require the assoc to be done with that command.
>

Driver uses assoc command to communicate STA capability to FW.
FW doesn't need further process Auth so it's converted to 802.11 format and sent directly

> > But that's pretty much business as usual for anybody but the vendor in
> > priorietary firmware land; I can't answer pretty much any question,
> > other than what I can glean from a driver.
>
> :)
>
> johannes