2022-01-10 03:41:44

by Seevalamuthu M (QUIC)

[permalink] [raw]
Subject: [PATCH] ath11k: Add support for dynamic vlan

Advertise AP-VLAN interface type for vlan support in driver.
Metadata information in dp_tx is added to notify firmware
that multicast/broadcast packets are encrypted in software.

Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1

Signed-off-by: Seevalamuthu Mariappan <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.c | 6 +++
drivers/net/wireless/ath/ath11k/dp_tx.c | 74 ++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath11k/dp_tx.h | 14 +++++++
drivers/net/wireless/ath/ath11k/hw.h | 1 +
drivers/net/wireless/ath/ath11k/mac.c | 5 +++
5 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 293563b..0b2407e 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -86,6 +86,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.num_vdevs = 16 + 1,
.num_peers = 512,
.supports_suspend = false,
+ .supports_ap_vlan = true,
.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
.supports_regdb = false,
.fix_l1ss = true,
@@ -150,6 +151,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.num_vdevs = 16 + 1,
.num_peers = 512,
.supports_suspend = false,
+ .supports_ap_vlan = true,
.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
.supports_regdb = false,
.fix_l1ss = true,
@@ -213,6 +215,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.num_vdevs = 16 + 1,
.num_peers = 512,
.supports_suspend = true,
+ .supports_ap_vlan = false,
.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
.supports_regdb = true,
.fix_l1ss = true,
@@ -276,6 +279,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.num_vdevs = 8,
.num_peers = 128,
.supports_suspend = false,
+ .supports_ap_vlan = true,
.hal_desc_sz = sizeof(struct hal_rx_desc_qcn9074),
.supports_regdb = false,
.fix_l1ss = true,
@@ -339,6 +343,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.num_vdevs = 16 + 1,
.num_peers = 512,
.supports_suspend = true,
+ .supports_ap_vlan = false,
.hal_desc_sz = sizeof(struct hal_rx_desc_wcn6855),
.supports_regdb = true,
.fix_l1ss = false,
@@ -401,6 +406,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.num_vdevs = 16 + 1,
.num_peers = 512,
.supports_suspend = true,
+ .supports_ap_vlan = false,
.hal_desc_sz = sizeof(struct hal_rx_desc_wcn6855),
.supports_regdb = true,
.fix_l1ss = false,
diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c
index 91d6244..211a604 100644
--- a/drivers/net/wireless/ath/ath11k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
@@ -78,6 +78,44 @@ enum hal_encrypt_type ath11k_dp_tx_get_encrypt_type(u32 cipher)
}
}

+#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;
+ struct htt_tx_msdu_desc_ext *desc_ext;
+ int ret;
+
+ htt_desc_size = sizeof(*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->info0 =
+ __cpu_to_le32(FIELD_PREP(HTT_TX_MSDU_DESC_INFO0_VALID_ENCRYPT_TYPE, 1) |
+ FIELD_PREP(HTT_TX_MSDU_DESC_INFO0_ENCRYPT_TYPE, 0) |
+ FIELD_PREP(HTT_TX_MSDU_DESC_INFO0_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)
{
@@ -95,6 +133,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
int ret;
u8 ring_selector = 0, ring_map = 0;
bool tcl_ring_retry;
+ u8 align_pad, htt_meta_size = 0;

if (unlikely(test_bit(ATH11K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags)))
return -ESHUTDOWN;
@@ -211,15 +250,42 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
goto fail_remove_idr;
}

+ /* Add metadata for sw encrypted vlan group traffic */
+ if (!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags) &&
+ !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
+ !info->control.hw_key &&
+ ieee80211_has_protected(hdr->frame_control)) {
+ /* 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_pull_skb;
+
+ 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);
ath11k_warn(ab, "failed to DMA map data Tx buffer\n");
ret = -ENOMEM;
- goto fail_remove_idr;
+ goto fail_pull_skb;
}

- ti.data_len = skb->len;
+ ti.data_len = skb->len - ti.pkt_offset;
skb_cb->paddr = ti.paddr;
skb_cb->vif = arvif->vif;
skb_cb->ar = ar;
@@ -274,6 +340,10 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
fail_unmap_dma:
dma_unmap_single(ab->dev, ti.paddr, ti.data_len, DMA_TO_DEVICE);

+fail_pull_skb:
+ if (ti.pkt_offset)
+ skb_pull(skb, ti.pkt_offset);
+
fail_remove_idr:
spin_lock_bh(&tx_ring->tx_idr_lock);
idr_remove(&tx_ring->txbuf_idr,
diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.h b/drivers/net/wireless/ath/ath11k/dp_tx.h
index e87d65b..95f8757 100644
--- a/drivers/net/wireless/ath/ath11k/dp_tx.h
+++ b/drivers/net/wireless/ath/ath11k/dp_tx.h
@@ -15,6 +15,20 @@ struct ath11k_dp_htt_wbm_tx_status {
int ack_rssi;
};

+#define HTT_TX_MSDU_DESC_INFO0_VALID_ENCRYPT_TYPE BIT(8)
+#define HTT_TX_MSDU_DESC_INFO0_ENCRYPT_TYPE GENMASK(16, 15)
+#define HTT_TX_MSDU_DESC_INFO0_HOST_TX_DESC_POOL BIT(31)
+
+struct htt_tx_msdu_desc_ext {
+ __le32 info0;
+ __le32 info1;
+ __le32 info2;
+ __le32 info3;
+ __le32 info4;
+ __le32 info5;
+ __le32 info6;
+} __packed;
+
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,
diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
index 29934b3..dfe8d4c 100644
--- a/drivers/net/wireless/ath/ath11k/hw.h
+++ b/drivers/net/wireless/ath/ath11k/hw.h
@@ -181,6 +181,7 @@ struct ath11k_hw_params {
u32 num_vdevs;
u32 num_peers;
bool supports_suspend;
+ bool supports_ap_vlan;
u32 hal_desc_sz;
bool supports_regdb;
bool fix_l1ss;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 07f499d..b035e29 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -8469,6 +8469,11 @@ static int __ath11k_mac_register(struct ath11k *ar)
*/
ar->hw->wiphy->interface_modes &= ~BIT(NL80211_IFTYPE_MONITOR);

+ if (ab->hw_params.supports_ap_vlan) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ }
+
/* Apply the regd received during initialization */
ret = ath11k_regd_update(ar);
if (ret) {
--
2.7.4



2022-01-18 02:24:17

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath11k: Add support for dynamic vlan

Seevalamuthu Mariappan <[email protected]> writes:

> Advertise AP-VLAN interface type for vlan support in driver.
> Metadata information in dp_tx is added to notify firmware
> that multicast/broadcast packets are encrypted in software.
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Seevalamuthu Mariappan <[email protected]>

[...]

> --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> @@ -78,6 +78,44 @@ enum hal_encrypt_type ath11k_dp_tx_get_encrypt_type(u32 cipher)
> }
> }
>
> +#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;
> +}

[...]

> @@ -211,15 +250,42 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
> goto fail_remove_idr;
> }
>
> + /* Add metadata for sw encrypted vlan group traffic */
> + if (!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags) &&
> + !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
> + !info->control.hw_key &&
> + ieee80211_has_protected(hdr->frame_control)) {
> + /* 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;

If you push the skb like that shouldn't you also reserve the room for it
in struct ieee80211_hw::extra_tx_headroom?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-02-09 06:33:40

by Seevalamuthu M (QUIC)

[permalink] [raw]
Subject: RE: [PATCH] ath11k: Add support for dynamic vlan

Hi Kalle,

> -----Original Message-----
> From: Kalle Valo <[email protected]>
> Sent: Monday, January 17, 2022 6:03 PM
> To: Seevalamuthu M (QUIC) <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] ath11k: Add support for dynamic vlan
>
> Seevalamuthu Mariappan <[email protected]> writes:
>
> > Advertise AP-VLAN interface type for vlan support in driver.
> > Metadata information in dp_tx is added to notify firmware that
> > multicast/broadcast packets are encrypted in software.
> >
> > Tested-on: IPQ8074 hw2.0 AHB
> > WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1
> >
> > Signed-off-by: Seevalamuthu Mariappan <[email protected]>
>
> [...]
>
> > --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> > +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> > @@ -78,6 +78,44 @@ enum hal_encrypt_type
> ath11k_dp_tx_get_encrypt_type(u32 cipher)
> > }
> > }
> >
> > +#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;
> > +}
>
> [...]
>
> > @@ -211,15 +250,42 @@ int ath11k_dp_tx(struct ath11k *ar, struct
> ath11k_vif *arvif,
> > goto fail_remove_idr;
> > }
> >
> > + /* Add metadata for sw encrypted vlan group traffic */
> > + if (!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab-
> >dev_flags) &&
> > + !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
> > + !info->control.hw_key &&
> > + ieee80211_has_protected(hdr->frame_control)) {
> > + /* 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;
>
> If you push the skb like that shouldn't you also reserve the room for it in
> struct ieee80211_hw::extra_tx_headroom?

[seevalam] : already this is handled using skb_cow_head() in ath11k_dp_metadata_align_skb(). Please correct me if I misunderstood your point.

>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp
> atches

Regards,
Seevalamuthu M

2022-05-07 19:52:51

by Seevalamuthu M (QUIC)

[permalink] [raw]
Subject: RE: [PATCH] ath11k: Add support for dynamic vlan



> -----Original Message-----
> From: Seevalamuthu M (QUIC) <[email protected]>
> Sent: Tuesday, February 8, 2022 11:00 AM
> To: Kalle Valo <[email protected]>; Seevalamuthu M (QUIC)
> <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: RE: [PATCH] ath11k: Add support for dynamic vlan
>
> Hi Kalle,
>
> > -----Original Message-----
> > From: Kalle Valo <[email protected]>
> > Sent: Monday, January 17, 2022 6:03 PM
> > To: Seevalamuthu M (QUIC) <[email protected]>
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH] ath11k: Add support for dynamic vlan
> >
> > Seevalamuthu Mariappan <[email protected]> writes:
> >
> > > Advertise AP-VLAN interface type for vlan support in driver.
> > > Metadata information in dp_tx is added to notify firmware that
> > > multicast/broadcast packets are encrypted in software.
> > >
> > > Tested-on: IPQ8074 hw2.0 AHB
> > > WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1
> > >
> > > Signed-off-by: Seevalamuthu Mariappan <[email protected]>
> >
> > [...]
> >
> > > --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> > > +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> > > @@ -78,6 +78,44 @@ enum hal_encrypt_type
> > ath11k_dp_tx_get_encrypt_type(u32 cipher)
> > > }
> > > }
> > >
> > > +#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;
> > > +}
> >
> > [...]
> >
> > > @@ -211,15 +250,42 @@ int ath11k_dp_tx(struct ath11k *ar, struct
> > ath11k_vif *arvif,
> > > goto fail_remove_idr;
> > > }
> > >
> > > + /* Add metadata for sw encrypted vlan group traffic */
> > > + if (!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab-
> > >dev_flags) &&
> > > + !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
> > > + !info->control.hw_key &&
> > > + ieee80211_has_protected(hdr->frame_control)) {
> > > + /* 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;
> >
> > If you push the skb like that shouldn't you also reserve the room for
> > it in struct ieee80211_hw::extra_tx_headroom?
>
> [seevalam] : already this is handled using skb_cow_head() in
> ath11k_dp_metadata_align_skb(). Please correct me if I misunderstood your
> point.

Hi Kalle,

Do you have any comments on this?

Regards,
Seevalamuthu M


2022-06-03 15:14:31

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath11k: Add support for dynamic vlan

Seevalamuthu Mariappan <[email protected]> writes:

> Advertise AP-VLAN interface type for vlan support in driver.
> Metadata information in dp_tx is added to notify firmware
> that multicast/broadcast packets are encrypted in software.
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Seevalamuthu Mariappan <[email protected]>
> ---
> drivers/net/wireless/ath/ath11k/core.c | 6 +++
> drivers/net/wireless/ath/ath11k/dp_tx.c | 74 ++++++++++++++++++++++++++++++++-
> drivers/net/wireless/ath/ath11k/dp_tx.h | 14 +++++++
> drivers/net/wireless/ath/ath11k/hw.h | 1 +
> drivers/net/wireless/ath/ath11k/mac.c | 5 +++
> 5 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
> index 293563b..0b2407e 100644
> --- a/drivers/net/wireless/ath/ath11k/core.c
> +++ b/drivers/net/wireless/ath/ath11k/core.c
> @@ -86,6 +86,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .num_vdevs = 16 + 1,
> .num_peers = 512,
> .supports_suspend = false,
> + .supports_ap_vlan = true,
> .hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
> .supports_regdb = false,
> .fix_l1ss = true,
> @@ -150,6 +151,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .num_vdevs = 16 + 1,
> .num_peers = 512,
> .supports_suspend = false,
> + .supports_ap_vlan = true,
> .hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
> .supports_regdb = false,
> .fix_l1ss = true,
> @@ -213,6 +215,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .num_vdevs = 16 + 1,
> .num_peers = 512,
> .supports_suspend = true,
> + .supports_ap_vlan = false,
> .hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
> .supports_regdb = true,
> .fix_l1ss = true,
> @@ -276,6 +279,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .num_vdevs = 8,
> .num_peers = 128,
> .supports_suspend = false,
> + .supports_ap_vlan = true,
> .hal_desc_sz = sizeof(struct hal_rx_desc_qcn9074),
> .supports_regdb = false,
> .fix_l1ss = true,
> @@ -339,6 +343,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .num_vdevs = 16 + 1,
> .num_peers = 512,
> .supports_suspend = true,
> + .supports_ap_vlan = false,
> .hal_desc_sz = sizeof(struct hal_rx_desc_wcn6855),
> .supports_regdb = true,
> .fix_l1ss = false,
> @@ -401,6 +406,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .num_vdevs = 16 + 1,
> .num_peers = 512,
> .supports_suspend = true,
> + .supports_ap_vlan = false,
> .hal_desc_sz = sizeof(struct hal_rx_desc_wcn6855),
> .supports_regdb = true,
> .fix_l1ss = false,

As this is an old patch, please make sure all entries in hw_params have
supports_ap_vlan.

> +static int ath11k_dp_metadata_align_skb(struct sk_buff *skb, u8 align_len)
> +{
> + if (unlikely(skb_cow_head(skb, align_len)))
> + return -ENOMEM;

The preferred style in ath11k is:

ret = skb_cow_head(skb, align_len);
if (unlikely(ret))
return ret;

But is the unlikely() really necessary here?

I doubt this applies anymore so please rebase and submit v2.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-06-03 17:53:20

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath11k: Add support for dynamic vlan

"Seevalamuthu M (QUIC)" <[email protected]> writes:

> Hi Kalle,
>
>> -----Original Message-----
>> From: Kalle Valo <[email protected]>
>> Sent: Monday, January 17, 2022 6:03 PM
>> To: Seevalamuthu M (QUIC) <[email protected]>
>> Cc: [email protected]; [email protected]
>> Subject: Re: [PATCH] ath11k: Add support for dynamic vlan
>>
>> Seevalamuthu Mariappan <[email protected]> writes:
>>
>> > Advertise AP-VLAN interface type for vlan support in driver.
>> > Metadata information in dp_tx is added to notify firmware that
>> > multicast/broadcast packets are encrypted in software.
>> >
>> > Tested-on: IPQ8074 hw2.0 AHB
>> > WLAN.HK.2.5.0.1-01073-QCAHKSWPL_SILICONZ-1
>> >
>> > Signed-off-by: Seevalamuthu Mariappan <[email protected]>
>>
>> [...]
>>
>> > --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
>> > +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
>> > @@ -78,6 +78,44 @@ enum hal_encrypt_type
>> ath11k_dp_tx_get_encrypt_type(u32 cipher)
>> > }
>> > }
>> >
>> > +#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;
>> > +}
>>
>> [...]
>>
>> > @@ -211,15 +250,42 @@ int ath11k_dp_tx(struct ath11k *ar, struct
>> ath11k_vif *arvif,
>> > goto fail_remove_idr;
>> > }
>> >
>> > + /* Add metadata for sw encrypted vlan group traffic */
>> > + if (!test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, &ar->ab-
>> >dev_flags) &&
>> > + !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
>> > + !info->control.hw_key &&
>> > + ieee80211_has_protected(hdr->frame_control)) {
>> > + /* 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;
>>
>> If you push the skb like that shouldn't you also reserve the room for it in
>> struct ieee80211_hw::extra_tx_headroom?
>
> [seevalam] : already this is handled using skb_cow_head() in
> ath11k_dp_metadata_align_skb().

Indeed, I missed that.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches