[PATCH v2] ath11k: add support for dynamic vlan (ap/vlan)
David Rapaň
david at rapan.cz
Fri May 23 04:14:46 PDT 2025
Hi Jeff, thanks for the feedback. I tried to fix all the problems you mentioned and now I have submitted v3.
________________________________________
From: Jeff Johnson <jeff.johnson at oss.qualcomm.com>
Sent: Wednesday, May 21, 2025 6:32 PM
To: David Rapaň; ath11k at lists.infradead.org
Cc: linux-wireless at vger.kernel.org; jjohnson at kernel.org; johannes at sipsolutions.net
Subject: Re: [PATCH v2] ath11k: add support for dynamic vlan (ap/vlan)
On 5/20/2025 2:16 PM, David Rapaň wrote:
> ath11k currently lacks support for dynamic vlan (AP/VLAN), so this patch
> adds feature advertisement in '__ath11k_mac_register' alongside
> all the other neccessary components supporting its function, including
> prerequisites for software encryption of VLAN group traffics.
> VLAN unicast packets shall be taking 8023 xmit path if encapsulation
> offload is enabled and multicast/broadcast then 80211 xmit path.
>
> Metadata info in dp_tx added to notify firmware that the
> multicast/broadcast packets are encrypted in software.
Use imperative mood in your commit text
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> Changes since v1:
> Move 'cpu_to_be16(ETH_P_PAE)' to earlier condition and add
> 'ieee80211_is_qos_nullfunc' check so QoS NULL Data frames are
> properly send as open type frames with TID 7 in ethernet encap.
> WIthout this change are as QoS Data with TID 0.
patch revision history goes after the "---"
>
> Tested, long-term in production environment using OpenWrt platform
> installed on multiple AX3600 with FT.
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.9.0.1-01385-QCAHKSWPL_SILICONZ-1
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.9.0.1-01977-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Seevalamuthu Mariappan <seevalam at codeaurora.org>
> Signed-off-by: David Rapan <david at rapan.cz>
Who authored this code? You are submitting it as the author, but the S-O-B
chain implies Seevalamuthu Mariappan was the author.
> ---
> drivers/net/wireless/ath/ath11k/core.h | 1 +
> drivers/net/wireless/ath/ath11k/dp_tx.c | 82 +++++++++-
> drivers/net/wireless/ath/ath11k/dp_tx.h | 197 ++++++++++++++++++++++++
> drivers/net/wireless/ath/ath11k/mac.c | 3 +
> net/mac80211/tx.c | 15 +++
> 5 files changed, 296 insertions(+), 2 deletions(-)
>
> base-commit: ff8069c7cf3eb0fcd53adebdf341b6aaa98bdd3b
>
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -120,6 +120,7 @@ struct ath11k_skb_cb {
> u32 cipher;
> struct ath11k *ar;
> struct ieee80211_vif *vif;
> + u32 pkt_offset;
> } __packed;
>
> struct ath11k_skb_rxcb {
> --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> @@ -79,6 +79,42 @@ enum hal_encrypt_type ath11k_dp_tx_get_e
> }
> }
>
> +#define HTT_META_DATA_ALIGNMENT 0x8
> +
> +static int ath11k_dp_metadata_align_skb(struct sk_buff *skb, u8 align_len)
> +{
> + if (unlikely(skb_cow_head(skb, align_len)))
> + return -ENOMEM;
> +
> + skb_push(skb, align_len);
> + memset(skb->data, 0, align_len);
> + return 0;
> +}
> +
> +static int ath11k_dp_prepare_htt_metadata(struct sk_buff *skb,
> + u8 *htt_metadata_size)
> +{
> + u8 htt_desc_size;
> + /* Size rounded of multiple of 8 bytes */
> + u8 htt_desc_size_aligned;
> + int ret;
> + struct htt_tx_msdu_desc_ext *desc_ext;
> +
> + htt_desc_size = sizeof(struct htt_tx_msdu_desc_ext);
> + htt_desc_size_aligned = ALIGN(htt_desc_size, HTT_META_DATA_ALIGNMENT);
> +
> + ret = ath11k_dp_metadata_align_skb(skb, htt_desc_size_aligned);
> + if (unlikely(ret))
> + return ret;
> +
> + desc_ext = (struct htt_tx_msdu_desc_ext *)skb->data;
> + desc_ext->valid_encrypt_type = 1;
> + desc_ext->encrypt_type = 0;
> + desc_ext->host_tx_desc_pool = 1;
> + *htt_metadata_size = htt_desc_size_aligned;
> + return 0;
> +}
> +
> int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
> struct ath11k_sta *arsta, struct sk_buff *skb)
> {
> @@ -97,6 +133,9 @@ int ath11k_dp_tx(struct ath11k *ar, stru
> u32 ring_selector = 0;
> u8 ring_map = 0;
> bool tcl_ring_retry;
> + bool is_diff_encap = false;
> + u8 align_pad;
> + u8 htt_meta_size = 0;
>
> if (unlikely(test_bit(ATH11K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags)))
> return -ESHUTDOWN;
> @@ -189,7 +228,12 @@ tcl_ring_sel:
>
> switch (ti.encap_type) {
> case HAL_TCL_ENCAP_TYPE_NATIVE_WIFI:
> - ath11k_dp_tx_encap_nwifi(skb);
> + if ((arvif->vif->offload_flags & IEEE80211_OFFLOAD_ENCAP_ENABLED) &&
> + (skb->protocol == cpu_to_be16(ETH_P_PAE) ||
> + ieee80211_is_qos_nullfunc(hdr->frame_control)))
> + is_diff_encap = true;
> + else
> + ath11k_dp_tx_encap_nwifi(skb);
> break;
> case HAL_TCL_ENCAP_TYPE_RAW:
> if (!test_bit(ATH11K_FLAG_RAW_MODE, &ab->dev_flags)) {
> @@ -208,6 +252,33 @@ tcl_ring_sel:
> goto fail_remove_idr;
> }
>
> + /* Add metadata for software encryption of vlan group traffic */
> + if ((!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags) &&
> + !(info->control.flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
> + !info->control.hw_key && ieee80211_has_protected(hdr->frame_control)) ||
> + is_diff_encap) {
> + /* HW requirement is that metadata should always point to a
> + * 8-byte aligned address. So we add alignment pad to start of
> + * buffer. HTT Metadata should be ensured to be multiple of 8-bytes
> + * to get 8-byte aligned start address along with align_pad added
> + */
> + align_pad = ((unsigned long)skb->data) & (HTT_META_DATA_ALIGNMENT - 1);
> + ret = ath11k_dp_metadata_align_skb(skb, align_pad);
> + if (unlikely(ret))
> + goto fail_remove_idr;
> +
> + ti.pkt_offset += align_pad;
> + ret = ath11k_dp_prepare_htt_metadata(skb, &htt_meta_size);
> + if (unlikely(ret))
> + goto fail_remove_idr;
> +
> + ti.pkt_offset += htt_meta_size;
> + ti.meta_data_flags |= HTT_TCL_META_DATA_VALID_HTT;
> + ti.flags0 |= FIELD_PREP(HAL_TCL_DATA_CMD_INFO1_TO_FW, 1);
> + ti.encap_type = HAL_TCL_ENCAP_TYPE_RAW;
> + ti.encrypt_type = HAL_ENCRYPT_TYPE_OPEN;
> + }
> +
> ti.paddr = dma_map_single(ab->dev, skb->data, skb->len, DMA_TO_DEVICE);
> if (unlikely(dma_mapping_error(ab->dev, ti.paddr))) {
> atomic_inc(&ab->soc_stats.tx_err.misc_fail);
> @@ -216,7 +287,8 @@ tcl_ring_sel:
> goto fail_remove_idr;
> }
>
> - ti.data_len = skb->len;
> + ti.data_len = skb->len - ti.pkt_offset;
> + skb_cb->pkt_offset = ti.pkt_offset;
> skb_cb->paddr = ti.paddr;
> skb_cb->vif = arvif->vif;
> skb_cb->ar = ar;
> @@ -272,6 +344,8 @@ fail_unmap_dma:
> dma_unmap_single(ab->dev, ti.paddr, ti.data_len, DMA_TO_DEVICE);
>
> fail_remove_idr:
> + if (ti.pkt_offset)
> + skb_pull(skb, ti.pkt_offset);
> spin_lock_bh(&tx_ring->tx_idr_lock);
> idr_remove(&tx_ring->txbuf_idr,
> FIELD_GET(DP_TX_DESC_ID_MSDU_ID, ti.desc_id));
> @@ -348,6 +422,10 @@ ath11k_dp_tx_htt_tx_complete_buf(struct
> return;
> }
>
> + if (skb_cb->pkt_offset)
> + /* Removing the alignment and htt meta data */
> + skb_pull(msdu, skb_cb->pkt_offset);
> +
> memset(&info->status, 0, sizeof(info->status));
>
> if (ts->acked) {
> --- a/drivers/net/wireless/ath/ath11k/dp_tx.h
> +++ b/drivers/net/wireless/ath/ath11k/dp_tx.h
> @@ -17,6 +17,203 @@ struct ath11k_dp_htt_wbm_tx_status {
> u16 peer_id;
> };
>
> +/* htt_tx_msdu_desc_ext
> + *
> + * valid_pwr
> + * If set, tx pwr spec is valid
> + *
> + * valid_mcs_mask
> + * If set, tx MCS mask is valid
> + *
> + * valid_nss_mask
> + * If set, tx Nss mask is valid
> + *
> + * valid_preamble_type
> + * If set, tx preamble spec is valid
> + *
> + * valid_retries
> + * If set, tx retries spec is valid
> + *
> + * valid_bw_info
> + * If set, tx dyn_bw and bw_mask are valid
> + *
> + * valid_guard_interval
> + * If set, tx guard intv spec is valid
> + *
> + * valid_chainmask
> + * If set, tx chainmask is valid
> + *
> + * valid_encrypt_type
> + * If set, encrypt type is valid
> + *
> + * valid_key_flags
> + * If set, key flags is valid
> + *
> + * valid_expire_tsf
> + * If set, tx expire TSF spec is valid
> + *
> + * valid_chanfreq
> + * If set, chanfreq is valid
> + *
> + * is_dsrc
> + * If set, MSDU is a DSRC frame
> + *
> + * guard_interval
> + * 0.4us, 0.8us, 1.6us, 3.2us
> + *
> + * encrypt_type
> + * 0 = NO_ENCRYPT,
> + * 1 = ENCRYPT,
> + * 2 ~ 3 - Reserved
> + *
> + * retry_limit
> + * Specify the maximum number of transmissions, including the
> + * initial transmission, to attempt before giving up if no ack
> + * is received.
> + * If the tx rate is specified, then all retries shall use the
> + * same rate as the initial transmission.
> + * If no tx rate is specified, the target can choose whether to
> + * retain the original rate during the retransmissions, or to
> + * fall back to a more robust rate.
> + *
> + * use_dcm_11ax
> + * If set, Use Dual subcarrier modulation.
> + * Valid only for 11ax preamble types HE_SU
> + * and HE_EXT_SU
> + *
> + * ltf_subtype_11ax
> + * Takes enum values of htt_11ax_ltf_subtype_t
> + * Valid only for 11ax preamble types HE_SU
> + * and HE_EXT_SU
> + *
> + * dyn_bw
> + * 0 = static bw, 1 = dynamic bw
> + *
> + * bw_mask
> + * Valid only if dyn_bw == 0 (static bw).
> + *
> + * host_tx_desc_pool
> + * If set, Firmware allocates tx_descriptors
> + * in WAL_BUFFERID_TX_HOST_DATA_EXP,instead
> + * of WAL_BUFFERID_TX_TCL_DATA_EXP.
> + * Use cases:
> + * Any time firmware uses TQM-BYPASS for Data
> + * TID, firmware expect host to set this bit.
> + *
> + * power
> + * Unit of the power field is 0.5 dbm
> + * signed value ranging from -64dbm to 63.5 dbm
> + *
> + * mcs_mask
> + * mcs bit mask of 0 ~ 11
> + * Setting more than one MCS isn't currently
> + * supported by the target but is supported
> + * in the interface in case in the future
> + * the target supports specifications of
> + * a limited set of MCS values.
> + *
> + * nss_mask
> + * Nss bit mask 0 ~ 7
> + * Setting more than one Nss isn't currently
> + * supported by the target but is supported
> + * in the interface in case in the future
> + * the target supports specifications of
> + * a limited set of Nss values.
> + *
> + * pream_type
> + * Preamble types
> + *
> + * update_peer_cache
> + * When set these custom values will be
> + * used for all packets, until the next
> + * update via this ext header.
> + * This is to make sure not all packets
> + * need to include this header.
> + *
> + * chain_mask
> + * Specify which chains to transmit from
> + *
> + * key_flags
> + * Key Index and related flags - used in mesh mode
> + *
> + * chanfreq
> + * Channel frequency: This identifies the desired channel
> + * frequency (in MHz) for tx frames. This is used by FW to help
> + * determine when it is safe to transmit or drop frames for
> + * off-channel operation.
> + * The default value of zero indicates to FW that the corresponding
> + * VDEV's home channel (if there is one) is the desired channel
> + * frequency.
> + *
> + * expire_tsf_lo
> + * tx expiry time (TSF) LSBs
> + *
> + * expire_tsf_hi
> + * tx expiry time (TSF) MSBs
> + *
> + * learning_frame
> + * When this flag is set, this frame will be dropped by FW
> + * rather than being enqueued to the Transmit Queue Manager (TQM) HW.
> + *
> + * send_as_standalone
> + * This will indicate if the msdu needs to be sent as a singleton PPDU,
> + * i.e. with no A-MSDU or A-MPDU aggregation.
> + * The scope is extended to other use-cases.
> + *
> + * is_host_opaque_valid
> + * Set this bit to 1 if the host_opaque_cookie is populated
> + * with valid information.
> + *
> + * host_opaque_cookie
> + * Host opaque cookie for special frames
> + */
> +struct htt_tx_msdu_desc_ext {
> + u32
> + valid_pwr : 1,
> + valid_mcs_mask : 1,
> + valid_nss_mask : 1,
> + valid_preamble_type : 1,
> + valid_retries : 1,
> + valid_bw_info : 1,
> + valid_guard_interval : 1,
> + valid_chainmask : 1,
> + valid_encrypt_type : 1,
> + valid_key_flags : 1,
> + valid_expire_tsf : 1,
> + valid_chanfreq : 1,
> + is_dsrc : 1,
> + guard_interval : 2,
> + encrypt_type : 2,
> + retry_limit : 4,
> + use_dcm_11ax : 1,
> + ltf_subtype_11ax : 2,
> + dyn_bw : 1,
> + bw_mask : 6,
> + host_tx_desc_pool : 1;
> + u32
> + power : 8,
> + mcs_mask : 12,
> + nss_mask : 8,
> + pream_type : 3,
> + update_peer_cache : 1;
> + u32
> + chain_mask : 8,
> + key_flags : 8,
> + chanfreq : 16;
> +
> + u32 expire_tsf_lo;
> + u32 expire_tsf_hi;
> +
> + u32
> + learning_frame : 1,
> + send_as_standalone : 1,
> + is_host_opaque_valid : 1,
> + rsvd0 : 29;
> + u32
> + host_opaque_cookie : 16,
> + rsvd1 : 16;
> +} __packed;
we should not be using bitmasks with packed in upstream code.
QSDK may have done this, but it's not ok upstream
and why isn't this being added to hal_desc.h?
> +
> void ath11k_dp_tx_update_txcompl(struct ath11k *ar, struct hal_tx_status *ts);
> int ath11k_dp_tx_htt_h2t_ver_req_msg(struct ath11k_base *ab);
> int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -10324,6 +10324,9 @@ static int __ath11k_mac_register(struct
> goto err_free_if_combs;
> }
>
> + ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
> + ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
> +
> if (!ab->hw_params.supports_monitor)
> /* There's a race between calling ieee80211_register_hw()
> * and here where the monitor mode is enabled for a little
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -40,6 +40,10 @@
>
> /* misc utils */
>
> +static void ieee80211_8023_xmit(struct ieee80211_sub_if_data *sdata,
> + struct net_device *dev, struct sta_info *sta,
> + struct ieee80211_key *key, struct sk_buff *skb);
> +
> static __le16 ieee80211_duration(struct ieee80211_tx_data *tx,
> struct sk_buff *skb, int group_addr,
> int next_frag_len)
> @@ -4257,6 +4261,7 @@ void __ieee80211_subif_start_xmit(struct
> {
> struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> struct ieee80211_local *local = sdata->local;
> + struct ieee80211_key *key = NULL;
> struct sta_info *sta;
> struct sk_buff *next;
> int len = skb->len;
> @@ -4281,5 +4286,15 @@ void __ieee80211_subif_start_xmit(struct
> if (IS_ERR(sta))
> sta = NULL;
>
> + if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
> + get_bss_sdata(sdata)->vif.offload_flags & IEEE80211_OFFLOAD_ENCAP_ENABLED &&
> + !is_multicast_ether_addr(skb->data)) {
> + if (sta)
> + key = rcu_dereference(sta->ptk[sta->ptk_idx]);
> + ieee80211_8023_xmit(sdata, dev, sta, key, skb);
> + rcu_read_unlock();
> + return;
> + }
> +
> skb_set_queue_mapping(skb, ieee80211_select_queue(sdata, sta, skb));
> ieee80211_aggr_check(sdata, sta, skb);
More information about the ath11k
mailing list