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

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

Hi Martin,

> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Friday, December 22, 2023 12:44 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH v2 19/21] wifi: rtl8xxxu: add hw crypto support for AP mode
> 

[...]

Zenm reported [1] his RTL8192EU and RTL8192FU don't work in station mode,
and cause is this patch. Please try if you can reproduce the symptom, and
apply my suggestion to see if help.

[1] https://lore.kernel.org/linux-wireless/20240112045104.12282-1-zenmchen@gmail.com/T/#me0940f522249becf49f25bc281f1992c523673f6

> 
> +static int rtl8xxxu_get_free_sec_cam(struct ieee80211_hw *hw)
> +{
> +       struct rtl8xxxu_priv *priv = hw->priv;

We need to reserve entries 0~3 for keys that aren't pairwise key. 

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

[...]

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

if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE)
	key->hw_key_idx = retval;
else
	key->hw_key_idx = key->keyidx;

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

Shouldn't swap these two statements? I missed that during reviewing.


>                 retval = 0;
>                 break;
>         default:
> --
> 2.39.2