| Message ID | 20231218143645.433356-1-martin.kaistra@linutronix.de |
|---|---|
| Headers | show |
| Series | wifi: rtl8xxxu: Add concurrent mode for 8188f | expand |
On 18/12/2023 16:36, Martin Kaistra 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 > Nice work! I'm glad to see this driver get more features.
On 18/12/2023 16:36, 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. The other chips seem to support more: RTL8723BU, RTL8192EU, RTL8192FU: 64 RTL8188EU: 32 RTL8710BU: 32 (even though it supports only 16 macid) I'm not sure about the RTL8723AU and RTL8192CU. Those drivers are older and different. Perhaps this should be a member of struct rtl8xxxu_fileops, like max_macid_num? > > 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> > --- > .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 4 ++ > .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++---- > 2 files changed, 42 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h > index 889ef7217f142..a0222d2666000 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 > @@ -1774,6 +1775,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_MAX_SEC_CAM_NUM 16 > > struct rtl8xxxu_priv { > struct ieee80211_hw *hw; > @@ -1907,6 +1909,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 { > @@ -1918,6 +1921,7 @@ struct rtl8xxxu_sta_info { > > struct rtl8xxxu_vif { > int port_num; > + u8 hw_key_idx; > }; > > struct rtl8xxxu_rx_urb { > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > index c2a577ebd061e..88730791091a7 100644 > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > @@ -4558,8 +4558,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) { > @@ -5545,13 +5547,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, > @@ -5593,10 +5596,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) { > @@ -5611,6 +5618,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; > + } cpu_to_le32(TXDESC_EN_DESC_ID) The vendor drivers only do this for data frames. Is it the same here? > } > > /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */ > @@ -5654,7 +5665,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); > > @@ -6656,6 +6666,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); > @@ -6832,11 +6843,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, RTL8XXXU_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]; > @@ -6848,9 +6867,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; > > @@ -6874,7 +6890,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); > @@ -6888,16 +6904,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:
> -----Original Message----- > From: Martin Kaistra <martin.kaistra@linutronix.de> > Sent: Monday, December 18, 2023 10:37 PM > 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 12/20] wifi: rtl8xxxu: support multiple interfaces in watchdog_callback() > > Check first whether priv->vifs[0] exists and is of type STATION, otherwise > go to priv->vifs[1]. > > Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> > --- > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > index f54d7b1647792..0d171f61ac8e8 100644 > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > @@ -7190,7 +7190,9 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work) This patch is fine. But, we need to extend this function to support multiple sta to rtl8xxxu_refresh_rate_mask(sta) at least. Ping-Ke
> -----Original Message----- > From: Martin Kaistra <martin.kaistra@linutronix.de> > Sent: Monday, December 18, 2023 10:37 PM > 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 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid() > > As sta_info->macid does not get set in station mode, we can simplify > this function by directly returning 0 if sta itself or sta_info is not > set. > > Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> > --- > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > index 3851fc90339e0..ad76cddef81b2 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; I checked where driver assign macid, and only if (vif->type == NL80211_IFTYPE_AP) { sta_info->macid = rtl8xxxu_acquire_macid(priv); That means STA mode can be macid == 0 always, right? This will be a problem. At least TX rate will be incorrect. Ping-Ke
> -----Original Message----- > From: Bitterblue Smith <rtl8821cerfe2@gmail.com> > Sent: Wednesday, December 20, 2023 6:03 AM > To: Martin Kaistra <martin.kaistra@linutronix.de>; linux-wireless@vger.kernel.org > Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih > <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Subject: Re: [PATCH 00/20] wifi: rtl8xxxu: Add concurrent mode for 8188f > > On 18/12/2023 16:36, Martin Kaistra 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 > > > > Nice work! I'm glad to see this driver get more features. Agree! It seems this driver can possible to support P2P as well. I have reviewed this patchset, and please feel free to add my reviewed-by Reviewed-by: Ping-Ke Shih <pkshih@realtek.com> if I have no comment on patches and you don't change them by v2. Ping-Ke
Am 19.12.23 um 23:29 schrieb Bitterblue Smith: > On 18/12/2023 16:36, 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. > > The other chips seem to support more: > > RTL8723BU, RTL8192EU, RTL8192FU: 64 > RTL8188EU: 32 > RTL8710BU: 32 (even though it supports only 16 macid) > > I'm not sure about the RTL8723AU and RTL8192CU. Those drivers are > older and different. > > Perhaps this should be a member of struct rtl8xxxu_fileops, like > max_macid_num? Sounds good to me, I will implement this for v2. > >> >> 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> >> --- >> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 4 ++ >> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 48 +++++++++++++++---- >> 2 files changed, 42 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h >> index 889ef7217f142..a0222d2666000 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 >> @@ -1774,6 +1775,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_MAX_SEC_CAM_NUM 16 >> >> struct rtl8xxxu_priv { >> struct ieee80211_hw *hw; >> @@ -1907,6 +1909,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 { >> @@ -1918,6 +1921,7 @@ struct rtl8xxxu_sta_info { >> >> struct rtl8xxxu_vif { >> int port_num; >> + u8 hw_key_idx; >> }; >> >> struct rtl8xxxu_rx_urb { >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> index c2a577ebd061e..88730791091a7 100644 >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> @@ -4558,8 +4558,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) { >> @@ -5545,13 +5547,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, >> @@ -5593,10 +5596,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) { >> @@ -5611,6 +5618,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; >> + } > > cpu_to_le32(TXDESC_EN_DESC_ID) > > The vendor drivers only do this for data frames. Is it the same here? > >> } >> >> /* (tx_info->flags & IEEE80211_TX_CTL_AMPDU) && */ >> @@ -5654,7 +5665,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); >> >> @@ -6656,6 +6666,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); >> @@ -6832,11 +6843,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, RTL8XXXU_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]; >> @@ -6848,9 +6867,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; >> >> @@ -6874,7 +6890,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); >> @@ -6888,16 +6904,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: >
Am 20.12.23 um 07:14 schrieb Ping-Ke Shih: > > >> -----Original Message----- >> From: Martin Kaistra <martin.kaistra@linutronix.de> >> Sent: Monday, December 18, 2023 10:37 PM >> 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 16/20] wifi: rtl8xxxu: support multiple interfaces in get_macid() >> >> As sta_info->macid does not get set in station mode, we can simplify >> this function by directly returning 0 if sta itself or sta_info is not >> set. >> >> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> >> --- >> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> index 3851fc90339e0..ad76cddef81b2 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; > > I checked where driver assign macid, and only > > if (vif->type == NL80211_IFTYPE_AP) { > sta_info->macid = rtl8xxxu_acquire_macid(priv); > > That means STA mode can be macid == 0 always, right? > This will be a problem. At least TX rate will be incorrect. Yes, currently macid for STA mode is always 0. Would it be enough to set macid in STA mode to either 0 or 1 depending on port_num? > > Ping-Ke >
On Wed, 2023-12-20 at 17:40 +0100, Martin Kaistra wrote: > > Am 20.12.23 um 07:14 schrieb Ping-Ke Shih: > > > > > > I checked where driver assign macid, and only > > > > if (vif->type == NL80211_IFTYPE_AP) { > > sta_info->macid = rtl8xxxu_acquire_macid(priv); > > > > That means STA mode can be macid == 0 always, right? > > This will be a problem. At least TX rate will be incorrect. > > Yes, currently macid for STA mode is always 0. Would it be enough to set macid > in STA mode to either 0 or 1 depending on port_num? > I am not very sure if macid 0 plays a special role, but others (macid >= 1) can be dynamically assigned to each stations. I think we can reserve macid 0 and 1 for port 0 and 1 respectively, and dynamically assign macid 2 or larger to TDLS or AP mode peer, like macid port num STA mode AP mode ------ ---------- ------------- --------------------------- 0 0 to/from AP broadcast 1 1 to/from AP X 2~ 0 or 1 to/from TDLS to/from connected station
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 Martin Kaistra (20): 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: 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: support multiple interfaces in get_macid() 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 | 16 +- .../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 2 +- .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 + .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 361 +++++++++++++----- 4 files changed, 283 insertions(+), 97 deletions(-)