Re: [PATCH 02/10] wifi: ath12k: ath12k_mac_op_tx(): MLO support

Baochen Qiang
2024-11-27T02:49:28Z

On 11/27/2024 1:11 AM, Kalle Valo wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
>
> For a frame transmission for an ML vif, mac80211 mentions transmit link id in
> the tx control info. Use it to convert the RA/TA to the corresponding link sta
> and link vif address before enqueueing the frame for transmission.
>
> For 802.3 data frames, always enqueue the frame on the primary (assoc) link id.
> Firmware does the link selection, builds 802.11 header and therefore the address
> translation too.
>
> Also ensure right link vif is used for WMI based management transmission and
> add comments to document when RCU read lock is held.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Co-developed-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> ---
> drivers/net/wireless/ath/ath12k/core.h | 1 +
> drivers/net/wireless/ath/ath12k/mac.c | 141 ++++++++++++++++++++++++-
> 2 files changed, 138 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 5be977008319..e246e3d3c162 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -122,6 +122,7 @@ struct ath12k_skb_cb {
> dma_addr_t paddr_ext_desc;
> u32 cipher;
> u8 flags;
> + u8 link_id;
> };
>
> struct ath12k_skb_rxcb {
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index a6fe998c177e..97a5f26cc577 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -6848,6 +6848,7 @@ static void ath12k_mgmt_over_wmi_tx_purge(struct ath12k *ar)
> static void ath12k_mgmt_over_wmi_tx_work(struct wiphy *wiphy, struct wiphy_work *work)
> {
> struct ath12k *ar = container_of(work, struct ath12k, wmi_mgmt_tx_work);
> + struct ath12k_hw *ah = ar->ah;
> struct ath12k_skb_cb *skb_cb;
> struct ath12k_vif *ahvif;
> struct ath12k_link_vif *arvif;
> @@ -6865,7 +6866,15 @@ static void ath12k_mgmt_over_wmi_tx_work(struct wiphy *wiphy, struct wiphy_work
> }
>
> ahvif = ath12k_vif_to_ahvif(skb_cb->vif);
> - arvif = &ahvif->deflink;
> + if (!(ahvif->links_map & BIT(skb_cb->link_id))) {
> + ath12k_warn(ar->ab,
> + "invalid linkid %u in mgmt over wmi tx with linkmap 0x%X\n",
s/0x%X/0x%x/ ?

> + skb_cb->link_id, ahvif->links_map);
> + ath12k_mgmt_over_wmi_tx_drop(ar, skb);
> + continue;
> + }
> +
> + arvif = wiphy_dereference(ah->hw->wiphy, ahvif->link[skb_cb->link_id]);
> if (ar->allocated_vdev_map & (1LL << arvif->vdev_id)) {
> ret = ath12k_mac_mgmt_tx_wmi(ar, arvif, skb);
> if (ret) {
> @@ -6875,9 +6884,10 @@ static void ath12k_mgmt_over_wmi_tx_work(struct wiphy *wiphy, struct wiphy_work
> }
> } else {
> ath12k_warn(ar->ab,
> - "dropping mgmt frame for vdev %d, is_started %d\n",
> + "dropping mgmt frame for vdev %d link_id %u, is_started %d\n",
> arvif->vdev_id,
> - arvif->is_started);
> + arvif->is_started,
> + skb_cb->link_id);
swap 'arvif->is_started' and 'skb_cb->link_id'.

> ath12k_mgmt_over_wmi_tx_drop(ar, skb);
> }
> }
> @@ -6936,6 +6946,105 @@ static void ath12k_mac_add_p2p_noa_ie(struct ath12k *ar,
> spin_unlock_bh(&ar->data_lock);
> }
>
> +/* Note: called under rcu_read_lock() */
> +static u8 ath12k_mac_get_tx_link(struct ieee80211_sta *sta, struct ieee80211_vif *vif,
> + u8 link, struct sk_buff *skb, u32 info_flags)
> +{
> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> + struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
> + struct ieee80211_link_sta *link_sta;
> + struct ieee80211_bss_conf *bss_conf;
> + struct ath12k_sta *ahsta;
better to assert RCU read lock here?

> +
> + /* Use the link id passed or the default vif link */
> + if (!sta) {
> + if (link != IEEE80211_LINK_UNSPECIFIED)
> + return link;
> +
> + return ahvif->deflink.link_id;
> + }
> +
> + ahsta = ath12k_sta_to_ahsta(sta);
> +
> + /* Below translation ensures we pass proper A2 & A3 for non ML clients.
> + * Also it assumes for now support only for MLO AP in this path
> + */
> + if (!sta->mlo) {
> + link = ahsta->deflink.link_id;
> +
> + if (info_flags & IEEE80211_TX_CTL_HW_80211_ENCAP)
> + return link;
> +
> + bss_conf = rcu_dereference(vif->link_conf[link]);
> + if (bss_conf) {
> + ether_addr_copy(hdr->addr2, bss_conf->addr);
> + if (!ieee80211_has_tods(hdr->frame_control) &&
> + !ieee80211_has_fromds(hdr->frame_control))
> + ether_addr_copy(hdr->addr3, bss_conf->addr);
> + }
> +
> + return link;
> + }
> +
> + /* enqueue eth enacap & data frames on primary link, FW does link
> + * selection and address translation.
> + */
> + if (info_flags & IEEE80211_TX_CTL_HW_80211_ENCAP ||
> + ieee80211_is_data(hdr->frame_control))
> + return ahsta->assoc_link_id;
> +
> + /* 802.11 frame cases */
> + if (link == IEEE80211_LINK_UNSPECIFIED)
> + link = ahsta->deflink.link_id;
> +
> + if (!ieee80211_is_mgmt(hdr->frame_control))
> + return link;
> +
> + /* Perform address conversion for ML STA Tx */
> + bss_conf = rcu_dereference(vif->link_conf[link]);
> + link_sta = rcu_dereference(sta->link[link]);
> +
> + if (bss_conf && link_sta) {
> + ether_addr_copy(hdr->addr1, link_sta->addr);
> + ether_addr_copy(hdr->addr2, bss_conf->addr);
> +
> + if (vif->type == NL80211_IFTYPE_STATION && bss_conf->bssid)
> + ether_addr_copy(hdr->addr3, bss_conf->bssid);
> + else if (vif->type == NL80211_IFTYPE_AP)
> + ether_addr_copy(hdr->addr3, bss_conf->addr);
> +
> + return link;
> + }
> +
> + if (bss_conf) {
> + /* In certain cases where a ML sta associated and added subset of
> + * links on which the ML AP is active, but now sends some frame
> + * (ex. Probe request) on a different link which is active in our
> + * MLD but was not added during previous association, we can
> + * still honor the Tx to that ML STA via the requested link.
> + * The control would reach here in such case only when that link
> + * address is same as the MLD address or in worst case clients
> + * used MLD address at TA wrongly which would have helped
> + * identify the ML sta object and pass it here.
> + * If the link address of that STA is different from MLD address,
> + * then the sta object would be NULL and control won't reach
> + * here but return at the start of the function itself with !sta
> + * check. Also this would not need any translation at hdr->addr1
> + * from MLD to link address since the RA is the MLD address
> + * (same as that link address ideally) already.
> + */
> + ether_addr_copy(hdr->addr2, bss_conf->addr);
> +
> + if (vif->type == NL80211_IFTYPE_STATION && bss_conf->bssid)
> + ether_addr_copy(hdr->addr3, bss_conf->bssid);
> + else if (vif->type == NL80211_IFTYPE_AP)
> + ether_addr_copy(hdr->addr3, bss_conf->addr);
> + }
> +
> + return link;
> +}
> +
> +/* Note: called under rcu_read_lock() */
> static void ath12k_mac_op_tx(struct ieee80211_hw *hw,
> struct ieee80211_tx_control *control,
> struct sk_buff *skb)
> @@ -6945,13 +7054,16 @@ static void ath12k_mac_op_tx(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif = info->control.vif;
> struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
> struct ath12k_link_vif *arvif = &ahvif->deflink;
> - struct ath12k *ar = arvif->ar;
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> struct ieee80211_key_conf *key = info->control.hw_key;
> + struct ieee80211_sta *sta = control->sta;
> u32 info_flags = info->flags;
> + struct ath12k *ar;
> bool is_prb_rsp;
> + u8 link_id;
> int ret;
>
better to assert RCU read lock here?

> + link_id = u32_get_bits(info->control.flags, IEEE80211_TX_CTRL_MLO_LINK);
> memset(skb_cb, 0, sizeof(*skb_cb));
> skb_cb->vif = vif;
>
> @@ -6960,6 +7072,27 @@ static void ath12k_mac_op_tx(struct ieee80211_hw *hw,
> skb_cb->flags |= ATH12K_SKB_CIPHER_SET;
> }
>
> + /* handle only for MLO case, use deflink for non MLO case */
> + if (vif->valid_links) {
better to use ieee80211_vif_is_mld() helper?

> + link_id = ath12k_mac_get_tx_link(sta, vif, link_id, skb, info_flags);
> + if (link_id >= IEEE80211_MLD_MAX_NUM_LINKS) {
> + ieee80211_free_txskb(hw, skb);
> + return;
> + }
> + } else {
> + link_id = 0;
> + }
> +
> + arvif = rcu_dereference(ahvif->link[link_id]);
> + if (!arvif || !arvif->ar) {
> + ath12k_warn(ahvif->ah, "failed to find arvif link id %u for frame transmission",
> + link_id);
> + ieee80211_free_txskb(hw, skb);
> + return;
> + }
> +
> + ar = arvif->ar;
> + skb_cb->link_id = link_id;
> is_prb_rsp = ieee80211_is_probe_resp(hdr->frame_control);
>
> if (info_flags & IEEE80211_TX_CTL_HW_80211_ENCAP) {