[PATCH v3 00/21] wifi: rtl8xxxu: Add concurrent mode for 8188f

[PATCH v3 00/21] wifi: rtl8xxxu: Add concurrent mode for 8188f

[Martin Kaistra]

This series adds the possibility to use two virtual interfaces on the
same channel. Supported combinations are STA+STA and STA+AP. The
conversion of the driver to support multiple interfaces is split into
individual patches to hopefully make it easier to understand what is
going on.

Thanks,
  Martin

changes from v2->v3:
- collect reviewed-by tags
- fix checkpatch warning for comment

changes from v1->v2:
- collect reviewed-by tags
- coding style changess
- ignore CFO for STA+STA
- extend watchdog_callback to call refresh_rate_mask for both interfaces
- remove port_num check for BEACON change notifications
- add macids for STA mode
- add number of sec cam entries to rtl8xxxu_fileops
- add comment to switch_ports about using the function in the future

v1: https://lore.kernel.org/linux-wireless/20231218143645.433356-1-martin.kaistra@linutronix.de/
v2: https://lore.kernel.org/linux-wireless/20231221164353.603258-1-martin.kaistra@linutronix.de/

Martin Kaistra (21):
  wifi: rtl8xxxu: remove assignment of priv->vif in
    rtl8xxxu_bss_info_changed()
  wifi: rtl8xxxu: prepare supporting two virtual interfaces
  wifi: rtl8xxxu: support setting linktype for both interfaces
  wifi: rtl8xxxu: 8188e: convert usage of priv->vif to priv->vifs[0]
  wifi: rtl8xxxu: support setting mac address register for both
    interfaces
  wifi: rtl8xxxu: extend wifi connected check to both interfaces
  wifi: rtl8xxxu: extend check for matching bssid to both interfaces
  wifi: rtl8xxxu: don't parse CFO, if both interfaces are connected in
    STA mode
  wifi: rtl8xxxu: support setting bssid register for multiple interfaces
  wifi: rtl8xxxu: support multiple interfaces in set_aifs()
  wifi: rtl8xxxu: support multiple interfaces in
    update_beacon_work_callback()
  wifi: rtl8xxxu: support multiple interfaces in configure_filter()
  wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()
  wifi: rtl8xxxu: support multiple interfaces in
    {add,remove}_interface()
  wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()
  wifi: rtl8xxxu: support multiple interface in start_ap()
  wifi: rtl8xxxu: add macids for STA mode
  wifi: rtl8xxxu: remove obsolete priv->vif
  wifi: rtl8xxxu: add hw crypto support for AP mode
  wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
  wifi: rtl8xxxu: declare concurrent mode support for 8188f

 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  18 +-
 .../realtek/rtl8xxxu/rtl8xxxu_8188e.c         |   2 +-
 .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |   2 +
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 380 ++++++++++++++----
 4 files changed, 311 insertions(+), 91 deletions(-)

-- 
2.39.2

[PATCH v3 01/21] wifi: rtl8xxxu: remove assignment of priv->vif in rtl8xxxu_bss_info_changed()

[Martin Kaistra]

priv->vif gets already set in rtl8xxxu_add_interface, there is no need
to set it also in rtl8xxxu_bss_info_changed().

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 43ee7592bc6e1..2dbed43ef22a5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5004,7 +5004,6 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 			rtl8xxxu_update_ra_report(rarpt, highest_rate, sgi, bw);
 
-			priv->vif = vif;
 			priv->rssi_level = RTL8XXXU_RATR_STA_INIT;
 
 			priv->fops->update_rate_mask(priv, ramask, 0, sgi,
-- 
2.39.2

Re: [PATCH v3 01/21] wifi: rtl8xxxu: remove assignment of priv->vif in rtl8xxxu_bss_info_changed()

[Kalle Valo]

Martin Kaistra <martin.kaistra@linutronix.de> wrote:

> priv->vif gets already set in rtl8xxxu_add_interface, there is no need
> to set it also in rtl8xxxu_bss_info_changed().
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

21 patches applied to wireless-next.git, thanks.

d55cb6d8a994 wifi: rtl8xxxu: remove assignment of priv->vif in rtl8xxxu_bss_info_changed()
2bbd7d584046 wifi: rtl8xxxu: prepare supporting two virtual interfaces
7f444692cde8 wifi: rtl8xxxu: support setting linktype for both interfaces
a047e46a7b98 wifi: rtl8xxxu: 8188e: convert usage of priv->vif to priv->vifs[0]
00add60cad3c wifi: rtl8xxxu: support setting mac address register for both interfaces
9aa776209ca3 wifi: rtl8xxxu: extend wifi connected check to both interfaces
80fd8687db41 wifi: rtl8xxxu: extend check for matching bssid to both interfaces
f86dd8eaf8da wifi: rtl8xxxu: don't parse CFO, if both interfaces are connected in STA mode
3ff7a05996f9 wifi: rtl8xxxu: support setting bssid register for multiple interfaces
43532c050f8e wifi: rtl8xxxu: support multiple interfaces in set_aifs()
05b22e9b7d84 wifi: rtl8xxxu: support multiple interfaces in update_beacon_work_callback()
6b7663828705 wifi: rtl8xxxu: support multiple interfaces in configure_filter()
3f9baa99f842 wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()
eef55f1545c9 wifi: rtl8xxxu: support multiple interfaces in {add,remove}_interface()
073401c3b6b9 wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()
61fdbd9e2a9d wifi: rtl8xxxu: support multiple interface in start_ap()
5ce0d7e8aee0 wifi: rtl8xxxu: add macids for STA mode
f232e9d91bb8 wifi: rtl8xxxu: remove obsolete priv->vif
b837f78fbffa wifi: rtl8xxxu: add hw crypto support for AP mode
69abad618efd wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent
1cd165adf314 wifi: rtl8xxxu: declare concurrent mode support for 8188f

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20231222101442.626837-2-martin.kaistra@linutronix.de/

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

[PATCH v3 02/21] wifi: rtl8xxxu: prepare supporting two virtual interfaces

[Martin Kaistra]

To prepare for concurrent mode, add an array ("vifs") to rtl8xxxu_priv
to keep track of both interfaces.

Keep the old priv->vif as long there are still users of it and let
priv->vifs[0] point to the same location.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h      |  2 ++
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 10 +++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 4695fb4e2d2db..b63fe084de92b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1897,6 +1897,8 @@ struct rtl8xxxu_priv {
 	 * is supported and no iface_combinations are provided.
 	 */
 	struct ieee80211_vif *vif;
+
+	struct ieee80211_vif *vifs[2];
 	struct delayed_work ra_watchdog;
 	struct work_struct c2hcmd_work;
 	struct sk_buff_head c2hcmd_queue;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 2dbed43ef22a5..5b7c20970a973 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6569,10 +6569,12 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 	int ret;
 	u8 val8;
 
-	if (!priv->vif)
+	if (!priv->vif) {
 		priv->vif = vif;
-	else
+		priv->vifs[0] = vif;
+	} else {
 		return -EOPNOTSUPP;
+	}
 
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
@@ -6622,8 +6624,10 @@ static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
 
 	dev_dbg(&priv->udev->dev, "%s\n", __func__);
 
-	if (priv->vif)
+	if (priv->vif) {
 		priv->vif = NULL;
+		priv->vifs[0] = NULL;
+	}
 }
 
 static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
-- 
2.39.2

[PATCH v3 03/21] wifi: rtl8xxxu: support setting linktype for both interfaces

[Martin Kaistra]

To prepare for concurrent mode, enhance the set_linktype function to be
able to set the linktype in the MSR register for both hardware ports.

Until the users of set_linktype can handle multiple interfaces, use
port_num = 0.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 37 +++++++++++--------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 5b7c20970a973..305d6dd585dfa 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1633,33 +1633,41 @@ rtl8xxxu_gen1_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
 }
 
 static void rtl8xxxu_set_linktype(struct rtl8xxxu_priv *priv,
-				  enum nl80211_iftype linktype)
+				  enum nl80211_iftype linktype, int port_num)
 {
-	u8 val8;
-
-	val8 = rtl8xxxu_read8(priv, REG_MSR);
-	val8 &= ~MSR_LINKTYPE_MASK;
+	u8 val8, type;
 
 	switch (linktype) {
 	case NL80211_IFTYPE_UNSPECIFIED:
-		val8 |= MSR_LINKTYPE_NONE;
+		type = MSR_LINKTYPE_NONE;
 		break;
 	case NL80211_IFTYPE_ADHOC:
-		val8 |= MSR_LINKTYPE_ADHOC;
+		type = MSR_LINKTYPE_ADHOC;
 		break;
 	case NL80211_IFTYPE_STATION:
-		val8 |= MSR_LINKTYPE_STATION;
+		type = MSR_LINKTYPE_STATION;
 		break;
 	case NL80211_IFTYPE_AP:
-		val8 |= MSR_LINKTYPE_AP;
+		type = MSR_LINKTYPE_AP;
 		break;
 	default:
-		goto out;
+		return;
+	}
+
+	switch (port_num) {
+	case 0:
+		val8 = rtl8xxxu_read8(priv, REG_MSR) & 0x0c;
+		val8 |= type;
+		break;
+	case 1:
+		val8 = rtl8xxxu_read8(priv, REG_MSR) & 0x03;
+		val8 |= type << 2;
+		break;
+	default:
+		return;
 	}
 
 	rtl8xxxu_write8(priv, REG_MSR, val8);
-out:
-	return;
 }
 
 static void
@@ -4236,7 +4244,6 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 	}
 
 	rtl8xxxu_set_mac(priv);
-	rtl8xxxu_set_linktype(priv, NL80211_IFTYPE_STATION);
 
 	/*
 	 * Configure initial WMAC settings
@@ -4964,7 +4971,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (changed & BSS_CHANGED_ASSOC) {
 		dev_dbg(dev, "Changed ASSOC: %i!\n", vif->cfg.assoc);
 
-		rtl8xxxu_set_linktype(priv, vif->type);
+		rtl8xxxu_set_linktype(priv, vif->type, 0);
 
 		if (vif->cfg.assoc) {
 			u32 ramask;
@@ -6610,7 +6617,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 		ret = -EOPNOTSUPP;
 	}
 
-	rtl8xxxu_set_linktype(priv, vif->type);
+	rtl8xxxu_set_linktype(priv, vif->type, 0);
 	ether_addr_copy(priv->mac_addr, vif->addr);
 	rtl8xxxu_set_mac(priv);
 
-- 
2.39.2

[PATCH v3 04/21] wifi: rtl8xxxu: 8188e: convert usage of priv->vif to priv->vifs[0]

[Martin Kaistra]

The driver currently does not support AP or concurrent mode for 8188e,
so just use priv->vifs[0] instead of priv->vif for now.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
index 6d0f975f891b7..cbeac9386ae51 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
@@ -1699,7 +1699,7 @@ void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *s
 	/* We only use macid 0, so only the first item is relevant.
 	 * AP mode will use more of them if it's ever implemented.
 	 */
-	if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION)
+	if (!priv->vifs[0] || priv->vifs[0]->type == NL80211_IFTYPE_STATION)
 		items = 1;
 
 	for (macid = 0; macid < items; macid++) {
-- 
2.39.2

[PATCH v3 05/21] wifi: rtl8xxxu: support setting mac address register for both interfaces

[Martin Kaistra]

To prepare for concurrent mode, enhance rtl8xxxu_set_mac() to write the
mac address of the respective interface to REG_MACID or REG_MACID1.

Remove the call to rtl8xxxu_set_mac() from the init function as we set
it in rtl8xxxu_add_interface() later anyway.

Until rtl8xxxu_add_interface() can handle both interfaces, call
rtl8xxxu_set_mac() with port_num = 0.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 305d6dd585dfa..c2ea8e92cd637 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3580,15 +3580,25 @@ void rtl8723a_phy_lc_calibrate(struct rtl8xxxu_priv *priv)
 		rtl8xxxu_write8(priv, REG_TXPAUSE, 0x00);
 }
 
-static int rtl8xxxu_set_mac(struct rtl8xxxu_priv *priv)
+static int rtl8xxxu_set_mac(struct rtl8xxxu_priv *priv, int port_num)
 {
 	int i;
 	u16 reg;
 
-	reg = REG_MACID;
+	switch (port_num) {
+	case 0:
+		reg = REG_MACID;
+		break;
+	case 1:
+		reg = REG_MACID1;
+		break;
+	default:
+		WARN_ONCE("%s: invalid port_num\n", __func__);
+		return -EINVAL;
+	}
 
 	for (i = 0; i < ETH_ALEN; i++)
-		rtl8xxxu_write8(priv, reg + i, priv->mac_addr[i]);
+		rtl8xxxu_write8(priv, reg + i, priv->vifs[port_num]->addr[i]);
 
 	return 0;
 }
@@ -4243,8 +4253,6 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 		rtl8xxxu_write32(priv, REG_HIMR, 0xffffffff);
 	}
 
-	rtl8xxxu_set_mac(priv);
-
 	/*
 	 * Configure initial WMAC settings
 	 */
@@ -6619,7 +6627,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 
 	rtl8xxxu_set_linktype(priv, vif->type, 0);
 	ether_addr_copy(priv->mac_addr, vif->addr);
-	rtl8xxxu_set_mac(priv);
+	rtl8xxxu_set_mac(priv, 0);
 
 	return ret;
 }
-- 
2.39.2

[PATCH v3 06/21] wifi: rtl8xxxu: extend wifi connected check to both interfaces

[Martin Kaistra]

There are multiple places in the code where the current connection
status of wifi is checked. The driver will support two interfaces soon
and either one of them (or both) could be connected.

Convert all uses of (vif && vif->cfg.assoc) to a new helper
function rtl8xxxu_is_assoc() which checks both interfaces.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 20 +++++++++----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c2ea8e92cd637..fd6b6e2eba038 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6043,18 +6043,20 @@ void rtl8723bu_update_bt_link_info(struct rtl8xxxu_priv *priv, u8 bt_info)
 		btcoex->bt_busy = false;
 }
 
+static inline bool rtl8xxxu_is_assoc(struct rtl8xxxu_priv *priv)
+{
+	return (priv->vifs[0] && priv->vifs[0]->cfg.assoc) ||
+	       (priv->vifs[1] && priv->vifs[1]->cfg.assoc);
+}
+
 static
 void rtl8723bu_handle_bt_inquiry(struct rtl8xxxu_priv *priv)
 {
-	struct ieee80211_vif *vif;
 	struct rtl8xxxu_btcoex *btcoex;
-	bool wifi_connected;
 
-	vif = priv->vif;
 	btcoex = &priv->bt_coex;
-	wifi_connected = (vif && vif->cfg.assoc);
 
-	if (!wifi_connected) {
+	if (!rtl8xxxu_is_assoc(priv)) {
 		rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
 		rtl8723bu_set_coex_with_type(priv, 0);
 	} else if (btcoex->has_sco || btcoex->has_hid || btcoex->has_a2dp) {
@@ -6072,15 +6074,11 @@ void rtl8723bu_handle_bt_inquiry(struct rtl8xxxu_priv *priv)
 static
 void rtl8723bu_handle_bt_info(struct rtl8xxxu_priv *priv)
 {
-	struct ieee80211_vif *vif;
 	struct rtl8xxxu_btcoex *btcoex;
-	bool wifi_connected;
 
-	vif = priv->vif;
 	btcoex = &priv->bt_coex;
-	wifi_connected = (vif && vif->cfg.assoc);
 
-	if (wifi_connected) {
+	if (rtl8xxxu_is_assoc(priv)) {
 		u32 val32 = 0;
 		u32 high_prio_tx = 0, high_prio_rx = 0;
 
@@ -7103,7 +7101,7 @@ static void rtl8xxxu_track_cfo(struct rtl8xxxu_priv *priv)
 	int cfo_khz_a, cfo_khz_b, cfo_average;
 	int crystal_cap;
 
-	if (!priv->vif || !priv->vif->cfg.assoc) {
+	if (!rtl8xxxu_is_assoc(priv)) {
 		/* Reset */
 		cfo->adjust = true;
 
-- 
2.39.2

[PATCH v3 07/21] wifi: rtl8xxxu: extend check for matching bssid to both interfaces

[Martin Kaistra]

The driver will support two interfaces soon, which both can be in
station mode, so extend the check, whether cfo information should be
parsed, to cover both interfaces.

For better code readability put the lines with priv->vifs[port_num] in a
separate function.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index fd6b6e2eba038..ca0d885eabb71 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
 	rtl8xxxu_send_beacon_frame(hw, vif);
 }
 
+static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv,
+						  struct ieee80211_hdr *hdr,
+						  int port_num)
+{
+	return priv->vifs[port_num] &&
+	       priv->vifs[port_num]->type == NL80211_IFTYPE_STATION &&
+	       priv->vifs[port_num]->cfg.assoc &&
+	       ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2);
+}
+
 void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
 				 struct ieee80211_rx_status *rx_status,
 				 struct rtl8723au_phy_stats *phy_stats,
@@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
 		rx_status->signal = priv->fops->cck_rssi(priv, phy_stats);
 	} else {
 		bool parse_cfo = priv->fops->set_crystal_cap &&
-				 priv->vif &&
-				 priv->vif->type == NL80211_IFTYPE_STATION &&
-				 priv->vif->cfg.assoc &&
 				 !crc_icv_err &&
 				 !ieee80211_is_ctl(hdr->frame_control) &&
-				 ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
+				 (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
+				  rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
 
 		if (parse_cfo) {
 			priv->cfo_tracking.cfo_tail[0] = phy_stats->path_cfotail[0];
@@ -5762,12 +5770,10 @@ static void jaguar2_rx_parse_phystats_type1(struct rtl8xxxu_priv *priv,
 					    bool crc_icv_err)
 {
 	bool parse_cfo = priv->fops->set_crystal_cap &&
-			 priv->vif &&
-			 priv->vif->type == NL80211_IFTYPE_STATION &&
-			 priv->vif->cfg.assoc &&
 			 !crc_icv_err &&
 			 !ieee80211_is_ctl(hdr->frame_control) &&
-			 ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
+			 (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
+			  rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
 	u8 pwdb_max = 0;
 	int rx_path;
 
-- 
2.39.2

[PATCH v3 08/21] wifi: rtl8xxxu: don't parse CFO, if both interfaces are connected in STA mode

[Martin Kaistra]

If both interfaces are in STATION mode and both are connected to an AP,
there might be conflicting CFO values for the two connections. Ignore
the CFO information in this case.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index ca0d885eabb71..6fff982f65253 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5716,6 +5716,14 @@ static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv,
 	       ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2);
 }
 
+static inline bool rtl8xxxu_is_sta_sta(struct rtl8xxxu_priv *priv)
+{
+	return (priv->vifs[0] && priv->vifs[0]->cfg.assoc &&
+		priv->vifs[0]->type == NL80211_IFTYPE_STATION) &&
+	       (priv->vifs[1] && priv->vifs[1]->cfg.assoc &&
+		priv->vifs[1]->type == NL80211_IFTYPE_STATION);
+}
+
 void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
 				 struct ieee80211_rx_status *rx_status,
 				 struct rtl8723au_phy_stats *phy_stats,
@@ -5734,6 +5742,7 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv,
 		bool parse_cfo = priv->fops->set_crystal_cap &&
 				 !crc_icv_err &&
 				 !ieee80211_is_ctl(hdr->frame_control) &&
+				 !rtl8xxxu_is_sta_sta(priv) &&
 				 (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
 				  rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
 
@@ -5772,6 +5781,7 @@ static void jaguar2_rx_parse_phystats_type1(struct rtl8xxxu_priv *priv,
 	bool parse_cfo = priv->fops->set_crystal_cap &&
 			 !crc_icv_err &&
 			 !ieee80211_is_ctl(hdr->frame_control) &&
+			 !rtl8xxxu_is_sta_sta(priv) &&
 			 (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) ||
 			  rtl8xxxu_is_packet_match_bssid(priv, hdr, 1));
 	u8 pwdb_max = 0;
-- 
2.39.2

[PATCH v3 09/21] wifi: rtl8xxxu: support setting bssid register for multiple interfaces

[Martin Kaistra]

To prepare for concurrent mode, enhance rtl8xxxu_set_bssid() to write the
BSSID of the respective interface to REG_BSSID or REG_BSSID1.

Like done with rtl8xxxu_set_mac(), call rtl8xxxu_set_bssid() with
port_num = 0, until the callers also support multiple interfaces.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 6fff982f65253..490f35d0d6199 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3603,14 +3603,24 @@ static int rtl8xxxu_set_mac(struct rtl8xxxu_priv *priv, int port_num)
 	return 0;
 }
 
-static int rtl8xxxu_set_bssid(struct rtl8xxxu_priv *priv, const u8 *bssid)
+static int rtl8xxxu_set_bssid(struct rtl8xxxu_priv *priv, const u8 *bssid, int port_num)
 {
 	int i;
 	u16 reg;
 
 	dev_dbg(&priv->udev->dev, "%s: (%pM)\n", __func__, bssid);
 
-	reg = REG_BSSID;
+	switch (port_num) {
+	case 0:
+		reg = REG_BSSID;
+		break;
+	case 1:
+		reg = REG_BSSID1;
+		break;
+	default:
+		WARN_ONCE("%s: invalid port_num\n", __func__);
+		return -EINVAL;
+	}
 
 	for (i = 0; i < ETH_ALEN; i++)
 		rtl8xxxu_write8(priv, reg + i, bssid[i]);
@@ -5068,7 +5078,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	if (changed & BSS_CHANGED_BSSID) {
 		dev_dbg(dev, "Changed BSSID!\n");
-		rtl8xxxu_set_bssid(priv, bss_conf->bssid);
+		rtl8xxxu_set_bssid(priv, bss_conf->bssid, 0);
 	}
 
 	if (changed & BSS_CHANGED_BASIC_RATES) {
@@ -5097,7 +5107,7 @@ static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	struct device *dev = &priv->udev->dev;
 
 	dev_dbg(dev, "Start AP mode\n");
-	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid);
+	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid, 0);
 	rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
 	priv->fops->report_connect(priv, RTL8XXXU_BC_MC_MACID, 0, true);
 
-- 
2.39.2

[PATCH v3 10/21] wifi: rtl8xxxu: support multiple interfaces in set_aifs()

[Martin Kaistra]

In concurrent mode supported by this driver, both interfaces will use
the same channel and same wireless mode.
It is therefore possible to get the wireless mode by checking the first
connected interface.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 490f35d0d6199..99fe567e0f75b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4913,14 +4913,20 @@ static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
 	u8 aifs, aifsn, sifs;
 	int i;
 
-	if (priv->vif) {
+	for (i = 0; i < ARRAY_SIZE(priv->vifs); i++) {
+		if (!priv->vifs[i])
+			continue;
+
 		struct ieee80211_sta *sta;
 
 		rcu_read_lock();
-		sta = ieee80211_find_sta(priv->vif, priv->vif->bss_conf.bssid);
+		sta = ieee80211_find_sta(priv->vifs[i], priv->vifs[i]->bss_conf.bssid);
 		if (sta)
 			wireless_mode = rtl8xxxu_wireless_mode(priv->hw, sta);
 		rcu_read_unlock();
+
+		if (wireless_mode)
+			break;
 	}
 
 	if (priv->hw->conf.chandef.chan->band == NL80211_BAND_5GHZ ||
-- 
2.39.2

[PATCH v3 11/21] wifi: rtl8xxxu: support multiple interfaces in update_beacon_work_callback()

[Martin Kaistra]

As we only want to support AP mode/sending beacons on port 0, it is
enough to replace priv->vif with priv->vifs[0].

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 99fe567e0f75b..680dffb9657e1 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5712,7 +5712,7 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
 	struct rtl8xxxu_priv *priv =
 		container_of(work, struct rtl8xxxu_priv, update_beacon_work);
 	struct ieee80211_hw *hw = priv->hw;
-	struct ieee80211_vif *vif = priv->vif;
+	struct ieee80211_vif *vif = priv->vifs[0];
 
 	if (!vif) {
 		WARN_ONCE(true, "no vif to update beacon\n");
-- 
2.39.2

[PATCH v3 12/21] wifi: rtl8xxxu: support multiple interfaces in configure_filter()

[Martin Kaistra]

As we only want to support AP mode/sending beacons on port 0, change
from priv->vif to priv->vifs[0] in the check for AP mode.
Additionally, if we are in AP mode, don't filter RX beacon and probe
response frames to still allow working STATION mode on the other
interface.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 680dffb9657e1..c5b71892369c9 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6794,8 +6794,8 @@ static void rtl8xxxu_configure_filter(struct ieee80211_hw *hw,
 	else
 		rcr |= RCR_CHECK_BSSID_BEACON | RCR_CHECK_BSSID_MATCH;
 
-	if (priv->vif && priv->vif->type == NL80211_IFTYPE_AP)
-		rcr &= ~RCR_CHECK_BSSID_MATCH;
+	if (priv->vifs[0] && priv->vifs[0]->type == NL80211_IFTYPE_AP)
+		rcr &= ~(RCR_CHECK_BSSID_MATCH | RCR_CHECK_BSSID_BEACON);
 
 	if (*total_flags & FIF_CONTROL)
 		rcr |= RCR_ACCEPT_CTRL_FRAME;
-- 
2.39.2

[PATCH v3 13/21] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()

[Martin Kaistra]

Check first whether priv->vifs[0] exists and is of type STATION, then go
to priv->vifs[1]. Make sure to call refresh_rate_mask for both
interfaces.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c5b71892369c9..fd0108668bcda 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -7200,11 +7200,15 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)
 {
 	struct ieee80211_vif *vif;
 	struct rtl8xxxu_priv *priv;
+	int i;
 
 	priv = container_of(work, struct rtl8xxxu_priv, ra_watchdog.work);
-	vif = priv->vif;
+	for (i = 0; i < ARRAY_SIZE(priv->vifs); i++) {
+		vif = priv->vifs[i];
+
+		if (!vif || vif->type != NL80211_IFTYPE_STATION)
+			continue;
 
-	if (vif && vif->type == NL80211_IFTYPE_STATION) {
 		int signal;
 		struct ieee80211_sta *sta;
 
@@ -7215,22 +7219,21 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)
 
 			dev_dbg(dev, "%s: no sta found\n", __func__);
 			rcu_read_unlock();
-			goto out;
+			continue;
 		}
 		rcu_read_unlock();
 
 		signal = ieee80211_ave_rssi(vif);
 
-		priv->fops->report_rssi(priv, 0,
+		priv->fops->report_rssi(priv, rtl8xxxu_get_macid(priv, sta),
 					rtl8xxxu_signal_to_snr(signal));
 
-		if (priv->fops->set_crystal_cap)
-			rtl8xxxu_track_cfo(priv);
-
 		rtl8xxxu_refresh_rate_mask(priv, signal, sta, false);
 	}
 
-out:
+	if (priv->fops->set_crystal_cap)
+		rtl8xxxu_track_cfo(priv);
+
 	schedule_delayed_work(&priv->ra_watchdog, 2 * HZ);
 }
 
-- 
2.39.2

RE: [PATCH v3 13/21] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()

[Ping-Ke Shih]

On Fri, Dec 22, 2023 at 6:16 PM Martin Kaistra
<martin.kaistra@linutronix.de> wrote:
>
> Check first whether priv->vifs[0] exists and is of type STATION, then go
> to priv->vifs[1]. Make sure to call refresh_rate_mask for both
> interfaces.
>
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

Logically, this patch is fine to me, but I would like to know if you have
tested two STATION interfaces connect two different AP? Especially, one AP
is 802.11g mode and the other is 802.11n mode, so we can clearly know this
patch is expected. 

Ping-Ke

Re: [PATCH v3 13/21] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()

[Martin Kaistra]

Am 25.12.23 um 06:22 schrieb Ping-Ke Shih:
> 
> On Fri, Dec 22, 2023 at 6:16 PM Martin Kaistra
> <martin.kaistra@linutronix.de> wrote:
>>
>> Check first whether priv->vifs[0] exists and is of type STATION, then go
>> to priv->vifs[1]. Make sure to call refresh_rate_mask for both
>> interfaces.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> 
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> Logically, this patch is fine to me, but I would like to know if you have
> tested two STATION interfaces connect two different AP? Especially, one AP
> is 802.11g mode and the other is 802.11n mode, so we can clearly know this
> patch is expected.

Yes, I did try to connect the two interfaces to two different AP (on the same 
channel) at the same time, and it seems to behave fine.

RE: [PATCH v3 13/21] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Monday, January 8, 2024 5:43 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Bitterblue Smith
> <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Subject: Re: [PATCH v3 13/21] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback()
> 
> Yes, I did try to connect the two interfaces to two different AP (on the same
> channel) at the same time, and it seems to behave fine.

Nice! Thanks for your work. :-)

Ping-Ke 

[PATCH v3 14/21] wifi: rtl8xxxu: support multiple interfaces in {add,remove}_interface()

[Martin Kaistra]

Add a custom struct to store in vif->drv_priv with a reference to
port_num and fill it when a new interface is added. Choose a free
port_num for the newly added interface.

As we only want to support AP mode/sending beacons on port 0, only change
the beacon settings if a new interface is actually assigned to port 0.

Call set_linktype() and set_mac() with the appropriate port_num.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  4 ++
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 52 +++++++++++--------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index b63fe084de92b..9d272da373a3c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1921,6 +1921,10 @@ struct rtl8xxxu_sta_info {
 	u8 macid;
 };
 
+struct rtl8xxxu_vif {
+	int port_num;
+};
+
 struct rtl8xxxu_rx_urb {
 	struct urb urb;
 	struct ieee80211_hw *hw;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index fd0108668bcda..4090db8abba7b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6610,28 +6610,33 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
 static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 				  struct ieee80211_vif *vif)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
-	int ret;
+	int port_num;
 	u8 val8;
 
-	if (!priv->vif) {
-		priv->vif = vif;
-		priv->vifs[0] = vif;
-	} else {
+	if (!priv->vifs[0])
+		port_num = 0;
+	else if (!priv->vifs[1])
+		port_num = 1;
+	else
 		return -EOPNOTSUPP;
-	}
 
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
-		rtl8xxxu_stop_tx_beacon(priv);
+		if (port_num == 0) {
+			rtl8xxxu_stop_tx_beacon(priv);
 
-		val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
-		val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
-			BEACON_DISABLE_TSF_UPDATE;
-		rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
-		ret = 0;
+			val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
+			val8 |= BEACON_ATIM | BEACON_FUNCTION_ENABLE |
+				BEACON_DISABLE_TSF_UPDATE;
+			rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
+		}
 		break;
 	case NL80211_IFTYPE_AP:
+		if (port_num == 1)
+			return -EOPNOTSUPP;
+
 		rtl8xxxu_write8(priv, REG_BEACON_CTRL,
 				BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
 		rtl8xxxu_write8(priv, REG_ATIMWND, 0x0c); /* 12ms */
@@ -6648,31 +6653,32 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 		val8 = rtl8xxxu_read8(priv, REG_CCK_CHECK);
 		val8 &= ~BIT_BCN_PORT_SEL;
 		rtl8xxxu_write8(priv, REG_CCK_CHECK, val8);
-
-		ret = 0;
 		break;
 	default:
-		ret = -EOPNOTSUPP;
+		return -EOPNOTSUPP;
 	}
 
-	rtl8xxxu_set_linktype(priv, vif->type, 0);
+	priv->vifs[port_num] = vif;
+	priv->vif = vif;
+	rtlvif->port_num = port_num;
+
+	rtl8xxxu_set_linktype(priv, vif->type, port_num);
 	ether_addr_copy(priv->mac_addr, vif->addr);
-	rtl8xxxu_set_mac(priv, 0);
+	rtl8xxxu_set_mac(priv, port_num);
 
-	return ret;
+	return 0;
 }
 
 static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
 				      struct ieee80211_vif *vif)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 
 	dev_dbg(&priv->udev->dev, "%s\n", __func__);
 
-	if (priv->vif) {
-		priv->vif = NULL;
-		priv->vifs[0] = NULL;
-	}
+	priv->vif = NULL;
+	priv->vifs[rtlvif->port_num] = NULL;
 }
 
 static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
@@ -7662,6 +7668,8 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 	if (ret)
 		goto err_set_intfdata;
 
+	hw->vif_data_size = sizeof(struct rtl8xxxu_vif);
+
 	hw->wiphy->max_scan_ssids = 1;
 	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
 	if (priv->fops->max_macid_num)
-- 
2.39.2

[PATCH v3 15/21] wifi: rtl8xxxu: support multiple interfaces in bss_info_changed()

[Martin Kaistra]

Call set_linktype and set_bssid now with correct port_num. Call
stop_tx_beacon only for port 0, as we don't support beacons on port 1.
Explicit changes to BEACON will only happen for AP type interfaces, so
we don't need an additional check there.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 4090db8abba7b..aa88e66c64d67 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4983,6 +4983,7 @@ static void
 rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			  struct ieee80211_bss_conf *bss_conf, u64 changed)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct device *dev = &priv->udev->dev;
 	struct ieee80211_sta *sta;
@@ -4995,7 +4996,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (changed & BSS_CHANGED_ASSOC) {
 		dev_dbg(dev, "Changed ASSOC: %i!\n", vif->cfg.assoc);
 
-		rtl8xxxu_set_linktype(priv, vif->type, 0);
+		rtl8xxxu_set_linktype(priv, vif->type, rtlvif->port_num);
 
 		if (vif->cfg.assoc) {
 			u32 ramask;
@@ -5042,7 +5043,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 			rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
 
-			rtl8xxxu_stop_tx_beacon(priv);
+			if (rtlvif->port_num == 0)
+				rtl8xxxu_stop_tx_beacon(priv);
 
 			/* joinbss sequence */
 			rtl8xxxu_write16(priv, REG_BCN_PSR_RPT,
@@ -5084,7 +5086,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	if (changed & BSS_CHANGED_BSSID) {
 		dev_dbg(dev, "Changed BSSID!\n");
-		rtl8xxxu_set_bssid(priv, bss_conf->bssid, 0);
+		rtl8xxxu_set_bssid(priv, bss_conf->bssid, rtlvif->port_num);
 	}
 
 	if (changed & BSS_CHANGED_BASIC_RATES) {
-- 
2.39.2

[PATCH v3 16/21] wifi: rtl8xxxu: support multiple interface in start_ap()

[Martin Kaistra]

Call set_bssid() with the correct port_num now.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index aa88e66c64d67..5e4e6c006cc1f 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5111,11 +5111,12 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 static int rtl8xxxu_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			     struct ieee80211_bss_conf *link_conf)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct device *dev = &priv->udev->dev;
 
 	dev_dbg(dev, "Start AP mode\n");
-	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid, 0);
+	rtl8xxxu_set_bssid(priv, vif->bss_conf.bssid, rtlvif->port_num);
 	rtl8xxxu_write16(priv, REG_BCN_INTERVAL, vif->bss_conf.beacon_int);
 	priv->fops->report_connect(priv, RTL8XXXU_BC_MC_MACID, 0, true);
 
-- 
2.39.2

[PATCH v3 17/21] wifi: rtl8xxxu: add macids for STA mode

[Martin Kaistra]

Until now, the driver only assigned a dedicated macid for connections
made in AP mode, in STA mode the return value of rtl8xxxu_get_macid()
was simply 0.
To differentiate between port 0 and 1, when both are in STA mode,
allocate a second macid (with value 1) and set sta_info->macid according
to the used port_num in rtl8xxxu_sta_add().

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  1 +
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 9d272da373a3c..6a58897446f4c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1774,6 +1774,7 @@ struct rtl8xxxu_cfo_tracking {
 #define RTL8XXXU_HW_LED_CONTROL	2
 #define RTL8XXXU_MAX_MAC_ID_NUM	128
 #define RTL8XXXU_BC_MC_MACID	0
+#define RTL8XXXU_BC_MC_MACID1	1
 
 struct rtl8xxxu_priv {
 	struct ieee80211_hw *hw;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 5e4e6c006cc1f..0b6eac14f60e5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4053,10 +4053,13 @@ static inline u8 rtl8xxxu_get_macid(struct rtl8xxxu_priv *priv,
 {
 	struct rtl8xxxu_sta_info *sta_info;
 
-	if (!priv->vif || priv->vif->type == NL80211_IFTYPE_STATION || !sta)
+	if (!sta)
 		return 0;
 
 	sta_info = (struct rtl8xxxu_sta_info *)sta->drv_priv;
+	if (!sta_info)
+		return 0;
+
 	return sta_info->macid;
 }
 
@@ -4536,6 +4539,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 		rtl8188e_ra_info_init_all(&priv->ra_info);
 
 	set_bit(RTL8XXXU_BC_MC_MACID, priv->mac_id_map);
+	set_bit(RTL8XXXU_BC_MC_MACID1, priv->mac_id_map);
 
 exit:
 	return ret;
@@ -7375,6 +7379,7 @@ static int rtl8xxxu_sta_add(struct ieee80211_hw *hw,
 			    struct ieee80211_sta *sta)
 {
 	struct rtl8xxxu_sta_info *sta_info = (struct rtl8xxxu_sta_info *)sta->drv_priv;
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 
 	if (vif->type == NL80211_IFTYPE_AP) {
@@ -7384,6 +7389,17 @@ static int rtl8xxxu_sta_add(struct ieee80211_hw *hw,
 
 		rtl8xxxu_refresh_rate_mask(priv, 0, sta, true);
 		priv->fops->report_connect(priv, sta_info->macid, H2C_MACID_ROLE_STA, true);
+	} else {
+		switch (rtlvif->port_num) {
+		case 0:
+			sta_info->macid = RTL8XXXU_BC_MC_MACID;
+			break;
+		case 1:
+			sta_info->macid = RTL8XXXU_BC_MC_MACID1;
+			break;
+		default:
+			break;
+		}
 	}
 
 	return 0;
-- 
2.39.2

[PATCH v3 18/21] wifi: rtl8xxxu: remove obsolete priv->vif

[Martin Kaistra]

Now that all uses of priv->vif have been converted to priv->vifs[]
remove the old attribute.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h      | 5 -----
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 --
 2 files changed, 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 6a58897446f4c..c5e6d8f7d26bd 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1893,11 +1893,6 @@ struct rtl8xxxu_priv {
 	u8 rssi_level;
 	DECLARE_BITMAP(tx_aggr_started, IEEE80211_NUM_TIDS);
 	DECLARE_BITMAP(tid_tx_operational, IEEE80211_NUM_TIDS);
-	/*
-	 * Only one virtual interface permitted because only STA mode
-	 * is supported and no iface_combinations are provided.
-	 */
-	struct ieee80211_vif *vif;
 
 	struct ieee80211_vif *vifs[2];
 	struct delayed_work ra_watchdog;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 0b6eac14f60e5..ecf54eb8dba61 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6666,7 +6666,6 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 	}
 
 	priv->vifs[port_num] = vif;
-	priv->vif = vif;
 	rtlvif->port_num = port_num;
 
 	rtl8xxxu_set_linktype(priv, vif->type, port_num);
@@ -6684,7 +6683,6 @@ static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
 
 	dev_dbg(&priv->udev->dev, "%s\n", __func__);
 
-	priv->vif = NULL;
 	priv->vifs[rtlvif->port_num] = NULL;
 }
 
-- 
2.39.2

[PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Martin Kaistra]

Add a custom function for allocating entries in the sec cam. This allows
us to store multiple keys with the same keyidx.

The maximum number of sec cam entries for 8188f is 16 according to the
vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
which might support more entries, can set a different number there.

Set the bssid as mac address for group keys instead of just using the
ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
for differentiating them from pairwise keys like in the vendor driver.

Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
broadcast/multicast packets in AP mode.

Finally, allow the usage of rtl8xxxu_set_key() for AP mode.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
 .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
 3 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index c5e6d8f7d26bd..62e6318bc0924 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
 #define DESC_RATE_ID_SHIFT		16
 #define DESC_RATE_ID_MASK		0xf
 #define TXDESC_NAVUSEHDR		BIT(20)
+#define TXDESC_EN_DESC_ID		BIT(21)
 #define TXDESC_SEC_RC4			0x00400000
 #define TXDESC_SEC_AES			0x00c00000
 #define TXDESC_PKT_OFFSET_SHIFT		26
@@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
 #define RTL8XXXU_MAX_MAC_ID_NUM	128
 #define RTL8XXXU_BC_MC_MACID	0
 #define RTL8XXXU_BC_MC_MACID1	1
+#define RTL8XXXU_MAX_SEC_CAM_NUM	64
 
 struct rtl8xxxu_priv {
 	struct ieee80211_hw *hw;
@@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
 	char led_name[32];
 	struct led_classdev led_cdev;
 	DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
+	DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
 };
 
 struct rtl8xxxu_sta_info {
@@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
 
 struct rtl8xxxu_vif {
 	int port_num;
+	u8 hw_key_idx;
 };
 
 struct rtl8xxxu_rx_urb {
@@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
 	u16 max_aggr_num;
 	u8 supports_ap:1;
 	u16 max_macid_num;
+	u16 max_sec_cam_num;
 	u32 adda_1t_init;
 	u32 adda_1t_path_on;
 	u32 adda_2t_path_on_a;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
index 1e1c8fa194cb8..574a5fe951543 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
@@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
 	.max_aggr_num = 0x0c14,
 	.supports_ap = 1,
 	.max_macid_num = 16,
+	.max_sec_cam_num = 16,
 	.adda_1t_init = 0x03c00014,
 	.adda_1t_path_on = 0x03c00014,
 	.trxff_boundary = 0x3f7f,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index ecf54eb8dba61..7aafae9fe76b8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
 	 * This is a bit of a hack - the lower bits of the cipher
 	 * suite selector happens to match the cipher index in the CAM
 	 */
-	addr = key->keyidx << CAM_CMD_KEY_SHIFT;
+	addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
 	ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
+	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
+		ctrl |= BIT(6);
 
 	for (j = 5; j >= 0; j--) {
 		switch (j) {
@@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	struct rtl8xxxu_tx_urb *tx_urb;
 	struct ieee80211_sta *sta = NULL;
 	struct ieee80211_vif *vif = tx_info->control.vif;
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct device *dev = &priv->udev->dev;
 	u32 queue, rts_rate;
 	u16 pktlen = skb->len;
 	int tx_desc_size = priv->fops->tx_desc_size;
 	u8 macid;
 	int ret;
-	bool ampdu_enable, sgi = false, short_preamble = false;
+	bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
 
 	if (skb_headroom(skb) < tx_desc_size) {
 		dev_warn(dev,
@@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 		tx_desc->txdw0 =
 			TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
 	if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
-	    is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
+	    is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
 		tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
+		bmc = true;
+	}
+
 
 	tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
+	macid = rtl8xxxu_get_macid(priv, sta);
 
 	if (tx_info->control.hw_key) {
 		switch (tx_info->control.hw_key->cipher) {
@@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 		default:
 			break;
 		}
+		if (bmc && rtlvif->hw_key_idx != 0xff) {
+			tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
+			macid = rtlvif->hw_key_idx;
+		}
 	}
 
 	/* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
@@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	else
 		rts_rate = 0;
 
-	macid = rtl8xxxu_get_macid(priv, sta);
 	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
 				ampdu_enable, rts_rate, macid);
 
@@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 
 	priv->vifs[port_num] = vif;
 	rtlvif->port_num = port_num;
+	rtlvif->hw_key_idx = 0xff;
 
 	rtl8xxxu_set_linktype(priv, vif->type, port_num);
 	ether_addr_copy(priv->mac_addr, vif->addr);
@@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
 	return 0;
 }
 
+static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
+{
+	struct rtl8xxxu_priv *priv = hw->priv;
+
+	return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
+}
+
 static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 			    struct ieee80211_vif *vif,
 			    struct ieee80211_sta *sta,
 			    struct ieee80211_key_conf *key)
 {
+	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
 	struct rtl8xxxu_priv *priv = hw->priv;
 	struct device *dev = &priv->udev->dev;
 	u8 mac_addr[ETH_ALEN];
@@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
 		__func__, cmd, key->cipher, key->keyidx);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return -EOPNOTSUPP;
-
 	if (key->keyidx > 3)
 		return -EOPNOTSUPP;
 
@@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		ether_addr_copy(mac_addr, sta->addr);
 	} else {
 		dev_dbg(dev, "%s: group key\n", __func__);
-		eth_broadcast_addr(mac_addr);
+		ether_addr_copy(mac_addr, vif->bss_conf.bssid);
 	}
 
 	val16 = rtl8xxxu_read16(priv, REG_CR);
@@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 	switch (cmd) {
 	case SET_KEY:
-		key->hw_key_idx = key->keyidx;
+
+		retval = rtl8xxxu_get_free_sec_cam(hw);
+		if (retval < 0)
+			return -EOPNOTSUPP;
+
+		key->hw_key_idx = retval;
+
+		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
+			rtlvif->hw_key_idx = key->hw_key_idx;
+
 		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
 		rtl8xxxu_cam_write(priv, key, mac_addr);
+		set_bit(key->hw_key_idx, priv->cam_map);
 		retval = 0;
 		break;
 	case DISABLE_KEY:
 		rtl8xxxu_write32(priv, REG_CAM_WRITE, 0x00000000);
 		val32 = CAM_CMD_POLLING | CAM_CMD_WRITE |
-			key->keyidx << CAM_CMD_KEY_SHIFT;
+			key->hw_key_idx << CAM_CMD_KEY_SHIFT;
 		rtl8xxxu_write32(priv, REG_CAM_CMD, val32);
+		rtlvif->hw_key_idx = 0xff;
+		clear_bit(key->hw_key_idx, priv->cam_map);
 		retval = 0;
 		break;
 	default:
-- 
2.39.2

Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Bitterblue Smith]

On 22/12/2023 12:14, Martin Kaistra wrote:
> Add a custom function for allocating entries in the sec cam. This allows
> us to store multiple keys with the same keyidx.
> 
> The maximum number of sec cam entries for 8188f is 16 according to the
> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
> which might support more entries, can set a different number there.
> 
> Set the bssid as mac address for group keys instead of just using the
> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
> for differentiating them from pairwise keys like in the vendor driver.
> 
> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
> broadcast/multicast packets in AP mode.
> 
> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
> 
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
>  .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>  3 files changed, 44 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index c5e6d8f7d26bd..62e6318bc0924 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>  #define DESC_RATE_ID_SHIFT		16
>  #define DESC_RATE_ID_MASK		0xf
>  #define TXDESC_NAVUSEHDR		BIT(20)
> +#define TXDESC_EN_DESC_ID		BIT(21)
>  #define TXDESC_SEC_RC4			0x00400000
>  #define TXDESC_SEC_AES			0x00c00000
>  #define TXDESC_PKT_OFFSET_SHIFT		26
> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>  #define RTL8XXXU_MAX_MAC_ID_NUM	128
>  #define RTL8XXXU_BC_MC_MACID	0
>  #define RTL8XXXU_BC_MC_MACID1	1
> +#define RTL8XXXU_MAX_SEC_CAM_NUM	64
>  
>  struct rtl8xxxu_priv {
>  	struct ieee80211_hw *hw;
> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>  	char led_name[32];
>  	struct led_classdev led_cdev;
>  	DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
> +	DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>  };
>  
>  struct rtl8xxxu_sta_info {
> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>  
>  struct rtl8xxxu_vif {
>  	int port_num;
> +	u8 hw_key_idx;
>  };
>  
>  struct rtl8xxxu_rx_urb {
> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>  	u16 max_aggr_num;
>  	u8 supports_ap:1;
>  	u16 max_macid_num;
> +	u16 max_sec_cam_num;
>  	u32 adda_1t_init;
>  	u32 adda_1t_path_on;
>  	u32 adda_2t_path_on_a;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> index 1e1c8fa194cb8..574a5fe951543 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>  	.max_aggr_num = 0x0c14,
>  	.supports_ap = 1,
>  	.max_macid_num = 16,
> +	.max_sec_cam_num = 16,
>  	.adda_1t_init = 0x03c00014,
>  	.adda_1t_path_on = 0x03c00014,
>  	.trxff_boundary = 0x3f7f,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index ecf54eb8dba61..7aafae9fe76b8 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>  	 * This is a bit of a hack - the lower bits of the cipher
>  	 * suite selector happens to match the cipher index in the CAM
>  	 */
> -	addr = key->keyidx << CAM_CMD_KEY_SHIFT;
> +	addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>  	ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
> +	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> +		ctrl |= BIT(6);
>  
>  	for (j = 5; j >= 0; j--) {
>  		switch (j) {
> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  	struct rtl8xxxu_tx_urb *tx_urb;
>  	struct ieee80211_sta *sta = NULL;
>  	struct ieee80211_vif *vif = tx_info->control.vif;
> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>  	struct device *dev = &priv->udev->dev;
>  	u32 queue, rts_rate;
>  	u16 pktlen = skb->len;
>  	int tx_desc_size = priv->fops->tx_desc_size;
>  	u8 macid;
>  	int ret;
> -	bool ampdu_enable, sgi = false, short_preamble = false;
> +	bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>  
>  	if (skb_headroom(skb) < tx_desc_size) {
>  		dev_warn(dev,
> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  		tx_desc->txdw0 =
>  			TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>  	if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
> -	    is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
> +	    is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>  		tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
> +		bmc = true;
> +	}
> +
>  
>  	tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
> +	macid = rtl8xxxu_get_macid(priv, sta);
>  
>  	if (tx_info->control.hw_key) {
>  		switch (tx_info->control.hw_key->cipher) {
> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  		default:
>  			break;
>  		}
> +		if (bmc && rtlvif->hw_key_idx != 0xff) {
> +			tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
> +			macid = rtlvif->hw_key_idx;
> +		}
>  	}
>  
>  	/* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  	else
>  		rts_rate = 0;
>  
> -	macid = rtl8xxxu_get_macid(priv, sta);
>  	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>  				ampdu_enable, rts_rate, macid);
>  
> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>  
>  	priv->vifs[port_num] = vif;
>  	rtlvif->port_num = port_num;
> +	rtlvif->hw_key_idx = 0xff;
>  
>  	rtl8xxxu_set_linktype(priv, vif->type, port_num);
>  	ether_addr_copy(priv->mac_addr, vif->addr);
> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>  	return 0;
>  }
>  
> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> +{
> +	struct rtl8xxxu_priv *priv = hw->priv;
> +
> +	return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
> +}
> +
>  static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  			    struct ieee80211_vif *vif,
>  			    struct ieee80211_sta *sta,
>  			    struct ieee80211_key_conf *key)
>  {
> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>  	struct rtl8xxxu_priv *priv = hw->priv;
>  	struct device *dev = &priv->udev->dev;
>  	u8 mac_addr[ETH_ALEN];
> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  	dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>  		__func__, cmd, key->cipher, key->keyidx);
>  
> -	if (vif->type != NL80211_IFTYPE_STATION)
> -		return -EOPNOTSUPP;
> -
>  	if (key->keyidx > 3)
>  		return -EOPNOTSUPP;
>  
> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  		ether_addr_copy(mac_addr, sta->addr);
>  	} else {
>  		dev_dbg(dev, "%s: group key\n", __func__);
> -		eth_broadcast_addr(mac_addr);
> +		ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>  	}
>  
>  	val16 = rtl8xxxu_read16(priv, REG_CR);
> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  
>  	switch (cmd) {
>  	case SET_KEY:
> -		key->hw_key_idx = key->keyidx;
> +
> +		retval = rtl8xxxu_get_free_sec_cam(hw);
> +		if (retval < 0)
> +			return -EOPNOTSUPP;
> +
> +		key->hw_key_idx = retval;
> +
> +		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> +			rtlvif->hw_key_idx = key->hw_key_idx;
> +
>  		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>  		rtl8xxxu_cam_write(priv, key, mac_addr);
> +		set_bit(key->hw_key_idx, priv->cam_map);

Hi Martin,

It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
when we use iwd. After the WPA2 handshake no more data is transmitted.

Before this patch, key->hw_key_idx was the same as key->keyidx. After
this patch, when we use iwd, the group key is installed first. It has
key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().

It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
first, with key->keyidx = 0, then the group key, with key->keyidx = 1.

This patch imitating rtw88 makes the old chips work again with iwd.
What do you think?

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index ee278f0548e4..f7b35655bec5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
 {
 	struct rtl8xxxu_priv *priv = hw->priv;
 
-	return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
+	return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
+				  RTL8XXXU_SEC_DEFAULT_KEY_NUM);
 }
 
 static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
@@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 	switch (cmd) {
 	case SET_KEY:
+		if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
+			retval = rtl8xxxu_get_free_sec_cam(hw);
+			if (retval < 0)
+				return -EOPNOTSUPP;
 
-		retval = rtl8xxxu_get_free_sec_cam(hw);
-		if (retval < 0)
-			return -EOPNOTSUPP;
-
-		key->hw_key_idx = retval;
+			key->hw_key_idx = retval;
+		} else {
+			key->hw_key_idx = key->keyidx;
+		}
 
 		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
 			rtlvif->hw_key_idx = key->hw_key_idx;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 4b05dba22e67..188f4bbe99cd 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
 #define RTL8XXXU_BC_MC_MACID	0
 #define RTL8XXXU_BC_MC_MACID1	1
 #define RTL8XXXU_MAX_SEC_CAM_NUM	64
+#define RTL8XXXU_SEC_DEFAULT_KEY_NUM	4
 
 struct rtl8xxxu_priv {
 	struct ieee80211_hw *hw;

RE: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Ping-Ke Shih]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 22/12/2023 12:14, Martin Kaistra wrote:
> > Add a custom function for allocating entries in the sec cam. This allows
> > us to store multiple keys with the same keyidx.
> >
> > The maximum number of sec cam entries for 8188f is 16 according to the
> > vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
> > which might support more entries, can set a different number there.
> >
> > Set the bssid as mac address for group keys instead of just using the
> > ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
> > for differentiating them from pairwise keys like in the vendor driver.
> >
> > Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
> > broadcast/multicast packets in AP mode.
> >
> > Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
> >
> > Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> > Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> > ---
> >  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
> >  .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
> >  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
> >  3 files changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > index c5e6d8f7d26bd..62e6318bc0924 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
> >  #define DESC_RATE_ID_SHIFT           16
> >  #define DESC_RATE_ID_MASK            0xf
> >  #define TXDESC_NAVUSEHDR             BIT(20)
> > +#define TXDESC_EN_DESC_ID            BIT(21)
> >  #define TXDESC_SEC_RC4                       0x00400000
> >  #define TXDESC_SEC_AES                       0x00c00000
> >  #define TXDESC_PKT_OFFSET_SHIFT              26
> > @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
> >  #define RTL8XXXU_MAX_MAC_ID_NUM      128
> >  #define RTL8XXXU_BC_MC_MACID 0
> >  #define RTL8XXXU_BC_MC_MACID1        1
> > +#define RTL8XXXU_MAX_SEC_CAM_NUM     64
> >
> >  struct rtl8xxxu_priv {
> >       struct ieee80211_hw *hw;
> > @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
> >       char led_name[32];
> >       struct led_classdev led_cdev;
> >       DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
> > +     DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
> >  };
> >
> >  struct rtl8xxxu_sta_info {
> > @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
> >
> >  struct rtl8xxxu_vif {
> >       int port_num;
> > +     u8 hw_key_idx;
> >  };
> >
> >  struct rtl8xxxu_rx_urb {
> > @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
> >       u16 max_aggr_num;
> >       u8 supports_ap:1;
> >       u16 max_macid_num;
> > +     u16 max_sec_cam_num;
> >       u32 adda_1t_init;
> >       u32 adda_1t_path_on;
> >       u32 adda_2t_path_on_a;
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> > index 1e1c8fa194cb8..574a5fe951543 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> > @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
> >       .max_aggr_num = 0x0c14,
> >       .supports_ap = 1,
> >       .max_macid_num = 16,
> > +     .max_sec_cam_num = 16,
> >       .adda_1t_init = 0x03c00014,
> >       .adda_1t_path_on = 0x03c00014,
> >       .trxff_boundary = 0x3f7f,
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > index ecf54eb8dba61..7aafae9fe76b8 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
> >        * This is a bit of a hack - the lower bits of the cipher
> >        * suite selector happens to match the cipher index in the CAM
> >        */
> > -     addr = key->keyidx << CAM_CMD_KEY_SHIFT;
> > +     addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
> >       ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
> > +     if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> > +             ctrl |= BIT(6);
> >
> >       for (j = 5; j >= 0; j--) {
> >               switch (j) {
> > @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >       struct rtl8xxxu_tx_urb *tx_urb;
> >       struct ieee80211_sta *sta = NULL;
> >       struct ieee80211_vif *vif = tx_info->control.vif;
> > +     struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >       struct device *dev = &priv->udev->dev;
> >       u32 queue, rts_rate;
> >       u16 pktlen = skb->len;
> >       int tx_desc_size = priv->fops->tx_desc_size;
> >       u8 macid;
> >       int ret;
> > -     bool ampdu_enable, sgi = false, short_preamble = false;
> > +     bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
> >
> >       if (skb_headroom(skb) < tx_desc_size) {
> >               dev_warn(dev,
> > @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >               tx_desc->txdw0 =
> >                       TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
> >       if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
> > -         is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
> > +         is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
> >               tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
> > +             bmc = true;
> > +     }
> > +
> >
> >       tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
> > +     macid = rtl8xxxu_get_macid(priv, sta);
> >
> >       if (tx_info->control.hw_key) {
> >               switch (tx_info->control.hw_key->cipher) {
> > @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >               default:
> >                       break;
> >               }
> > +             if (bmc && rtlvif->hw_key_idx != 0xff) {
> > +                     tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
> > +                     macid = rtlvif->hw_key_idx;
> > +             }
> >       }
> >
> >       /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
> > @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >       else
> >               rts_rate = 0;
> >
> > -     macid = rtl8xxxu_get_macid(priv, sta);
> >       priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
> >                               ampdu_enable, rts_rate, macid);
> >
> > @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
> >
> >       priv->vifs[port_num] = vif;
> >       rtlvif->port_num = port_num;
> > +     rtlvif->hw_key_idx = 0xff;
> >
> >       rtl8xxxu_set_linktype(priv, vif->type, port_num);
> >       ether_addr_copy(priv->mac_addr, vif->addr);
> > @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
> >       return 0;
> >  }
> >
> > +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> > +{
> > +     struct rtl8xxxu_priv *priv = hw->priv;
> > +
> > +     return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
> > +}
> > +
> >  static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >                           struct ieee80211_vif *vif,
> >                           struct ieee80211_sta *sta,
> >                           struct ieee80211_key_conf *key)
> >  {
> > +     struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >       struct rtl8xxxu_priv *priv = hw->priv;
> >       struct device *dev = &priv->udev->dev;
> >       u8 mac_addr[ETH_ALEN];
> > @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >       dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
> >               __func__, cmd, key->cipher, key->keyidx);
> >
> > -     if (vif->type != NL80211_IFTYPE_STATION)
> > -             return -EOPNOTSUPP;
> > -
> >       if (key->keyidx > 3)
> >               return -EOPNOTSUPP;
> >
> > @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >               ether_addr_copy(mac_addr, sta->addr);
> >       } else {
> >               dev_dbg(dev, "%s: group key\n", __func__);
> > -             eth_broadcast_addr(mac_addr);
> > +             ether_addr_copy(mac_addr, vif->bss_conf.bssid);
> >       }
> >
> >       val16 = rtl8xxxu_read16(priv, REG_CR);
> > @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >
> >       switch (cmd) {
> >       case SET_KEY:
> > -             key->hw_key_idx = key->keyidx;
> > +
> > +             retval = rtl8xxxu_get_free_sec_cam(hw);
> > +             if (retval < 0)
> > +                     return -EOPNOTSUPP;
> > +
> > +             key->hw_key_idx = retval;
> > +
> > +             if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> > +                     rtlvif->hw_key_idx = key->hw_key_idx;
> > +
> >               key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> >               rtl8xxxu_cam_write(priv, key, mac_addr);
> > +             set_bit(key->hw_key_idx, priv->cam_map);
> 
> Hi Martin,
> 
> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
> when we use iwd. After the WPA2 handshake no more data is transmitted.
> 
> Before this patch, key->hw_key_idx was the same as key->keyidx. After
> this patch, when we use iwd, the group key is installed first. It has
> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
> 
> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
> 
> This patch imitating rtw88 makes the old chips work again with iwd.
> What do you think?

This question is not to me, but the patch to reserve 4 CAMs for default key
looks good to me. 

> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index ee278f0548e4..f7b35655bec5 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>  {
>         struct rtl8xxxu_priv *priv = hw->priv;
> 
> -       return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
> +       return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
> +                                 RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>  }
> 
>  static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> 
>         switch (cmd) {
>         case SET_KEY:
> +               if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
> +                       retval = rtl8xxxu_get_free_sec_cam(hw);
> +                       if (retval < 0)
> +                               return -EOPNOTSUPP;
> 
> -               retval = rtl8xxxu_get_free_sec_cam(hw);
> -               if (retval < 0)
> -                       return -EOPNOTSUPP;
> -
> -               key->hw_key_idx = retval;
> +                       key->hw_key_idx = retval;
> +               } else {
> +                       key->hw_key_idx = key->keyidx;
> +               }
> 
>                 if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>                         rtlvif->hw_key_idx = key->hw_key_idx;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 4b05dba22e67..188f4bbe99cd 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>  #define RTL8XXXU_BC_MC_MACID   0
>  #define RTL8XXXU_BC_MC_MACID1  1
>  #define RTL8XXXU_MAX_SEC_CAM_NUM       64
> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM   4
> 
>  struct rtl8xxxu_priv {
>         struct ieee80211_hw *hw;

Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Martin Kaistra]

Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
> On 22/12/2023 12:14, Martin Kaistra wrote:
>> Add a custom function for allocating entries in the sec cam. This allows
>> us to store multiple keys with the same keyidx.
>>
>> The maximum number of sec cam entries for 8188f is 16 according to the
>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
>> which might support more entries, can set a different number there.
>>
>> Set the bssid as mac address for group keys instead of just using the
>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>> for differentiating them from pairwise keys like in the vendor driver.
>>
>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>> broadcast/multicast packets in AP mode.
>>
>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>
>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>> ---
>>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
>>   .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>   3 files changed, 44 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index c5e6d8f7d26bd..62e6318bc0924 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>   #define DESC_RATE_ID_SHIFT		16
>>   #define DESC_RATE_ID_MASK		0xf
>>   #define TXDESC_NAVUSEHDR		BIT(20)
>> +#define TXDESC_EN_DESC_ID		BIT(21)
>>   #define TXDESC_SEC_RC4			0x00400000
>>   #define TXDESC_SEC_AES			0x00c00000
>>   #define TXDESC_PKT_OFFSET_SHIFT		26
>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>>   #define RTL8XXXU_MAX_MAC_ID_NUM	128
>>   #define RTL8XXXU_BC_MC_MACID	0
>>   #define RTL8XXXU_BC_MC_MACID1	1
>> +#define RTL8XXXU_MAX_SEC_CAM_NUM	64
>>   
>>   struct rtl8xxxu_priv {
>>   	struct ieee80211_hw *hw;
>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>>   	char led_name[32];
>>   	struct led_classdev led_cdev;
>>   	DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>> +	DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>   };
>>   
>>   struct rtl8xxxu_sta_info {
>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>>   
>>   struct rtl8xxxu_vif {
>>   	int port_num;
>> +	u8 hw_key_idx;
>>   };
>>   
>>   struct rtl8xxxu_rx_urb {
>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>>   	u16 max_aggr_num;
>>   	u8 supports_ap:1;
>>   	u16 max_macid_num;
>> +	u16 max_sec_cam_num;
>>   	u32 adda_1t_init;
>>   	u32 adda_1t_path_on;
>>   	u32 adda_2t_path_on_a;
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> index 1e1c8fa194cb8..574a5fe951543 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>   	.max_aggr_num = 0x0c14,
>>   	.supports_ap = 1,
>>   	.max_macid_num = 16,
>> +	.max_sec_cam_num = 16,
>>   	.adda_1t_init = 0x03c00014,
>>   	.adda_1t_path_on = 0x03c00014,
>>   	.trxff_boundary = 0x3f7f,
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index ecf54eb8dba61..7aafae9fe76b8 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>>   	 * This is a bit of a hack - the lower bits of the cipher
>>   	 * suite selector happens to match the cipher index in the CAM
>>   	 */
>> -	addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>> +	addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>   	ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>> +	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>> +		ctrl |= BIT(6);
>>   
>>   	for (j = 5; j >= 0; j--) {
>>   		switch (j) {
>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   	struct rtl8xxxu_tx_urb *tx_urb;
>>   	struct ieee80211_sta *sta = NULL;
>>   	struct ieee80211_vif *vif = tx_info->control.vif;
>> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>   	struct device *dev = &priv->udev->dev;
>>   	u32 queue, rts_rate;
>>   	u16 pktlen = skb->len;
>>   	int tx_desc_size = priv->fops->tx_desc_size;
>>   	u8 macid;
>>   	int ret;
>> -	bool ampdu_enable, sgi = false, short_preamble = false;
>> +	bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>   
>>   	if (skb_headroom(skb) < tx_desc_size) {
>>   		dev_warn(dev,
>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   		tx_desc->txdw0 =
>>   			TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>>   	if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>> -	    is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>> +	    is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>   		tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>> +		bmc = true;
>> +	}
>> +
>>   
>>   	tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>> +	macid = rtl8xxxu_get_macid(priv, sta);
>>   
>>   	if (tx_info->control.hw_key) {
>>   		switch (tx_info->control.hw_key->cipher) {
>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   		default:
>>   			break;
>>   		}
>> +		if (bmc && rtlvif->hw_key_idx != 0xff) {
>> +			tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>> +			macid = rtlvif->hw_key_idx;
>> +		}
>>   	}
>>   
>>   	/* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>   	else
>>   		rts_rate = 0;
>>   
>> -	macid = rtl8xxxu_get_macid(priv, sta);
>>   	priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>>   				ampdu_enable, rts_rate, macid);
>>   
>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>   
>>   	priv->vifs[port_num] = vif;
>>   	rtlvif->port_num = port_num;
>> +	rtlvif->hw_key_idx = 0xff;
>>   
>>   	rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>   	ether_addr_copy(priv->mac_addr, vif->addr);
>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>>   	return 0;
>>   }
>>   
>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>> +{
>> +	struct rtl8xxxu_priv *priv = hw->priv;
>> +
>> +	return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>> +}
>> +
>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   			    struct ieee80211_vif *vif,
>>   			    struct ieee80211_sta *sta,
>>   			    struct ieee80211_key_conf *key)
>>   {
>> +	struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>   	struct rtl8xxxu_priv *priv = hw->priv;
>>   	struct device *dev = &priv->udev->dev;
>>   	u8 mac_addr[ETH_ALEN];
>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   	dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>   		__func__, cmd, key->cipher, key->keyidx);
>>   
>> -	if (vif->type != NL80211_IFTYPE_STATION)
>> -		return -EOPNOTSUPP;
>> -
>>   	if (key->keyidx > 3)
>>   		return -EOPNOTSUPP;
>>   
>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   		ether_addr_copy(mac_addr, sta->addr);
>>   	} else {
>>   		dev_dbg(dev, "%s: group key\n", __func__);
>> -		eth_broadcast_addr(mac_addr);
>> +		ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>   	}
>>   
>>   	val16 = rtl8xxxu_read16(priv, REG_CR);
>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   
>>   	switch (cmd) {
>>   	case SET_KEY:
>> -		key->hw_key_idx = key->keyidx;
>> +
>> +		retval = rtl8xxxu_get_free_sec_cam(hw);
>> +		if (retval < 0)
>> +			return -EOPNOTSUPP;
>> +
>> +		key->hw_key_idx = retval;
>> +
>> +		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>> +			rtlvif->hw_key_idx = key->hw_key_idx;
>> +
>>   		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>   		rtl8xxxu_cam_write(priv, key, mac_addr);
>> +		set_bit(key->hw_key_idx, priv->cam_map);
> 
> Hi Martin,

Hi Bitterblue,

> 
> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
> when we use iwd. After the WPA2 handshake no more data is transmitted.
> 
> Before this patch, key->hw_key_idx was the same as key->keyidx. After
> this patch, when we use iwd, the group key is installed first. It has
> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
> 
> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
> 
> This patch imitating rtw88 makes the old chips work again with iwd.
> What do you think?

So you reserve the first 4 entries for group keys and use key->keyidx as 
key->hw_key_idx directly for those, right? Does that work if 2 virtual 
interfaces are used at the same time?
I will do some tests in the next days, but I suspect this be an issue.

> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index ee278f0548e4..f7b35655bec5 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>   {
>   	struct rtl8xxxu_priv *priv = hw->priv;
>   
> -	return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
> +	return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
> +				  RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>   }
>   
>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>   
>   	switch (cmd) {
>   	case SET_KEY:
> +		if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
> +			retval = rtl8xxxu_get_free_sec_cam(hw);
> +			if (retval < 0)
> +				return -EOPNOTSUPP;
>   
> -		retval = rtl8xxxu_get_free_sec_cam(hw);
> -		if (retval < 0)
> -			return -EOPNOTSUPP;
> -
> -		key->hw_key_idx = retval;
> +			key->hw_key_idx = retval;
> +		} else {
> +			key->hw_key_idx = key->keyidx;
> +		}
>   
>   		if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>   			rtlvif->hw_key_idx = key->hw_key_idx;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 4b05dba22e67..188f4bbe99cd 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>   #define RTL8XXXU_BC_MC_MACID	0
>   #define RTL8XXXU_BC_MC_MACID1	1
>   #define RTL8XXXU_MAX_SEC_CAM_NUM	64
> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM	4
>   
>   struct rtl8xxxu_priv {
>   	struct ieee80211_hw *hw;

RE: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Ping-Ke Shih]

Martin Kaistra <martin.kaistra@linutronix.de> wrote:
> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
> > On 22/12/2023 12:14, Martin Kaistra wrote:
> >> Add a custom function for allocating entries in the sec cam. This allows
> >> us to store multiple keys with the same keyidx.
> >>
> >> The maximum number of sec cam entries for 8188f is 16 according to the
> >> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
> >> which might support more entries, can set a different number there.
> >>
> >> Set the bssid as mac address for group keys instead of just using the
> >> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
> >> for differentiating them from pairwise keys like in the vendor driver.
> >>
> >> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
> >> broadcast/multicast packets in AP mode.
> >>
> >> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
> >>
> >> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> >> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> >> ---
> >>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
> >>   .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
> >>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
> >>   3 files changed, 44 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> index c5e6d8f7d26bd..62e6318bc0924 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
> >>   #define DESC_RATE_ID_SHIFT         16
> >>   #define DESC_RATE_ID_MASK          0xf
> >>   #define TXDESC_NAVUSEHDR           BIT(20)
> >> +#define TXDESC_EN_DESC_ID           BIT(21)
> >>   #define TXDESC_SEC_RC4                     0x00400000
> >>   #define TXDESC_SEC_AES                     0x00c00000
> >>   #define TXDESC_PKT_OFFSET_SHIFT            26
> >> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
> >>   #define RTL8XXXU_MAX_MAC_ID_NUM    128
> >>   #define RTL8XXXU_BC_MC_MACID       0
> >>   #define RTL8XXXU_BC_MC_MACID1      1
> >> +#define RTL8XXXU_MAX_SEC_CAM_NUM    64
> >>
> >>   struct rtl8xxxu_priv {
> >>      struct ieee80211_hw *hw;
> >> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
> >>      char led_name[32];
> >>      struct led_classdev led_cdev;
> >>      DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
> >> +    DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
> >>   };
> >>
> >>   struct rtl8xxxu_sta_info {
> >> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
> >>
> >>   struct rtl8xxxu_vif {
> >>      int port_num;
> >> +    u8 hw_key_idx;
> >>   };
> >>
> >>   struct rtl8xxxu_rx_urb {
> >> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
> >>      u16 max_aggr_num;
> >>      u8 supports_ap:1;
> >>      u16 max_macid_num;
> >> +    u16 max_sec_cam_num;
> >>      u32 adda_1t_init;
> >>      u32 adda_1t_path_on;
> >>      u32 adda_2t_path_on_a;
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >> index 1e1c8fa194cb8..574a5fe951543 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
> >>      .max_aggr_num = 0x0c14,
> >>      .supports_ap = 1,
> >>      .max_macid_num = 16,
> >> +    .max_sec_cam_num = 16,
> >>      .adda_1t_init = 0x03c00014,
> >>      .adda_1t_path_on = 0x03c00014,
> >>      .trxff_boundary = 0x3f7f,
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> index ecf54eb8dba61..7aafae9fe76b8 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
> >>       * This is a bit of a hack - the lower bits of the cipher
> >>       * suite selector happens to match the cipher index in the CAM
> >>       */
> >> -    addr = key->keyidx << CAM_CMD_KEY_SHIFT;
> >> +    addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
> >>      ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
> >> +    if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> >> +            ctrl |= BIT(6);
> >>
> >>      for (j = 5; j >= 0; j--) {
> >>              switch (j) {
> >> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>      struct rtl8xxxu_tx_urb *tx_urb;
> >>      struct ieee80211_sta *sta = NULL;
> >>      struct ieee80211_vif *vif = tx_info->control.vif;
> >> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >>      struct device *dev = &priv->udev->dev;
> >>      u32 queue, rts_rate;
> >>      u16 pktlen = skb->len;
> >>      int tx_desc_size = priv->fops->tx_desc_size;
> >>      u8 macid;
> >>      int ret;
> >> -    bool ampdu_enable, sgi = false, short_preamble = false;
> >> +    bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
> >>
> >>      if (skb_headroom(skb) < tx_desc_size) {
> >>              dev_warn(dev,
> >> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>              tx_desc->txdw0 =
> >>                      TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
> >>      if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
> >> -        is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
> >> +        is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
> >>              tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
> >> +            bmc = true;
> >> +    }
> >> +
> >>
> >>      tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
> >> +    macid = rtl8xxxu_get_macid(priv, sta);
> >>
> >>      if (tx_info->control.hw_key) {
> >>              switch (tx_info->control.hw_key->cipher) {
> >> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>              default:
> >>                      break;
> >>              }
> >> +            if (bmc && rtlvif->hw_key_idx != 0xff) {
> >> +                    tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
> >> +                    macid = rtlvif->hw_key_idx;
> >> +            }
> >>      }
> >>
> >>      /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
> >> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>      else
> >>              rts_rate = 0;
> >>
> >> -    macid = rtl8xxxu_get_macid(priv, sta);
> >>      priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
> >>                              ampdu_enable, rts_rate, macid);
> >>
> >> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
> >>
> >>      priv->vifs[port_num] = vif;
> >>      rtlvif->port_num = port_num;
> >> +    rtlvif->hw_key_idx = 0xff;
> >>
> >>      rtl8xxxu_set_linktype(priv, vif->type, port_num);
> >>      ether_addr_copy(priv->mac_addr, vif->addr);
> >> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
> >>      return 0;
> >>   }
> >>
> >> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> >> +{
> >> +    struct rtl8xxxu_priv *priv = hw->priv;
> >> +
> >> +    return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
> >> +}
> >> +
> >>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >>                          struct ieee80211_vif *vif,
> >>                          struct ieee80211_sta *sta,
> >>                          struct ieee80211_key_conf *key)
> >>   {
> >> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >>      struct rtl8xxxu_priv *priv = hw->priv;
> >>      struct device *dev = &priv->udev->dev;
> >>      u8 mac_addr[ETH_ALEN];
> >> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >>      dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
> >>              __func__, cmd, key->cipher, key->keyidx);
> >>
> >> -    if (vif->type != NL80211_IFTYPE_STATION)
> >> -            return -EOPNOTSUPP;
> >> -
> >>      if (key->keyidx > 3)
> >>              return -EOPNOTSUPP;
> >>
> >> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >>              ether_addr_copy(mac_addr, sta->addr);
> >>      } else {
> >>              dev_dbg(dev, "%s: group key\n", __func__);
> >> -            eth_broadcast_addr(mac_addr);
> >> +            ether_addr_copy(mac_addr, vif->bss_conf.bssid);
> >>      }
> >>
> >>      val16 = rtl8xxxu_read16(priv, REG_CR);
> >> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >>
> >>      switch (cmd) {
> >>      case SET_KEY:
> >> -            key->hw_key_idx = key->keyidx;
> >> +
> >> +            retval = rtl8xxxu_get_free_sec_cam(hw);
> >> +            if (retval < 0)
> >> +                    return -EOPNOTSUPP;
> >> +
> >> +            key->hw_key_idx = retval;
> >> +
> >> +            if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> >> +                    rtlvif->hw_key_idx = key->hw_key_idx;
> >> +
> >>              key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> >>              rtl8xxxu_cam_write(priv, key, mac_addr);
> >> +            set_bit(key->hw_key_idx, priv->cam_map);
> >
> > Hi Martin,
> 
> Hi Bitterblue,
> 
> >
> > It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
> > when we use iwd. After the WPA2 handshake no more data is transmitted.
> >
> > Before this patch, key->hw_key_idx was the same as key->keyidx. After
> > this patch, when we use iwd, the group key is installed first. It has
> > key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
> > installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
> > Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
> >
> > It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
> > first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
> >
> > This patch imitating rtw88 makes the old chips work again with iwd.
> > What do you think?
> 
> So you reserve the first 4 entries for group keys and use key->keyidx as
> key->hw_key_idx directly for those, right? Does that work if 2 virtual
> interfaces are used at the same time?
> I will do some tests in the next days, but I suspect this be an issue.

Ah. I'm not aware that rtl8xxxu can support 2 virtual interfaces, and
only RTL8188FU declares .supports_concurrent = 1, so maybe we can have
special handling for this chip?

I'd share some information about security CAM for reference. Since data is
quite old (10+ years), I can't guarantee all are correct.

1. default key switch in rtl8xxxu_set_key()
   SEC_CFG_TXBC_USE_DEFKEY and SEC_CFG_RXBC_USE_DEFKEY mean that
   broadcast/multicast packets use default key (CAM entries 0~3 for
   keyidx 0~3).
   For two interfaces case, that'd a problem, so maybe we should use
   software encryption/decryption for non-pairwise key.

	val8 = SEC_CFG_TX_SEC_ENABLE | SEC_CFG_TXBC_USE_DEFKEY |
		SEC_CFG_RX_SEC_ENABLE | SEC_CFG_RXBC_USE_DEFKEY;
	val8 |= SEC_CFG_TX_USE_DEFKEY | SEC_CFG_RX_USE_DEFKEY;
	rtl8xxxu_write8(priv, REG_SECURITY_CFG, val8);

2. group key (GK) field of security CAM in rtl8xxxu_cam_write()
   The group key field of security CAM is BIT(6) which isn't supported
   by earlier chips (sorry I have no data about the exact chips).

   If a chip can support group key field, the CAM layout with default key
   enabled for two interfaces (STA-1+AP-1) can be

   STA-1 <-> AP-10
   AP-1  <-> STA-20

   CAM  GK   MAC    KEY
    0
    1   1    00:    AP-1 (GTK)  // should be on entry 0~3 depends on key_idx
    2
    3
    4
    5   1    AP-10  AP-10 (GTK)  // can be on any entry after 4
    6

    (pairwise key can be on any entry after 4)

	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
		ctrl |= BIT(6);

My perspective are
1. currently default key is enabled, so we should treat CAM entry 0~3 as
   special cases. That means rtl8xxxu_get_free_sec_cam() should be modified
   as Bitterblue's version.

2. For two interfaces, I guess RTL8188FU can support GK bit, so it'd be worth
   to try the method of example 2 above.

   If we want earlier chips can support two interfaces, I think we should 
   disable default key. Seemingly, it is not possible that using hardware
   crypto when operating one interface and then switching to use software
   crypto when operating two interfaces. That means broadcast/multicast
   packets should be by software crypto entirely if we want to support two
   interfaces.

3. I think rtw88 has similar problem for two interfaces.

> 
> >
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> > index ee278f0548e4..f7b35655bec5 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> > @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> >   {
> >       struct rtl8xxxu_priv *priv = hw->priv;
> >
> > -     return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
> > +     return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
> > +                               RTL8XXXU_SEC_DEFAULT_KEY_NUM);
> >   }
> >
> >   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> > @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >
> >       switch (cmd) {
> >       case SET_KEY:
> > +             if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
> > +                     retval = rtl8xxxu_get_free_sec_cam(hw);
> > +                     if (retval < 0)
> > +                             return -EOPNOTSUPP;
> >
> > -             retval = rtl8xxxu_get_free_sec_cam(hw);
> > -             if (retval < 0)
> > -                     return -EOPNOTSUPP;
> > -
> > -             key->hw_key_idx = retval;
> > +                     key->hw_key_idx = retval;
> > +             } else {
> > +                     key->hw_key_idx = key->keyidx;
> > +             }
> >
> >               if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> >                       rtlvif->hw_key_idx = key->hw_key_idx;
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > index 4b05dba22e67..188f4bbe99cd 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
> >   #define RTL8XXXU_BC_MC_MACID        0
> >   #define RTL8XXXU_BC_MC_MACID1       1
> >   #define RTL8XXXU_MAX_SEC_CAM_NUM    64
> > +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
> >
> >   struct rtl8xxxu_priv {
> >       struct ieee80211_hw *hw;

Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Bitterblue Smith]

On 25/02/2026 04:28, Ping-Ke Shih wrote:
> Martin Kaistra <martin.kaistra@linutronix.de> wrote:
>> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
>>> On 22/12/2023 12:14, Martin Kaistra wrote:
>>>> Add a custom function for allocating entries in the sec cam. This allows
>>>> us to store multiple keys with the same keyidx.
>>>>
>>>> The maximum number of sec cam entries for 8188f is 16 according to the
>>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
>>>> which might support more entries, can set a different number there.
>>>>
>>>> Set the bssid as mac address for group keys instead of just using the
>>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>>>> for differentiating them from pairwise keys like in the vendor driver.
>>>>
>>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>>>> broadcast/multicast packets in AP mode.
>>>>
>>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>>>
>>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>>>> ---
>>>>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
>>>>   .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
>>>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>>>   3 files changed, 44 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> index c5e6d8f7d26bd..62e6318bc0924 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>>>   #define DESC_RATE_ID_SHIFT         16
>>>>   #define DESC_RATE_ID_MASK          0xf
>>>>   #define TXDESC_NAVUSEHDR           BIT(20)
>>>> +#define TXDESC_EN_DESC_ID           BIT(21)
>>>>   #define TXDESC_SEC_RC4                     0x00400000
>>>>   #define TXDESC_SEC_AES                     0x00c00000
>>>>   #define TXDESC_PKT_OFFSET_SHIFT            26
>>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>>>>   #define RTL8XXXU_MAX_MAC_ID_NUM    128
>>>>   #define RTL8XXXU_BC_MC_MACID       0
>>>>   #define RTL8XXXU_BC_MC_MACID1      1
>>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM    64
>>>>
>>>>   struct rtl8xxxu_priv {
>>>>      struct ieee80211_hw *hw;
>>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>>>>      char led_name[32];
>>>>      struct led_classdev led_cdev;
>>>>      DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>>>> +    DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>>>   };
>>>>
>>>>   struct rtl8xxxu_sta_info {
>>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>>>>
>>>>   struct rtl8xxxu_vif {
>>>>      int port_num;
>>>> +    u8 hw_key_idx;
>>>>   };
>>>>
>>>>   struct rtl8xxxu_rx_urb {
>>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>>>>      u16 max_aggr_num;
>>>>      u8 supports_ap:1;
>>>>      u16 max_macid_num;
>>>> +    u16 max_sec_cam_num;
>>>>      u32 adda_1t_init;
>>>>      u32 adda_1t_path_on;
>>>>      u32 adda_2t_path_on_a;
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> index 1e1c8fa194cb8..574a5fe951543 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>>>      .max_aggr_num = 0x0c14,
>>>>      .supports_ap = 1,
>>>>      .max_macid_num = 16,
>>>> +    .max_sec_cam_num = 16,
>>>>      .adda_1t_init = 0x03c00014,
>>>>      .adda_1t_path_on = 0x03c00014,
>>>>      .trxff_boundary = 0x3f7f,
>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> index ecf54eb8dba61..7aafae9fe76b8 100644
>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>>>>       * This is a bit of a hack - the lower bits of the cipher
>>>>       * suite selector happens to match the cipher index in the CAM
>>>>       */
>>>> -    addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>>>> +    addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>>>      ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>>>> +    if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>> +            ctrl |= BIT(6);
>>>>
>>>>      for (j = 5; j >= 0; j--) {
>>>>              switch (j) {
>>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>      struct rtl8xxxu_tx_urb *tx_urb;
>>>>      struct ieee80211_sta *sta = NULL;
>>>>      struct ieee80211_vif *vif = tx_info->control.vif;
>>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>>      struct device *dev = &priv->udev->dev;
>>>>      u32 queue, rts_rate;
>>>>      u16 pktlen = skb->len;
>>>>      int tx_desc_size = priv->fops->tx_desc_size;
>>>>      u8 macid;
>>>>      int ret;
>>>> -    bool ampdu_enable, sgi = false, short_preamble = false;
>>>> +    bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>>>
>>>>      if (skb_headroom(skb) < tx_desc_size) {
>>>>              dev_warn(dev,
>>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>              tx_desc->txdw0 =
>>>>                      TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>>>>      if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>>>> -        is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>>>> +        is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>>>              tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>>>> +            bmc = true;
>>>> +    }
>>>> +
>>>>
>>>>      tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>>>> +    macid = rtl8xxxu_get_macid(priv, sta);
>>>>
>>>>      if (tx_info->control.hw_key) {
>>>>              switch (tx_info->control.hw_key->cipher) {
>>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>              default:
>>>>                      break;
>>>>              }
>>>> +            if (bmc && rtlvif->hw_key_idx != 0xff) {
>>>> +                    tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>>>> +                    macid = rtlvif->hw_key_idx;
>>>> +            }
>>>>      }
>>>>
>>>>      /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>      else
>>>>              rts_rate = 0;
>>>>
>>>> -    macid = rtl8xxxu_get_macid(priv, sta);
>>>>      priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>>>>                              ampdu_enable, rts_rate, macid);
>>>>
>>>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>>>
>>>>      priv->vifs[port_num] = vif;
>>>>      rtlvif->port_num = port_num;
>>>> +    rtlvif->hw_key_idx = 0xff;
>>>>
>>>>      rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>>>      ether_addr_copy(priv->mac_addr, vif->addr);
>>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>>>>      return 0;
>>>>   }
>>>>
>>>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>>> +{
>>>> +    struct rtl8xxxu_priv *priv = hw->priv;
>>>> +
>>>> +    return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>>>> +}
>>>> +
>>>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>                          struct ieee80211_vif *vif,
>>>>                          struct ieee80211_sta *sta,
>>>>                          struct ieee80211_key_conf *key)
>>>>   {
>>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>>      struct rtl8xxxu_priv *priv = hw->priv;
>>>>      struct device *dev = &priv->udev->dev;
>>>>      u8 mac_addr[ETH_ALEN];
>>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>      dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>>>              __func__, cmd, key->cipher, key->keyidx);
>>>>
>>>> -    if (vif->type != NL80211_IFTYPE_STATION)
>>>> -            return -EOPNOTSUPP;
>>>> -
>>>>      if (key->keyidx > 3)
>>>>              return -EOPNOTSUPP;
>>>>
>>>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>              ether_addr_copy(mac_addr, sta->addr);
>>>>      } else {
>>>>              dev_dbg(dev, "%s: group key\n", __func__);
>>>> -            eth_broadcast_addr(mac_addr);
>>>> +            ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>>>      }
>>>>
>>>>      val16 = rtl8xxxu_read16(priv, REG_CR);
>>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>>
>>>>      switch (cmd) {
>>>>      case SET_KEY:
>>>> -            key->hw_key_idx = key->keyidx;
>>>> +
>>>> +            retval = rtl8xxxu_get_free_sec_cam(hw);
>>>> +            if (retval < 0)
>>>> +                    return -EOPNOTSUPP;
>>>> +
>>>> +            key->hw_key_idx = retval;
>>>> +
>>>> +            if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>> +                    rtlvif->hw_key_idx = key->hw_key_idx;
>>>> +
>>>>              key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>>>              rtl8xxxu_cam_write(priv, key, mac_addr);
>>>> +            set_bit(key->hw_key_idx, priv->cam_map);
>>>
>>> Hi Martin,
>>
>> Hi Bitterblue,
>>
>>>
>>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
>>> when we use iwd. After the WPA2 handshake no more data is transmitted.
>>>
>>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
>>> this patch, when we use iwd, the group key is installed first. It has
>>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
>>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
>>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
>>>
>>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
>>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
>>>
>>> This patch imitating rtw88 makes the old chips work again with iwd.
>>> What do you think?
>>
>> So you reserve the first 4 entries for group keys and use key->keyidx as
>> key->hw_key_idx directly for those, right? Does that work if 2 virtual
>> interfaces are used at the same time?
>> I will do some tests in the next days, but I suspect this be an issue.
> 
> Ah. I'm not aware that rtl8xxxu can support 2 virtual interfaces, and
> only RTL8188FU declares .supports_concurrent = 1, so maybe we can have
> special handling for this chip?

I would like to enable supports_concurrent for more chips, if I can test
this feature.

> 
> I'd share some information about security CAM for reference. Since data is
> quite old (10+ years), I can't guarantee all are correct.
> 
> 1. default key switch in rtl8xxxu_set_key()
>    SEC_CFG_TXBC_USE_DEFKEY and SEC_CFG_RXBC_USE_DEFKEY mean that
>    broadcast/multicast packets use default key (CAM entries 0~3 for
>    keyidx 0~3).
>    For two interfaces case, that'd a problem, so maybe we should use
>    software encryption/decryption for non-pairwise key.
> 
> 	val8 = SEC_CFG_TX_SEC_ENABLE | SEC_CFG_TXBC_USE_DEFKEY |
> 		SEC_CFG_RX_SEC_ENABLE | SEC_CFG_RXBC_USE_DEFKEY;
> 	val8 |= SEC_CFG_TX_USE_DEFKEY | SEC_CFG_RX_USE_DEFKEY;
> 	rtl8xxxu_write8(priv, REG_SECURITY_CFG, val8);
> 
> 2. group key (GK) field of security CAM in rtl8xxxu_cam_write()
>    The group key field of security CAM is BIT(6) which isn't supported
>    by earlier chips (sorry I have no data about the exact chips).
> 

BIT 6 is used in the rtl8188eu driver, and not used in the rtl8192cu driver,
so I assume it's just the older RTL8188CU/RTL8192CU and RTL8723AU that don't
support it.

>    If a chip can support group key field, the CAM layout with default key
>    enabled for two interfaces (STA-1+AP-1) can be
> 
>    STA-1 <-> AP-10
>    AP-1  <-> STA-20
> 
>    CAM  GK   MAC    KEY
>     0
>     1   1    00:    AP-1 (GTK)  // should be on entry 0~3 depends on key_idx
>     2
>     3
>     4
>     5   1    AP-10  AP-10 (GTK)  // can be on any entry after 4
>     6
> 

What are the numbers 1, 10, and 20 in "AP-1", "AP-10", "STA-20" ?

>     (pairwise key can be on any entry after 4)
> 
> 	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> 		ctrl |= BIT(6);
> 
> My perspective are
> 1. currently default key is enabled, so we should treat CAM entry 0~3 as
>    special cases. That means rtl8xxxu_get_free_sec_cam() should be modified
>    as Bitterblue's version.
> 
> 2. For two interfaces, I guess RTL8188FU can support GK bit, so it'd be worth
>    to try the method of example 2 above.
> 
>    If we want earlier chips can support two interfaces, I think we should 
>    disable default key.

Indeed, the rtl8192cu driver (out of kernel) doesn't set SEC_CFG_TXBC_USE_DEFKEY
and SEC_CFG_RXBC_USE_DEFKEY when concurrent mode is enabled.

> Seemingly, it is not possible that using hardware
>    crypto when operating one interface and then switching to use software>    crypto when operating two interfaces. That means broadcast/multicast
>    packets should be by software crypto entirely if we want to support two
>    interfaces.
> 
> 3. I think rtw88 has similar problem for two interfaces.
> 
>>
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>> index ee278f0548e4..f7b35655bec5 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>>   {
>>>       struct rtl8xxxu_priv *priv = hw->priv;
>>>
>>> -     return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>>> +     return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
>>> +                               RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>>>   }
>>>
>>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>
>>>       switch (cmd) {
>>>       case SET_KEY:
>>> +             if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
>>> +                     retval = rtl8xxxu_get_free_sec_cam(hw);
>>> +                     if (retval < 0)
>>> +                             return -EOPNOTSUPP;
>>>
>>> -             retval = rtl8xxxu_get_free_sec_cam(hw);
>>> -             if (retval < 0)
>>> -                     return -EOPNOTSUPP;
>>> -
>>> -             key->hw_key_idx = retval;
>>> +                     key->hw_key_idx = retval;
>>> +             } else {
>>> +                     key->hw_key_idx = key->keyidx;
>>> +             }
>>>
>>>               if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>                       rtlvif->hw_key_idx = key->hw_key_idx;
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> index 4b05dba22e67..188f4bbe99cd 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>>>   #define RTL8XXXU_BC_MC_MACID        0
>>>   #define RTL8XXXU_BC_MC_MACID1       1
>>>   #define RTL8XXXU_MAX_SEC_CAM_NUM    64
>>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
>>>
>>>   struct rtl8xxxu_priv {
>>>       struct ieee80211_hw *hw;
> 

RE: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Ping-Ke Shih]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 25/02/2026 04:28, Ping-Ke Shih wrote:
> > Martin Kaistra <martin.kaistra@linutronix.de> wrote:
> >> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
> >>> On 22/12/2023 12:14, Martin Kaistra wrote:
> >>>> Add a custom function for allocating entries in the sec cam. This allows
> >>>> us to store multiple keys with the same keyidx.
> >>>>
> >>>> The maximum number of sec cam entries for 8188f is 16 according to the
> >>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
> >>>> which might support more entries, can set a different number there.
> >>>>
> >>>> Set the bssid as mac address for group keys instead of just using the
> >>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
> >>>> for differentiating them from pairwise keys like in the vendor driver.
> >>>>
> >>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
> >>>> broadcast/multicast packets in AP mode.
> >>>>
> >>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
> >>>>
> >>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
> >>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
> >>>> ---
> >>>>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
> >>>>   .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
> >>>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
> >>>>   3 files changed, 44 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>>> index c5e6d8f7d26bd..62e6318bc0924 100644
> >>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
> >>>>   #define DESC_RATE_ID_SHIFT         16
> >>>>   #define DESC_RATE_ID_MASK          0xf
> >>>>   #define TXDESC_NAVUSEHDR           BIT(20)
> >>>> +#define TXDESC_EN_DESC_ID           BIT(21)
> >>>>   #define TXDESC_SEC_RC4                     0x00400000
> >>>>   #define TXDESC_SEC_AES                     0x00c00000
> >>>>   #define TXDESC_PKT_OFFSET_SHIFT            26
> >>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
> >>>>   #define RTL8XXXU_MAX_MAC_ID_NUM    128
> >>>>   #define RTL8XXXU_BC_MC_MACID       0
> >>>>   #define RTL8XXXU_BC_MC_MACID1      1
> >>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM    64
> >>>>
> >>>>   struct rtl8xxxu_priv {
> >>>>      struct ieee80211_hw *hw;
> >>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
> >>>>      char led_name[32];
> >>>>      struct led_classdev led_cdev;
> >>>>      DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
> >>>> +    DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
> >>>>   };
> >>>>
> >>>>   struct rtl8xxxu_sta_info {
> >>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
> >>>>
> >>>>   struct rtl8xxxu_vif {
> >>>>      int port_num;
> >>>> +    u8 hw_key_idx;
> >>>>   };
> >>>>
> >>>>   struct rtl8xxxu_rx_urb {
> >>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
> >>>>      u16 max_aggr_num;
> >>>>      u8 supports_ap:1;
> >>>>      u16 max_macid_num;
> >>>> +    u16 max_sec_cam_num;
> >>>>      u32 adda_1t_init;
> >>>>      u32 adda_1t_path_on;
> >>>>      u32 adda_2t_path_on_a;
> >>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >>>> index 1e1c8fa194cb8..574a5fe951543 100644
> >>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> >>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
> >>>>      .max_aggr_num = 0x0c14,
> >>>>      .supports_ap = 1,
> >>>>      .max_macid_num = 16,
> >>>> +    .max_sec_cam_num = 16,
> >>>>      .adda_1t_init = 0x03c00014,
> >>>>      .adda_1t_path_on = 0x03c00014,
> >>>>      .trxff_boundary = 0x3f7f,
> >>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>>> index ecf54eb8dba61..7aafae9fe76b8 100644
> >>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv
> *priv,
> >>>>       * This is a bit of a hack - the lower bits of the cipher
> >>>>       * suite selector happens to match the cipher index in the CAM
> >>>>       */
> >>>> -    addr = key->keyidx << CAM_CMD_KEY_SHIFT;
> >>>> +    addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
> >>>>      ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
> >>>> +    if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> >>>> +            ctrl |= BIT(6);
> >>>>
> >>>>      for (j = 5; j >= 0; j--) {
> >>>>              switch (j) {
> >>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>>      struct rtl8xxxu_tx_urb *tx_urb;
> >>>>      struct ieee80211_sta *sta = NULL;
> >>>>      struct ieee80211_vif *vif = tx_info->control.vif;
> >>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >>>>      struct device *dev = &priv->udev->dev;
> >>>>      u32 queue, rts_rate;
> >>>>      u16 pktlen = skb->len;
> >>>>      int tx_desc_size = priv->fops->tx_desc_size;
> >>>>      u8 macid;
> >>>>      int ret;
> >>>> -    bool ampdu_enable, sgi = false, short_preamble = false;
> >>>> +    bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
> >>>>
> >>>>      if (skb_headroom(skb) < tx_desc_size) {
> >>>>              dev_warn(dev,
> >>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>>              tx_desc->txdw0 =
> >>>>                      TXDESC_OWN | TXDESC_FIRST_SEGMENT |
> TXDESC_LAST_SEGMENT;
> >>>>      if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
> >>>> -        is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
> >>>> +        is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
> >>>>              tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
> >>>> +            bmc = true;
> >>>> +    }
> >>>> +
> >>>>
> >>>>      tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
> >>>> +    macid = rtl8xxxu_get_macid(priv, sta);
> >>>>
> >>>>      if (tx_info->control.hw_key) {
> >>>>              switch (tx_info->control.hw_key->cipher) {
> >>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>>              default:
> >>>>                      break;
> >>>>              }
> >>>> +            if (bmc && rtlvif->hw_key_idx != 0xff) {
> >>>> +                    tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
> >>>> +                    macid = rtlvif->hw_key_idx;
> >>>> +            }
> >>>>      }
> >>>>
> >>>>      /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
> >>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
> >>>>      else
> >>>>              rts_rate = 0;
> >>>>
> >>>> -    macid = rtl8xxxu_get_macid(priv, sta);
> >>>>      priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi,
> short_preamble,
> >>>>                              ampdu_enable, rts_rate, macid);
> >>>>
> >>>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw
> *hw,
> >>>>
> >>>>      priv->vifs[port_num] = vif;
> >>>>      rtlvif->port_num = port_num;
> >>>> +    rtlvif->hw_key_idx = 0xff;
> >>>>
> >>>>      rtl8xxxu_set_linktype(priv, vif->type, port_num);
> >>>>      ether_addr_copy(priv->mac_addr, vif->addr);
> >>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct
> ieee80211_hw *hw, u32 rts)
> >>>>      return 0;
> >>>>   }
> >>>>
> >>>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> >>>> +{
> >>>> +    struct rtl8xxxu_priv *priv = hw->priv;
> >>>> +
> >>>> +    return find_first_zero_bit(priv->cam_map,
> priv->fops->max_sec_cam_num);
> >>>> +}
> >>>> +
> >>>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd
> cmd,
> >>>>                          struct ieee80211_vif *vif,
> >>>>                          struct ieee80211_sta *sta,
> >>>>                          struct ieee80211_key_conf *key)
> >>>>   {
> >>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
> >>>>      struct rtl8xxxu_priv *priv = hw->priv;
> >>>>      struct device *dev = &priv->udev->dev;
> >>>>      u8 mac_addr[ETH_ALEN];
> >>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>>      dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
> >>>>              __func__, cmd, key->cipher, key->keyidx);
> >>>>
> >>>> -    if (vif->type != NL80211_IFTYPE_STATION)
> >>>> -            return -EOPNOTSUPP;
> >>>> -
> >>>>      if (key->keyidx > 3)
> >>>>              return -EOPNOTSUPP;
> >>>>
> >>>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>>              ether_addr_copy(mac_addr, sta->addr);
> >>>>      } else {
> >>>>              dev_dbg(dev, "%s: group key\n", __func__);
> >>>> -            eth_broadcast_addr(mac_addr);
> >>>> +            ether_addr_copy(mac_addr, vif->bss_conf.bssid);
> >>>>      }
> >>>>
> >>>>      val16 = rtl8xxxu_read16(priv, REG_CR);
> >>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>>
> >>>>      switch (cmd) {
> >>>>      case SET_KEY:
> >>>> -            key->hw_key_idx = key->keyidx;
> >>>> +
> >>>> +            retval = rtl8xxxu_get_free_sec_cam(hw);
> >>>> +            if (retval < 0)
> >>>> +                    return -EOPNOTSUPP;
> >>>> +
> >>>> +            key->hw_key_idx = retval;
> >>>> +
> >>>> +            if (vif->type == NL80211_IFTYPE_AP && !(key->flags &
> IEEE80211_KEY_FLAG_PAIRWISE))
> >>>> +                    rtlvif->hw_key_idx = key->hw_key_idx;
> >>>> +
> >>>>              key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> >>>>              rtl8xxxu_cam_write(priv, key, mac_addr);
> >>>> +            set_bit(key->hw_key_idx, priv->cam_map);
> >>>
> >>> Hi Martin,
> >>
> >> Hi Bitterblue,
> >>
> >>>
> >>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
> >>> when we use iwd. After the WPA2 handshake no more data is transmitted.
> >>>
> >>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
> >>> this patch, when we use iwd, the group key is installed first. It has
> >>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
> >>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
> >>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
> >>>
> >>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
> >>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
> >>>
> >>> This patch imitating rtw88 makes the old chips work again with iwd.
> >>> What do you think?
> >>
> >> So you reserve the first 4 entries for group keys and use key->keyidx as
> >> key->hw_key_idx directly for those, right? Does that work if 2 virtual
> >> interfaces are used at the same time?
> >> I will do some tests in the next days, but I suspect this be an issue.
> >
> > Ah. I'm not aware that rtl8xxxu can support 2 virtual interfaces, and
> > only RTL8188FU declares .supports_concurrent = 1, so maybe we can have
> > special handling for this chip?
> 
> I would like to enable supports_concurrent for more chips, if I can test
> this feature.
> 
> >
> > I'd share some information about security CAM for reference. Since data is
> > quite old (10+ years), I can't guarantee all are correct.
> >
> > 1. default key switch in rtl8xxxu_set_key()
> >    SEC_CFG_TXBC_USE_DEFKEY and SEC_CFG_RXBC_USE_DEFKEY mean that
> >    broadcast/multicast packets use default key (CAM entries 0~3 for
> >    keyidx 0~3).
> >    For two interfaces case, that'd a problem, so maybe we should use
> >    software encryption/decryption for non-pairwise key.
> >
> >       val8 = SEC_CFG_TX_SEC_ENABLE | SEC_CFG_TXBC_USE_DEFKEY |
> >               SEC_CFG_RX_SEC_ENABLE | SEC_CFG_RXBC_USE_DEFKEY;
> >       val8 |= SEC_CFG_TX_USE_DEFKEY | SEC_CFG_RX_USE_DEFKEY;
> >       rtl8xxxu_write8(priv, REG_SECURITY_CFG, val8);
> >
> > 2. group key (GK) field of security CAM in rtl8xxxu_cam_write()
> >    The group key field of security CAM is BIT(6) which isn't supported
> >    by earlier chips (sorry I have no data about the exact chips).
> >
> 
> BIT 6 is used in the rtl8188eu driver, and not used in the rtl8192cu driver,
> so I assume it's just the older RTL8188CU/RTL8192CU and RTL8723AU that don't
> support it.
> 
> >    If a chip can support group key field, the CAM layout with default key
> >    enabled for two interfaces (STA-1+AP-1) can be
> >
> >    STA-1 <-> AP-10
> >    AP-1  <-> STA-20
> >
> >    CAM  GK   MAC    KEY
> >     0
> >     1   1    00:    AP-1 (GTK)  // should be on entry 0~3 depends on key_idx
> >     2
> >     3
> >     4
> >     5   1    AP-10  AP-10 (GTK)  // can be on any entry after 4
> >     6
> >
> 
> What are the numbers 1, 10, and 20 in "AP-1", "AP-10", "STA-20" ?

Just represent different roles. 

For rtl8xxxu, two interfaces are STA-1 and AP-1, and STA-1 connect to AP-10,
and AP-1 is connected from STA-20.

I don't add pairwise key to above example, because it is simple, just one
entry to a PTK. Instead, I only list group key in the example.

For AP role (AP-1), it can send broadcast packets to many stations, so
use default key.
(but it should not receive a broadcast packet from stations)

For station role (STA-1), it should receive broadcast packets from
specific AP-10, so add an entry with AP-10's MAC address.

> 
> >     (pairwise key can be on any entry after 4)
> >
> >       if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
> >               ctrl |= BIT(6);
> >
> > My perspective are
> > 1. currently default key is enabled, so we should treat CAM entry 0~3 as
> >    special cases. That means rtl8xxxu_get_free_sec_cam() should be modified
> >    as Bitterblue's version.
> >
> > 2. For two interfaces, I guess RTL8188FU can support GK bit, so it'd be worth
> >    to try the method of example 2 above.
> >
> >    If we want earlier chips can support two interfaces, I think we should
> >    disable default key.
> 
> Indeed, the rtl8192cu driver (out of kernel) doesn't set SEC_CFG_TXBC_USE_DEFKEY
> and SEC_CFG_RXBC_USE_DEFKEY when concurrent mode is enabled.
> 
> > Seemingly, it is not possible that using hardware
> >    crypto when operating one interface and then switching to use software>
> crypto when operating two interfaces. That means broadcast/multicast
> >    packets should be by software crypto entirely if we want to support two
> >    interfaces.
> >
> > 3. I think rtw88 has similar problem for two interfaces.
> >
> >>
> >>>
> >>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >>> index ee278f0548e4..f7b35655bec5 100644
> >>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> >>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct
> ieee80211_hw *hw)
> >>>   {
> >>>       struct rtl8xxxu_priv *priv = hw->priv;
> >>>
> >>> -     return find_first_zero_bit(priv->cam_map,
> priv->fops->max_sec_cam_num);
> >>> +     return find_next_zero_bit(priv->cam_map,
> priv->fops->max_sec_cam_num,
> >>> +                               RTL8XXXU_SEC_DEFAULT_KEY_NUM);
> >>>   }
> >>>
> >>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd
> cmd,
> >>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
> enum set_key_cmd cmd,
> >>>
> >>>       switch (cmd) {
> >>>       case SET_KEY:
> >>> +             if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
> >>> +                     retval = rtl8xxxu_get_free_sec_cam(hw);
> >>> +                     if (retval < 0)
> >>> +                             return -EOPNOTSUPP;
> >>>
> >>> -             retval = rtl8xxxu_get_free_sec_cam(hw);
> >>> -             if (retval < 0)
> >>> -                     return -EOPNOTSUPP;
> >>> -
> >>> -             key->hw_key_idx = retval;
> >>> +                     key->hw_key_idx = retval;
> >>> +             } else {
> >>> +                     key->hw_key_idx = key->keyidx;
> >>> +             }
> >>>
> >>>               if (vif->type == NL80211_IFTYPE_AP && !(key->flags &
> IEEE80211_KEY_FLAG_PAIRWISE))
> >>>                       rtlvif->hw_key_idx = key->hw_key_idx;
> >>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>> index 4b05dba22e67..188f4bbe99cd 100644
> >>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> >>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
> >>>   #define RTL8XXXU_BC_MC_MACID        0
> >>>   #define RTL8XXXU_BC_MC_MACID1       1
> >>>   #define RTL8XXXU_MAX_SEC_CAM_NUM    64
> >>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
> >>>
> >>>   struct rtl8xxxu_priv {
> >>>       struct ieee80211_hw *hw;
> >

Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Bitterblue Smith]

On 11/03/2026 04:38, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> On 25/02/2026 04:28, Ping-Ke Shih wrote:
>>> Martin Kaistra <martin.kaistra@linutronix.de> wrote:
>>>> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
>>>>> On 22/12/2023 12:14, Martin Kaistra wrote:
>>>>>> Add a custom function for allocating entries in the sec cam. This allows
>>>>>> us to store multiple keys with the same keyidx.
>>>>>>
>>>>>> The maximum number of sec cam entries for 8188f is 16 according to the
>>>>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
>>>>>> which might support more entries, can set a different number there.
>>>>>>
>>>>>> Set the bssid as mac address for group keys instead of just using the
>>>>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>>>>>> for differentiating them from pairwise keys like in the vendor driver.
>>>>>>
>>>>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>>>>>> broadcast/multicast packets in AP mode.
>>>>>>
>>>>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>>>>>
>>>>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>>>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>>>>>> ---
>>>>>>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
>>>>>>   .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
>>>>>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>>>>>   3 files changed, 44 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>>>> index c5e6d8f7d26bd..62e6318bc0924 100644
>>>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>>>>>   #define DESC_RATE_ID_SHIFT         16
>>>>>>   #define DESC_RATE_ID_MASK          0xf
>>>>>>   #define TXDESC_NAVUSEHDR           BIT(20)
>>>>>> +#define TXDESC_EN_DESC_ID           BIT(21)
>>>>>>   #define TXDESC_SEC_RC4                     0x00400000
>>>>>>   #define TXDESC_SEC_AES                     0x00c00000
>>>>>>   #define TXDESC_PKT_OFFSET_SHIFT            26
>>>>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>>>>>>   #define RTL8XXXU_MAX_MAC_ID_NUM    128
>>>>>>   #define RTL8XXXU_BC_MC_MACID       0
>>>>>>   #define RTL8XXXU_BC_MC_MACID1      1
>>>>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM    64
>>>>>>
>>>>>>   struct rtl8xxxu_priv {
>>>>>>      struct ieee80211_hw *hw;
>>>>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>>>>>>      char led_name[32];
>>>>>>      struct led_classdev led_cdev;
>>>>>>      DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>>>>>> +    DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>>>>>   };
>>>>>>
>>>>>>   struct rtl8xxxu_sta_info {
>>>>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>>>>>>
>>>>>>   struct rtl8xxxu_vif {
>>>>>>      int port_num;
>>>>>> +    u8 hw_key_idx;
>>>>>>   };
>>>>>>
>>>>>>   struct rtl8xxxu_rx_urb {
>>>>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>>>>>>      u16 max_aggr_num;
>>>>>>      u8 supports_ap:1;
>>>>>>      u16 max_macid_num;
>>>>>> +    u16 max_sec_cam_num;
>>>>>>      u32 adda_1t_init;
>>>>>>      u32 adda_1t_path_on;
>>>>>>      u32 adda_2t_path_on_a;
>>>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>>>> index 1e1c8fa194cb8..574a5fe951543 100644
>>>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>>>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>>>>>      .max_aggr_num = 0x0c14,
>>>>>>      .supports_ap = 1,
>>>>>>      .max_macid_num = 16,
>>>>>> +    .max_sec_cam_num = 16,
>>>>>>      .adda_1t_init = 0x03c00014,
>>>>>>      .adda_1t_path_on = 0x03c00014,
>>>>>>      .trxff_boundary = 0x3f7f,
>>>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>>>> index ecf54eb8dba61..7aafae9fe76b8 100644
>>>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>>>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv
>> *priv,
>>>>>>       * This is a bit of a hack - the lower bits of the cipher
>>>>>>       * suite selector happens to match the cipher index in the CAM
>>>>>>       */
>>>>>> -    addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>>>>>> +    addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>>>>>      ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>>>>>> +    if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>>>> +            ctrl |= BIT(6);
>>>>>>
>>>>>>      for (j = 5; j >= 0; j--) {
>>>>>>              switch (j) {
>>>>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>>>      struct rtl8xxxu_tx_urb *tx_urb;
>>>>>>      struct ieee80211_sta *sta = NULL;
>>>>>>      struct ieee80211_vif *vif = tx_info->control.vif;
>>>>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>>>>      struct device *dev = &priv->udev->dev;
>>>>>>      u32 queue, rts_rate;
>>>>>>      u16 pktlen = skb->len;
>>>>>>      int tx_desc_size = priv->fops->tx_desc_size;
>>>>>>      u8 macid;
>>>>>>      int ret;
>>>>>> -    bool ampdu_enable, sgi = false, short_preamble = false;
>>>>>> +    bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>>>>>
>>>>>>      if (skb_headroom(skb) < tx_desc_size) {
>>>>>>              dev_warn(dev,
>>>>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>>>              tx_desc->txdw0 =
>>>>>>                      TXDESC_OWN | TXDESC_FIRST_SEGMENT |
>> TXDESC_LAST_SEGMENT;
>>>>>>      if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>>>>>> -        is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>>>>>> +        is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>>>>>              tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>>>>>> +            bmc = true;
>>>>>> +    }
>>>>>> +
>>>>>>
>>>>>>      tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>>>>>> +    macid = rtl8xxxu_get_macid(priv, sta);
>>>>>>
>>>>>>      if (tx_info->control.hw_key) {
>>>>>>              switch (tx_info->control.hw_key->cipher) {
>>>>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>>>              default:
>>>>>>                      break;
>>>>>>              }
>>>>>> +            if (bmc && rtlvif->hw_key_idx != 0xff) {
>>>>>> +                    tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>>>>>> +                    macid = rtlvif->hw_key_idx;
>>>>>> +            }
>>>>>>      }
>>>>>>
>>>>>>      /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>>>>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>>>>      else
>>>>>>              rts_rate = 0;
>>>>>>
>>>>>> -    macid = rtl8xxxu_get_macid(priv, sta);
>>>>>>      priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi,
>> short_preamble,
>>>>>>                              ampdu_enable, rts_rate, macid);
>>>>>>
>>>>>> @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw
>> *hw,
>>>>>>
>>>>>>      priv->vifs[port_num] = vif;
>>>>>>      rtlvif->port_num = port_num;
>>>>>> +    rtlvif->hw_key_idx = 0xff;
>>>>>>
>>>>>>      rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>>>>>      ether_addr_copy(priv->mac_addr, vif->addr);
>>>>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct
>> ieee80211_hw *hw, u32 rts)
>>>>>>      return 0;
>>>>>>   }
>>>>>>
>>>>>> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>>>>> +{
>>>>>> +    struct rtl8xxxu_priv *priv = hw->priv;
>>>>>> +
>>>>>> +    return find_first_zero_bit(priv->cam_map,
>> priv->fops->max_sec_cam_num);
>>>>>> +}
>>>>>> +
>>>>>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd
>> cmd,
>>>>>>                          struct ieee80211_vif *vif,
>>>>>>                          struct ieee80211_sta *sta,
>>>>>>                          struct ieee80211_key_conf *key)
>>>>>>   {
>>>>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>>>>      struct rtl8xxxu_priv *priv = hw->priv;
>>>>>>      struct device *dev = &priv->udev->dev;
>>>>>>      u8 mac_addr[ETH_ALEN];
>>>>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
>> enum set_key_cmd cmd,
>>>>>>      dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>>>>>              __func__, cmd, key->cipher, key->keyidx);
>>>>>>
>>>>>> -    if (vif->type != NL80211_IFTYPE_STATION)
>>>>>> -            return -EOPNOTSUPP;
>>>>>> -
>>>>>>      if (key->keyidx > 3)
>>>>>>              return -EOPNOTSUPP;
>>>>>>
>>>>>> @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
>> enum set_key_cmd cmd,
>>>>>>              ether_addr_copy(mac_addr, sta->addr);
>>>>>>      } else {
>>>>>>              dev_dbg(dev, "%s: group key\n", __func__);
>>>>>> -            eth_broadcast_addr(mac_addr);
>>>>>> +            ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>>>>>      }
>>>>>>
>>>>>>      val16 = rtl8xxxu_read16(priv, REG_CR);
>>>>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
>> enum set_key_cmd cmd,
>>>>>>
>>>>>>      switch (cmd) {
>>>>>>      case SET_KEY:
>>>>>> -            key->hw_key_idx = key->keyidx;
>>>>>> +
>>>>>> +            retval = rtl8xxxu_get_free_sec_cam(hw);
>>>>>> +            if (retval < 0)
>>>>>> +                    return -EOPNOTSUPP;
>>>>>> +
>>>>>> +            key->hw_key_idx = retval;
>>>>>> +
>>>>>> +            if (vif->type == NL80211_IFTYPE_AP && !(key->flags &
>> IEEE80211_KEY_FLAG_PAIRWISE))
>>>>>> +                    rtlvif->hw_key_idx = key->hw_key_idx;
>>>>>> +
>>>>>>              key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>>>>>              rtl8xxxu_cam_write(priv, key, mac_addr);
>>>>>> +            set_bit(key->hw_key_idx, priv->cam_map);
>>>>>
>>>>> Hi Martin,
>>>>
>>>> Hi Bitterblue,
>>>>
>>>>>
>>>>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
>>>>> when we use iwd. After the WPA2 handshake no more data is transmitted.
>>>>>
>>>>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
>>>>> this patch, when we use iwd, the group key is installed first. It has
>>>>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
>>>>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
>>>>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
>>>>>
>>>>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
>>>>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
>>>>>
>>>>> This patch imitating rtw88 makes the old chips work again with iwd.
>>>>> What do you think?
>>>>
>>>> So you reserve the first 4 entries for group keys and use key->keyidx as
>>>> key->hw_key_idx directly for those, right? Does that work if 2 virtual
>>>> interfaces are used at the same time?
>>>> I will do some tests in the next days, but I suspect this be an issue.
>>>
>>> Ah. I'm not aware that rtl8xxxu can support 2 virtual interfaces, and
>>> only RTL8188FU declares .supports_concurrent = 1, so maybe we can have
>>> special handling for this chip?
>>
>> I would like to enable supports_concurrent for more chips, if I can test
>> this feature.
>>
>>>
>>> I'd share some information about security CAM for reference. Since data is
>>> quite old (10+ years), I can't guarantee all are correct.
>>>
>>> 1. default key switch in rtl8xxxu_set_key()
>>>    SEC_CFG_TXBC_USE_DEFKEY and SEC_CFG_RXBC_USE_DEFKEY mean that
>>>    broadcast/multicast packets use default key (CAM entries 0~3 for
>>>    keyidx 0~3).
>>>    For two interfaces case, that'd a problem, so maybe we should use
>>>    software encryption/decryption for non-pairwise key.
>>>
>>>       val8 = SEC_CFG_TX_SEC_ENABLE | SEC_CFG_TXBC_USE_DEFKEY |
>>>               SEC_CFG_RX_SEC_ENABLE | SEC_CFG_RXBC_USE_DEFKEY;
>>>       val8 |= SEC_CFG_TX_USE_DEFKEY | SEC_CFG_RX_USE_DEFKEY;
>>>       rtl8xxxu_write8(priv, REG_SECURITY_CFG, val8);
>>>
>>> 2. group key (GK) field of security CAM in rtl8xxxu_cam_write()
>>>    The group key field of security CAM is BIT(6) which isn't supported
>>>    by earlier chips (sorry I have no data about the exact chips).
>>>
>>
>> BIT 6 is used in the rtl8188eu driver, and not used in the rtl8192cu driver,
>> so I assume it's just the older RTL8188CU/RTL8192CU and RTL8723AU that don't
>> support it.
>>
>>>    If a chip can support group key field, the CAM layout with default key
>>>    enabled for two interfaces (STA-1+AP-1) can be
>>>
>>>    STA-1 <-> AP-10
>>>    AP-1  <-> STA-20
>>>
>>>    CAM  GK   MAC    KEY
>>>     0
>>>     1   1    00:    AP-1 (GTK)  // should be on entry 0~3 depends on key_idx
>>>     2
>>>     3
>>>     4
>>>     5   1    AP-10  AP-10 (GTK)  // can be on any entry after 4
>>>     6
>>>
>>
>> What are the numbers 1, 10, and 20 in "AP-1", "AP-10", "STA-20" ?
> 
> Just represent different roles. 
> 
> For rtl8xxxu, two interfaces are STA-1 and AP-1, and STA-1 connect to AP-10,
> and AP-1 is connected from STA-20.
> 
> I don't add pairwise key to above example, because it is simple, just one
> entry to a PTK. Instead, I only list group key in the example.
> 
> For AP role (AP-1), it can send broadcast packets to many stations, so
> use default key.
> (but it should not receive a broadcast packet from stations)
> 
> For station role (STA-1), it should receive broadcast packets from
> specific AP-10, so add an entry with AP-10's MAC address.
> 

Oh, now I understand, thank you.

>>
>>>     (pairwise key can be on any entry after 4)
>>>
>>>       if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>>               ctrl |= BIT(6);
>>>
>>> My perspective are
>>> 1. currently default key is enabled, so we should treat CAM entry 0~3 as
>>>    special cases. That means rtl8xxxu_get_free_sec_cam() should be modified
>>>    as Bitterblue's version.
>>>
>>> 2. For two interfaces, I guess RTL8188FU can support GK bit, so it'd be worth
>>>    to try the method of example 2 above.
>>>
>>>    If we want earlier chips can support two interfaces, I think we should
>>>    disable default key.
>>
>> Indeed, the rtl8192cu driver (out of kernel) doesn't set SEC_CFG_TXBC_USE_DEFKEY
>> and SEC_CFG_RXBC_USE_DEFKEY when concurrent mode is enabled.
>>
>>> Seemingly, it is not possible that using hardware
>>>    crypto when operating one interface and then switching to use software>
>> crypto when operating two interfaces. That means broadcast/multicast
>>>    packets should be by software crypto entirely if we want to support two
>>>    interfaces.
>>>
>>> 3. I think rtw88 has similar problem for two interfaces.
>>>
>>>>
>>>>>
>>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>>> b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>>>> index ee278f0548e4..f7b35655bec5 100644
>>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>>>>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct
>> ieee80211_hw *hw)
>>>>>   {
>>>>>       struct rtl8xxxu_priv *priv = hw->priv;
>>>>>
>>>>> -     return find_first_zero_bit(priv->cam_map,
>> priv->fops->max_sec_cam_num);
>>>>> +     return find_next_zero_bit(priv->cam_map,
>> priv->fops->max_sec_cam_num,
>>>>> +                               RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>>>>>   }
>>>>>
>>>>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd
>> cmd,
>>>>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw,
>> enum set_key_cmd cmd,
>>>>>
>>>>>       switch (cmd) {
>>>>>       case SET_KEY:
>>>>> +             if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
>>>>> +                     retval = rtl8xxxu_get_free_sec_cam(hw);
>>>>> +                     if (retval < 0)
>>>>> +                             return -EOPNOTSUPP;
>>>>>
>>>>> -             retval = rtl8xxxu_get_free_sec_cam(hw);
>>>>> -             if (retval < 0)
>>>>> -                     return -EOPNOTSUPP;
>>>>> -
>>>>> -             key->hw_key_idx = retval;
>>>>> +                     key->hw_key_idx = retval;
>>>>> +             } else {
>>>>> +                     key->hw_key_idx = key->keyidx;
>>>>> +             }
>>>>>
>>>>>               if (vif->type == NL80211_IFTYPE_AP && !(key->flags &
>> IEEE80211_KEY_FLAG_PAIRWISE))
>>>>>                       rtlvif->hw_key_idx = key->hw_key_idx;
>>>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>>> index 4b05dba22e67..188f4bbe99cd 100644
>>>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>>>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>>>>>   #define RTL8XXXU_BC_MC_MACID        0
>>>>>   #define RTL8XXXU_BC_MC_MACID1       1
>>>>>   #define RTL8XXXU_MAX_SEC_CAM_NUM    64
>>>>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM 4
>>>>>
>>>>>   struct rtl8xxxu_priv {
>>>>>       struct ieee80211_hw *hw;
>>>
> 

Re: [PATCH v3 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode

[Bitterblue Smith]

On 24/02/2026 17:43, Martin Kaistra wrote:
> Am 21.02.26 um 23:09 schrieb Bitterblue Smith:
>> On 22/12/2023 12:14, Martin Kaistra wrote:
>>> Add a custom function for allocating entries in the sec cam. This allows
>>> us to store multiple keys with the same keyidx.
>>>
>>> The maximum number of sec cam entries for 8188f is 16 according to the
>>> vendor driver. Add the number to rtl8xxxu_fileops, so that other chips
>>> which might support more entries, can set a different number there.
>>>
>>> Set the bssid as mac address for group keys instead of just using the
>>> ethernet broadcast address and use BIT(6) in the sec cam ctrl entry
>>> for differentiating them from pairwise keys like in the vendor driver.
>>>
>>> Add the TXDESC_EN_DESC_ID bit and the hw_key_idx to tx
>>> broadcast/multicast packets in AP mode.
>>>
>>> Finally, allow the usage of rtl8xxxu_set_key() for AP mode.
>>>
>>> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
>>> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
>>> ---
>>>   .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++
>>>   .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
>>>   .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++----
>>>   3 files changed, 44 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> index c5e6d8f7d26bd..62e6318bc0924 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>>> @@ -498,6 +498,7 @@ struct rtl8xxxu_txdesc40 {
>>>   #define DESC_RATE_ID_SHIFT        16
>>>   #define DESC_RATE_ID_MASK        0xf
>>>   #define TXDESC_NAVUSEHDR        BIT(20)
>>> +#define TXDESC_EN_DESC_ID        BIT(21)
>>>   #define TXDESC_SEC_RC4            0x00400000
>>>   #define TXDESC_SEC_AES            0x00c00000
>>>   #define TXDESC_PKT_OFFSET_SHIFT        26
>>> @@ -1775,6 +1776,7 @@ struct rtl8xxxu_cfo_tracking {
>>>   #define RTL8XXXU_MAX_MAC_ID_NUM    128
>>>   #define RTL8XXXU_BC_MC_MACID    0
>>>   #define RTL8XXXU_BC_MC_MACID1    1
>>> +#define RTL8XXXU_MAX_SEC_CAM_NUM    64
>>>     struct rtl8xxxu_priv {
>>>       struct ieee80211_hw *hw;
>>> @@ -1908,6 +1910,7 @@ struct rtl8xxxu_priv {
>>>       char led_name[32];
>>>       struct led_classdev led_cdev;
>>>       DECLARE_BITMAP(mac_id_map, RTL8XXXU_MAX_MAC_ID_NUM);
>>> +    DECLARE_BITMAP(cam_map, RTL8XXXU_MAX_SEC_CAM_NUM);
>>>   };
>>>     struct rtl8xxxu_sta_info {
>>> @@ -1919,6 +1922,7 @@ struct rtl8xxxu_sta_info {
>>>     struct rtl8xxxu_vif {
>>>       int port_num;
>>> +    u8 hw_key_idx;
>>>   };
>>>     struct rtl8xxxu_rx_urb {
>>> @@ -1993,6 +1997,7 @@ struct rtl8xxxu_fileops {
>>>       u16 max_aggr_num;
>>>       u8 supports_ap:1;
>>>       u16 max_macid_num;
>>> +    u16 max_sec_cam_num;
>>>       u32 adda_1t_init;
>>>       u32 adda_1t_path_on;
>>>       u32 adda_2t_path_on_a;
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>> index 1e1c8fa194cb8..574a5fe951543 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>>> @@ -1751,6 +1751,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>>       .max_aggr_num = 0x0c14,
>>>       .supports_ap = 1,
>>>       .max_macid_num = 16,
>>> +    .max_sec_cam_num = 16,
>>>       .adda_1t_init = 0x03c00014,
>>>       .adda_1t_path_on = 0x03c00014,
>>>       .trxff_boundary = 0x3f7f,
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index ecf54eb8dba61..7aafae9fe76b8 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -4559,8 +4559,10 @@ static void rtl8xxxu_cam_write(struct rtl8xxxu_priv *priv,
>>>        * This is a bit of a hack - the lower bits of the cipher
>>>        * suite selector happens to match the cipher index in the CAM
>>>        */
>>> -    addr = key->keyidx << CAM_CMD_KEY_SHIFT;
>>> +    addr = key->hw_key_idx << CAM_CMD_KEY_SHIFT;
>>>       ctrl = (key->cipher & 0x0f) << 2 | key->keyidx | CAM_WRITE_VALID;
>>> +    if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>> +        ctrl |= BIT(6);
>>>         for (j = 5; j >= 0; j--) {
>>>           switch (j) {
>>> @@ -5546,13 +5548,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>       struct rtl8xxxu_tx_urb *tx_urb;
>>>       struct ieee80211_sta *sta = NULL;
>>>       struct ieee80211_vif *vif = tx_info->control.vif;
>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>       struct device *dev = &priv->udev->dev;
>>>       u32 queue, rts_rate;
>>>       u16 pktlen = skb->len;
>>>       int tx_desc_size = priv->fops->tx_desc_size;
>>>       u8 macid;
>>>       int ret;
>>> -    bool ampdu_enable, sgi = false, short_preamble = false;
>>> +    bool ampdu_enable, sgi = false, short_preamble = false, bmc = false;
>>>         if (skb_headroom(skb) < tx_desc_size) {
>>>           dev_warn(dev,
>>> @@ -5594,10 +5597,14 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>           tx_desc->txdw0 =
>>>               TXDESC_OWN | TXDESC_FIRST_SEGMENT | TXDESC_LAST_SEGMENT;
>>>       if (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ||
>>> -        is_broadcast_ether_addr(ieee80211_get_DA(hdr)))
>>> +        is_broadcast_ether_addr(ieee80211_get_DA(hdr))) {
>>>           tx_desc->txdw0 |= TXDESC_BROADMULTICAST;
>>> +        bmc = true;
>>> +    }
>>> +
>>>         tx_desc->txdw1 = cpu_to_le32(queue << TXDESC_QUEUE_SHIFT);
>>> +    macid = rtl8xxxu_get_macid(priv, sta);
>>>         if (tx_info->control.hw_key) {
>>>           switch (tx_info->control.hw_key->cipher) {
>>> @@ -5612,6 +5619,10 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>           default:
>>>               break;
>>>           }
>>> +        if (bmc && rtlvif->hw_key_idx != 0xff) {
>>> +            tx_desc->txdw1 |= TXDESC_EN_DESC_ID;
>>> +            macid = rtlvif->hw_key_idx;
>>> +        }
>>>       }
>>>         /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */
>>> @@ -5655,7 +5666,6 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>>>       else
>>>           rts_rate = 0;
>>>   -    macid = rtl8xxxu_get_macid(priv, sta);
>>>       priv->fops->fill_txdesc(hw, hdr, tx_info, tx_desc, sgi, short_preamble,
>>>                   ampdu_enable, rts_rate, macid);
>>>   @@ -6667,6 +6677,7 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>>>         priv->vifs[port_num] = vif;
>>>       rtlvif->port_num = port_num;
>>> +    rtlvif->hw_key_idx = 0xff;
>>>         rtl8xxxu_set_linktype(priv, vif->type, port_num);
>>>       ether_addr_copy(priv->mac_addr, vif->addr);
>>> @@ -6843,11 +6854,19 @@ static int rtl8xxxu_set_rts_threshold(struct ieee80211_hw *hw, u32 rts)
>>>       return 0;
>>>   }
>>>   +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>> +{
>>> +    struct rtl8xxxu_priv *priv = hw->priv;
>>> +
>>> +    return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>>> +}
>>> +
>>>   static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>                   struct ieee80211_vif *vif,
>>>                   struct ieee80211_sta *sta,
>>>                   struct ieee80211_key_conf *key)
>>>   {
>>> +    struct rtl8xxxu_vif *rtlvif = (struct rtl8xxxu_vif *)vif->drv_priv;
>>>       struct rtl8xxxu_priv *priv = hw->priv;
>>>       struct device *dev = &priv->udev->dev;
>>>       u8 mac_addr[ETH_ALEN];
>>> @@ -6859,9 +6878,6 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>       dev_dbg(dev, "%s: cmd %02x, cipher %08x, index %i\n",
>>>           __func__, cmd, key->cipher, key->keyidx);
>>>   -    if (vif->type != NL80211_IFTYPE_STATION)
>>> -        return -EOPNOTSUPP;
>>> -
>>>       if (key->keyidx > 3)
>>>           return -EOPNOTSUPP;
>>>   @@ -6885,7 +6901,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>           ether_addr_copy(mac_addr, sta->addr);
>>>       } else {
>>>           dev_dbg(dev, "%s: group key\n", __func__);
>>> -        eth_broadcast_addr(mac_addr);
>>> +        ether_addr_copy(mac_addr, vif->bss_conf.bssid);
>>>       }
>>>         val16 = rtl8xxxu_read16(priv, REG_CR);
>>> @@ -6899,16 +6915,28 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>>         switch (cmd) {
>>>       case SET_KEY:
>>> -        key->hw_key_idx = key->keyidx;
>>> +
>>> +        retval = rtl8xxxu_get_free_sec_cam(hw);
>>> +        if (retval < 0)
>>> +            return -EOPNOTSUPP;
>>> +
>>> +        key->hw_key_idx = retval;
>>> +
>>> +        if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>> +            rtlvif->hw_key_idx = key->hw_key_idx;
>>> +
>>>           key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>>>           rtl8xxxu_cam_write(priv, key, mac_addr);
>>> +        set_bit(key->hw_key_idx, priv->cam_map);
>>
>> Hi Martin,
> 
> Hi Bitterblue,
> 
>>
>> It turns out RTL8188CUS and RTL8192CU don't like this patch, specifically
>> when we use iwd. After the WPA2 handshake no more data is transmitted.
>>
>> Before this patch, key->hw_key_idx was the same as key->keyidx. After
>> this patch, when we use iwd, the group key is installed first. It has
>> key->keyidx = 1, but it gets key->hw_key_idx = 0. The pairwise key is
>> installed second. It has key->keyidx = 0, but it gets key->hw_key_idx = 1.
>> Both keyidx and hw_key_idx are passed to the chip in rtl8xxxu_cam_write().
>>
>> It's fine with wpa_supplicant. wpa_supplicant installs the pairwise key
>> first, with key->keyidx = 0, then the group key, with key->keyidx = 1.
>>
>> This patch imitating rtw88 makes the old chips work again with iwd.
>> What do you think?
> 
> So you reserve the first 4 entries for group keys and use key->keyidx as key->hw_key_idx directly for those, right? Does that work if 2 virtual interfaces are used at the same time?

Yes. I don't know if that works with 2 interfaces. How can I test?
I never tried to use 2 interfaces at the same time.

> I will do some tests in the next days, but I suspect this be an issue.
> 
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> index ee278f0548e4..f7b35655bec5 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
>> @@ -6942,7 +6942,8 @@ static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
>>   {
>>       struct rtl8xxxu_priv *priv = hw->priv;
>>   -    return find_first_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num);
>> +    return find_next_zero_bit(priv->cam_map, priv->fops->max_sec_cam_num,
>> +                  RTL8XXXU_SEC_DEFAULT_KEY_NUM);
>>   }
>>     static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> @@ -6999,12 +7000,15 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>         switch (cmd) {
>>       case SET_KEY:
>> +        if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
>> +            retval = rtl8xxxu_get_free_sec_cam(hw);
>> +            if (retval < 0)
>> +                return -EOPNOTSUPP;
>>   -        retval = rtl8xxxu_get_free_sec_cam(hw);
>> -        if (retval < 0)
>> -            return -EOPNOTSUPP;
>> -
>> -        key->hw_key_idx = retval;
>> +            key->hw_key_idx = retval;
>> +        } else {
>> +            key->hw_key_idx = key->keyidx;
>> +        }
>>             if (vif->type == NL80211_IFTYPE_AP && !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
>>               rtlvif->hw_key_idx = key->hw_key_idx;
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index 4b05dba22e67..188f4bbe99cd 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -1788,6 +1788,7 @@ struct rtl8xxxu_cfo_tracking {
>>   #define RTL8XXXU_BC_MC_MACID    0
>>   #define RTL8XXXU_BC_MC_MACID1    1
>>   #define RTL8XXXU_MAX_SEC_CAM_NUM    64
>> +#define RTL8XXXU_SEC_DEFAULT_KEY_NUM    4
>>     struct rtl8xxxu_priv {
>>       struct ieee80211_hw *hw;
> 

[PATCH v3 20/21] wifi: rtl8xxxu: make supporting AP mode only on port 0 transparent

[Martin Kaistra]

When the driver is used for concurrent mode, both virtual interfaces can
be set to station or AP mode, though only one can be in AP mode at the
same time.

In order to keep the code simple, use only hw port 0 for AP mode. When
an interface is added in AP mode which would be assigned to port 1, use
a switch_port function to transparently swap the mapping between virtual
interface and hw port.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 91 ++++++++++++++++++-
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 7aafae9fe76b8..0178b441dce53 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6624,6 +6624,91 @@ static int rtl8xxxu_submit_int_urb(struct ieee80211_hw *hw)
 	return ret;
 }
 
+static void rtl8xxxu_switch_ports(struct rtl8xxxu_priv *priv)
+{
+	u8 macid[ETH_ALEN], bssid[ETH_ALEN], macid_1[ETH_ALEN], bssid_1[ETH_ALEN];
+	u8 msr, bcn_ctrl, bcn_ctrl_1, atimwnd[2], atimwnd_1[2];
+	struct rtl8xxxu_vif *rtlvif;
+	struct ieee80211_vif *vif;
+	u8 tsftr[8], tsftr_1[8];
+	int i;
+
+	msr = rtl8xxxu_read8(priv, REG_MSR);
+	bcn_ctrl = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
+	bcn_ctrl_1 = rtl8xxxu_read8(priv, REG_BEACON_CTRL_1);
+
+	for (i = 0; i < ARRAY_SIZE(atimwnd); i++)
+		atimwnd[i] = rtl8xxxu_read8(priv, REG_ATIMWND + i);
+	for (i = 0; i < ARRAY_SIZE(atimwnd_1); i++)
+		atimwnd_1[i] = rtl8xxxu_read8(priv, REG_ATIMWND_1 + i);
+
+	for (i = 0; i < ARRAY_SIZE(tsftr); i++)
+		tsftr[i] = rtl8xxxu_read8(priv, REG_TSFTR + i);
+	for (i = 0; i < ARRAY_SIZE(tsftr); i++)
+		tsftr_1[i] = rtl8xxxu_read8(priv, REG_TSFTR1 + i);
+
+	for (i = 0; i < ARRAY_SIZE(macid); i++)
+		macid[i] = rtl8xxxu_read8(priv, REG_MACID + i);
+
+	for (i = 0; i < ARRAY_SIZE(bssid); i++)
+		bssid[i] = rtl8xxxu_read8(priv, REG_BSSID + i);
+
+	for (i = 0; i < ARRAY_SIZE(macid_1); i++)
+		macid_1[i] = rtl8xxxu_read8(priv, REG_MACID1 + i);
+
+	for (i = 0; i < ARRAY_SIZE(bssid_1); i++)
+		bssid_1[i] = rtl8xxxu_read8(priv, REG_BSSID1 + i);
+
+	/* disable bcn function, disable update TSF */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL, (bcn_ctrl &
+			(~BEACON_FUNCTION_ENABLE)) | BEACON_DISABLE_TSF_UPDATE);
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL_1, (bcn_ctrl_1 &
+			(~BEACON_FUNCTION_ENABLE)) | BEACON_DISABLE_TSF_UPDATE);
+
+	/* switch msr */
+	msr = (msr & 0xf0) | ((msr & 0x03) << 2) | ((msr & 0x0c) >> 2);
+	rtl8xxxu_write8(priv, REG_MSR, msr);
+
+	/* write port0 */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL, bcn_ctrl_1 & ~BEACON_FUNCTION_ENABLE);
+	for (i = 0; i < ARRAY_SIZE(atimwnd_1); i++)
+		rtl8xxxu_write8(priv, REG_ATIMWND + i, atimwnd_1[i]);
+	for (i = 0; i < ARRAY_SIZE(tsftr_1); i++)
+		rtl8xxxu_write8(priv, REG_TSFTR + i, tsftr_1[i]);
+	for (i = 0; i < ARRAY_SIZE(macid_1); i++)
+		rtl8xxxu_write8(priv, REG_MACID + i, macid_1[i]);
+	for (i = 0; i < ARRAY_SIZE(bssid_1); i++)
+		rtl8xxxu_write8(priv, REG_BSSID + i, bssid_1[i]);
+
+	/* write port1 */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL_1, bcn_ctrl & ~BEACON_FUNCTION_ENABLE);
+	for (i = 0; i < ARRAY_SIZE(atimwnd); i++)
+		rtl8xxxu_write8(priv, REG_ATIMWND_1 + i, atimwnd[i]);
+	for (i = 0; i < ARRAY_SIZE(tsftr); i++)
+		rtl8xxxu_write8(priv, REG_TSFTR1 + i, tsftr[i]);
+	for (i = 0; i < ARRAY_SIZE(macid); i++)
+		rtl8xxxu_write8(priv, REG_MACID1 + i, macid[i]);
+	for (i = 0; i < ARRAY_SIZE(bssid); i++)
+		rtl8xxxu_write8(priv, REG_BSSID1 + i, bssid[i]);
+
+	/* write bcn ctl */
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL, bcn_ctrl_1);
+	rtl8xxxu_write8(priv, REG_BEACON_CTRL_1, bcn_ctrl);
+
+	vif = priv->vifs[0];
+	priv->vifs[0] = priv->vifs[1];
+	priv->vifs[1] = vif;
+
+	/* priv->vifs[0] is NULL here, based on how this function is currently
+	 * called from rtl8xxxu_add_interface().
+	 * When this function will be used in the future for a different
+	 * scenario, please check whether vifs[0] or vifs[1] can be NULL and if
+	 * necessary add code to set port_num = 1.
+	 */
+	rtlvif = (struct rtl8xxxu_vif *)priv->vifs[1]->drv_priv;
+	rtlvif->port_num = 1;
+}
+
 static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 				  struct ieee80211_vif *vif)
 {
@@ -6651,8 +6736,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
 		}
 		break;
 	case NL80211_IFTYPE_AP:
-		if (port_num == 1)
-			return -EOPNOTSUPP;
+		if (port_num == 1) {
+			rtl8xxxu_switch_ports(priv);
+			port_num = 0;
+		}
 
 		rtl8xxxu_write8(priv, REG_BEACON_CTRL,
 				BEACON_DISABLE_TSF_UPDATE | BEACON_CTRL_MBSSID);
-- 
2.39.2

[PATCH v3 21/21] wifi: rtl8xxxu: declare concurrent mode support for 8188f

[Martin Kaistra]

Everything is in place now for concurrent mode, we can tell the system
that we support it.
We will allow a maximum of 2 virtual interfaces, one of them can be in
AP mode.

Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>
Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  1 +
 .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  1 +
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 62e6318bc0924..803c76b3209c4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1992,6 +1992,7 @@ struct rtl8xxxu_fileops {
 	u8 init_reg_rxfltmap:1;
 	u8 init_reg_pkt_life_time:1;
 	u8 init_reg_hmtfr:1;
+	u8 supports_concurrent:1;
 	u8 ampdu_max_time;
 	u8 ustime_tsf_edca;
 	u16 max_aggr_num;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
index 574a5fe951543..464216d007ce8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
@@ -1752,6 +1752,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
 	.supports_ap = 1,
 	.max_macid_num = 16,
 	.max_sec_cam_num = 16,
+	.supports_concurrent = 1,
 	.adda_1t_init = 0x03c00014,
 	.adda_1t_path_on = 0x03c00014,
 	.trxff_boundary = 0x3f7f,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 0178b441dce53..5fddbd6594a24 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -7665,6 +7665,20 @@ static void rtl8xxxu_deinit_led(struct rtl8xxxu_priv *priv)
 	led_classdev_unregister(led);
 }
 
+struct ieee80211_iface_limit rtl8xxxu_limits[] = {
+	{ .max = 2, .types = BIT(NL80211_IFTYPE_STATION), },
+	{ .max = 1, .types = BIT(NL80211_IFTYPE_AP), },
+};
+
+struct ieee80211_iface_combination rtl8xxxu_combinations[] = {
+	{
+		.limits = rtl8xxxu_limits,
+		.n_limits = ARRAY_SIZE(rtl8xxxu_limits),
+		.max_interfaces = 2,
+		.num_different_channels = 1,
+	},
+};
+
 static int rtl8xxxu_probe(struct usb_interface *interface,
 			  const struct usb_device_id *id)
 {
@@ -7811,6 +7825,11 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 		hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP);
 	hw->queues = 4;
 
+	if (priv->fops->supports_concurrent) {
+		hw->wiphy->iface_combinations = rtl8xxxu_combinations;
+		hw->wiphy->n_iface_combinations = ARRAY_SIZE(rtl8xxxu_combinations);
+	}
+
 	sband = &rtl8xxxu_supported_band;
 	sband->ht_cap.ht_supported = true;
 	sband->ht_cap.ampdu_factor = IEEE80211_HT_MAX_AMPDU_64K;
-- 
2.39.2

[PATCH v3 21/21] wifi: rtl8xxxu: declare concurrent mode support for 8188f

[Zenm Chen]

Hi,

The rtl8xxxu driver can't establish the connection after applying this series
of the patches. Please help me, thanks!

OS: Arch Linux (kernel version: 6.6.10-arch1-1)
devices tested: MERCUSYS MW300UM (RTL8192EU)
                MERCURY MW310UH  (RTL8192FU)

dmesg:
[  161.984182] usb 1-4: new high-speed USB device number 3 using ehci-pci
[  162.131919] usb 1-4: New USB device found, idVendor=0bda, idProduct=818b, bcdDevice= 2.00
[  162.131930] usb 1-4: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[  162.131935] usb 1-4: Product: 802.11n NIC 
[  162.131938] usb 1-4: Manufacturer: Realtek 
[  162.131941] usb 1-4: SerialNumber: 00e04c000001
[  162.355271] usb 1-4: RTL8192EU rev B (SMIC) romver 0, 2T2R, TX queues 3, WiFi=1, BT=0, GPS=0, HI PA=0
[  162.355288] usb 1-4: RTL8192EU MAC: 00:eb:d8:76:23:92
[  162.355294] usb 1-4: rtl8xxxu: Loading firmware rtlwifi/rtl8192eu_nic.bin
[  162.356370] usb 1-4: Firmware revision 35.7 (signature 0x92e1)
[  163.453122] usb 1-4: Enabling HT_20_40 on the 2.4GHz band
[  163.455327] usbcore: registered new interface driver rtl8xxxu
[  163.475365] rtl8xxxu 1-4:1.0 wlp0s18f2u4: renamed from wlan0
[  186.034061] wlp0s18f2u4: authenticate with bc:e6:3f:5f:9c:cd
[  186.045683] wlp0s18f2u4: send auth to bc:e6:3f:5f:9c:cd (try 1/3)
[  186.050060] wlp0s18f2u4: authenticated
[  186.054184] wlp0s18f2u4: associate with bc:e6:3f:5f:9c:cd (try 1/3)
[  186.077089] wlp0s18f2u4: RX AssocResp from bc:e6:3f:5f:9c:cd (capab=0x411 status=0 aid=1)
[  186.078853] usb 1-4: rtl8xxxu_bss_info_changed: HT supported
[  186.081741] wlp0s18f2u4: associated
[  186.218451] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd
[  195.553755] warning: `ThreadPoolForeg' uses wireless extensions which will stop working for Wi-Fi 7 hardware; use nl80211
[  231.035382] wlp0s18f2u4: deauthenticating from bc:e6:3f:5f:9c:cd by local choice (Reason: 3=DEAUTH_LEAVING)
[  233.770396] wlp0s18f2u4: authenticate with bc:e6:3f:5f:9c:cd
[  233.782081] wlp0s18f2u4: send auth to bc:e6:3f:5f:9c:cd (try 1/3)
[  233.784161] wlp0s18f2u4: authenticated
[  233.787506] wlp0s18f2u4: associate with bc:e6:3f:5f:9c:cd (try 1/3)
[  233.803705] wlp0s18f2u4: RX AssocResp from bc:e6:3f:5f:9c:cd (capab=0x411 status=0 aid=1)
[  233.805501] usb 1-4: rtl8xxxu_bss_info_changed: HT supported
[  233.808383] wlp0s18f2u4: associated
[  233.836597] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd
[  279.032159] wlp0s18f2u4: deauthenticating from bc:e6:3f:5f:9c:cd by local choice (Reason: 3=DEAUTH_LEAVING)
[  280.657559] wlp0s18f2u4: authenticate with bc:e6:3f:5f:9c:cd
[  280.669301] wlp0s18f2u4: send auth to bc:e6:3f:5f:9c:cd (try 1/3)
[  280.671303] wlp0s18f2u4: authenticated
[  280.674082] wlp0s18f2u4: associate with bc:e6:3f:5f:9c:cd (try 1/3)
[  280.688088] wlp0s18f2u4: RX AssocResp from bc:e6:3f:5f:9c:cd (capab=0x411 status=0 aid=1)
[  280.689881] usb 1-4: rtl8xxxu_bss_info_changed: HT supported
[  280.692761] wlp0s18f2u4: associated
[  280.743720] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd
[  326.035411] wlp0s18f2u4: deauthenticating from bc:e6:3f:5f:9c:cd by local choice (Reason: 3=DEAUTH_LEAVING)
[  328.697827] wlp0s18f2u4: authenticate with bc:e6:3f:5f:9c:cd
[  328.709441] wlp0s18f2u4: send auth to bc:e6:3f:5f:9c:cd (try 1/3)
[  328.716384] wlp0s18f2u4: authenticated
[  328.720811] wlp0s18f2u4: associate with bc:e6:3f:5f:9c:cd (try 1/3)
[  328.739267] wlp0s18f2u4: RX AssocResp from bc:e6:3f:5f:9c:cd (capab=0x411 status=0 aid=1)
[  328.741111] usb 1-4: rtl8xxxu_bss_info_changed: HT supported
[  328.744014] wlp0s18f2u4: associated
[  328.765823] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd
[  374.035605] wlp0s18f2u4: deauthenticating from bc:e6:3f:5f:9c:cd by local choice (Reason: 3=DEAUTH_LEAVING)
[  675.415603] wlp0s18f2u4: authenticate with bc:e6:3f:5f:9c:cd
[  675.427238] wlp0s18f2u4: send auth to bc:e6:3f:5f:9c:cd (try 1/3)
[  675.434465] wlp0s18f2u4: authenticated
[  675.437328] wlp0s18f2u4: associate with bc:e6:3f:5f:9c:cd (try 1/3)
[  675.451227] wlp0s18f2u4: RX AssocResp from bc:e6:3f:5f:9c:cd (capab=0x411 status=0 aid=1)
[  675.453061] usb 1-4: rtl8xxxu_bss_info_changed: HT supported
[  675.455938] wlp0s18f2u4: associated
[  675.508092] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd
[  721.037516] wlp0s18f2u4: deauthenticating from bc:e6:3f:5f:9c:cd by local choice (Reason: 3=DEAUTH_LEAVING)
[  723.744620] wlp0s18f2u4: authenticate with bc:e6:3f:5f:9c:cd
[  723.756204] wlp0s18f2u4: send auth to bc:e6:3f:5f:9c:cd (try 1/3)
[  723.758400] wlp0s18f2u4: authenticated
[  723.760735] wlp0s18f2u4: associate with bc:e6:3f:5f:9c:cd (try 1/3)
[  723.774653] wlp0s18f2u4: RX AssocResp from bc:e6:3f:5f:9c:cd (capab=0x411 status=0 aid=1)
[  723.776369] usb 1-4: rtl8xxxu_bss_info_changed: HT supported
[  723.779249] wlp0s18f2u4: associated
[  723.847090] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd

RE: [PATCH v3 21/21] wifi: rtl8xxxu: declare concurrent mode support for 8188f

[Ping-Ke Shih]

> -----Original Message-----
> From: Zenm Chen <zenmchen@gmail.com>
> Sent: Friday, January 12, 2024 12:36 AM
> To: martin.kaistra@linutronix.de
> Cc: Jes.Sorensen@gmail.com; bigeasy@linutronix.de; kvalo@kernel.org; linux-wireless@vger.kernel.org;
> Ping-Ke Shih <pkshih@realtek.com>; rtl8821cerfe2@gmail.com
> Subject: [PATCH v3 21/21] wifi: rtl8xxxu: declare concurrent mode support for 8188f
> 
> Hi,
> 
> The rtl8xxxu driver can't establish the connection after applying this series
> of the patches. Please help me, thanks!
> 
> OS: Arch Linux (kernel version: 6.6.10-arch1-1)
> devices tested: MERCUSYS MW300UM (RTL8192EU)
>                 MERCURY MW310UH  (RTL8192FU)
> 

[...]

> [  186.081741] wlp0s18f2u4: associated
> [  186.218451] wlp0s18f2u4: Limiting TX power to 30 (30 - 0) dBm as advertised by bc:e6:3f:5f:9c:cd
> [  195.553755] warning: `ThreadPoolForeg' uses wireless extensions which will stop working for Wi-Fi 7
> hardware; use nl80211
> [  231.035382] wlp0s18f2u4: deauthenticating from bc:e6:3f:5f:9c:cd by local choice (Reason:
> 3=DEAUTH_LEAVING)

Locally trying to disconnect after 30 seconds. Can you try non-secure
connection and specify static IP to see if it can work? 

I reviewed the patchset again, no obvious change if you are using a vif with
station mode only. The only one I'm not very sure is a commit related to
security CAM:
    Fixes: b837f78fbffa ("wifi: rtl8xxxu: add hw crypto support for AP mode")

Since each patch can be seen as individual one, could you help to bisect
the cause? 

Ping-Ke

[PATCH v3 21/21] wifi: rtl8xxxu: declare concurrent mode support for 8188f

[Zenm Chen]

Hi Ping-Ke,

> Locally trying to disconnect after 30 seconds. Can you try non-secure
> connection and specify static IP to see if it can work? 

I tried using a non-secure connection and keeping the DHCP enabled and it 
can work.

> 
> I reviewed the patchset again, no obvious change if you are using a vif with
> station mode only. The only one I'm not very sure is a commit related to
> security CAM:
>     Fixes: b837f78fbffa ("wifi: rtl8xxxu: add hw crypto support for AP mode")

I tried applying all the patches except this one, it works as usual now. I
think we find the one which caused this issue, thank you very much! :)

> 
> Since each patch can be seen as individual one, could you help to bisect
> the cause? 
> 
> Ping-Ke

RE: [PATCH v3 00/21] wifi: rtl8xxxu: Add concurrent mode for 8188f

[Ping-Ke Shih]

On Fri, Dec 22, 2023 at 6:15 PM Martin Kaistra
<martin.kaistra@linutronix.de> wrote:
>
> This series adds the possibility to use two virtual interfaces on the
> same channel. Supported combinations are STA+STA and STA+AP. The
> conversion of the driver to support multiple interfaces is split into
> individual patches to hopefully make it easier to understand what is
> going on.
>
> Thanks,
>   Martin
>
> changes from v2->v3:
> - collect reviewed-by tags
> - fix checkpatch warning for comment

Thanks for this great work! I have added my reviewed-by to all patches.