* [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
@ 2024-03-06 2:00 David Lin
2024-03-06 2:00 ` [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode David Lin
` (3 more replies)
0 siblings, 4 replies; 44+ messages in thread
From: David Lin @ 2024-03-06 2:00 UTC (permalink / raw
To: linux-wireless
Cc: linux-kernel, briannorris, kvalo, francesco, tsung-hsien.hsieh,
David Lin, rafael.beims, Francesco Dolcini
With host mlme:
Tested-by: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD
Without host mlme:
Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # 88W8997-SD
This series add host based MLME support to the mwifiex driver, this
enables WPA3 support in both client and AP mode.
To enable WPA3, a firmware with corresponding V2 Key API support is
required.
The feature is currently only enabled on NXP IW416 (SD8978), and it
was internally validated by NXP QA team. Other NXP Wi-Fi chips
supported in current mwifiex are not affected by this change.
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode
2024-03-06 2:00 [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme David Lin
@ 2024-03-06 2:00 ` David Lin
2024-03-16 0:00 ` Brian Norris
2024-03-06 2:00 ` [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
` (2 subsequent siblings)
3 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-06 2:00 UTC (permalink / raw
To: linux-wireless
Cc: linux-kernel, briannorris, kvalo, francesco, tsung-hsien.hsieh,
David Lin, Francesco Dolcini
Add host based MLME to enable WPA3 functionalities in client mode.
This feature required a firmware with the corresponding V2 Key API
support. The feature (WPA3) is currently enabled and verified only
on IW416. Also, verified no regression with change when host MLME
is disabled.
Signed-off-by: David Lin <yu-hao.lin@nxp.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode
2024-03-06 2:00 [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme David Lin
2024-03-06 2:00 ` [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode David Lin
@ 2024-03-06 2:00 ` David Lin
2024-03-16 0:45 ` Brian Norris
2024-03-15 9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
2024-03-16 0:07 ` Brian Norris
3 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-06 2:00 UTC (permalink / raw
To: linux-wireless
Cc: linux-kernel, briannorris, kvalo, francesco, tsung-hsien.hsieh,
David Lin, Francesco Dolcini
Add host based MLME to enable WPA3 functionalities in AP mode.
This feature required a firmware with the corresponding V2 Key API
support. The feature (WPA3) is currently enabled and verified only
on IW416. Also, verified no regression with change when host MLME
is disabled.
Signed-off-by: David Lin <yu-hao.lin@nxp.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v9:
- remove unnecessary goto target.
v8:
- first full and complete patch to support host based MLME for AP
mode.
---
.../net/wireless/marvell/mwifiex/cfg80211.c | 79 +++++++-
drivers/net/wireless/marvell/mwifiex/cmdevt.c | 2 +
drivers/net/wireless/marvell/mwifiex/fw.h | 21 +++
drivers/net/wireless/marvell/mwifiex/ioctl.h | 5 +
.../wireless/marvell/mwifiex/sta_cmdresp.c | 2 +
.../net/wireless/marvell/mwifiex/uap_cmd.c | 171 ++++++++++++++++++
drivers/net/wireless/marvell/mwifiex/util.c | 24 +++
7 files changed, 301 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index bcf4f87dcaab..316f2ada9bf8 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -221,6 +221,26 @@ mwifiex_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
return 0;
}
+ if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
+ if (ieee80211_is_auth(mgmt->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "auth: send auth to %pM\n", mgmt->da);
+ if (ieee80211_is_deauth(mgmt->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "auth: send deauth to %pM\n", mgmt->da);
+ if (ieee80211_is_disassoc(mgmt->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "assoc: send disassoc to %pM\n", mgmt->da);
+ if (ieee80211_is_assoc_resp(mgmt->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "assoc: send assoc resp to %pM\n",
+ mgmt->da);
+ if (ieee80211_is_reassoc_resp(mgmt->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "assoc: send reassoc resp to %pM\n",
+ mgmt->da);
+ }
+
pkt_len = len + ETH_ALEN;
skb = dev_alloc_skb(MWIFIEX_MIN_DATA_HEADER_LEN +
MWIFIEX_MGMT_FRAME_HEADER_SIZE +
@@ -505,6 +525,9 @@ mwifiex_cfg80211_set_default_mgmt_key(struct wiphy *wiphy,
wiphy_dbg(wiphy, "set default mgmt key, key index=%d\n", key_index);
+ if (priv->adapter->host_mlme_enabled)
+ return 0;
+
memset(&encrypt_key, 0, sizeof(struct mwifiex_ds_encrypt_key));
encrypt_key.key_len = WLAN_KEY_LEN_CCMP;
encrypt_key.key_index = key_index;
@@ -1712,7 +1735,7 @@ static const u32 mwifiex_cipher_suites[] = {
};
/* Supported mgmt frame types to be advertised to cfg80211 */
-static const struct ieee80211_txrx_stypes
+static struct ieee80211_txrx_stypes
mwifiex_mgmt_stypes[NUM_NL80211_IFTYPES] = {
[NL80211_IFTYPE_STATION] = {
.tx = BIT(IEEE80211_STYPE_ACTION >> 4) |
@@ -3951,12 +3974,43 @@ mwifiex_cfg80211_tdls_cancel_chan_switch(struct wiphy *wiphy,
}
}
+static int
+mwifiex_cfg80211_uap_add_station(struct mwifiex_private *priv, const u8 *mac,
+ struct station_parameters *params)
+{
+ struct mwifiex_sta_info add_sta;
+ int ret;
+
+ memcpy(add_sta.peer_mac, mac, ETH_ALEN);
+ add_sta.params = params;
+
+ ret = mwifiex_send_cmd(priv, HostCmd_CMD_ADD_NEW_STATION,
+ HostCmd_ACT_ADD_STA, 0, (void *)&add_sta, true);
+
+ if (!ret) {
+ struct station_info *sinfo;
+
+ sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+ if (!sinfo)
+ return -ENOMEM;
+
+ cfg80211_new_sta(priv->netdev, mac, sinfo, GFP_KERNEL);
+ kfree(sinfo);
+ }
+
+ return ret;
+}
+
static int
mwifiex_cfg80211_add_station(struct wiphy *wiphy, struct net_device *dev,
const u8 *mac, struct station_parameters *params)
{
struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
+ if (priv->adapter->host_mlme_enabled &&
+ (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP))
+ return mwifiex_cfg80211_uap_add_station(priv, mac, params);
+
if (!(params->sta_flags_set & BIT(NL80211_STA_FLAG_TDLS_PEER)))
return -EOPNOTSUPP;
@@ -4194,6 +4248,10 @@ mwifiex_cfg80211_change_station(struct wiphy *wiphy, struct net_device *dev,
int ret;
struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
+ if (priv->adapter->host_mlme_enabled &&
+ (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP))
+ return 0;
+
/* we support change_station handler only for TDLS peers*/
if (!(params->sta_flags_set & BIT(NL80211_STA_FLAG_TDLS_PEER)))
return -EOPNOTSUPP;
@@ -4663,6 +4721,17 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
}
wiphy->max_scan_ssids = MWIFIEX_MAX_SSID_LIST_LENGTH;
wiphy->max_scan_ie_len = MWIFIEX_MAX_VSIE_LEN;
+ if (adapter->host_mlme_enabled) {
+ mwifiex_mgmt_stypes[NL80211_IFTYPE_AP].tx = 0xffff;
+ mwifiex_mgmt_stypes[NL80211_IFTYPE_AP].rx =
+ BIT(IEEE80211_STYPE_ASSOC_REQ >> 4) |
+ BIT(IEEE80211_STYPE_REASSOC_REQ >> 4) |
+ BIT(IEEE80211_STYPE_PROBE_REQ >> 4) |
+ BIT(IEEE80211_STYPE_DISASSOC >> 4) |
+ BIT(IEEE80211_STYPE_AUTH >> 4) |
+ BIT(IEEE80211_STYPE_DEAUTH >> 4) |
+ BIT(IEEE80211_STYPE_ACTION >> 4);
+ }
wiphy->mgmt_stypes = mwifiex_mgmt_stypes;
wiphy->max_remain_on_channel_duration = 5000;
wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
@@ -4705,14 +4774,18 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter *adapter)
ether_addr_copy(wiphy->perm_addr, adapter->perm_addr);
wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;
- wiphy->flags |= WIPHY_FLAG_HAVE_AP_SME |
- WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD |
+ wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD |
WIPHY_FLAG_AP_UAPSD |
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL |
WIPHY_FLAG_HAS_CHANNEL_SWITCH |
WIPHY_FLAG_NETNS_OK |
WIPHY_FLAG_PS_ON_BY_DEFAULT;
+ if (adapter->host_mlme_enabled)
+ wiphy->flags |= WIPHY_FLAG_REPORTS_OBSS;
+ else
+ wiphy->flags |= WIPHY_FLAG_HAVE_AP_SME;
+
if (ISSUPP_TDLS_ENABLED(adapter->fw_cap_info))
wiphy->flags |= WIPHY_FLAG_SUPPORTS_TDLS |
WIPHY_FLAG_TDLS_EXTERNAL_SETUP;
diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index da983e27023c..ea6ebc9c23ef 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -635,6 +635,8 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
case HostCmd_CMD_UAP_STA_DEAUTH:
case HOST_CMD_APCMD_SYS_RESET:
case HOST_CMD_APCMD_STA_LIST:
+ case HostCmd_CMD_CHAN_REPORT_REQUEST:
+ case HostCmd_CMD_ADD_NEW_STATION:
ret = mwifiex_uap_prepare_cmd(priv, cmd_no, cmd_action,
cmd_oid, data_buf,
cmd_ptr);
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 0f89b86aa527..65799ae3bc72 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -211,6 +211,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
#define TLV_TYPE_CHAN_ATTR_CFG (PROPRIETARY_TLV_BASE_ID + 237)
#define TLV_TYPE_MAX_CONN (PROPRIETARY_TLV_BASE_ID + 279)
#define TLV_TYPE_HOST_MLME (PROPRIETARY_TLV_BASE_ID + 307)
+#define TLV_TYPE_UAP_STA_FLAGS (PROPRIETARY_TLV_BASE_ID + 313)
#define TLV_TYPE_SAE_PWE_MODE (PROPRIETARY_TLV_BASE_ID + 339)
#define MWIFIEX_TX_DATA_BUF_SIZE_2K 2048
@@ -407,6 +408,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
#define HostCmd_CMD_STA_CONFIGURE 0x023f
#define HostCmd_CMD_CHAN_REGION_CFG 0x0242
#define HostCmd_CMD_PACKET_AGGR_CTRL 0x0251
+#define HostCmd_CMD_ADD_NEW_STATION 0x025f
#define PROTOCOL_NO_SECURITY 0x01
#define PROTOCOL_STATIC_WEP 0x02
@@ -417,6 +419,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
#define KEY_MGMT_NONE 0x04
#define KEY_MGMT_PSK 0x02
#define KEY_MGMT_EAP 0x01
+#define KEY_MGMT_SAE 0x400
#define CIPHER_TKIP 0x04
#define CIPHER_AES_CCMP 0x08
#define VALID_CIPHER_BITMAP 0x0c
@@ -502,6 +505,9 @@ enum mwifiex_channel_flags {
#define HostCmd_ACT_GET_TX 0x0008
#define HostCmd_ACT_GET_BOTH 0x000c
+#define HostCmd_ACT_REMOVE_STA 0x0
+#define HostCmd_ACT_ADD_STA 0x1
+
#define RF_ANTENNA_AUTO 0xFFFF
#define HostCmd_SET_SEQ_NO_BSS_INFO(seq, num, type) \
@@ -2331,6 +2337,20 @@ struct host_cmd_ds_sta_configure {
u8 tlv_buffer[];
} __packed;
+struct mwifiex_ie_types_sta_flag {
+ struct mwifiex_ie_types_header header;
+ __le32 sta_flags;
+} __packed;
+
+struct host_cmd_ds_add_station {
+ __le16 action;
+ __le16 aid;
+ u8 peer_mac[ETH_ALEN];
+ __le32 listen_interval;
+ __le16 cap_info;
+ u8 tlv[];
+} __packed;
+
struct host_cmd_ds_command {
__le16 command;
__le16 size;
@@ -2409,6 +2429,7 @@ struct host_cmd_ds_command {
struct host_cmd_ds_chan_region_cfg reg_cfg;
struct host_cmd_ds_pkt_aggr_ctrl pkt_aggr_ctrl;
struct host_cmd_ds_sta_configure sta_cfg;
+ struct host_cmd_ds_add_station sta_info;
} params;
} __packed;
diff --git a/drivers/net/wireless/marvell/mwifiex/ioctl.h b/drivers/net/wireless/marvell/mwifiex/ioctl.h
index e8825f302de8..516159b721d3 100644
--- a/drivers/net/wireless/marvell/mwifiex/ioctl.h
+++ b/drivers/net/wireless/marvell/mwifiex/ioctl.h
@@ -158,6 +158,11 @@ struct mwifiex_bss_info {
u8 bssid[ETH_ALEN];
};
+struct mwifiex_sta_info {
+ u8 peer_mac[ETH_ALEN];
+ struct station_parameters *params;
+};
+
#define MAX_NUM_TID 8
#define MAX_RX_WINSIZE 64
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 7b69d27e0c0e..9c53825f222d 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -1398,6 +1398,8 @@ int mwifiex_process_sta_cmdresp(struct mwifiex_private *priv, u16 cmdresp_no,
break;
case HostCmd_CMD_UAP_STA_DEAUTH:
break;
+ case HostCmd_CMD_ADD_NEW_STATION:
+ break;
case HOST_CMD_APCMD_SYS_RESET:
break;
case HostCmd_CMD_MEF_CFG:
diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
index 491e36611909..073c665183b3 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
@@ -72,6 +72,10 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
bss_config->key_mgmt = KEY_MGMT_PSK;
}
break;
+ case WLAN_AKM_SUITE_SAE:
+ bss_config->protocol = PROTOCOL_WPA2;
+ bss_config->key_mgmt = KEY_MGMT_SAE;
+ break;
default:
break;
}
@@ -751,6 +755,28 @@ mwifiex_cmd_uap_sys_config(struct host_cmd_ds_command *cmd, u16 cmd_action,
return 0;
}
+/* This function prepares AP start up command with or without host MLME
+ */
+static void mwifiex_cmd_uap_bss_start(struct mwifiex_private *priv,
+ struct host_cmd_ds_command *cmd)
+{
+ struct mwifiex_ie_types_host_mlme *tlv;
+ int size;
+
+ cmd->command = cpu_to_le16(HostCmd_CMD_UAP_BSS_START);
+ size = S_DS_GEN;
+
+ if (priv->adapter->host_mlme_enabled) {
+ tlv = (struct mwifiex_ie_types_host_mlme *)((u8 *)cmd + size);
+ tlv->header.type = cpu_to_le16(TLV_TYPE_HOST_MLME);
+ tlv->header.len = cpu_to_le16(sizeof(tlv->host_mlme));
+ tlv->host_mlme = 1;
+ size += sizeof(struct mwifiex_ie_types_host_mlme);
+ }
+
+ cmd->size = cpu_to_le16(size);
+}
+
/* This function prepares AP specific deauth command with mac supplied in
* function parameter.
*/
@@ -768,6 +794,144 @@ static int mwifiex_cmd_uap_sta_deauth(struct mwifiex_private *priv,
return 0;
}
+/* This function prepares AP specific add station command.
+ */
+static int mwifiex_cmd_uap_add_station(struct mwifiex_private *priv,
+ struct host_cmd_ds_command *cmd,
+ u16 cmd_action, void *data_buf)
+{
+ struct host_cmd_ds_add_station *new_sta = &cmd->params.sta_info;
+ struct mwifiex_sta_info *add_sta = (struct mwifiex_sta_info *)data_buf;
+ struct station_parameters *params = add_sta->params;
+ struct mwifiex_sta_node *sta_ptr;
+ u8 *pos;
+ u8 qos_capa;
+ u16 header_len = sizeof(struct mwifiex_ie_types_header);
+ u16 tlv_len;
+ int size;
+ struct mwifiex_ie_types_data *tlv;
+ struct mwifiex_ie_types_sta_flag *sta_flag;
+ int i;
+
+ cmd->command = cpu_to_le16(HostCmd_CMD_ADD_NEW_STATION);
+ new_sta->action = cpu_to_le16(cmd_action);
+ size = sizeof(struct host_cmd_ds_add_station) + S_DS_GEN;
+
+ if (cmd_action == HostCmd_ACT_ADD_STA)
+ sta_ptr = mwifiex_add_sta_entry(priv, add_sta->peer_mac);
+ else
+ sta_ptr = mwifiex_get_sta_entry(priv, add_sta->peer_mac);
+
+ if (!sta_ptr)
+ return -1;
+
+ memcpy(new_sta->peer_mac, add_sta->peer_mac, ETH_ALEN);
+
+ if (cmd_action == HostCmd_ACT_REMOVE_STA) {
+ cmd->size = cpu_to_le16(size);
+ return 0;
+ }
+
+ new_sta->aid = cpu_to_le16(params->aid);
+ new_sta->listen_interval = cpu_to_le32(params->listen_interval);
+ new_sta->cap_info = cpu_to_le16(params->capability);
+
+ pos = new_sta->tlv;
+
+ if (params->sta_flags_set & NL80211_STA_FLAG_WME)
+ sta_ptr->is_wmm_enabled = 1;
+ sta_flag = (struct mwifiex_ie_types_sta_flag *)pos;
+ sta_flag->header.type = cpu_to_le16(TLV_TYPE_UAP_STA_FLAGS);
+ sta_flag->header.len = cpu_to_le16(sizeof(__le32));
+ sta_flag->sta_flags = cpu_to_le32(params->sta_flags_set);
+ pos += sizeof(struct mwifiex_ie_types_sta_flag);
+ size += sizeof(struct mwifiex_ie_types_sta_flag);
+
+ if (params->ext_capab_len) {
+ tlv = (struct mwifiex_ie_types_data *)pos;
+ tlv->header.type = cpu_to_le16(WLAN_EID_EXT_CAPABILITY);
+ tlv_len = params->ext_capab_len;
+ tlv->header.len = cpu_to_le16(tlv_len);
+ memcpy(tlv->data, params->ext_capab, tlv_len);
+ pos += (header_len + tlv_len);
+ size += (header_len + tlv_len);
+ }
+
+ if (params->link_sta_params.supported_rates_len) {
+ tlv = (struct mwifiex_ie_types_data *)pos;
+ tlv->header.type = cpu_to_le16(WLAN_EID_SUPP_RATES);
+ tlv_len = params->link_sta_params.supported_rates_len;
+ tlv->header.len = cpu_to_le16(tlv_len);
+ memcpy(tlv->data,
+ params->link_sta_params.supported_rates, tlv_len);
+ pos += (header_len + tlv_len);
+ size += (header_len + tlv_len);
+ }
+
+ if (params->uapsd_queues || params->max_sp) {
+ tlv = (struct mwifiex_ie_types_data *)pos;
+ tlv->header.type = cpu_to_le16(WLAN_EID_QOS_CAPA);
+ tlv_len = sizeof(qos_capa);
+ tlv->header.len = cpu_to_le16(tlv_len);
+ qos_capa = params->uapsd_queues | (params->max_sp << 5);
+ memcpy(tlv->data, &qos_capa, tlv_len);
+ pos += (header_len + tlv_len);
+ size += (header_len + tlv_len);
+ sta_ptr->is_wmm_enabled = 1;
+ }
+
+ if (params->link_sta_params.ht_capa) {
+ tlv = (struct mwifiex_ie_types_data *)pos;
+ tlv->header.type = cpu_to_le16(WLAN_EID_HT_CAPABILITY);
+ tlv_len = sizeof(struct ieee80211_ht_cap);
+ tlv->header.len = cpu_to_le16(tlv_len);
+ memcpy(tlv->data, params->link_sta_params.ht_capa, tlv_len);
+ pos += (header_len + tlv_len);
+ size += (header_len + tlv_len);
+ sta_ptr->is_11n_enabled = 1;
+ sta_ptr->max_amsdu =
+ le16_to_cpu(params->link_sta_params.ht_capa->cap_info) &
+ IEEE80211_HT_CAP_MAX_AMSDU ?
+ MWIFIEX_TX_DATA_BUF_SIZE_8K :
+ MWIFIEX_TX_DATA_BUF_SIZE_4K;
+ }
+
+ if (params->link_sta_params.vht_capa) {
+ tlv = (struct mwifiex_ie_types_data *)pos;
+ tlv->header.type = cpu_to_le16(WLAN_EID_VHT_CAPABILITY);
+ tlv_len = sizeof(struct ieee80211_vht_cap);
+ tlv->header.len = cpu_to_le16(tlv_len);
+ memcpy(tlv->data, params->link_sta_params.vht_capa, tlv_len);
+ pos += (header_len + tlv_len);
+ size += (header_len + tlv_len);
+ sta_ptr->is_11ac_enabled = 1;
+ }
+
+ if (params->link_sta_params.opmode_notif_used) {
+ tlv = (struct mwifiex_ie_types_data *)pos;
+ tlv->header.type = cpu_to_le16(WLAN_EID_OPMODE_NOTIF);
+ tlv_len = sizeof(u8);
+ tlv->header.len = cpu_to_le16(tlv_len);
+ memcpy(tlv->data, ¶ms->link_sta_params.opmode_notif,
+ tlv_len);
+ pos += (header_len + tlv_len);
+ size += (header_len + tlv_len);
+ }
+
+ for (i = 0; i < MAX_NUM_TID; i++) {
+ if (sta_ptr->is_11n_enabled)
+ sta_ptr->ampdu_sta[i] =
+ priv->aggr_prio_tbl[i].ampdu_user;
+ else
+ sta_ptr->ampdu_sta[i] = BA_STREAM_NOT_ALLOWED;
+ }
+
+ memset(sta_ptr->rx_seq, 0xff, sizeof(sta_ptr->rx_seq));
+ cmd->size = cpu_to_le16(size);
+
+ return 0;
+}
+
/* This function prepares the AP specific commands before sending them
* to the firmware.
* This is a generic function which calls specific command preparation
@@ -785,6 +949,8 @@ int mwifiex_uap_prepare_cmd(struct mwifiex_private *priv, u16 cmd_no,
return -1;
break;
case HostCmd_CMD_UAP_BSS_START:
+ mwifiex_cmd_uap_bss_start(priv, cmd);
+ break;
case HostCmd_CMD_UAP_BSS_STOP:
case HOST_CMD_APCMD_SYS_RESET:
case HOST_CMD_APCMD_STA_LIST:
@@ -800,6 +966,11 @@ int mwifiex_uap_prepare_cmd(struct mwifiex_private *priv, u16 cmd_no,
data_buf))
return -1;
break;
+ case HostCmd_CMD_ADD_NEW_STATION:
+ if (mwifiex_cmd_uap_add_station(priv, cmd, cmd_action,
+ data_buf))
+ return -1;
+ break;
default:
mwifiex_dbg(priv->adapter, ERROR,
"PREP_CMD: unknown cmd %#x\n", cmd_no);
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index af82f0401c63..c0614c246b5f 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -497,6 +497,30 @@ mwifiex_process_mgmt_packet(struct mwifiex_private *priv,
cfg80211_rx_mlme_mgmt(priv->netdev, skb->data, pkt_len);
}
+ if (priv->adapter->host_mlme_enabled &&
+ (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP)) {
+ if (ieee80211_is_auth(ieee_hdr->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "auth: receive auth from %pM\n",
+ ieee_hdr->addr2);
+ if (ieee80211_is_deauth(ieee_hdr->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "auth: receive deauth from %pM\n",
+ ieee_hdr->addr2);
+ if (ieee80211_is_disassoc(ieee_hdr->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "assoc: receive disassoc from %pM\n",
+ ieee_hdr->addr2);
+ if (ieee80211_is_assoc_req(ieee_hdr->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "assoc: receive assoc req from %pM\n",
+ ieee_hdr->addr2);
+ if (ieee80211_is_reassoc_req(ieee_hdr->frame_control))
+ mwifiex_dbg(priv->adapter, MSG,
+ "assoc: receive reassoc req from %pM\n",
+ ieee_hdr->addr2);
+ }
+
cfg80211_rx_mgmt(&priv->wdev, priv->roc_cfg.chan.center_freq,
CAL_RSSI(rx_pd->snr, rx_pd->nf), skb->data, pkt_len,
0);
--
2.34.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-06 2:00 [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme David Lin
2024-03-06 2:00 ` [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode David Lin
2024-03-06 2:00 ` [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
@ 2024-03-15 9:49 ` Francesco Dolcini
2024-03-15 23:59 ` Brian Norris
` (2 more replies)
2024-03-16 0:07 ` Brian Norris
3 siblings, 3 replies; 44+ messages in thread
From: Francesco Dolcini @ 2024-03-15 9:49 UTC (permalink / raw
To: briannorris, kvalo
Cc: linux-wireless, linux-kernel, David Lin, francesco,
tsung-hsien.hsieh, rafael.beims, Francesco Dolcini
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.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-15 9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
@ 2024-03-15 23:59 ` Brian Norris
2024-03-18 11:28 ` Francesco Dolcini
2024-03-16 0:49 ` Brian Norris
2024-03-18 9:24 ` Kalle Valo
2 siblings, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-15 23:59 UTC (permalink / raw
To: Francesco Dolcini
Cc: kvalo, linux-wireless, linux-kernel, David Lin, tsung-hsien.hsieh,
rafael.beims, Francesco Dolcini
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode
2024-03-06 2:00 ` [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode David Lin
@ 2024-03-16 0:00 ` Brian Norris
2024-03-18 2:00 ` [EXT] " David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-16 0:00 UTC (permalink / raw
To: David Lin
Cc: linux-wireless, linux-kernel, kvalo, francesco, tsung-hsien.hsieh,
Francesco Dolcini
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 <yu-hao.lin@nxp.com>
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>
> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-06 2:00 [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme David Lin
` (2 preceding siblings ...)
2024-03-15 9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
@ 2024-03-16 0:07 ` Brian Norris
2024-03-18 2:20 ` [EXT] " David Lin
3 siblings, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-16 0:07 UTC (permalink / raw
To: David Lin
Cc: linux-wireless, linux-kernel, kvalo, francesco, tsung-hsien.hsieh,
rafael.beims, Francesco Dolcini
On Wed, Mar 06, 2024 at 10:00:51AM +0800, David Lin wrote:
> With host mlme:
> Tested-by: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD
> Without host mlme:
> Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # 88W8997-SD
>
> This series add host based MLME support to the mwifiex driver, this
> enables WPA3 support in both client and AP mode.
> To enable WPA3, a firmware with corresponding V2 Key API support is
> required.
> The feature is currently only enabled on NXP IW416 (SD8978), and it
> was internally validated by NXP QA team. Other NXP Wi-Fi chips
> supported in current mwifiex are not affected by this change.
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode
2024-03-06 2:00 ` [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
@ 2024-03-16 0:45 ` Brian Norris
2024-03-18 2:04 ` [EXT] " David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-16 0:45 UTC (permalink / raw
To: David Lin
Cc: linux-wireless, linux-kernel, kvalo, francesco, tsung-hsien.hsieh,
Francesco Dolcini
On Wed, Mar 06, 2024 at 10:00:53AM +0800, David Lin wrote:
> Add host based MLME to enable WPA3 functionalities in AP mode.
> This feature required a firmware with the corresponding V2 Key API
> support. The feature (WPA3) is currently enabled and verified only
> on IW416. Also, verified no regression with change when host MLME
> is disabled.
>
> Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Quick pass for now; nothing jumps out at me today, but I'll give a
better look/Ack next week:
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -3951,12 +3974,43 @@ mwifiex_cfg80211_tdls_cancel_chan_switch(struct wiphy *wiphy,
> }
> }
>
> +static int
> +mwifiex_cfg80211_uap_add_station(struct mwifiex_private *priv, const u8 *mac,
> + struct station_parameters *params)
> +{
> + struct mwifiex_sta_info add_sta;
> + int ret;
> +
> + memcpy(add_sta.peer_mac, mac, ETH_ALEN);
> + add_sta.params = params;
> +
> + ret = mwifiex_send_cmd(priv, HostCmd_CMD_ADD_NEW_STATION,
> + HostCmd_ACT_ADD_STA, 0, (void *)&add_sta, true);
> +
> + if (!ret) {
> + struct station_info *sinfo;
> +
> + sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
Couldn't this just be stack allocation?
struct staion_info sinfo;
cfg80211_new_sta(priv->netdev, mac, &sinfo, GFP_KERNEL);
I'm not sure you need to kzalloc() something here, if you're freeing it
a few lines later.
> + if (!sinfo)
> + return -ENOMEM;
> +
> + cfg80211_new_sta(priv->netdev, mac, sinfo, GFP_KERNEL);
> + kfree(sinfo);
> + }
> +
> + return ret;
> +}
Brian
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-15 9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
2024-03-15 23:59 ` Brian Norris
@ 2024-03-16 0:49 ` Brian Norris
2024-03-19 12:12 ` Johannes Berg
2024-03-18 9:24 ` Kalle Valo
2 siblings, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-16 0:49 UTC (permalink / raw
To: Francesco Dolcini
Cc: kvalo, linux-wireless, linux-kernel, David Lin, tsung-hsien.hsieh,
rafael.beims, Francesco Dolcini, Johannes Berg
+ 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode
2024-03-16 0:00 ` Brian Norris
@ 2024-03-18 2:00 ` David Lin
2024-03-18 11:41 ` Francesco Dolcini
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-18 2:00 UTC (permalink / raw
To: Brian Norris
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
Francesco Dolcini
> From: Brian Norris <briannorris@chromium.org>
> Sent: Saturday, March 16, 2024 8:00 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: [EXT] Re: [PATCH 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 <yu-hao.lin@nxp.com>
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> >
> > 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode
2024-03-16 0:45 ` Brian Norris
@ 2024-03-18 2:04 ` David Lin
2024-04-18 3:37 ` David Lin
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-18 2:04 UTC (permalink / raw
To: Brian Norris
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
Francesco Dolcini
> From: Brian Norris <briannorris@chromium.org>
> Sent: Saturday, March 16, 2024 8:45 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: [EXT] Re: [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Wed, Mar 06, 2024 at 10:00:53AM +0800, David Lin wrote:
> > Add host based MLME to enable WPA3 functionalities in AP mode.
> > This feature required a firmware with the corresponding V2 Key API
> > support. The feature (WPA3) is currently enabled and verified only on
> > IW416. Also, verified no regression with change when host MLME is
> > disabled.
> >
> > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>
> Quick pass for now; nothing jumps out at me today, but I'll give a better
> look/Ack next week:
>
> > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>
>
> > @@ -3951,12 +3974,43 @@
> mwifiex_cfg80211_tdls_cancel_chan_switch(struct wiphy *wiphy,
> > }
> > }
> >
> > +static int
> > +mwifiex_cfg80211_uap_add_station(struct mwifiex_private *priv, const u8
> *mac,
> > + struct station_parameters *params) {
> > + struct mwifiex_sta_info add_sta;
> > + int ret;
> > +
> > + memcpy(add_sta.peer_mac, mac, ETH_ALEN);
> > + add_sta.params = params;
> > +
> > + ret = mwifiex_send_cmd(priv, HostCmd_CMD_ADD_NEW_STATION,
> > + HostCmd_ACT_ADD_STA, 0, (void
> *)&add_sta,
> > + true);
> > +
> > + if (!ret) {
> > + struct station_info *sinfo;
> > +
> > + sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
>
> Couldn't this just be stack allocation?
>
> struct staion_info sinfo;
>
> cfg80211_new_sta(priv->netdev, mac, &sinfo,
> GFP_KERNEL);
>
> I'm not sure you need to kzalloc() something here, if you're freeing it a few
> lines later.
>
Will modify it in patch v10.
>
> > + if (!sinfo)
> > + return -ENOMEM;
> > +
> > + cfg80211_new_sta(priv->netdev, mac, sinfo, GFP_KERNEL);
> > + kfree(sinfo);
> > + }
> > +
> > + return ret;
> > +}
>
> Brian
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-16 0:07 ` Brian Norris
@ 2024-03-18 2:20 ` David Lin
2024-03-18 11:45 ` Francesco Dolcini
2024-03-20 21:13 ` Brian Norris
0 siblings, 2 replies; 44+ messages in thread
From: David Lin @ 2024-03-18 2:20 UTC (permalink / raw
To: Brian Norris
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh, rafael.beims,
Francesco Dolcini
> From: Brian Norris <briannorris@chromium.org>
> Sent: Saturday, March 16, 2024 8:08 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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: <rafael.beims@toradex.com> #Verdin AM62 IW416 SD Without
> > host mlme:
> > Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> #
> > 88W8997-SD
> >
> > This series add host based MLME support to the mwifiex driver, this
> > enables WPA3 support in both client and AP mode.
> > To enable WPA3, a firmware with corresponding V2 Key API support is
> > required.
> > The feature is currently only enabled on NXP IW416 (SD8978), and it
> > was internally validated by NXP QA team. Other NXP Wi-Fi chips
> > supported in current mwifiex are not affected by this change.
>
> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-15 9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
2024-03-15 23:59 ` Brian Norris
2024-03-16 0:49 ` Brian Norris
@ 2024-03-18 9:24 ` Kalle Valo
2024-03-19 1:40 ` [EXT] " David Lin
2024-03-20 21:28 ` Brian Norris
2 siblings, 2 replies; 44+ messages in thread
From: Kalle Valo @ 2024-03-18 9:24 UTC (permalink / raw
To: Francesco Dolcini
Cc: briannorris, linux-wireless, linux-kernel, David Lin,
tsung-hsien.hsieh, rafael.beims, Francesco Dolcini
Francesco Dolcini <francesco@dolcini.it> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-15 23:59 ` Brian Norris
@ 2024-03-18 11:28 ` Francesco Dolcini
2024-03-19 10:33 ` Kalle Valo
0 siblings, 1 reply; 44+ messages in thread
From: Francesco Dolcini @ 2024-03-18 11:28 UTC (permalink / raw
To: Brian Norris
Cc: Francesco Dolcini, kvalo, linux-wireless, linux-kernel, David Lin,
tsung-hsien.hsieh, rafael.beims, Francesco Dolcini
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode
2024-03-18 2:00 ` [EXT] " David Lin
@ 2024-03-18 11:41 ` Francesco Dolcini
2024-03-19 1:36 ` [EXT] " David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Francesco Dolcini @ 2024-03-18 11:41 UTC (permalink / raw
To: David Lin
Cc: Brian Norris, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, kvalo@kernel.org,
francesco@dolcini.it, Pete Hsieh, Francesco Dolcini
Hello David,
On Mon, Mar 18, 2024 at 02:00:35AM +0000, David Lin wrote:
> > From: Brian Norris <briannorris@chromium.org>
...
> > > .../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).
What I explicitly asked was to not add code in a patch, and fix the
newly added code in a following patch. What you are supposed to do is to
just amend the original code when you get review feedback.
Splitting a big patch into multiple patches is welcome to easier review,
and this needs to be done breaking down in logical pieces keeping in
mind also bisect-ability.
This [1] is an example of the addition of a relatively big new driver,
and you can see that the series is broken down in smaller patches like
"Add skeleton PowerVR driver", with intermediate steps that were
non-functional, but they were building fine, they were correct and they
were enabling more effective code review.
Unfortunately, as Brian agreed here, there was no easy way to do it for
this patch.
Francesco
[1] https://lore.kernel.org/all/cover.1700668843.git.donald.robson@imgtec.com/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-18 2:20 ` [EXT] " David Lin
@ 2024-03-18 11:45 ` Francesco Dolcini
2024-03-20 21:13 ` Brian Norris
1 sibling, 0 replies; 44+ messages in thread
From: Francesco Dolcini @ 2024-03-18 11:45 UTC (permalink / raw
To: David Lin, Brian Norris
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh, rafael.beims,
Francesco Dolcini
Hello David and Brian,
On Mon, Mar 18, 2024 at 02:20:56AM +0000, David Lin wrote:
> > From: Brian Norris <briannorris@chromium.org>
> > 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode
2024-03-18 11:41 ` Francesco Dolcini
@ 2024-03-19 1:36 ` David Lin
0 siblings, 0 replies; 44+ messages in thread
From: David Lin @ 2024-03-19 1:36 UTC (permalink / raw
To: Francesco Dolcini
Cc: Brian Norris, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, kvalo@kernel.org, Pete Hsieh,
Francesco Dolcini
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Monday, March 18, 2024 7:42 PM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: Brian Norris <briannorris@chromium.org>; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; kvalo@kernel.org; francesco@dolcini.it; Pete
> Hsieh <tsung-hsien.hsieh@nxp.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: [EXT] Re: [PATCH 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
>
>
> Hello David,
>
> On Mon, Mar 18, 2024 at 02:00:35AM +0000, David Lin wrote:
> > > From: Brian Norris <briannorris@chromium.org>
> ...
> > > > .../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).
>
> What I explicitly asked was to not add code in a patch, and fix the newly added
> code in a following patch. What you are supposed to do is to just amend the
> original code when you get review feedback.
>
Yes. I will do that for patch v10 and keep all 'Review-by:' and 'Tested-by:' tags.
> Splitting a big patch into multiple patches is welcome to easier review, and this
> needs to be done breaking down in logical pieces keeping in mind also
> bisect-ability.
>
> This [1] is an example of the addition of a relatively big new driver, and you
> can see that the series is broken down in smaller patches like "Add skeleton
> PowerVR driver", with intermediate steps that were non-functional, but they
> were building fine, they were correct and they were enabling more effective
> code review.
>
> Unfortunately, as Brian agreed here, there was no easy way to do it for this
> patch.
>
> Francesco
>
> [1]
> https://lore.kern/
> el.org%2Fall%2Fcover.1700668843.git.donald.robson%40imgtec.com%2F&data
> =05%7C02%7Cyu-hao.lin%40nxp.com%7C6ce04f812a5b4dd7e74908dc47405cc
> 7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6384635889617189
> 65%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
> CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=HPgEuiKfmtEicp
> PfTz57piOdOoT0iQfo5qnPp9p6jlY%3D&reserved=0
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-18 9:24 ` Kalle Valo
@ 2024-03-19 1:40 ` David Lin
2024-03-20 21:28 ` Brian Norris
1 sibling, 0 replies; 44+ messages in thread
From: David Lin @ 2024-03-19 1:40 UTC (permalink / raw
To: Kalle Valo, Francesco Dolcini
Cc: briannorris@chromium.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, Pete Hsieh, rafael.beims,
Francesco Dolcini
> From: Kalle Valo <kvalo@kernel.org>
> Sent: Monday, March 18, 2024 5:25 PM
> To: Francesco Dolcini <francesco@dolcini.it>
> Cc: briannorris@chromium.org; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; David Lin <yu-hao.lin@nxp.com>; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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 <francesco@dolcini.it> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-18 11:28 ` Francesco Dolcini
@ 2024-03-19 10:33 ` Kalle Valo
2024-03-20 21:06 ` Brian Norris
0 siblings, 1 reply; 44+ messages in thread
From: Kalle Valo @ 2024-03-19 10:33 UTC (permalink / raw
To: Francesco Dolcini
Cc: Brian Norris, linux-wireless, linux-kernel, David Lin,
tsung-hsien.hsieh, rafael.beims, Francesco Dolcini
Francesco Dolcini <francesco@dolcini.it> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-16 0:49 ` Brian Norris
@ 2024-03-19 12:12 ` Johannes Berg
2024-03-20 0:59 ` [EXT] " David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Johannes Berg @ 2024-03-19 12:12 UTC (permalink / raw
To: Brian Norris, Francesco Dolcini
Cc: kvalo, linux-wireless, linux-kernel, David Lin, tsung-hsien.hsieh,
rafael.beims, Francesco Dolcini
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-19 12:12 ` Johannes Berg
@ 2024-03-20 0:59 ` David Lin
2024-03-20 1:10 ` David Lin
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-20 0:59 UTC (permalink / raw
To: Johannes Berg, Brian Norris, Francesco Dolcini
Cc: kvalo@kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, Pete Hsieh, rafael.beims,
Francesco Dolcini
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Tuesday, March 19, 2024 8:13 PM
> To: Brian Norris <briannorris@chromium.org>; Francesco Dolcini
> <francesco@dolcini.it>
> Cc: kvalo@kernel.org; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; David Lin <yu-hao.lin@nxp.com>; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 0:59 ` [EXT] " David Lin
@ 2024-03-20 1:10 ` David Lin
2024-03-20 9:12 ` Johannes Berg
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-20 1:10 UTC (permalink / raw
To: Johannes Berg, Brian Norris, Francesco Dolcini
Cc: kvalo@kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, Pete Hsieh, rafael.beims,
Francesco Dolcini
> From: David Lin
> Sent: Wednesday, March 20, 2024 9:00 AM
> To: Johannes Berg <johannes@sipsolutions.net>; Brian Norris
> <briannorris@chromium.org>; Francesco Dolcini <francesco@dolcini.it>
> Cc: kvalo@kernel.org; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; Pete Hsieh <tsung-hsien.hsieh@nxp.com>;
> rafael.beims <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> > From: Johannes Berg <johannes@sipsolutions.net>
> > Sent: Tuesday, March 19, 2024 8:13 PM
> > To: Brian Norris <briannorris@chromium.org>; Francesco Dolcini
> > <francesco@dolcini.it>
> > Cc: kvalo@kernel.org; linux-wireless@vger.kernel.org;
> > linux-kernel@vger.kernel.org; David Lin <yu-hao.lin@nxp.com>; Pete
> > Hsieh <tsung-hsien.hsieh@nxp.com>; rafael.beims
> > <rafael.beims@toradex.com>; Francesco Dolcini
> > <francesco.dolcini@toradex.com>
> > 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?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 1:10 ` David Lin
@ 2024-03-20 9:12 ` Johannes Berg
2024-03-20 21:50 ` Brian Norris
0 siblings, 1 reply; 44+ messages in thread
From: Johannes Berg @ 2024-03-20 9:12 UTC (permalink / raw
To: David Lin, Brian Norris, Francesco Dolcini
Cc: kvalo@kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, Pete Hsieh, rafael.beims,
Francesco Dolcini
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-19 10:33 ` Kalle Valo
@ 2024-03-20 21:06 ` Brian Norris
0 siblings, 0 replies; 44+ messages in thread
From: Brian Norris @ 2024-03-20 21:06 UTC (permalink / raw
To: Kalle Valo
Cc: Francesco Dolcini, linux-wireless, linux-kernel, David Lin,
tsung-hsien.hsieh, rafael.beims, Francesco Dolcini
On Tue, Mar 19, 2024 at 12:33:55PM +0200, Kalle Valo wrote:
> Francesco Dolcini <francesco@dolcini.it> 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.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-18 2:20 ` [EXT] " David Lin
2024-03-18 11:45 ` Francesco Dolcini
@ 2024-03-20 21:13 ` Brian Norris
2024-03-21 2:12 ` David Lin
1 sibling, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-20 21:13 UTC (permalink / raw
To: David Lin
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh, rafael.beims,
Francesco Dolcini
Hi David,
On Mon, Mar 18, 2024 at 02:20:56AM +0000, David Lin wrote:
> > From: Brian Norris <briannorris@chromium.org>
> > 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-18 9:24 ` Kalle Valo
2024-03-19 1:40 ` [EXT] " David Lin
@ 2024-03-20 21:28 ` Brian Norris
2024-03-21 2:14 ` [EXT] " David Lin
1 sibling, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-20 21:28 UTC (permalink / raw
To: Kalle Valo
Cc: Francesco Dolcini, linux-wireless, linux-kernel, David Lin,
tsung-hsien.hsieh, rafael.beims, Francesco Dolcini
On Mon, Mar 18, 2024 at 11:24:34AM +0200, Kalle Valo wrote:
> Francesco Dolcini <francesco@dolcini.it> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 9:12 ` Johannes Berg
@ 2024-03-20 21:50 ` Brian Norris
2024-03-21 4:07 ` David Lin
2024-03-25 15:58 ` Johannes Berg
0 siblings, 2 replies; 44+ messages in thread
From: Brian Norris @ 2024-03-20 21:50 UTC (permalink / raw
To: Johannes Berg
Cc: David Lin, Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 21:13 ` Brian Norris
@ 2024-03-21 2:12 ` David Lin
0 siblings, 0 replies; 44+ messages in thread
From: David Lin @ 2024-03-21 2:12 UTC (permalink / raw
To: Brian Norris
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh, rafael.beims,
Francesco Dolcini
> From: Brian Norris <briannorris@chromium.org>
> Sent: Thursday, March 21, 2024 5:13 AM
> To: David Lin <yu-hao.lin@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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 <briannorris@chromium.org>
>
> > > 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.
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 21:28 ` Brian Norris
@ 2024-03-21 2:14 ` David Lin
0 siblings, 0 replies; 44+ messages in thread
From: David Lin @ 2024-03-21 2:14 UTC (permalink / raw
To: Brian Norris, Kalle Valo
Cc: Francesco Dolcini, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, Pete Hsieh, rafael.beims,
Francesco Dolcini
> From: Brian Norris <briannorris@chromium.org>
> Sent: Thursday, March 21, 2024 5:29 AM
> To: Kalle Valo <kvalo@kernel.org>
> Cc: Francesco Dolcini <francesco@dolcini.it>; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; David Lin <yu-hao.lin@nxp.com>; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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 Mon, Mar 18, 2024 at 11:24:34AM +0200, Kalle Valo wrote:
> > Francesco Dolcini <francesco@dolcini.it> 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
No matter with or without host mlme, this patch is tested by NXP QA, Rafael, Francesco and myself.
Thanks,
David
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 21:50 ` Brian Norris
@ 2024-03-21 4:07 ` David Lin
2024-03-23 1:06 ` Brian Norris
2024-03-25 15:58 ` Johannes Berg
1 sibling, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-21 4:07 UTC (permalink / raw
To: Brian Norris, Johannes Berg
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
> From: Brian Norris <briannorris@chromium.org>
> Sent: Thursday, March 21, 2024 5:50 AM
> To: Johannes Berg <johannes@sipsolutions.net>
> Cc: David Lin <yu-hao.lin@nxp.com>; Francesco Dolcini <francesco@dolcini.it>;
> kvalo@kernel.org; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; Pete Hsieh <tsung-hsien.hsieh@nxp.com>;
> rafael.beims <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-21 4:07 ` David Lin
@ 2024-03-23 1:06 ` Brian Norris
2024-03-25 16:15 ` Johannes Berg
0 siblings, 1 reply; 44+ messages in thread
From: Brian Norris @ 2024-03-23 1:06 UTC (permalink / raw
To: David Lin
Cc: Johannes Berg, Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
On Thu, Mar 21, 2024 at 04:07:58AM +0000, David Lin wrote:
> > From: Brian Norris <briannorris@chromium.org>
> >
> > 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.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-20 21:50 ` Brian Norris
2024-03-21 4:07 ` David Lin
@ 2024-03-25 15:58 ` Johannes Berg
2024-03-29 9:58 ` David Lin
1 sibling, 1 reply; 44+ messages in thread
From: Johannes Berg @ 2024-03-25 15:58 UTC (permalink / raw
To: Brian Norris
Cc: David Lin, Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-23 1:06 ` Brian Norris
@ 2024-03-25 16:15 ` Johannes Berg
2024-03-29 10:06 ` David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Johannes Berg @ 2024-03-25 16:15 UTC (permalink / raw
To: Brian Norris, David Lin
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
On Fri, 2024-03-22 at 18:06 -0700, Brian Norris wrote:
> 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.
Maybe, maybe not, actually. The auth command _is_ somewhat special in
that it mostly hands stuff down from userspace via cfg80211, but does
require sending frames. As long as you don't have full offload, at
least.
The way I see it, the issue here isn't necessarily the fact that this
uses the auth command (and then requires assoc, of course), but that we
see here this is "growing" towards a more mac80211-like model, with the
code duplication (albeit little that it is today) implied by that. To
me, it seems like the firmware is moving into the "oh we can't do all
_that_ in firmware" territory, and that brings it closer to mac80211.
At the same time, as you say, mac80211 is doing more and more offload
capability, so it seems like apart from "today the firmware requires an
assoc command rather than assoc frame processing in the host", it's
actually not _that_ far apart any more!
Now that may be an issue in the short term, but I wouldn't be surprised
at all if desiring to implement FILS and other new features in this
space would make the driver move to assoc frame processing in the host
as well, because it's getting more and more complex, just like auth.
At which point - yeah the APIs are still significantly different, but
again we'd end up implementing something that exists in mac80211 today
and taking it into mwifiex?
> 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.
Not sure that makes a difference for mac80211 in itself, obviously
changes in this space would then affect mwifiex, but that shouldn't be
much of an issue.
I'm less worried about this individual patch than what it says about the
direction this driver and firmware are taking, and I fear we'll end up
in a situation where over time this driver actually gets to a point
where it should be using mac80211, but because it's such a piece-meal
affair (auth frames now, etc.) and large architectural change, they'd
never actually do that.
To be fair, that might also require firmware API changes in some way. I
used to think that was something we should never require, but I'm not so
sure now any more - certainly we've changed our (Intel) FW API in
support of Linux architecture many times, and overall that's for a
better product (on Linux at least.)
Also: David, I'd appreciate if you actually took this discussion
seriously; so far you've not really contributed any technical arguments.
johannes
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-25 15:58 ` Johannes Berg
@ 2024-03-29 9:58 ` David Lin
0 siblings, 0 replies; 44+ messages in thread
From: David Lin @ 2024-03-29 9:58 UTC (permalink / raw
To: Johannes Berg, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Monday, March 25, 2024 11:59 PM
> To: Brian Norris <briannorris@chromium.org>
> Cc: David Lin <yu-hao.lin@nxp.com>; Francesco Dolcini
> <francesco@dolcini.it>; kvalo@kernel.org; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; Pete Hsieh <tsung-hsien.hsieh@nxp.com>;
> rafael.beims <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> 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
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-25 16:15 ` Johannes Berg
@ 2024-03-29 10:06 ` David Lin
2024-04-02 17:38 ` Brian Norris
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-03-29 10:06 UTC (permalink / raw
To: Johannes Berg, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Tuesday, March 26, 2024 12:15 AM
> To: Brian Norris <briannorris@chromium.org>; David Lin
> <yu-hao.lin@nxp.com>
> Cc: Francesco Dolcini <francesco@dolcini.it>; kvalo@kernel.org;
> linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> 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 Fri, 2024-03-22 at 18:06 -0700, Brian Norris wrote:
> > 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.
>
> Maybe, maybe not, actually. The auth command _is_ somewhat special in
> that it mostly hands stuff down from userspace via cfg80211, but does
> require sending frames. As long as you don't have full offload, at least.
>
> The way I see it, the issue here isn't necessarily the fact that this uses the
> auth command (and then requires assoc, of course), but that we see here
> this is "growing" towards a more mac80211-like model, with the code
> duplication (albeit little that it is today) implied by that. To me, it seems like
> the firmware is moving into the "oh we can't do all _that_ in firmware"
> territory, and that brings it closer to mac80211.
>
> At the same time, as you say, mac80211 is doing more and more offload
> capability, so it seems like apart from "today the firmware requires an assoc
> command rather than assoc frame processing in the host", it's actually not
> _that_ far apart any more!
>
> Now that may be an issue in the short term, but I wouldn't be surprised at
> all if desiring to implement FILS and other new features in this space would
> make the driver move to assoc frame processing in the host as well, because
> it's getting more and more complex, just like auth.
>
> At which point - yeah the APIs are still significantly different, but again we'd
> end up implementing something that exists in mac80211 today and taking it
> into mwifiex?
>
Mwifiex is designed based on a "Thick FW" architecture.
As security features become more complicated, this patch adds WPA3 support by offloading to wpa_supplicant/hostapd
With this patch, auth, assoc and key handshakes are handled in wpa_supplicant/hostapd.
Wpa_supplicant communicates peer capability and derived keys to driver/FW via cfg80211 assoc and add_key commands.
The cfg80211 commands are standard. Implementation between driver and firmware is vendor specific. It shall not break any existing architecture.
> > 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.
>
> Not sure that makes a difference for mac80211 in itself, obviously changes in
> this space would then affect mwifiex, but that shouldn't be much of an
> issue.
>
With this patch Mwifiex still a non-mac80211 implementation.
Driver communicates with wpa_supplicant/hostapd via cfg80211
I think how driver/FW communicate each other is proprietary, I don't see a dependency with mac80211 here
> I'm less worried about this individual patch than what it says about the
> direction this driver and firmware are taking, and I fear we'll end up in a
> situation where over time this driver actually gets to a point where it should
> be using mac80211, but because it's such a piece-meal affair (auth frames
> now, etc.) and large architectural change, they'd never actually do that.
>
> To be fair, that might also require firmware API changes in some way. I used
> to think that was something we should never require, but I'm not so sure
> now any more - certainly we've changed our (Intel) FW API in support of
> Linux architecture many times, and overall that's for a better product (on
> Linux at least.)
>
> Also: David, I'd appreciate if you actually took this discussion seriously; so
> far you've not really contributed any technical arguments.
>
> Johannes
I think we are using standard cfg80211 commands. How it's handled between driver/FW is proprietary, it's carefully verified and shall not impact other features or break any architecture.
We do not see a need why driver has to be redesigned based on mac80211. We can keep adding new features using cfg80211.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-03-29 10:06 ` David Lin
@ 2024-04-02 17:38 ` Brian Norris
2024-04-10 7:30 ` David Lin
2024-04-10 7:52 ` Johannes Berg
0 siblings, 2 replies; 44+ messages in thread
From: Brian Norris @ 2024-04-02 17:38 UTC (permalink / raw
To: David Lin, Johannes Berg
Cc: Johannes Berg, Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
Hi Johannes and David,
On Fri, Mar 29, 2024 at 10:06:19AM +0000, David Lin wrote:
> > From: Johannes Berg <johannes@sipsolutions.net>
> >
> > The way I see it, the issue here isn't necessarily the fact that this uses the
> > auth command (and then requires assoc, of course), but that we see here
> > this is "growing" towards a more mac80211-like model, with the code
> > duplication (albeit little that it is today) implied by that. To me, it seems like
> > the firmware is moving into the "oh we can't do all _that_ in firmware"
> > territory, and that brings it closer to mac80211.
> >
> > At the same time, as you say, mac80211 is doing more and more offload
> > capability, so it seems like apart from "today the firmware requires an assoc
> > command rather than assoc frame processing in the host", it's actually not
> > _that_ far apart any more!
> >
> > Now that may be an issue in the short term, but I wouldn't be surprised at
> > all if desiring to implement FILS and other new features in this space would
> > make the driver move to assoc frame processing in the host as well, because
> > it's getting more and more complex, just like auth.
> >
> > At which point - yeah the APIs are still significantly different, but again we'd
> > end up implementing something that exists in mac80211 today and taking it
> > into mwifiex?
So it seems there's 2 possible sticking points: code duplication, and
overall trend in the specs and implementation that might increase
duplication?
To me, it seems like the duplication is minimal today, or at least, not
much as a result of anything in this patch proposal. There's some
repetition of 802.11 definitions already, but that's probably
orthogonal.
I have less understanding and foresight on the "trend" questions,
although David seems to somewhat agree in his response below -- that NXP
intends to handle modern security features in the host more and more,
which could indeed mean a bit more framing-related duplication.
> Mwifiex is designed based on a "Thick FW" architecture.
> As security features become more complicated, this patch adds WPA3 support by offloading to wpa_supplicant/hostapd
> With this patch, auth, assoc and key handshakes are handled in wpa_supplicant/hostapd.
> Wpa_supplicant communicates peer capability and derived keys to driver/FW via cfg80211 assoc and add_key commands.
> The cfg80211 commands are standard. Implementation between driver and firmware is vendor specific. It shall not break any existing architecture.
>
> > > 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.
> >
> > Not sure that makes a difference for mac80211 in itself, obviously changes in
> > this space would then affect mwifiex, but that shouldn't be much of an
> > issue.
> >
>
> With this patch Mwifiex still a non-mac80211 implementation.
> Driver communicates with wpa_supplicant/hostapd via cfg80211
> I think how driver/FW communicate each other is proprietary, I don't see a dependency with mac80211 here
David, I may have pointed in the wrong direction by claiming "conflict"
with mac80211. I believe Johannes's concerns are in code duplication.
Pretty much all other actively-maintained Linux WiFi drivers are based
on mac80211, so that we don't all have to implement the same frame
construction and parsing code. mwifiex is somewhat of an outlier in
actively adding new features while remaining a cfg80211-only driver.
But for myself, I'm not even fully convinced mac80211-style stuff makes
sense here. Even just looking at the auth() stuff in patch 1, this
driver is far from a thin layer that allows mac80211 to handle the
802.11 details. Just look at the 'priv->host_mlme_reg' and
HostCmd_CMD_MGMT_FRAME_REG stuff -- it seems that the simple act of
sending a single 802.11 frame requires opting into some FW-specific
command mask. This feels "thick", like David mentioned above.
> > I'm less worried about this individual patch than what it says about the
> > direction this driver and firmware are taking, and I fear we'll end up in a
> > situation where over time this driver actually gets to a point where it should
> > be using mac80211, but because it's such a piece-meal affair (auth frames
> > now, etc.) and large architectural change, they'd never actually do that.
> >
> > To be fair, that might also require firmware API changes in some way. I used
> > to think that was something we should never require, but I'm not so sure
> > now any more - certainly we've changed our (Intel) FW API in support of
> > Linux architecture many times, and overall that's for a better product (on
> > Linux at least.)
Yeah, I feel like I see hints of stuff (in public driver code, and in
internal discussions with vendors) where things really work best in
mac80211 land if firmware is developed with *some* consideration for the
mac80211 Linux architecture (or else already is a very thin firmware
from the start). I don't feel like Marvell/NXP has that consideration at
all, and so trying to drag its firmware into mac80211 regardless would
bring its own unique pain. But that's just a lightly-educated feeling.
> > Also: David, I'd appreciate if you actually took this discussion seriously; so
> > far you've not really contributed any technical arguments.
>
> I think we are using standard cfg80211 commands. How it's handled
> between driver/FW is proprietary, it's carefully verified and shall
> not impact other features or break any architecture.
David, repeating the "carefully verified" stuff doesn't really help the
subject at hand, and it's not really a technical argument either, when
it's not accompanied by any proofs. Being careful and running a good QA
cycle are excellent and appreciated, but that's not what we're talking
about any more. We're trying to understand what the firmware
architecture is like, and what driver architecture matches your future
development best, with the needs of the Linux community in mind.
(Now, as an example useful argument: if you claimed that implementing a
mac80211 driver with a generalized ieee80211_ops::tx() function would
introduce unvetted combinations of control logic that cannot be
reconciled with your HostCmd protocols, or that QA can't properly vet
that ... well, that would be a start of a technical argument. But it
would require more than a sentence or two to describe why that's
impossible or difficult.)
> We do not see a need why driver has to be redesigned based on
> mac80211. We can keep adding new features using cfg80211.
All in all, other than being a little grumpy about reviewing new
features here, I'm still leaning toward David's statement here -- that I
don't see why it *must* be rewritten toward mac80211. But I still defer
to Johannes, and I'm just trying to be a bit of a referee, or the
proverbial "devil's advocate".
Brian
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-02 17:38 ` Brian Norris
@ 2024-04-10 7:30 ` David Lin
2024-04-10 7:55 ` Johannes Berg
2024-04-10 7:52 ` Johannes Berg
1 sibling, 1 reply; 44+ messages in thread
From: David Lin @ 2024-04-10 7:30 UTC (permalink / raw
To: Brian Norris, Johannes Berg
Cc: Johannes Berg, Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
Hi Johannes and Brian,
I think this patch is used to leverage MLME of wpa_supplicant and hostapd. It won't affect the usage of cfg80211 for mwifiex. I wonder if I can prepare patch v10.
David
> From: Brian Norris <briannorris@chromium.org>
> Sent: Wednesday, April 3, 2024 1:39 AM
> To: David Lin <yu-hao.lin@nxp.com>; Johannes Berg
> <johannes@sipsolutions.net>
> Cc: Johannes Berg <johannes@sipsolutions.net>; Francesco Dolcini
> <francesco@dolcini.it>; kvalo@kernel.org; linux-wireless@vger.kernel.org;
> linux-kernel@vger.kernel.org; Pete Hsieh <tsung-hsien.hsieh@nxp.com>;
> rafael.beims <rafael.beims@toradex.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> Hi Johannes and David,
>
> On Fri, Mar 29, 2024 at 10:06:19AM +0000, David Lin wrote:
> > > From: Johannes Berg <johannes@sipsolutions.net>
> > >
> > > The way I see it, the issue here isn't necessarily the fact that
> > > this uses the auth command (and then requires assoc, of course), but
> > > that we see here this is "growing" towards a more mac80211-like
> > > model, with the code duplication (albeit little that it is today)
> > > implied by that. To me, it seems like the firmware is moving into the "oh
> we can't do all _that_ in firmware"
> > > territory, and that brings it closer to mac80211.
> > >
> > > At the same time, as you say, mac80211 is doing more and more
> > > offload capability, so it seems like apart from "today the firmware
> > > requires an assoc command rather than assoc frame processing in the
> > > host", it's actually not _that_ far apart any more!
> > >
> > > Now that may be an issue in the short term, but I wouldn't be
> > > surprised at all if desiring to implement FILS and other new
> > > features in this space would make the driver move to assoc frame
> > > processing in the host as well, because it's getting more and more complex,
> just like auth.
> > >
> > > At which point - yeah the APIs are still significantly different,
> > > but again we'd end up implementing something that exists in mac80211
> > > today and taking it into mwifiex?
>
> So it seems there's 2 possible sticking points: code duplication, and overall
> trend in the specs and implementation that might increase duplication?
>
> To me, it seems like the duplication is minimal today, or at least, not much as a
> result of anything in this patch proposal. There's some repetition of 802.11
> definitions already, but that's probably orthogonal.
>
> I have less understanding and foresight on the "trend" questions, although
> David seems to somewhat agree in his response below -- that NXP intends to
> handle modern security features in the host more and more, which could
> indeed mean a bit more framing-related duplication.
>
> > Mwifiex is designed based on a "Thick FW" architecture.
> > As security features become more complicated, this patch adds WPA3
> > support by offloading to wpa_supplicant/hostapd With this patch, auth, assoc
> and key handshakes are handled in wpa_supplicant/hostapd.
> > Wpa_supplicant communicates peer capability and derived keys to
> driver/FW via cfg80211 assoc and add_key commands.
> > The cfg80211 commands are standard. Implementation between driver and
> firmware is vendor specific. It shall not break any existing architecture.
> >
> > > > 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.
> > >
> > > Not sure that makes a difference for mac80211 in itself, obviously
> > > changes in this space would then affect mwifiex, but that shouldn't
> > > be much of an issue.
> > >
> >
> > With this patch Mwifiex still a non-mac80211 implementation.
> > Driver communicates with wpa_supplicant/hostapd via cfg80211 I think
> > how driver/FW communicate each other is proprietary, I don't see a
> > dependency with mac80211 here
>
> David, I may have pointed in the wrong direction by claiming "conflict"
> with mac80211. I believe Johannes's concerns are in code duplication.
> Pretty much all other actively-maintained Linux WiFi drivers are based on
> mac80211, so that we don't all have to implement the same frame
> construction and parsing code. mwifiex is somewhat of an outlier in actively
> adding new features while remaining a cfg80211-only driver.
>
> But for myself, I'm not even fully convinced mac80211-style stuff makes sense
> here. Even just looking at the auth() stuff in patch 1, this driver is far from a
> thin layer that allows mac80211 to handle the
> 802.11 details. Just look at the 'priv->host_mlme_reg' and
> HostCmd_CMD_MGMT_FRAME_REG stuff -- it seems that the simple act of
> sending a single 802.11 frame requires opting into some FW-specific command
> mask. This feels "thick", like David mentioned above.
>
> > > I'm less worried about this individual patch than what it says about
> > > the direction this driver and firmware are taking, and I fear we'll
> > > end up in a situation where over time this driver actually gets to a
> > > point where it should be using mac80211, but because it's such a
> > > piece-meal affair (auth frames now, etc.) and large architectural change,
> they'd never actually do that.
> > >
> > > To be fair, that might also require firmware API changes in some
> > > way. I used to think that was something we should never require, but
> > > I'm not so sure now any more - certainly we've changed our (Intel)
> > > FW API in support of Linux architecture many times, and overall
> > > that's for a better product (on Linux at least.)
>
> Yeah, I feel like I see hints of stuff (in public driver code, and in internal
> discussions with vendors) where things really work best in
> mac80211 land if firmware is developed with *some* consideration for the
> mac80211 Linux architecture (or else already is a very thin firmware from the
> start). I don't feel like Marvell/NXP has that consideration at all, and so trying
> to drag its firmware into mac80211 regardless would bring its own unique pain.
> But that's just a lightly-educated feeling.
>
> > > Also: David, I'd appreciate if you actually took this discussion
> > > seriously; so far you've not really contributed any technical arguments.
> >
> > I think we are using standard cfg80211 commands. How it's handled
> > between driver/FW is proprietary, it's carefully verified and shall
> > not impact other features or break any architecture.
>
> David, repeating the "carefully verified" stuff doesn't really help the subject at
> hand, and it's not really a technical argument either, when it's not
> accompanied by any proofs. Being careful and running a good QA cycle are
> excellent and appreciated, but that's not what we're talking about any more.
> We're trying to understand what the firmware architecture is like, and what
> driver architecture matches your future development best, with the needs of
> the Linux community in mind.
>
> (Now, as an example useful argument: if you claimed that implementing a
> mac80211 driver with a generalized ieee80211_ops::tx() function would
> introduce unvetted combinations of control logic that cannot be reconciled
> with your HostCmd protocols, or that QA can't properly vet that ... well, that
> would be a start of a technical argument. But it would require more than a
> sentence or two to describe why that's impossible or difficult.)
>
> > We do not see a need why driver has to be redesigned based on
> > mac80211. We can keep adding new features using cfg80211.
>
> All in all, other than being a little grumpy about reviewing new features here,
> I'm still leaning toward David's statement here -- that I don't see why it *must*
> be rewritten toward mac80211. But I still defer to Johannes, and I'm just trying
> to be a bit of a referee, or the proverbial "devil's advocate".
>
> Brian
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-02 17:38 ` Brian Norris
2024-04-10 7:30 ` David Lin
@ 2024-04-10 7:52 ` Johannes Berg
1 sibling, 0 replies; 44+ messages in thread
From: Johannes Berg @ 2024-04-10 7:52 UTC (permalink / raw
To: Brian Norris, David Lin
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
Hi Brian,
> So it seems there's 2 possible sticking points: code duplication, and
> overall trend in the specs and implementation that might increase
> duplication?
>
> To me, it seems like the duplication is minimal today, or at least, not
> much as a result of anything in this patch proposal. There's some
> repetition of 802.11 definitions already, but that's probably
> orthogonal.
Agree.
> I have less understanding and foresight on the "trend" questions,
> although David seems to somewhat agree in his response below -- that NXP
> intends to handle modern security features in the host more and more,
> which could indeed mean a bit more framing-related duplication.
We'll see, but I don't _actually_ think this will significantly change
the architecture here.
In any case we can always kick the can further down the road.
> > With this patch Mwifiex still a non-mac80211 implementation.
> > Driver communicates with wpa_supplicant/hostapd via cfg80211
> > I think how driver/FW communicate each other is proprietary, I don't see a dependency with mac80211 here
>
> David, I may have pointed in the wrong direction by claiming "conflict"
> with mac80211. I believe Johannes's concerns are in code duplication.
Partially, yes, but also architecturally it should fit in.
> Pretty much all other actively-maintained Linux WiFi drivers are based
> on mac80211, so that we don't all have to implement the same frame
> construction and parsing code. mwifiex is somewhat of an outlier in
> actively adding new features while remaining a cfg80211-only driver.
I'd say though that "actively maintained" part really is because full-
MAC devices that are supported are very few now with Broadcom having
essentially dropped out. I suspect there are other full-MAC chips and/or
firmwares on the market, but few, if any, supported upstream. There's no
particular reason this must be the case, it's just the way hardware
architecture seems to be going.
And as you said before, mac80211 is doing more and more offloads too, so
the line ends up blurring from both sides.
Which then again is part of my concern, if we blur the line from both
sides we need to be even more careful to not grow into a parallel zone
too much.
> But for myself, I'm not even fully convinced mac80211-style stuff makes
> sense here. Even just looking at the auth() stuff in patch 1, this
> driver is far from a thin layer that allows mac80211 to handle the
> 802.11 details. Just look at the 'priv->host_mlme_reg' and
> HostCmd_CMD_MGMT_FRAME_REG stuff -- it seems that the simple act of
> sending a single 802.11 frame requires opting into some FW-specific
> command mask.
I actually agree.
> This feels "thick", like David mentioned above.
This is kind of getting to the core of the thread: I, for one, don't
think he made that argument. He just *claimed*, without much argument
for that claim, that "Mwifiex is designed based on a "Thick FW"
architecture."
> > I think we are using standard cfg80211 commands. How it's handled
> > between driver/FW is proprietary, it's carefully verified and shall
> > not impact other features or break any architecture.
>
> David, repeating the "carefully verified" stuff doesn't really help the
> subject at hand, and it's not really a technical argument either,
Nor does "we are using standard cfg80211 commands", FWIW. That doesn't
mean we need to think the architecture is good. Taking this argument to
the extreme, it'd be entirely possible to put a (modified) copy of
mac80211 into a driver and claim "we are using standard cfg80211
commands". It's a non-argument.
And really here we get to the core of the issue: Brian, you have now
mostly done the actual _technical_ analysis (and defence) of this patch
that I was waiting for _David_ to do.
johannes
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-10 7:30 ` David Lin
@ 2024-04-10 7:55 ` Johannes Berg
2024-04-10 10:33 ` David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Johannes Berg @ 2024-04-10 7:55 UTC (permalink / raw
To: David Lin, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
On Wed, 2024-04-10 at 07:30 +0000, David Lin wrote:
> Hi Johannes and Brian,
>
> I think this patch is used to leverage MLME of wpa_supplicant and hostapd. It won't affect the usage of cfg80211 for mwifiex. I wonder if I can prepare patch v10.
No. That sentence tells me you've _still_ not understood any of the
technical arguments in the thread, you're _still_ arguing with
completely uninteresting arguments. Where before you had "it's well
tested" and "it uses 'standard' APIs" now you're saying "it doesn't
affect anyone else". All of that is obvious, and also completely besides
the point.
Please go back and actually _understand_ the discussion. Also actually
_participate_ in the discussion too, so far you've pretty much only made
empty arguments. Once you've understood the concerns and can explain why
they don't apply, _then_ you can resend the patch.
johannes
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-10 7:55 ` Johannes Berg
@ 2024-04-10 10:33 ` David Lin
2024-04-10 17:56 ` Johannes Berg
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-04-10 10:33 UTC (permalink / raw
To: Johannes Berg, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, April 10, 2024 3:55 PM
> To: David Lin <yu-hao.lin@nxp.com>; Brian Norris
> <briannorris@chromium.org>
> Cc: Francesco Dolcini <francesco@dolcini.it>; kvalo@kernel.org;
> linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> On Wed, 2024-04-10 at 07:30 +0000, David Lin wrote:
> > Hi Johannes and Brian,
> >
> > I think this patch is used to leverage MLME of wpa_supplicant and
> hostapd. It won't affect the usage of cfg80211 for mwifiex. I wonder if I can
> prepare patch v10.
>
> No. That sentence tells me you've _still_ not understood any of the technical
> arguments in the thread, you're _still_ arguing with completely uninteresting
> arguments. Where before you had "it's well tested" and "it uses 'standard'
> APIs" now you're saying "it doesn't affect anyone else". All of that is obvious,
> and also completely besides the point.
>
> Please go back and actually _understand_ the discussion. Also actually
> _participate_ in the discussion too, so far you've pretty much only made empty
> arguments. Once you've understood the concerns and can explain why they
> don't apply, _then_ you can resend the patch.
>
> johannes
Take Rx data path as an example,
In current FW, BA stream setup and de-ampdu are handled in FW. Packet is converted to 802.3 before passing to host. Ampdu reordering is handled in host driver (Mwifiex) due to memory consideration. We used to work on a driver that uses RX_FLAG_8023. It was on an older Wi-Fi part which has more memory and powerful processor. But with this chip buffer required for reordering doesn’t fit FW memory.
Ampdu reordering needs MAC 802.11 header, FW keeps the MAC header in packet descriptor since packet already 802.3 when arrive at driver. As packet descriptor only accessible in the driver, Mwifiex handles the reordering instead of using mac80211 reordering.
With current FW design, to apply mac802.11 we either change FW to pass packet in 802.11 format or driver needs to do the conversion back to 802.11 (which I think doesn’t make sense)
Given existing FW design, we think it’s difficult to apply mac80211. Hope this make valid arguments.
David
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-10 10:33 ` David Lin
@ 2024-04-10 17:56 ` Johannes Berg
2024-04-11 7:57 ` David Lin
0 siblings, 1 reply; 44+ messages in thread
From: Johannes Berg @ 2024-04-10 17:56 UTC (permalink / raw
To: David Lin, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
On Wed, 2024-04-10 at 10:33 +0000, David Lin wrote:
>
>
> Take Rx data path as an example,
> In current FW, BA stream setup and de-ampdu are handled in FW. Packet is converted to 802.3 before passing to host. Ampdu reordering is handled in host driver (Mwifiex) due to memory consideration. We used to work on a driver that uses RX_FLAG_8023. It was on an older Wi-Fi part which has more memory and powerful processor. But with this chip buffer required for reordering doesn’t fit FW memory.
>
> Ampdu reordering needs MAC 802.11 header, FW keeps the MAC header in packet descriptor since packet already 802.3 when arrive at driver. As packet descriptor only accessible in the driver, Mwifiex handles the reordering instead of using mac80211 reordering.
>
> With current FW design, to apply mac802.11 we either change FW to pass packet in 802.11 format or driver needs to do the conversion back to 802.11 (which I think doesn’t make sense)
>
> Given existing FW design, we think it’s difficult to apply mac80211.
Hmm, I don't think so? If you have a mac80211 driver with RX_FLAG_8023,
then of course mac80211 cannot do reordering, and you have to do it
somewhere below. But that doesn't mean you necessarily _have_ to do it
in hardware/firmware? You could do it in the driver, which you also have
to do in a cfg80211 driver anyway, and that's OK. Due to usage of RSS,
iwlwifi/mvm even does it internally, although it doesn't even have
RX_FLAG_8023.
But that goes into the direction I was talking about: the boundaries are
getting fuzzier all the time, because offloads happen and get supported
in mac80211. So if you have offloads for header conversion and only get
some reordering data "out of band", then you have to handle that in the
driver. Using mac80211 doesn't mean you have to use _all_ of it.
Originally, mac80211 didn't even have RX_FLAG_8023 after all. But
obviously adding something like that also requires more upstream
engagement :)
I think the question is about the design, but I also think the
differences in the association process are much more fundamental, and
you _don't_ (seem to) handle that in the way mac80211 does/expects,
though you also don't seem to handle it in the way most other full-MAC
devices do (which [can] offload even BSS selection.)
The thing I want you to understand is the relative architecture and how
your work fits into this. I'm not telling you have to change it right
now or do this work differently, I just want to make sure you actually
understand it. The above argument goes some way, showing you've actually
looked at the differences mac80211 would imply, where before I felt you
were pretty much handwaving it away as if irrelevant to the discussion.
johannes
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-10 17:56 ` Johannes Berg
@ 2024-04-11 7:57 ` David Lin
2024-04-11 8:02 ` Johannes Berg
0 siblings, 1 reply; 44+ messages in thread
From: David Lin @ 2024-04-11 7:57 UTC (permalink / raw
To: Johannes Berg, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Thursday, April 11, 2024 1:57 AM
> To: David Lin <yu-hao.lin@nxp.com>; Brian Norris
> <briannorris@chromium.org>
> Cc: Francesco Dolcini <francesco@dolcini.it>; kvalo@kernel.org;
> linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; rafael.beims <rafael.beims@toradex.com>;
> Francesco Dolcini <francesco.dolcini@toradex.com>
> Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> On Wed, 2024-04-10 at 10:33 +0000, David Lin wrote:
> >
> >
> > Take Rx data path as an example,
> > In current FW, BA stream setup and de-ampdu are handled in FW. Packet is
> converted to 802.3 before passing to host. Ampdu reordering is handled in host
> driver (Mwifiex) due to memory consideration. We used to work on a driver
> that uses RX_FLAG_8023. It was on an older Wi-Fi part which has more
> memory and powerful processor. But with this chip buffer required for
> reordering doesn’t fit FW memory.
> >
> > Ampdu reordering needs MAC 802.11 header, FW keeps the MAC header in
> packet descriptor since packet already 802.3 when arrive at driver. As packet
> descriptor only accessible in the driver, Mwifiex handles the reordering instead
> of using mac80211 reordering.
> >
> > With current FW design, to apply mac802.11 we either change FW to pass
> > packet in 802.11 format or driver needs to do the conversion back to
> > 802.11 (which I think doesn’t make sense)
> >
> > Given existing FW design, we think it’s difficult to apply mac80211.
>
> Hmm, I don't think so? If you have a mac80211 driver with RX_FLAG_8023,
> then of course mac80211 cannot do reordering, and you have to do it
> somewhere below. But that doesn't mean you necessarily _have_ to do it in
> hardware/firmware? You could do it in the driver, which you also have to do in
> a cfg80211 driver anyway, and that's OK. Due to usage of RSS, iwlwifi/mvm
> even does it internally, although it doesn't even have RX_FLAG_8023.
>
> But that goes into the direction I was talking about: the boundaries are getting
> fuzzier all the time, because offloads happen and get supported in mac80211.
> So if you have offloads for header conversion and only get some reordering
> data "out of band", then you have to handle that in the driver. Using mac80211
> doesn't mean you have to use _all_ of it.
Following jobs are done by FW:
1. MAC 802.11 Rx processes except for BA buffering/reordering and AMSDU.
2. Convert 802.11 frame to 802.3 frame.
3. Embedded MAC 802.11 header information to Rx descriptor for driver to do BA buffering/reordering.
If this FW wants to leverage mac80211:
1. Use 802.11 frame:
Driver should restructure 802.11 frame and mac80211 will redo jobs done by FW. I think this does not make sense.
2. Use 802.3 frame:
Driver still needs to do BA buffering/reordering (mac80211 can help with AMSDU with flag IEEE80211_RX_AMSDU,
but cfg80211 also supports function ieee80211_amsdu_to_8023s() to help this job).
If this is the case, it becomes redundant to pass 802.3 frame to kernel stack via mac80211.
So I think cfg80211 will be more suitable for existing FW.
> Originally, mac80211 didn't even have RX_FLAG_8023 after all. But obviously
> adding something like that also requires more upstream engagement :)
>
Yes, we requested 802.3 fast forwarding of mac80211 due to our previous fully Rx offload FW with very powerful WiFi SoC.
> I think the question is about the design, but I also think the differences in the
> association process are much more fundamental, and you _don't_ (seem to)
> handle that in the way mac80211 does/expects, though you also don't seem to
> handle it in the way most other full-MAC devices do (which [can] offload even
> BSS selection.)
>
FW only supports add station command for AP mode. Association command is used to request and add client station to FW.
FW will help to connect to AP and reply result to driver.
This is another reason that we need to use cfg80211 instead of mac80211. For mac80211, management frames are passed
through ieee80211_ops.tx and station is added via ieee80211_ops.sta_add.
This way can't work with FW for client mode, FW can't not be bypassed for association process for client mode.
> The thing I want you to understand is the relative architecture and how your
> work fits into this. I'm not telling you have to change it right now or do this
> work differently, I just want to make sure you actually understand it. The above
> argument goes some way, showing you've actually looked at the differences
> mac80211 would imply, where before I felt you were pretty much handwaving
> it away as if irrelevant to the discussion.
>
> johannes
Thanks for your comments and suggestions. I wonder can I prepare patch v10 to let this patch be accepted?
David
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
2024-04-11 7:57 ` David Lin
@ 2024-04-11 8:02 ` Johannes Berg
0 siblings, 0 replies; 44+ messages in thread
From: Johannes Berg @ 2024-04-11 8:02 UTC (permalink / raw
To: David Lin, Brian Norris
Cc: Francesco Dolcini, kvalo@kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Pete Hsieh, rafael.beims, Francesco Dolcini
On Thu, 2024-04-11 at 07:57 +0000, David Lin wrote:
>
>
> Following jobs are done by FW:
>
> 1. MAC 802.11 Rx processes except for BA buffering/reordering and AMSDU.
> 2. Convert 802.11 frame to 802.3 frame.
> 3. Embedded MAC 802.11 header information to Rx descriptor for driver to do BA buffering/reordering.
>
> If this FW wants to leverage mac80211:
>
> 1. Use 802.11 frame:
> Driver should restructure 802.11 frame and mac80211 will redo jobs done by FW. I think this does not make sense.
> 2. Use 802.3 frame:
> Driver still needs to do BA buffering/reordering (mac80211 can help with AMSDU with flag IEEE80211_RX_AMSDU,
> but cfg80211 also supports function ieee80211_amsdu_to_8023s() to help this job).
> If this is the case, it becomes redundant to pass 802.3 frame to kernel stack via mac80211.
>
> So I think cfg80211 will be more suitable for existing FW.
I agree with most of the above, but I don't really think this is an
argument either way. It just means the datapath won't be hugely
different between such two drivers, and we have to look for details to
make a decision elsewhere.
> > I think the question is about the design, but I also think the differences in the
> > association process are much more fundamental, and you _don't_ (seem to)
> > handle that in the way mac80211 does/expects, though you also don't seem to
> > handle it in the way most other full-MAC devices do (which [can] offload even
> > BSS selection.)
> >
>
> FW only supports add station command for AP mode. Association command is used to request and add client station to FW.
> FW will help to connect to AP and reply result to driver.
> This is another reason that we need to use cfg80211 instead of mac80211. For mac80211, management frames are passed
> through ieee80211_ops.tx and station is added via ieee80211_ops.sta_add.
> This way can't work with FW for client mode, FW can't not be bypassed for association process for client mode.
Right, this is the important distinction here, and indeed that leave
pretty much no choice other than doing a cfg80211 driver, and I don't
think it'd make sense in mac80211 to support offloading that either.
> Thanks for your comments and suggestions. I wonder can I prepare patch v10 to let this patch be accepted?
Yes please.
johannes
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [EXT] Re: [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode
2024-03-18 2:04 ` [EXT] " David Lin
@ 2024-04-18 3:37 ` David Lin
0 siblings, 0 replies; 44+ messages in thread
From: David Lin @ 2024-04-18 3:37 UTC (permalink / raw
To: David Lin, Brian Norris
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
kvalo@kernel.org, francesco@dolcini.it, Pete Hsieh,
Francesco Dolcini
> From: David Lin <yu-hao.lin@nxp.com>
> Sent: Monday, March 18, 2024 10:04 AM
> To: Brian Norris <briannorris@chromium.org>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>; Francesco Dolcini
> <francesco.dolcini@toradex.com>
> Subject: RE: [EXT] Re: [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP
> mode
> >
> > From: Brian Norris <briannorris@chromium.org>
> > Sent: Saturday, March 16, 2024 8:45 AM
> > To: David Lin <yu-hao.lin@nxp.com>
> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > kvalo@kernel.org; francesco@dolcini.it; Pete Hsieh
> > <tsung-hsien.hsieh@nxp.com>; Francesco Dolcini
> > <francesco.dolcini@toradex.com>
> > Subject: [EXT] Re: [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP
> > mode
> >
> > Caution: This is an external email. Please take care when clicking
> > links or opening attachments. When in doubt, report the message using
> > the 'Report this email' button
> >
> >
> > On Wed, Mar 06, 2024 at 10:00:53AM +0800, David Lin wrote:
> > > Add host based MLME to enable WPA3 functionalities in AP mode.
> > > This feature required a firmware with the corresponding V2 Key API
> > > support. The feature (WPA3) is currently enabled and verified only
> > > on IW416. Also, verified no regression with change when host MLME is
> > > disabled.
> > >
> > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> >
> > Quick pass for now; nothing jumps out at me today, but I'll give a
> > better look/Ack next week:
> >
> > > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> >
> >
> > > @@ -3951,12 +3974,43 @@
> > mwifiex_cfg80211_tdls_cancel_chan_switch(struct wiphy *wiphy,
> > > }
> > > }
> > >
> > > +static int
> > > +mwifiex_cfg80211_uap_add_station(struct mwifiex_private *priv,
> > > +const u8
> > *mac,
> > > + struct station_parameters *params) {
> > > + struct mwifiex_sta_info add_sta;
> > > + int ret;
> > > +
> > > + memcpy(add_sta.peer_mac, mac, ETH_ALEN);
> > > + add_sta.params = params;
> > > +
> > > + ret = mwifiex_send_cmd(priv, HostCmd_CMD_ADD_NEW_STATION,
> > > + HostCmd_ACT_ADD_STA, 0, (void
> > *)&add_sta,
> > > + true);
> > > +
> > > + if (!ret) {
> > > + struct station_info *sinfo;
> > > +
> > > + sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
> >
> > Couldn't this just be stack allocation?
> >
> > struct staion_info sinfo;
> >
> > cfg80211_new_sta(priv->netdev, mac, &sinfo,
> > GFP_KERNEL);
> >
> > I'm not sure you need to kzalloc() something here, if you're freeing
> > it a few lines later.
> >
>
> Will modify it in patch v10.
>
This modification will let stack overflow. Patch v10 will keep original code.
> >
> > > + if (!sinfo)
> > > + return -ENOMEM;
> > > +
> > > + cfg80211_new_sta(priv->netdev, mac, sinfo,
> GFP_KERNEL);
> > > + kfree(sinfo);
> > > + }
> > > +
> > > + return ret;
> > > +}
> >
> > Brian
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2024-04-18 3:37 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-06 2:00 [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme David Lin
2024-03-06 2:00 ` [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode David Lin
2024-03-16 0:00 ` Brian Norris
2024-03-18 2:00 ` [EXT] " David Lin
2024-03-18 11:41 ` Francesco Dolcini
2024-03-19 1:36 ` [EXT] " David Lin
2024-03-06 2:00 ` [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
2024-03-16 0:45 ` Brian Norris
2024-03-18 2:04 ` [EXT] " David Lin
2024-04-18 3:37 ` David Lin
2024-03-15 9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
2024-03-15 23:59 ` Brian Norris
2024-03-18 11:28 ` Francesco Dolcini
2024-03-19 10:33 ` Kalle Valo
2024-03-20 21:06 ` Brian Norris
2024-03-16 0:49 ` Brian Norris
2024-03-19 12:12 ` Johannes Berg
2024-03-20 0:59 ` [EXT] " David Lin
2024-03-20 1:10 ` David Lin
2024-03-20 9:12 ` Johannes Berg
2024-03-20 21:50 ` Brian Norris
2024-03-21 4:07 ` David Lin
2024-03-23 1:06 ` Brian Norris
2024-03-25 16:15 ` Johannes Berg
2024-03-29 10:06 ` David Lin
2024-04-02 17:38 ` Brian Norris
2024-04-10 7:30 ` David Lin
2024-04-10 7:55 ` Johannes Berg
2024-04-10 10:33 ` David Lin
2024-04-10 17:56 ` Johannes Berg
2024-04-11 7:57 ` David Lin
2024-04-11 8:02 ` Johannes Berg
2024-04-10 7:52 ` Johannes Berg
2024-03-25 15:58 ` Johannes Berg
2024-03-29 9:58 ` David Lin
2024-03-18 9:24 ` Kalle Valo
2024-03-19 1:40 ` [EXT] " David Lin
2024-03-20 21:28 ` Brian Norris
2024-03-21 2:14 ` [EXT] " David Lin
2024-03-16 0:07 ` Brian Norris
2024-03-18 2:20 ` [EXT] " David Lin
2024-03-18 11:45 ` Francesco Dolcini
2024-03-20 21:13 ` Brian Norris
2024-03-21 2:12 ` David Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).