2022-09-21 02:06:29

by Ian Lin

[permalink] [raw]
Subject: [PATCH 1/3] brcmfmac: Support DPP feature

From: Kurt Lee <[email protected]>

Let driver parse DPP frames from upper layer and do conresponding
configuration to firmware.
This change supports DPP handshake based on wpa_supplicant v2.9.

Signed-off-by: Kurt Lee <[email protected]>
Signed-off-by: Ian Lin <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 85 ++++++++++++-------
.../broadcom/brcm80211/brcmfmac/p2p.c | 72 ++++++++++++----
.../broadcom/brcm80211/brcmfmac/p2p.h | 4 +-
.../broadcom/brcm80211/include/brcmu_wifi.h | 5 ++
4 files changed, 117 insertions(+), 49 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 7c72ea26a7d7..4a8aceda8fe8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -64,6 +64,9 @@
#define RSN_CAP_MFPC_MASK BIT(7)
#define RSN_PMKID_COUNT_LEN 2

+#define DPP_AKM_SUITE_TYPE 2
+#define WLAN_AKM_SUITE_DPP SUITE(WLAN_OUI_WFA, DPP_AKM_SUITE_TYPE)
+
#define VNDR_IE_CMD_LEN 4 /* length of the set command
* string :"add", "del" (+ NUL)
*/
@@ -1816,6 +1819,9 @@ brcmf_set_key_mgmt(struct net_device *ndev, struct cfg80211_connect_params *sme)
val = WPA2_AUTH_PSK | WPA2_AUTH_FT;
profile->is_ft = true;
break;
+ case WLAN_AKM_SUITE_DPP:
+ val = WFA_AUTH_DPP;
+ break;
default:
bphy_err(drvr, "invalid akm suite (%d)\n",
sme->crypto.akm_suites[0]);
@@ -4144,6 +4150,12 @@ static bool brcmf_valid_wpa_oui(u8 *oui, bool is_rsn_ie)
return (memcmp(oui, WPA_OUI, TLV_OUI_LEN) == 0);
}

+static bool brcmf_valid_dpp_suite(u8 *oui)
+{
+ return (memcmp(oui, WFA_OUI, TLV_OUI_LEN) == 0 &&
+ *(oui + TLV_OUI_LEN) == DPP_AKM_SUITE_TYPE);
+}
+
static s32
brcmf_configure_wpaie(struct brcmf_if *ifp,
const struct brcmf_vs_tlv *wpa_ie,
@@ -4257,42 +4269,47 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
goto exit;
}
for (i = 0; i < count; i++) {
- if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
+ if (brcmf_valid_dpp_suite(&data[offset])) {
+ wpa_auth |= WFA_AUTH_DPP;
+ offset += TLV_OUI_LEN;
+ } else if (brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
+ offset += TLV_OUI_LEN;
+ switch (data[offset]) {
+ case RSN_AKM_NONE:
+ brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
+ wpa_auth |= WPA_AUTH_NONE;
+ break;
+ case RSN_AKM_UNSPECIFIED:
+ brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
+ is_rsn_ie ?
+ (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
+ (wpa_auth |= WPA_AUTH_UNSPECIFIED);
+ break;
+ case RSN_AKM_PSK:
+ brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
+ is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
+ (wpa_auth |= WPA_AUTH_PSK);
+ break;
+ case RSN_AKM_SHA256_PSK:
+ brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
+ wpa_auth |= WPA2_AUTH_PSK_SHA256;
+ break;
+ case RSN_AKM_SHA256_1X:
+ brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
+ wpa_auth |= WPA2_AUTH_1X_SHA256;
+ break;
+ case RSN_AKM_SAE:
+ brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
+ wpa_auth |= WPA3_AUTH_SAE_PSK;
+ break;
+ default:
+ bphy_err(drvr, "Invalid key mgmt info\n");
+ }
+ } else {
err = -EINVAL;
bphy_err(drvr, "ivalid OUI\n");
goto exit;
}
- offset += TLV_OUI_LEN;
- switch (data[offset]) {
- case RSN_AKM_NONE:
- brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
- wpa_auth |= WPA_AUTH_NONE;
- break;
- case RSN_AKM_UNSPECIFIED:
- brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
- is_rsn_ie ? (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
- (wpa_auth |= WPA_AUTH_UNSPECIFIED);
- break;
- case RSN_AKM_PSK:
- brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
- is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
- (wpa_auth |= WPA_AUTH_PSK);
- break;
- case RSN_AKM_SHA256_PSK:
- brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
- wpa_auth |= WPA2_AUTH_PSK_SHA256;
- break;
- case RSN_AKM_SHA256_1X:
- brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
- wpa_auth |= WPA2_AUTH_1X_SHA256;
- break;
- case RSN_AKM_SAE:
- brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
- wpa_auth |= WPA3_AUTH_SAE_PSK;
- break;
- default:
- bphy_err(drvr, "Invalid key mgmt info\n");
- }
offset++;
}

@@ -4312,10 +4329,12 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
*/
if (!(wpa_auth & (WPA2_AUTH_PSK_SHA256 |
WPA2_AUTH_1X_SHA256 |
+ WFA_AUTH_DPP |
WPA3_AUTH_SAE_PSK))) {
err = -EINVAL;
goto exit;
}
+
/* Firmware has requirement that WPA2_AUTH_PSK/
* WPA2_AUTH_UNSPECIFIED be set, if SHA256 OUI
* is to be included in the rsn ie.
@@ -5225,7 +5244,7 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
*cookie, le16_to_cpu(action_frame->len), freq);

ack = brcmf_p2p_send_action_frame(cfg, cfg_to_ndev(cfg),
- af_params);
+ af_params, vif);

cfg80211_mgmt_tx_status(wdev, *cookie, buf, len, ack,
GFP_KERNEL);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 479041f070f9..4636fc27e915 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -231,7 +231,35 @@ static bool brcmf_p2p_is_pub_action(void *frame, u32 frame_len)
if (pact_frm->category == P2P_PUB_AF_CATEGORY &&
pact_frm->action == P2P_PUB_AF_ACTION &&
pact_frm->oui_type == P2P_VER &&
- memcmp(pact_frm->oui, P2P_OUI, P2P_OUI_LEN) == 0)
+ memcmp(pact_frm->oui, WFA_OUI, P2P_OUI_LEN) == 0)
+ return true;
+
+ return false;
+}
+
+/**
+ * brcmf_p2p_is_dpp_pub_action() - true if dpp public type frame.
+ *
+ * @frame: action frame data.
+ * @frame_len: length of action frame data.
+ *
+ * Determine if action frame is dpp public action type
+ */
+static bool brcmf_p2p_is_dpp_pub_action(void *frame, u32 frame_len)
+{
+ struct brcmf_p2p_pub_act_frame *pact_frm;
+
+ if (!frame)
+ return false;
+
+ pact_frm = (struct brcmf_p2p_pub_act_frame *)frame;
+ if (frame_len < sizeof(struct brcmf_p2p_pub_act_frame) - 1)
+ return false;
+
+ if (pact_frm->category == WLAN_CATEGORY_PUBLIC &&
+ pact_frm->action == WLAN_PUB_ACTION_VENDOR_SPECIFIC &&
+ pact_frm->oui_type == DPP_VER &&
+ memcmp(pact_frm->oui, WFA_OUI, TLV_OUI_LEN) == 0)
return true;

return false;
@@ -991,6 +1019,8 @@ int brcmf_p2p_remain_on_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
if (err)
goto exit;

+ p2p->remin_on_channel_wdev = wdev;
+
memcpy(&p2p->remain_on_channel, channel, sizeof(*channel));
*cookie = p2p->remain_on_channel_cookie;
cfg80211_ready_on_channel(wdev, *cookie, channel, duration, GFP_KERNEL);
@@ -1014,6 +1044,7 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
{
struct brcmf_cfg80211_info *cfg = ifp->drvr->config;
struct brcmf_p2p_info *p2p = &cfg->p2p;
+ struct wireless_dev *wdev = p2p->remin_on_channel_wdev;

brcmf_dbg(TRACE, "Enter\n");
if (test_and_clear_bit(BRCMF_P2P_STATUS_DISCOVER_LISTEN,
@@ -1026,10 +1057,16 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
complete(&p2p->wait_next_af);
}

- cfg80211_remain_on_channel_expired(&ifp->vif->wdev,
+ wdev = p2p->remin_on_channel_wdev ?
+ p2p->remin_on_channel_wdev :
+ &ifp->vif->wdev;
+
+ cfg80211_remain_on_channel_expired(wdev,
p2p->remain_on_channel_cookie,
&p2p->remain_on_channel,
GFP_KERNEL);
+ p2p->remin_on_channel_wdev = NULL;
+
}
return 0;
}
@@ -1531,6 +1568,7 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
*
* @p2p: p2p info struct for vif.
* @af_params: action frame data/info.
+ * @vif: vif to send
*
* Send an action frame immediately without doing channel synchronization.
*
@@ -1539,12 +1577,17 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
* frame is transmitted.
*/
static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
- struct brcmf_fil_af_params_le *af_params)
+ struct brcmf_fil_af_params_le *af_params,
+ struct brcmf_cfg80211_vif *vif
+ )
{
struct brcmf_pub *drvr = p2p->cfg->pub;
- struct brcmf_cfg80211_vif *vif;
- struct brcmf_p2p_action_frame *p2p_af;
s32 err = 0;
+ struct brcmf_fil_action_frame_le *action_frame;
+ u16 action_frame_len;
+
+ action_frame = &af_params->action_frame;
+ action_frame_len = le16_to_cpu(action_frame->len);

brcmf_dbg(TRACE, "Enter\n");

@@ -1552,13 +1595,6 @@ static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);

- /* check if it is a p2p_presence response */
- p2p_af = (struct brcmf_p2p_action_frame *)af_params->action_frame.data;
- if (p2p_af->subtype == P2P_AF_PRESENCE_RSP)
- vif = p2p->bss_idx[P2PAPI_BSSCFG_CONNECTION].vif;
- else
- vif = p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif;
-
err = brcmf_fil_bsscfg_data_set(vif->ifp, "actframe", af_params,
sizeof(*af_params));
if (err) {
@@ -1714,10 +1750,13 @@ static bool brcmf_p2p_check_dwell_overflow(u32 requested_dwell,
* @cfg: driver private data for cfg80211 interface.
* @ndev: net device to transmit on.
* @af_params: configuration data for action frame.
+ * @vif: virtual interface to send
*/
bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
struct net_device *ndev,
- struct brcmf_fil_af_params_le *af_params)
+ struct brcmf_fil_af_params_le *af_params,
+ struct brcmf_cfg80211_vif *vif
+ )
{
struct brcmf_p2p_info *p2p = &cfg->p2p;
struct brcmf_if *ifp = netdev_priv(ndev);
@@ -1789,7 +1828,9 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
goto exit;
}
} else if (brcmf_p2p_is_p2p_action(action_frame->data,
- action_frame_len)) {
+ action_frame_len) ||
+ brcmf_p2p_is_dpp_pub_action(action_frame->data,
+ action_frame_len)) {
/* do not configure anything. it will be */
/* sent with a default configuration */
} else {
@@ -1857,7 +1898,7 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
if (af_params->channel)
msleep(P2P_AF_RETRY_DELAY_TIME);

- ack = !brcmf_p2p_tx_action_frame(p2p, af_params);
+ ack = !brcmf_p2p_tx_action_frame(p2p, af_params, vif);
tx_retry++;
dwell_overflow = brcmf_p2p_check_dwell_overflow(requested_dwell,
dwell_jiffies);
@@ -2508,6 +2549,7 @@ s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced)

pri_ifp = brcmf_get_ifp(cfg->pub, 0);
p2p->bss_idx[P2PAPI_BSSCFG_PRIMARY].vif = pri_ifp->vif;
+ init_completion(&p2p->send_af_done);

if (p2pdev_forced) {
err_ptr = brcmf_p2p_create_p2pdev(p2p, NULL, NULL);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
index d2ecee565bf2..bbc455238707 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
@@ -138,6 +138,7 @@ struct brcmf_p2p_info {
bool block_gon_req_tx;
bool p2pdev_dynamically;
bool wait_for_offchan_complete;
+ struct wireless_dev *remin_on_channel_wdev;
};

s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced);
@@ -170,7 +171,8 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
void *data);
bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
struct net_device *ndev,
- struct brcmf_fil_af_params_le *af_params);
+ struct brcmf_fil_af_params_le *af_params,
+ struct brcmf_cfg80211_vif *vif);
bool brcmf_p2p_scan_finding_common_channel(struct brcmf_cfg80211_info *cfg,
struct brcmf_bss_info_le *bi);
s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
index 7552bdb91991..3a9cad3730b8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
@@ -233,6 +233,11 @@ static inline bool ac_bitmap_tst(u8 bitmap, int prec)

#define WPA3_AUTH_SAE_PSK 0x40000 /* SAE with 4-way handshake */

+#define WFA_AUTH_DPP 0x200000 /* WFA DPP AUTH */
+
+#define WFA_OUI "\x50\x6F\x9A" /* WFA OUI */
+#define DPP_VER 0x1A /* WFA DPP v1.0 */
+
#define DOT11_DEFAULT_RTS_LEN 2347
#define DOT11_DEFAULT_FRAG_LEN 2346

--
2.25.0


2022-09-21 04:58:35

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] brcmfmac: Support DPP feature

Hi Ian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.0-rc6 next-20220920]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Ian-Lin/brcmfmac-Support-DPP-feature-series/20220921-101658
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220921/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/a4592ec96782290389bab0b4ca9cd9bc0ae4672a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ian-Lin/brcmfmac-Support-DPP-feature-series/20220921-101658
git checkout a4592ec96782290389bab0b4ca9cd9bc0ae4672a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/wireless/broadcom/brcm80211/brcmfmac/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c: In function 'brcmf_p2p_tx_action_frame':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c:1587:13: warning: variable 'action_frame_len' set but not used [-Wunused-but-set-variable]
1587 | u16 action_frame_len;
| ^~~~~~~~~~~~~~~~


vim +/action_frame_len +1587 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c

1564
1565
1566 /**
1567 * brcmf_p2p_tx_action_frame() - send action frame over fil.
1568 *
1569 * @p2p: p2p info struct for vif.
1570 * @af_params: action frame data/info.
1571 * @vif: vif to send
1572 *
1573 * Send an action frame immediately without doing channel synchronization.
1574 *
1575 * This function waits for a completion event before returning.
1576 * The WLC_E_ACTION_FRAME_COMPLETE event will be received when the action
1577 * frame is transmitted.
1578 */
1579 static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
1580 struct brcmf_fil_af_params_le *af_params,
1581 struct brcmf_cfg80211_vif *vif
1582 )
1583 {
1584 struct brcmf_pub *drvr = p2p->cfg->pub;
1585 s32 err = 0;
1586 struct brcmf_fil_action_frame_le *action_frame;
> 1587 u16 action_frame_len;
1588
1589 action_frame = &af_params->action_frame;
1590 action_frame_len = le16_to_cpu(action_frame->len);
1591
1592 brcmf_dbg(TRACE, "Enter\n");
1593
1594 reinit_completion(&p2p->send_af_done);
1595 clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
1596 clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);
1597
1598 err = brcmf_fil_bsscfg_data_set(vif->ifp, "actframe", af_params,
1599 sizeof(*af_params));
1600 if (err) {
1601 bphy_err(drvr, " sending action frame has failed\n");
1602 goto exit;
1603 }
1604
1605 p2p->af_sent_channel = le32_to_cpu(af_params->channel);
1606 p2p->af_tx_sent_jiffies = jiffies;
1607
1608 if (test_bit(BRCMF_P2P_STATUS_DISCOVER_LISTEN, &p2p->status) &&
1609 p2p->af_sent_channel ==
1610 ieee80211_frequency_to_channel(p2p->remain_on_channel.center_freq))
1611 p2p->wait_for_offchan_complete = false;
1612 else
1613 p2p->wait_for_offchan_complete = true;
1614
1615 brcmf_dbg(TRACE, "Waiting for %s tx completion event\n",
1616 (p2p->wait_for_offchan_complete) ?
1617 "off-channel" : "on-channel");
1618
1619 wait_for_completion_timeout(&p2p->send_af_done, P2P_AF_MAX_WAIT_TIME);
1620
1621 if (test_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status)) {
1622 brcmf_dbg(TRACE, "TX action frame operation is success\n");
1623 } else {
1624 err = -EIO;
1625 brcmf_dbg(TRACE, "TX action frame operation has failed\n");
1626 }
1627 /* clear status bit for action tx */
1628 clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
1629 clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);
1630
1631 exit:
1632 return err;
1633 }
1634

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-09-21 17:30:26

by Franky Lin

[permalink] [raw]
Subject: Re: [PATCH 1/3] brcmfmac: Support DPP feature

On Tue, Sep 20, 2022 at 7:04 PM Ian Lin <[email protected]> wrote:
>
> From: Kurt Lee <[email protected]>
>
> Let driver parse DPP frames from upper layer and do conresponding

corresponding

> configuration to firmware.
> This change supports DPP handshake based on wpa_supplicant v2.9.
>
> Signed-off-by: Kurt Lee <[email protected]>
> Signed-off-by: Ian Lin <[email protected]>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 85 ++++++++++++-------
> .../broadcom/brcm80211/brcmfmac/p2p.c | 72 ++++++++++++----
> .../broadcom/brcm80211/brcmfmac/p2p.h | 4 +-
> .../broadcom/brcm80211/include/brcmu_wifi.h | 5 ++
> 4 files changed, 117 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 7c72ea26a7d7..4a8aceda8fe8 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -64,6 +64,9 @@
> #define RSN_CAP_MFPC_MASK BIT(7)
> #define RSN_PMKID_COUNT_LEN 2
>
> +#define DPP_AKM_SUITE_TYPE 2
> +#define WLAN_AKM_SUITE_DPP SUITE(WLAN_OUI_WFA, DPP_AKM_SUITE_TYPE)

Please use WLAN_AKM_SUITE_WFA_DPP.


> +
> #define VNDR_IE_CMD_LEN 4 /* length of the set command
> * string :"add", "del" (+ NUL)
> */
> @@ -1816,6 +1819,9 @@ brcmf_set_key_mgmt(struct net_device *ndev, struct cfg80211_connect_params *sme)
> val = WPA2_AUTH_PSK | WPA2_AUTH_FT;
> profile->is_ft = true;
> break;
> + case WLAN_AKM_SUITE_DPP:
> + val = WFA_AUTH_DPP;
> + break;
> default:
> bphy_err(drvr, "invalid akm suite (%d)\n",
> sme->crypto.akm_suites[0]);
> @@ -4144,6 +4150,12 @@ static bool brcmf_valid_wpa_oui(u8 *oui, bool is_rsn_ie)
> return (memcmp(oui, WPA_OUI, TLV_OUI_LEN) == 0);
> }
>
> +static bool brcmf_valid_dpp_suite(u8 *oui)
> +{
> + return (memcmp(oui, WFA_OUI, TLV_OUI_LEN) == 0 &&
> + *(oui + TLV_OUI_LEN) == DPP_AKM_SUITE_TYPE);
> +}
> +
> static s32
> brcmf_configure_wpaie(struct brcmf_if *ifp,
> const struct brcmf_vs_tlv *wpa_ie,
> @@ -4257,42 +4269,47 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
> goto exit;
> }
> for (i = 0; i < count; i++) {
> - if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
> + if (brcmf_valid_dpp_suite(&data[offset])) {
> + wpa_auth |= WFA_AUTH_DPP;
> + offset += TLV_OUI_LEN;
> + } else if (brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
> + offset += TLV_OUI_LEN;
> + switch (data[offset]) {
> + case RSN_AKM_NONE:
> + brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
> + wpa_auth |= WPA_AUTH_NONE;
> + break;
> + case RSN_AKM_UNSPECIFIED:
> + brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
> + is_rsn_ie ?
> + (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
> + (wpa_auth |= WPA_AUTH_UNSPECIFIED);
> + break;
> + case RSN_AKM_PSK:
> + brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
> + is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
> + (wpa_auth |= WPA_AUTH_PSK);
> + break;
> + case RSN_AKM_SHA256_PSK:
> + brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
> + wpa_auth |= WPA2_AUTH_PSK_SHA256;
> + break;
> + case RSN_AKM_SHA256_1X:
> + brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
> + wpa_auth |= WPA2_AUTH_1X_SHA256;
> + break;
> + case RSN_AKM_SAE:
> + brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
> + wpa_auth |= WPA3_AUTH_SAE_PSK;
> + break;
> + default:
> + bphy_err(drvr, "Invalid key mgmt info\n");
> + }
> + } else {

Only check for invalid case here
if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie) &&
!brcmf_valid_dpp_suite(&data[offset])) {

And keep the switch below but add a new case for DPP_AKM_SUITE_TYPE.

> err = -EINVAL;
> bphy_err(drvr, "ivalid OUI\n");
> goto exit;
> }
> - offset += TLV_OUI_LEN;
> - switch (data[offset]) {
> - case RSN_AKM_NONE:
> - brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
> - wpa_auth |= WPA_AUTH_NONE;
> - break;
> - case RSN_AKM_UNSPECIFIED:
> - brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
> - is_rsn_ie ? (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
> - (wpa_auth |= WPA_AUTH_UNSPECIFIED);
> - break;
> - case RSN_AKM_PSK:
> - brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
> - is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
> - (wpa_auth |= WPA_AUTH_PSK);
> - break;
> - case RSN_AKM_SHA256_PSK:
> - brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
> - wpa_auth |= WPA2_AUTH_PSK_SHA256;
> - break;
> - case RSN_AKM_SHA256_1X:
> - brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
> - wpa_auth |= WPA2_AUTH_1X_SHA256;
> - break;
> - case RSN_AKM_SAE:
> - brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
> - wpa_auth |= WPA3_AUTH_SAE_PSK;
> - break;
> - default:
> - bphy_err(drvr, "Invalid key mgmt info\n");
> - }
> offset++;
> }
>
> @@ -4312,10 +4329,12 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
> */
> if (!(wpa_auth & (WPA2_AUTH_PSK_SHA256 |
> WPA2_AUTH_1X_SHA256 |
> + WFA_AUTH_DPP |
> WPA3_AUTH_SAE_PSK))) {
> err = -EINVAL;
> goto exit;
> }
> +
> /* Firmware has requirement that WPA2_AUTH_PSK/
> * WPA2_AUTH_UNSPECIFIED be set, if SHA256 OUI
> * is to be included in the rsn ie.
> @@ -5225,7 +5244,7 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> *cookie, le16_to_cpu(action_frame->len), freq);
>
> ack = brcmf_p2p_send_action_frame(cfg, cfg_to_ndev(cfg),
> - af_params);
> + af_params, vif);
>
> cfg80211_mgmt_tx_status(wdev, *cookie, buf, len, ack,
> GFP_KERNEL);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> index 479041f070f9..4636fc27e915 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> @@ -231,7 +231,35 @@ static bool brcmf_p2p_is_pub_action(void *frame, u32 frame_len)
> if (pact_frm->category == P2P_PUB_AF_CATEGORY &&
> pact_frm->action == P2P_PUB_AF_ACTION &&
> pact_frm->oui_type == P2P_VER &&
> - memcmp(pact_frm->oui, P2P_OUI, P2P_OUI_LEN) == 0)
> + memcmp(pact_frm->oui, WFA_OUI, P2P_OUI_LEN) == 0)

This change is irrelevant although the two macros are the same. Please
have a separate patch to clean up P2P_OUI.

> + return true;
> +
> + return false;
> +}
> +
> +/**
> + * brcmf_p2p_is_dpp_pub_action() - true if dpp public type frame.
> + *
> + * @frame: action frame data.
> + * @frame_len: length of action frame data.
> + *
> + * Determine if action frame is dpp public action type
> + */
> +static bool brcmf_p2p_is_dpp_pub_action(void *frame, u32 frame_len)
> +{
> + struct brcmf_p2p_pub_act_frame *pact_frm;
> +
> + if (!frame)
> + return false;
> +
> + pact_frm = (struct brcmf_p2p_pub_act_frame *)frame;
> + if (frame_len < sizeof(struct brcmf_p2p_pub_act_frame) - 1)
> + return false;
> +
> + if (pact_frm->category == WLAN_CATEGORY_PUBLIC &&
> + pact_frm->action == WLAN_PUB_ACTION_VENDOR_SPECIFIC &&
> + pact_frm->oui_type == DPP_VER &&
> + memcmp(pact_frm->oui, WFA_OUI, TLV_OUI_LEN) == 0)
> return true;
>
> return false;
> @@ -991,6 +1019,8 @@ int brcmf_p2p_remain_on_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
> if (err)
> goto exit;
>
> + p2p->remin_on_channel_wdev = wdev;
> +
> memcpy(&p2p->remain_on_channel, channel, sizeof(*channel));
> *cookie = p2p->remain_on_channel_cookie;
> cfg80211_ready_on_channel(wdev, *cookie, channel, duration, GFP_KERNEL);
> @@ -1014,6 +1044,7 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
> {
> struct brcmf_cfg80211_info *cfg = ifp->drvr->config;
> struct brcmf_p2p_info *p2p = &cfg->p2p;
> + struct wireless_dev *wdev = p2p->remin_on_channel_wdev;
>
> brcmf_dbg(TRACE, "Enter\n");
> if (test_and_clear_bit(BRCMF_P2P_STATUS_DISCOVER_LISTEN,
> @@ -1026,10 +1057,16 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
> complete(&p2p->wait_next_af);
> }
>
> - cfg80211_remain_on_channel_expired(&ifp->vif->wdev,
> + wdev = p2p->remin_on_channel_wdev ?
> + p2p->remin_on_channel_wdev :
> + &ifp->vif->wdev;
> +
> + cfg80211_remain_on_channel_expired(wdev,
> p2p->remain_on_channel_cookie,
> &p2p->remain_on_channel,
> GFP_KERNEL);
> + p2p->remin_on_channel_wdev = NULL;
> +
> }
> return 0;
> }
> @@ -1531,6 +1568,7 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
> *
> * @p2p: p2p info struct for vif.
> * @af_params: action frame data/info.
> + * @vif: vif to send
> *
> * Send an action frame immediately without doing channel synchronization.
> *
> @@ -1539,12 +1577,17 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
> * frame is transmitted.
> */
> static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
> - struct brcmf_fil_af_params_le *af_params)
> + struct brcmf_fil_af_params_le *af_params,
> + struct brcmf_cfg80211_vif *vif
> + )
> {
> struct brcmf_pub *drvr = p2p->cfg->pub;
> - struct brcmf_cfg80211_vif *vif;
> - struct brcmf_p2p_action_frame *p2p_af;
> s32 err = 0;
> + struct brcmf_fil_action_frame_le *action_frame;
> + u16 action_frame_len;
> +
> + action_frame = &af_params->action_frame;
> + action_frame_len = le16_to_cpu(action_frame->len);
>
> brcmf_dbg(TRACE, "Enter\n");
>
> @@ -1552,13 +1595,6 @@ static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
> clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
> clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);
>
> - /* check if it is a p2p_presence response */
> - p2p_af = (struct brcmf_p2p_action_frame *)af_params->action_frame.data;
> - if (p2p_af->subtype == P2P_AF_PRESENCE_RSP)
> - vif = p2p->bss_idx[P2PAPI_BSSCFG_CONNECTION].vif;
> - else
> - vif = p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif;
> -
> err = brcmf_fil_bsscfg_data_set(vif->ifp, "actframe", af_params,
> sizeof(*af_params));
> if (err) {
> @@ -1714,10 +1750,13 @@ static bool brcmf_p2p_check_dwell_overflow(u32 requested_dwell,
> * @cfg: driver private data for cfg80211 interface.
> * @ndev: net device to transmit on.
> * @af_params: configuration data for action frame.
> + * @vif: virtual interface to send
> */
> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> struct net_device *ndev,
> - struct brcmf_fil_af_params_le *af_params)
> + struct brcmf_fil_af_params_le *af_params,
> + struct brcmf_cfg80211_vif *vif
> + )
> {
> struct brcmf_p2p_info *p2p = &cfg->p2p;
> struct brcmf_if *ifp = netdev_priv(ndev);
> @@ -1789,7 +1828,9 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> goto exit;
> }
> } else if (brcmf_p2p_is_p2p_action(action_frame->data,
> - action_frame_len)) {
> + action_frame_len) ||
> + brcmf_p2p_is_dpp_pub_action(action_frame->data,
> + action_frame_len)) {
> /* do not configure anything. it will be */
> /* sent with a default configuration */
> } else {
> @@ -1857,7 +1898,7 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> if (af_params->channel)
> msleep(P2P_AF_RETRY_DELAY_TIME);
>
> - ack = !brcmf_p2p_tx_action_frame(p2p, af_params);
> + ack = !brcmf_p2p_tx_action_frame(p2p, af_params, vif);
> tx_retry++;
> dwell_overflow = brcmf_p2p_check_dwell_overflow(requested_dwell,
> dwell_jiffies);
> @@ -2508,6 +2549,7 @@ s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced)
>
> pri_ifp = brcmf_get_ifp(cfg->pub, 0);
> p2p->bss_idx[P2PAPI_BSSCFG_PRIMARY].vif = pri_ifp->vif;
> + init_completion(&p2p->send_af_done);
>
> if (p2pdev_forced) {
> err_ptr = brcmf_p2p_create_p2pdev(p2p, NULL, NULL);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
> index d2ecee565bf2..bbc455238707 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
> @@ -138,6 +138,7 @@ struct brcmf_p2p_info {
> bool block_gon_req_tx;
> bool p2pdev_dynamically;
> bool wait_for_offchan_complete;
> + struct wireless_dev *remin_on_channel_wdev;

Docstring needs update. Also s/remin/remain

> };
>
> s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced);
> @@ -170,7 +171,8 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
> void *data);
> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> struct net_device *ndev,
> - struct brcmf_fil_af_params_le *af_params);
> + struct brcmf_fil_af_params_le *af_params,
> + struct brcmf_cfg80211_vif *vif);
> bool brcmf_p2p_scan_finding_common_channel(struct brcmf_cfg80211_info *cfg,
> struct brcmf_bss_info_le *bi);
> s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
> diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
> index 7552bdb91991..3a9cad3730b8 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
> @@ -233,6 +233,11 @@ static inline bool ac_bitmap_tst(u8 bitmap, int prec)
>
> #define WPA3_AUTH_SAE_PSK 0x40000 /* SAE with 4-way handshake */
>
> +#define WFA_AUTH_DPP 0x200000 /* WFA DPP AUTH */

This is incompatible with Broadcom's bit definitions. Please use a per
vendor approach.

> +
> +#define WFA_OUI "\x50\x6F\x9A" /* WFA OUI */
> +#define DPP_VER 0x1A /* WFA DPP v1.0 */
> +
> #define DOT11_DEFAULT_RTS_LEN 2347
> #define DOT11_DEFAULT_FRAG_LEN 2346
>
> --
> 2.25.0
>

--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.


Attachments:
smime.p7s (4.10 kB)
S/MIME Cryptographic Signature

2022-09-23 08:27:22

by Ian Lin

[permalink] [raw]
Subject: Re: [PATCH 1/3] brcmfmac: Support DPP feature



On 9/22/2022 1:10 AM, Franky Lin wrote:
> On Tue, Sep 20, 2022 at 7:04 PM Ian Lin <[email protected]> wrote:
>> From: Kurt Lee <[email protected]>
>>
>> Let driver parse DPP frames from upper layer and do conresponding
> corresponding
Will fix in next version.

>> configuration to firmware.
>> This change supports DPP handshake based on wpa_supplicant v2.9.
>>
>> Signed-off-by: Kurt Lee <[email protected]>
>> Signed-off-by: Ian Lin <[email protected]>
>> ---
>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 85 ++++++++++++-------
>> .../broadcom/brcm80211/brcmfmac/p2p.c | 72 ++++++++++++----
>> .../broadcom/brcm80211/brcmfmac/p2p.h | 4 +-
>> .../broadcom/brcm80211/include/brcmu_wifi.h | 5 ++
>> 4 files changed, 117 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 7c72ea26a7d7..4a8aceda8fe8 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -64,6 +64,9 @@
>> #define RSN_CAP_MFPC_MASK BIT(7)
>> #define RSN_PMKID_COUNT_LEN 2
>>
>> +#define DPP_AKM_SUITE_TYPE 2
>> +#define WLAN_AKM_SUITE_DPP SUITE(WLAN_OUI_WFA, DPP_AKM_SUITE_TYPE)
> Please use WLAN_AKM_SUITE_WFA_DPP.
Will fix in next version.

>
>> +
>> #define VNDR_IE_CMD_LEN 4 /* length of the set command
>> * string :"add", "del" (+ NUL)
>> */
>> @@ -1816,6 +1819,9 @@ brcmf_set_key_mgmt(struct net_device *ndev, struct cfg80211_connect_params *sme)
>> val = WPA2_AUTH_PSK | WPA2_AUTH_FT;
>> profile->is_ft = true;
>> break;
>> + case WLAN_AKM_SUITE_DPP:
>> + val = WFA_AUTH_DPP;
>> + break;
>> default:
>> bphy_err(drvr, "invalid akm suite (%d)\n",
>> sme->crypto.akm_suites[0]);
>> @@ -4144,6 +4150,12 @@ static bool brcmf_valid_wpa_oui(u8 *oui, bool is_rsn_ie)
>> return (memcmp(oui, WPA_OUI, TLV_OUI_LEN) == 0);
>> }
>>
>> +static bool brcmf_valid_dpp_suite(u8 *oui)
>> +{
>> + return (memcmp(oui, WFA_OUI, TLV_OUI_LEN) == 0 &&
>> + *(oui + TLV_OUI_LEN) == DPP_AKM_SUITE_TYPE);
>> +}
>> +
>> static s32
>> brcmf_configure_wpaie(struct brcmf_if *ifp,
>> const struct brcmf_vs_tlv *wpa_ie,
>> @@ -4257,42 +4269,47 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
>> goto exit;
>> }
>> for (i = 0; i < count; i++) {
>> - if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
>> + if (brcmf_valid_dpp_suite(&data[offset])) {
>> + wpa_auth |= WFA_AUTH_DPP;
>> + offset += TLV_OUI_LEN;
>> + } else if (brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
>> + offset += TLV_OUI_LEN;
>> + switch (data[offset]) {
>> + case RSN_AKM_NONE:
>> + brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
>> + wpa_auth |= WPA_AUTH_NONE;
>> + break;
>> + case RSN_AKM_UNSPECIFIED:
>> + brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
>> + is_rsn_ie ?
>> + (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
>> + (wpa_auth |= WPA_AUTH_UNSPECIFIED);
>> + break;
>> + case RSN_AKM_PSK:
>> + brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
>> + is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
>> + (wpa_auth |= WPA_AUTH_PSK);
>> + break;
>> + case RSN_AKM_SHA256_PSK:
>> + brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
>> + wpa_auth |= WPA2_AUTH_PSK_SHA256;
>> + break;
>> + case RSN_AKM_SHA256_1X:
>> + brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
>> + wpa_auth |= WPA2_AUTH_1X_SHA256;
>> + break;
>> + case RSN_AKM_SAE:
>> + brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
>> + wpa_auth |= WPA3_AUTH_SAE_PSK;
>> + break;
>> + default:
>> + bphy_err(drvr, "Invalid key mgmt info\n");
>> + }
>> + } else {
> Only check for invalid case here
> if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie) &&
> !brcmf_valid_dpp_suite(&data[offset])) {
>
> And keep the switch below but add a new case for DPP_AKM_SUITE_TYPE.
The two authentication type refer to equal value: DPP_AKM_SUITE_TYPE &
RSN_AKM_PSK (both =2)
So I can not directly place DPP_AKM_SUITE_TYPE into switch case.

It's more reasonable that we firstly check if it's DPP by
brcmf_valid_dpp_suite
 - need to check WFA_OUI and DPP_AKM_SUITE_TYPE
Then go to the other cases.

>> err = -EINVAL;
>> bphy_err(drvr, "ivalid OUI\n");
>> goto exit;
>> }
>> - offset += TLV_OUI_LEN;
>> - switch (data[offset]) {
>> - case RSN_AKM_NONE:
>> - brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
>> - wpa_auth |= WPA_AUTH_NONE;
>> - break;
>> - case RSN_AKM_UNSPECIFIED:
>> - brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
>> - is_rsn_ie ? (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
>> - (wpa_auth |= WPA_AUTH_UNSPECIFIED);
>> - break;
>> - case RSN_AKM_PSK:
>> - brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
>> - is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
>> - (wpa_auth |= WPA_AUTH_PSK);
>> - break;
>> - case RSN_AKM_SHA256_PSK:
>> - brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
>> - wpa_auth |= WPA2_AUTH_PSK_SHA256;
>> - break;
>> - case RSN_AKM_SHA256_1X:
>> - brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
>> - wpa_auth |= WPA2_AUTH_1X_SHA256;
>> - break;
>> - case RSN_AKM_SAE:
>> - brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
>> - wpa_auth |= WPA3_AUTH_SAE_PSK;
>> - break;
>> - default:
>> - bphy_err(drvr, "Invalid key mgmt info\n");
>> - }
>> offset++;
>> }
>>
>> @@ -4312,10 +4329,12 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
>> */
>> if (!(wpa_auth & (WPA2_AUTH_PSK_SHA256 |
>> WPA2_AUTH_1X_SHA256 |
>> + WFA_AUTH_DPP |
>> WPA3_AUTH_SAE_PSK))) {
>> err = -EINVAL;
>> goto exit;
>> }
>> +
>> /* Firmware has requirement that WPA2_AUTH_PSK/
>> * WPA2_AUTH_UNSPECIFIED be set, if SHA256 OUI
>> * is to be included in the rsn ie.
>> @@ -5225,7 +5244,7 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>> *cookie, le16_to_cpu(action_frame->len), freq);
>>
>> ack = brcmf_p2p_send_action_frame(cfg, cfg_to_ndev(cfg),
>> - af_params);
>> + af_params, vif);
>>
>> cfg80211_mgmt_tx_status(wdev, *cookie, buf, len, ack,
>> GFP_KERNEL);
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> index 479041f070f9..4636fc27e915 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> @@ -231,7 +231,35 @@ static bool brcmf_p2p_is_pub_action(void *frame, u32 frame_len)
>> if (pact_frm->category == P2P_PUB_AF_CATEGORY &&
>> pact_frm->action == P2P_PUB_AF_ACTION &&
>> pact_frm->oui_type == P2P_VER &&
>> - memcmp(pact_frm->oui, P2P_OUI, P2P_OUI_LEN) == 0)
>> + memcmp(pact_frm->oui, WFA_OUI, P2P_OUI_LEN) == 0)
> This change is irrelevant although the two macros are the same. Please
> have a separate patch to clean up P2P_OUI.
Will fix in next version.

>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +/**
>> + * brcmf_p2p_is_dpp_pub_action() - true if dpp public type frame.
>> + *
>> + * @frame: action frame data.
>> + * @frame_len: length of action frame data.
>> + *
>> + * Determine if action frame is dpp public action type
>> + */
>> +static bool brcmf_p2p_is_dpp_pub_action(void *frame, u32 frame_len)
>> +{
>> + struct brcmf_p2p_pub_act_frame *pact_frm;
>> +
>> + if (!frame)
>> + return false;
>> +
>> + pact_frm = (struct brcmf_p2p_pub_act_frame *)frame;
>> + if (frame_len < sizeof(struct brcmf_p2p_pub_act_frame) - 1)
>> + return false;
>> +
>> + if (pact_frm->category == WLAN_CATEGORY_PUBLIC &&
>> + pact_frm->action == WLAN_PUB_ACTION_VENDOR_SPECIFIC &&
>> + pact_frm->oui_type == DPP_VER &&
>> + memcmp(pact_frm->oui, WFA_OUI, TLV_OUI_LEN) == 0)
>> return true;
>>
>> return false;
>> @@ -991,6 +1019,8 @@ int brcmf_p2p_remain_on_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
>> if (err)
>> goto exit;
>>
>> + p2p->remin_on_channel_wdev = wdev;
>> +
>> memcpy(&p2p->remain_on_channel, channel, sizeof(*channel));
>> *cookie = p2p->remain_on_channel_cookie;
>> cfg80211_ready_on_channel(wdev, *cookie, channel, duration, GFP_KERNEL);
>> @@ -1014,6 +1044,7 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
>> {
>> struct brcmf_cfg80211_info *cfg = ifp->drvr->config;
>> struct brcmf_p2p_info *p2p = &cfg->p2p;
>> + struct wireless_dev *wdev = p2p->remin_on_channel_wdev;
>>
>> brcmf_dbg(TRACE, "Enter\n");
>> if (test_and_clear_bit(BRCMF_P2P_STATUS_DISCOVER_LISTEN,
>> @@ -1026,10 +1057,16 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
>> complete(&p2p->wait_next_af);
>> }
>>
>> - cfg80211_remain_on_channel_expired(&ifp->vif->wdev,
>> + wdev = p2p->remin_on_channel_wdev ?
>> + p2p->remin_on_channel_wdev :
>> + &ifp->vif->wdev;
>> +
>> + cfg80211_remain_on_channel_expired(wdev,
>> p2p->remain_on_channel_cookie,
>> &p2p->remain_on_channel,
>> GFP_KERNEL);
>> + p2p->remin_on_channel_wdev = NULL;
>> +
>> }
>> return 0;
>> }
>> @@ -1531,6 +1568,7 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
>> *
>> * @p2p: p2p info struct for vif.
>> * @af_params: action frame data/info.
>> + * @vif: vif to send
>> *
>> * Send an action frame immediately without doing channel synchronization.
>> *
>> @@ -1539,12 +1577,17 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
>> * frame is transmitted.
>> */
>> static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
>> - struct brcmf_fil_af_params_le *af_params)
>> + struct brcmf_fil_af_params_le *af_params,
>> + struct brcmf_cfg80211_vif *vif
>> + )
>> {
>> struct brcmf_pub *drvr = p2p->cfg->pub;
>> - struct brcmf_cfg80211_vif *vif;
>> - struct brcmf_p2p_action_frame *p2p_af;
>> s32 err = 0;
>> + struct brcmf_fil_action_frame_le *action_frame;
>> + u16 action_frame_len;
>> +
>> + action_frame = &af_params->action_frame;
>> + action_frame_len = le16_to_cpu(action_frame->len);
>>
>> brcmf_dbg(TRACE, "Enter\n");
>>
>> @@ -1552,13 +1595,6 @@ static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
>> clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
>> clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);
>>
>> - /* check if it is a p2p_presence response */
>> - p2p_af = (struct brcmf_p2p_action_frame *)af_params->action_frame.data;
>> - if (p2p_af->subtype == P2P_AF_PRESENCE_RSP)
>> - vif = p2p->bss_idx[P2PAPI_BSSCFG_CONNECTION].vif;
>> - else
>> - vif = p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif;
>> -
>> err = brcmf_fil_bsscfg_data_set(vif->ifp, "actframe", af_params,
>> sizeof(*af_params));
>> if (err) {
>> @@ -1714,10 +1750,13 @@ static bool brcmf_p2p_check_dwell_overflow(u32 requested_dwell,
>> * @cfg: driver private data for cfg80211 interface.
>> * @ndev: net device to transmit on.
>> * @af_params: configuration data for action frame.
>> + * @vif: virtual interface to send
>> */
>> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
>> struct net_device *ndev,
>> - struct brcmf_fil_af_params_le *af_params)
>> + struct brcmf_fil_af_params_le *af_params,
>> + struct brcmf_cfg80211_vif *vif
>> + )
>> {
>> struct brcmf_p2p_info *p2p = &cfg->p2p;
>> struct brcmf_if *ifp = netdev_priv(ndev);
>> @@ -1789,7 +1828,9 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
>> goto exit;
>> }
>> } else if (brcmf_p2p_is_p2p_action(action_frame->data,
>> - action_frame_len)) {
>> + action_frame_len) ||
>> + brcmf_p2p_is_dpp_pub_action(action_frame->data,
>> + action_frame_len)) {
>> /* do not configure anything. it will be */
>> /* sent with a default configuration */
>> } else {
>> @@ -1857,7 +1898,7 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
>> if (af_params->channel)
>> msleep(P2P_AF_RETRY_DELAY_TIME);
>>
>> - ack = !brcmf_p2p_tx_action_frame(p2p, af_params);
>> + ack = !brcmf_p2p_tx_action_frame(p2p, af_params, vif);
>> tx_retry++;
>> dwell_overflow = brcmf_p2p_check_dwell_overflow(requested_dwell,
>> dwell_jiffies);
>> @@ -2508,6 +2549,7 @@ s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced)
>>
>> pri_ifp = brcmf_get_ifp(cfg->pub, 0);
>> p2p->bss_idx[P2PAPI_BSSCFG_PRIMARY].vif = pri_ifp->vif;
>> + init_completion(&p2p->send_af_done);
>>
>> if (p2pdev_forced) {
>> err_ptr = brcmf_p2p_create_p2pdev(p2p, NULL, NULL);
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
>> index d2ecee565bf2..bbc455238707 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
>> @@ -138,6 +138,7 @@ struct brcmf_p2p_info {
>> bool block_gon_req_tx;
>> bool p2pdev_dynamically;
>> bool wait_for_offchan_complete;
>> + struct wireless_dev *remin_on_channel_wdev;
> Docstring needs update. Also s/remin/remain
Will fix in next version.

>> };
>>
>> s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced);
>> @@ -170,7 +171,8 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
>> void *data);
>> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
>> struct net_device *ndev,
>> - struct brcmf_fil_af_params_le *af_params);
>> + struct brcmf_fil_af_params_le *af_params,
>> + struct brcmf_cfg80211_vif *vif);
>> bool brcmf_p2p_scan_finding_common_channel(struct brcmf_cfg80211_info *cfg,
>> struct brcmf_bss_info_le *bi);
>> s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
>> index 7552bdb91991..3a9cad3730b8 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
>> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
>> @@ -233,6 +233,11 @@ static inline bool ac_bitmap_tst(u8 bitmap, int prec)
>>
>> #define WPA3_AUTH_SAE_PSK 0x40000 /* SAE with 4-way handshake */
>>
>> +#define WFA_AUTH_DPP 0x200000 /* WFA DPP AUTH */
> This is incompatible with Broadcom's bit definitions. Please use a per
> vendor approach.
We had extended the bit definition.
The authentication mode will be set to our FW so it's FW-dependent.
Do you suggest I change the name? like CY_WFA_AUTH_DPP?

Thank you for the review.

>> +
>> +#define WFA_OUI "\x50\x6F\x9A" /* WFA OUI */
>> +#define DPP_VER 0x1A /* WFA DPP v1.0 */
>> +
>> #define DOT11_DEFAULT_RTS_LEN 2347
>> #define DOT11_DEFAULT_FRAG_LEN 2346
>>
>> --
>> 2.25.0
>>

2022-09-23 16:42:51

by Franky Lin

[permalink] [raw]
Subject: Re: [PATCH 1/3] brcmfmac: Support DPP feature

On Fri, Sep 23, 2022 at 1:20 AM Lin Ian (CSSITB CSS ICW SW WFS / EE)
<[email protected]> wrote:
>
>
>
> On 9/22/2022 1:10 AM, Franky Lin wrote:
> > On Tue, Sep 20, 2022 at 7:04 PM Ian Lin <[email protected]> wrote:
> >> From: Kurt Lee <[email protected]>
> >>
> >> Let driver parse DPP frames from upper layer and do conresponding
> > corresponding
> Will fix in next version.
>
> >> configuration to firmware.
> >> This change supports DPP handshake based on wpa_supplicant v2.9.
> >>
> >> Signed-off-by: Kurt Lee <[email protected]>
> >> Signed-off-by: Ian Lin <[email protected]>
> >> ---
> >> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 85 ++++++++++++-------
> >> .../broadcom/brcm80211/brcmfmac/p2p.c | 72 ++++++++++++----
> >> .../broadcom/brcm80211/brcmfmac/p2p.h | 4 +-
> >> .../broadcom/brcm80211/include/brcmu_wifi.h | 5 ++
> >> 4 files changed, 117 insertions(+), 49 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >> index 7c72ea26a7d7..4a8aceda8fe8 100644
> >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> >> @@ -64,6 +64,9 @@
> >> #define RSN_CAP_MFPC_MASK BIT(7)
> >> #define RSN_PMKID_COUNT_LEN 2
> >>
> >> +#define DPP_AKM_SUITE_TYPE 2
> >> +#define WLAN_AKM_SUITE_DPP SUITE(WLAN_OUI_WFA, DPP_AKM_SUITE_TYPE)
> > Please use WLAN_AKM_SUITE_WFA_DPP.
> Will fix in next version.
>
> >
> >> +
> >> #define VNDR_IE_CMD_LEN 4 /* length of the set command
> >> * string :"add", "del" (+ NUL)
> >> */
> >> @@ -1816,6 +1819,9 @@ brcmf_set_key_mgmt(struct net_device *ndev, struct cfg80211_connect_params *sme)
> >> val = WPA2_AUTH_PSK | WPA2_AUTH_FT;
> >> profile->is_ft = true;
> >> break;
> >> + case WLAN_AKM_SUITE_DPP:
> >> + val = WFA_AUTH_DPP;
> >> + break;
> >> default:
> >> bphy_err(drvr, "invalid akm suite (%d)\n",
> >> sme->crypto.akm_suites[0]);
> >> @@ -4144,6 +4150,12 @@ static bool brcmf_valid_wpa_oui(u8 *oui, bool is_rsn_ie)
> >> return (memcmp(oui, WPA_OUI, TLV_OUI_LEN) == 0);
> >> }
> >>
> >> +static bool brcmf_valid_dpp_suite(u8 *oui)
> >> +{
> >> + return (memcmp(oui, WFA_OUI, TLV_OUI_LEN) == 0 &&
> >> + *(oui + TLV_OUI_LEN) == DPP_AKM_SUITE_TYPE);
> >> +}
> >> +
> >> static s32
> >> brcmf_configure_wpaie(struct brcmf_if *ifp,
> >> const struct brcmf_vs_tlv *wpa_ie,
> >> @@ -4257,42 +4269,47 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
> >> goto exit;
> >> }
> >> for (i = 0; i < count; i++) {
> >> - if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
> >> + if (brcmf_valid_dpp_suite(&data[offset])) {
> >> + wpa_auth |= WFA_AUTH_DPP;
> >> + offset += TLV_OUI_LEN;
> >> + } else if (brcmf_valid_wpa_oui(&data[offset], is_rsn_ie)) {
> >> + offset += TLV_OUI_LEN;
> >> + switch (data[offset]) {
> >> + case RSN_AKM_NONE:
> >> + brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
> >> + wpa_auth |= WPA_AUTH_NONE;
> >> + break;
> >> + case RSN_AKM_UNSPECIFIED:
> >> + brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
> >> + is_rsn_ie ?
> >> + (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
> >> + (wpa_auth |= WPA_AUTH_UNSPECIFIED);
> >> + break;
> >> + case RSN_AKM_PSK:
> >> + brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
> >> + is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
> >> + (wpa_auth |= WPA_AUTH_PSK);
> >> + break;
> >> + case RSN_AKM_SHA256_PSK:
> >> + brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
> >> + wpa_auth |= WPA2_AUTH_PSK_SHA256;
> >> + break;
> >> + case RSN_AKM_SHA256_1X:
> >> + brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
> >> + wpa_auth |= WPA2_AUTH_1X_SHA256;
> >> + break;
> >> + case RSN_AKM_SAE:
> >> + brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
> >> + wpa_auth |= WPA3_AUTH_SAE_PSK;
> >> + break;
> >> + default:
> >> + bphy_err(drvr, "Invalid key mgmt info\n");
> >> + }
> >> + } else {
> > Only check for invalid case here
> > if (!brcmf_valid_wpa_oui(&data[offset], is_rsn_ie) &&
> > !brcmf_valid_dpp_suite(&data[offset])) {
> >
> > And keep the switch below but add a new case for DPP_AKM_SUITE_TYPE.
> The two authentication type refer to equal value: DPP_AKM_SUITE_TYPE &
> RSN_AKM_PSK (both =2)
> So I can not directly place DPP_AKM_SUITE_TYPE into switch case.
>
> It's more reasonable that we firstly check if it's DPP by
> brcmf_valid_dpp_suite
> - need to check WFA_OUI and DPP_AKM_SUITE_TYPE
> Then go to the other cases.
>
> >> err = -EINVAL;
> >> bphy_err(drvr, "ivalid OUI\n");
> >> goto exit;
> >> }
> >> - offset += TLV_OUI_LEN;
> >> - switch (data[offset]) {
> >> - case RSN_AKM_NONE:
> >> - brcmf_dbg(TRACE, "RSN_AKM_NONE\n");
> >> - wpa_auth |= WPA_AUTH_NONE;
> >> - break;
> >> - case RSN_AKM_UNSPECIFIED:
> >> - brcmf_dbg(TRACE, "RSN_AKM_UNSPECIFIED\n");
> >> - is_rsn_ie ? (wpa_auth |= WPA2_AUTH_UNSPECIFIED) :
> >> - (wpa_auth |= WPA_AUTH_UNSPECIFIED);
> >> - break;
> >> - case RSN_AKM_PSK:
> >> - brcmf_dbg(TRACE, "RSN_AKM_PSK\n");
> >> - is_rsn_ie ? (wpa_auth |= WPA2_AUTH_PSK) :
> >> - (wpa_auth |= WPA_AUTH_PSK);
> >> - break;
> >> - case RSN_AKM_SHA256_PSK:
> >> - brcmf_dbg(TRACE, "RSN_AKM_MFP_PSK\n");
> >> - wpa_auth |= WPA2_AUTH_PSK_SHA256;
> >> - break;
> >> - case RSN_AKM_SHA256_1X:
> >> - brcmf_dbg(TRACE, "RSN_AKM_MFP_1X\n");
> >> - wpa_auth |= WPA2_AUTH_1X_SHA256;
> >> - break;
> >> - case RSN_AKM_SAE:
> >> - brcmf_dbg(TRACE, "RSN_AKM_SAE\n");
> >> - wpa_auth |= WPA3_AUTH_SAE_PSK;
> >> - break;
> >> - default:
> >> - bphy_err(drvr, "Invalid key mgmt info\n");
> >> - }
> >> offset++;
> >> }
> >>
> >> @@ -4312,10 +4329,12 @@ brcmf_configure_wpaie(struct brcmf_if *ifp,
> >> */
> >> if (!(wpa_auth & (WPA2_AUTH_PSK_SHA256 |
> >> WPA2_AUTH_1X_SHA256 |
> >> + WFA_AUTH_DPP |
> >> WPA3_AUTH_SAE_PSK))) {
> >> err = -EINVAL;
> >> goto exit;
> >> }
> >> +
> >> /* Firmware has requirement that WPA2_AUTH_PSK/
> >> * WPA2_AUTH_UNSPECIFIED be set, if SHA256 OUI
> >> * is to be included in the rsn ie.
> >> @@ -5225,7 +5244,7 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> >> *cookie, le16_to_cpu(action_frame->len), freq);
> >>
> >> ack = brcmf_p2p_send_action_frame(cfg, cfg_to_ndev(cfg),
> >> - af_params);
> >> + af_params, vif);
> >>
> >> cfg80211_mgmt_tx_status(wdev, *cookie, buf, len, ack,
> >> GFP_KERNEL);
> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> >> index 479041f070f9..4636fc27e915 100644
> >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> >> @@ -231,7 +231,35 @@ static bool brcmf_p2p_is_pub_action(void *frame, u32 frame_len)
> >> if (pact_frm->category == P2P_PUB_AF_CATEGORY &&
> >> pact_frm->action == P2P_PUB_AF_ACTION &&
> >> pact_frm->oui_type == P2P_VER &&
> >> - memcmp(pact_frm->oui, P2P_OUI, P2P_OUI_LEN) == 0)
> >> + memcmp(pact_frm->oui, WFA_OUI, P2P_OUI_LEN) == 0)
> > This change is irrelevant although the two macros are the same. Please
> > have a separate patch to clean up P2P_OUI.
> Will fix in next version.
>
> >> + return true;
> >> +
> >> + return false;
> >> +}
> >> +
> >> +/**
> >> + * brcmf_p2p_is_dpp_pub_action() - true if dpp public type frame.
> >> + *
> >> + * @frame: action frame data.
> >> + * @frame_len: length of action frame data.
> >> + *
> >> + * Determine if action frame is dpp public action type
> >> + */
> >> +static bool brcmf_p2p_is_dpp_pub_action(void *frame, u32 frame_len)
> >> +{
> >> + struct brcmf_p2p_pub_act_frame *pact_frm;
> >> +
> >> + if (!frame)
> >> + return false;
> >> +
> >> + pact_frm = (struct brcmf_p2p_pub_act_frame *)frame;
> >> + if (frame_len < sizeof(struct brcmf_p2p_pub_act_frame) - 1)
> >> + return false;
> >> +
> >> + if (pact_frm->category == WLAN_CATEGORY_PUBLIC &&
> >> + pact_frm->action == WLAN_PUB_ACTION_VENDOR_SPECIFIC &&
> >> + pact_frm->oui_type == DPP_VER &&
> >> + memcmp(pact_frm->oui, WFA_OUI, TLV_OUI_LEN) == 0)
> >> return true;
> >>
> >> return false;
> >> @@ -991,6 +1019,8 @@ int brcmf_p2p_remain_on_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
> >> if (err)
> >> goto exit;
> >>
> >> + p2p->remin_on_channel_wdev = wdev;
> >> +
> >> memcpy(&p2p->remain_on_channel, channel, sizeof(*channel));
> >> *cookie = p2p->remain_on_channel_cookie;
> >> cfg80211_ready_on_channel(wdev, *cookie, channel, duration, GFP_KERNEL);
> >> @@ -1014,6 +1044,7 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
> >> {
> >> struct brcmf_cfg80211_info *cfg = ifp->drvr->config;
> >> struct brcmf_p2p_info *p2p = &cfg->p2p;
> >> + struct wireless_dev *wdev = p2p->remin_on_channel_wdev;
> >>
> >> brcmf_dbg(TRACE, "Enter\n");
> >> if (test_and_clear_bit(BRCMF_P2P_STATUS_DISCOVER_LISTEN,
> >> @@ -1026,10 +1057,16 @@ int brcmf_p2p_notify_listen_complete(struct brcmf_if *ifp,
> >> complete(&p2p->wait_next_af);
> >> }
> >>
> >> - cfg80211_remain_on_channel_expired(&ifp->vif->wdev,
> >> + wdev = p2p->remin_on_channel_wdev ?
> >> + p2p->remin_on_channel_wdev :
> >> + &ifp->vif->wdev;
> >> +
> >> + cfg80211_remain_on_channel_expired(wdev,
> >> p2p->remain_on_channel_cookie,
> >> &p2p->remain_on_channel,
> >> GFP_KERNEL);
> >> + p2p->remin_on_channel_wdev = NULL;
> >> +
> >> }
> >> return 0;
> >> }
> >> @@ -1531,6 +1568,7 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
> >> *
> >> * @p2p: p2p info struct for vif.
> >> * @af_params: action frame data/info.
> >> + * @vif: vif to send
> >> *
> >> * Send an action frame immediately without doing channel synchronization.
> >> *
> >> @@ -1539,12 +1577,17 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
> >> * frame is transmitted.
> >> */
> >> static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
> >> - struct brcmf_fil_af_params_le *af_params)
> >> + struct brcmf_fil_af_params_le *af_params,
> >> + struct brcmf_cfg80211_vif *vif
> >> + )
> >> {
> >> struct brcmf_pub *drvr = p2p->cfg->pub;
> >> - struct brcmf_cfg80211_vif *vif;
> >> - struct brcmf_p2p_action_frame *p2p_af;
> >> s32 err = 0;
> >> + struct brcmf_fil_action_frame_le *action_frame;
> >> + u16 action_frame_len;
> >> +
> >> + action_frame = &af_params->action_frame;
> >> + action_frame_len = le16_to_cpu(action_frame->len);
> >>
> >> brcmf_dbg(TRACE, "Enter\n");
> >>
> >> @@ -1552,13 +1595,6 @@ static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
> >> clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
> >> clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);
> >>
> >> - /* check if it is a p2p_presence response */
> >> - p2p_af = (struct brcmf_p2p_action_frame *)af_params->action_frame.data;
> >> - if (p2p_af->subtype == P2P_AF_PRESENCE_RSP)
> >> - vif = p2p->bss_idx[P2PAPI_BSSCFG_CONNECTION].vif;
> >> - else
> >> - vif = p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif;
> >> -
> >> err = brcmf_fil_bsscfg_data_set(vif->ifp, "actframe", af_params,
> >> sizeof(*af_params));
> >> if (err) {
> >> @@ -1714,10 +1750,13 @@ static bool brcmf_p2p_check_dwell_overflow(u32 requested_dwell,
> >> * @cfg: driver private data for cfg80211 interface.
> >> * @ndev: net device to transmit on.
> >> * @af_params: configuration data for action frame.
> >> + * @vif: virtual interface to send
> >> */
> >> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> >> struct net_device *ndev,
> >> - struct brcmf_fil_af_params_le *af_params)
> >> + struct brcmf_fil_af_params_le *af_params,
> >> + struct brcmf_cfg80211_vif *vif
> >> + )
> >> {
> >> struct brcmf_p2p_info *p2p = &cfg->p2p;
> >> struct brcmf_if *ifp = netdev_priv(ndev);
> >> @@ -1789,7 +1828,9 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> >> goto exit;
> >> }
> >> } else if (brcmf_p2p_is_p2p_action(action_frame->data,
> >> - action_frame_len)) {
> >> + action_frame_len) ||
> >> + brcmf_p2p_is_dpp_pub_action(action_frame->data,
> >> + action_frame_len)) {
> >> /* do not configure anything. it will be */
> >> /* sent with a default configuration */
> >> } else {
> >> @@ -1857,7 +1898,7 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> >> if (af_params->channel)
> >> msleep(P2P_AF_RETRY_DELAY_TIME);
> >>
> >> - ack = !brcmf_p2p_tx_action_frame(p2p, af_params);
> >> + ack = !brcmf_p2p_tx_action_frame(p2p, af_params, vif);
> >> tx_retry++;
> >> dwell_overflow = brcmf_p2p_check_dwell_overflow(requested_dwell,
> >> dwell_jiffies);
> >> @@ -2508,6 +2549,7 @@ s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced)
> >>
> >> pri_ifp = brcmf_get_ifp(cfg->pub, 0);
> >> p2p->bss_idx[P2PAPI_BSSCFG_PRIMARY].vif = pri_ifp->vif;
> >> + init_completion(&p2p->send_af_done);
> >>
> >> if (p2pdev_forced) {
> >> err_ptr = brcmf_p2p_create_p2pdev(p2p, NULL, NULL);
> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
> >> index d2ecee565bf2..bbc455238707 100644
> >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
> >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.h
> >> @@ -138,6 +138,7 @@ struct brcmf_p2p_info {
> >> bool block_gon_req_tx;
> >> bool p2pdev_dynamically;
> >> bool wait_for_offchan_complete;
> >> + struct wireless_dev *remin_on_channel_wdev;
> > Docstring needs update. Also s/remin/remain
> Will fix in next version.
>
> >> };
> >>
> >> s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced);
> >> @@ -170,7 +171,8 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
> >> void *data);
> >> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
> >> struct net_device *ndev,
> >> - struct brcmf_fil_af_params_le *af_params);
> >> + struct brcmf_fil_af_params_le *af_params,
> >> + struct brcmf_cfg80211_vif *vif);
> >> bool brcmf_p2p_scan_finding_common_channel(struct brcmf_cfg80211_info *cfg,
> >> struct brcmf_bss_info_le *bi);
> >> s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
> >> index 7552bdb91991..3a9cad3730b8 100644
> >> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
> >> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
> >> @@ -233,6 +233,11 @@ static inline bool ac_bitmap_tst(u8 bitmap, int prec)
> >>
> >> #define WPA3_AUTH_SAE_PSK 0x40000 /* SAE with 4-way handshake */
> >>
> >> +#define WFA_AUTH_DPP 0x200000 /* WFA DPP AUTH */
> > This is incompatible with Broadcom's bit definitions. Please use a per
> > vendor approach.
> We had extended the bit definition.
> The authentication mode will be set to our FW so it's FW-dependent.
> Do you suggest I change the name? like CY_WFA_AUTH_DPP?

Being firmware dependent is exactly the problem here. The user
functions of this macro are in common code path so this bit could go
to firmware from any vendor. A mechanism should be in place to only
set this bit when the driver is working with a infineon/cypress
firmware.

Regards,
- Franky


>
> Thank you for the review.
>
> >> +
> >> +#define WFA_OUI "\x50\x6F\x9A" /* WFA OUI */
> >> +#define DPP_VER 0x1A /* WFA DPP v1.0 */
> >> +
> >> #define DOT11_DEFAULT_RTS_LEN 2347
> >> #define DOT11_DEFAULT_FRAG_LEN 2346
> >>
> >> --
> >> 2.25.0
> >>
>


Attachments:
smime.p7s (4.10 kB)
S/MIME Cryptographic Signature

2022-09-28 04:34:41

by Ian Lin

[permalink] [raw]
Subject: Re: [PATCH 1/3] brcmfmac: Support DPP feature



On 9/24/2022 12:36 AM, Franky Lin wrote:
>
>>>> };
>>>>
>>>> s32 brcmf_p2p_attach(struct brcmf_cfg80211_info *cfg, bool p2pdev_forced);
>>>> @@ -170,7 +171,8 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
>>>> void *data);
>>>> bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
>>>> struct net_device *ndev,
>>>> - struct brcmf_fil_af_params_le *af_params);
>>>> + struct brcmf_fil_af_params_le *af_params,
>>>> + struct brcmf_cfg80211_vif *vif);
>>>> bool brcmf_p2p_scan_finding_common_channel(struct brcmf_cfg80211_info *cfg,
>>>> struct brcmf_bss_info_le *bi);
>>>> s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
>>>> index 7552bdb91991..3a9cad3730b8 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
>>>> @@ -233,6 +233,11 @@ static inline bool ac_bitmap_tst(u8 bitmap, int prec)
>>>>
>>>> #define WPA3_AUTH_SAE_PSK 0x40000 /* SAE with 4-way handshake */
>>>>
>>>> +#define WFA_AUTH_DPP 0x200000 /* WFA DPP AUTH */
>>> This is incompatible with Broadcom's bit definitions. Please use a per
>>> vendor approach.
>> We had extended the bit definition.
>> The authentication mode will be set to our FW so it's FW-dependent.
>> Do you suggest I change the name? like CY_WFA_AUTH_DPP?
> Being firmware dependent is exactly the problem here. The user
> functions of this macro are in common code path so this bit could go
> to firmware from any vendor. A mechanism should be in place to only
> set this bit when the driver is working with a infineon/cypress
> firmware.
>
> Regards,
> - Franky
>

Currently we don't have such mechanism.
Please abandon this review series.
Thank you.