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

[Ping-Ke Shih <pkshih@realtek.com>]

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