diff mbox series

[v2] wifi: ath12k: Use msdu_end to check MCBC

Message ID 20230509033638.3228-1-quic_bqiang@quicinc.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series [v2] wifi: ath12k: Use msdu_end to check MCBC | expand

Commit Message

Baochen Qiang May 9, 2023, 3:36 a.m. UTC 
We are seeing a very low TCP throughput testing with some specific
tools. This is because for sub-frames of an AMSDU, MCBC flag in
mpdu_start may be not valid, and as a result those frames would be
dropped by kernel.

Add a new helper to get it from msdu_end.

Since original helper is not used for now, add __maybe_unused
attribute to make GCC happy.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
---
v2:
 1. add a new helper according to Vasanth's comment.
 2. change to use __le16_to_cpu instead of __le32_to_cpu

 drivers/net/wireless/ath/ath12k/dp_rx.c | 13 ++++++++++---
 drivers/net/wireless/ath/ath12k/hal.c   | 14 ++++++++++++++
 drivers/net/wireless/ath/ath12k/hal.h   |  1 +
 3 files changed, 25 insertions(+), 3 deletions(-)


base-commit: 858793444bc68ad8e8fa73d20055aa8da8ab591b

Comments

Kalle Valo May 26, 2023, 9:32 a.m. UTC | #1 
Baochen Qiang <quic_bqiang@quicinc.com> writes:

> We are seeing a very low TCP throughput testing with some specific
> tools. This is because for sub-frames of an AMSDU, MCBC flag in
> mpdu_start may be not valid, and as a result those frames would be
> dropped by kernel.
>
> Add a new helper to get it from msdu_end.
>
> Since original helper is not used for now, add __maybe_unused
> attribute to make GCC happy.
>
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> ---
> v2:
>  1. add a new helper according to Vasanth's comment.
>  2. change to use __le16_to_cpu instead of __le32_to_cpu
>
>  drivers/net/wireless/ath/ath12k/dp_rx.c | 13 ++++++++++---
>  drivers/net/wireless/ath/ath12k/hal.c   | 14 ++++++++++++++
>  drivers/net/wireless/ath/ath12k/hal.h   |  1 +
>  3 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
> index 8c8162fbe5c6..972034b7f159 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
> @@ -193,13 +193,20 @@ static void ath12k_dp_rxdesc_set_msdu_len(struct ath12k_base *ab,
>  	ab->hw_params->hal_ops->rx_desc_set_msdu_len(desc, len);
>  }
>  
> -static bool ath12k_dp_rx_h_is_mcbc(struct ath12k_base *ab,
> -				   struct hal_rx_desc *desc)
> +static __maybe_unused bool ath12k_dp_rx_h_is_mcbc(struct ath12k_base *ab,
> +						  struct hal_rx_desc *desc)
>  {
>  	return (ath12k_dp_rx_h_first_msdu(ab, desc) &&
>  		ab->hw_params->hal_ops->rx_desc_is_mcbc(desc));
>  }

Using __maybe_unused is usually a bad idea, it should be used only on
very special cases.

But why do you leave struct hal_ops::rx_desc_is_mcbc? Nobody is using it
and we should not have dead code lying around.
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
index 8c8162fbe5c6..972034b7f159 100644
--- a/drivers/net/wireless/ath/ath12k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
@@ -193,13 +193,20 @@  static void ath12k_dp_rxdesc_set_msdu_len(struct ath12k_base *ab,
 	ab->hw_params->hal_ops->rx_desc_set_msdu_len(desc, len);
 }
 
-static bool ath12k_dp_rx_h_is_mcbc(struct ath12k_base *ab,
-				   struct hal_rx_desc *desc)
+static __maybe_unused bool ath12k_dp_rx_h_is_mcbc(struct ath12k_base *ab,
+						  struct hal_rx_desc *desc)
 {
 	return (ath12k_dp_rx_h_first_msdu(ab, desc) &&
 		ab->hw_params->hal_ops->rx_desc_is_mcbc(desc));
 }
 
+static bool ath12k_dp_rx_h_is_da_mcbc(struct ath12k_base *ab,
+				      struct hal_rx_desc *desc)
+{
+	return (ath12k_dp_rx_h_first_msdu(ab, desc) &&
+		ab->hw_params->hal_ops->rx_desc_is_da_mcbc(desc));
+}
+
 static bool ath12k_dp_rxdesc_mac_addr2_valid(struct ath12k_base *ab,
 					     struct hal_rx_desc *desc)
 {
@@ -2208,7 +2215,7 @@  static void ath12k_dp_rx_h_mpdu(struct ath12k *ar,
 
 	/* PN for multicast packets will be checked in mac80211 */
 	rxcb = ATH12K_SKB_RXCB(msdu);
-	fill_crypto_hdr = ath12k_dp_rx_h_is_mcbc(ar->ab, rx_desc);
+	fill_crypto_hdr = ath12k_dp_rx_h_is_da_mcbc(ar->ab, rx_desc);
 	rxcb->is_mcbc = fill_crypto_hdr;
 
 	if (rxcb->is_mcbc)
diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
index 0ec53afe9915..b25fa1200656 100644
--- a/drivers/net/wireless/ath/ath12k/hal.c
+++ b/drivers/net/wireless/ath/ath12k/hal.c
@@ -453,6 +453,12 @@  static bool ath12k_hw_qcn9274_rx_desc_is_mcbc(struct hal_rx_desc *desc)
 	       RX_MPDU_START_INFO6_MCAST_BCAST;
 }
 
+static bool ath12k_hw_qcn9274_rx_desc_is_da_mcbc(struct hal_rx_desc *desc)
+{
+	return __le16_to_cpu(desc->u.qcn9274.msdu_end.info5) &
+	       RX_MSDU_END_INFO5_DA_IS_MCBC;
+}
+
 static void ath12k_hw_qcn9274_rx_desc_get_dot11_hdr(struct hal_rx_desc *desc,
 						    struct ieee80211_hdr *hdr)
 {
@@ -709,6 +715,7 @@  const struct hal_ops hal_qcn9274_ops = {
 	.rx_desc_mac_addr2_valid = ath12k_hw_qcn9274_rx_desc_mac_addr2_valid,
 	.rx_desc_mpdu_start_addr2 = ath12k_hw_qcn9274_rx_desc_mpdu_start_addr2,
 	.rx_desc_is_mcbc = ath12k_hw_qcn9274_rx_desc_is_mcbc,
+	.rx_desc_is_da_mcbc = ath12k_hw_qcn9274_rx_desc_is_da_mcbc,
 	.rx_desc_get_dot11_hdr = ath12k_hw_qcn9274_rx_desc_get_dot11_hdr,
 	.rx_desc_get_crypto_header = ath12k_hw_qcn9274_rx_desc_get_crypto_hdr,
 	.rx_desc_get_mpdu_frame_ctl = ath12k_hw_qcn9274_rx_desc_get_mpdu_frame_ctl,
@@ -893,6 +900,12 @@  static bool ath12k_hw_wcn7850_rx_desc_is_mcbc(struct hal_rx_desc *desc)
 	       RX_MPDU_START_INFO6_MCAST_BCAST;
 }
 
+static bool ath12k_hw_wcn7850_rx_desc_is_da_mcbc(struct hal_rx_desc *desc)
+{
+	return __le16_to_cpu(desc->u.wcn7850.msdu_end.info5) &
+	       RX_MSDU_END_INFO5_DA_IS_MCBC;
+}
+
 static void ath12k_hw_wcn7850_rx_desc_get_dot11_hdr(struct hal_rx_desc *desc,
 						    struct ieee80211_hdr *hdr)
 {
@@ -1164,6 +1177,7 @@  const struct hal_ops hal_wcn7850_ops = {
 	.rx_desc_mac_addr2_valid = ath12k_hw_wcn7850_rx_desc_mac_addr2_valid,
 	.rx_desc_mpdu_start_addr2 = ath12k_hw_wcn7850_rx_desc_mpdu_start_addr2,
 	.rx_desc_is_mcbc = ath12k_hw_wcn7850_rx_desc_is_mcbc,
+	.rx_desc_is_da_mcbc = ath12k_hw_wcn7850_rx_desc_is_da_mcbc,
 	.rx_desc_get_dot11_hdr = ath12k_hw_wcn7850_rx_desc_get_dot11_hdr,
 	.rx_desc_get_crypto_header = ath12k_hw_wcn7850_rx_desc_get_crypto_hdr,
 	.rx_desc_get_mpdu_frame_ctl = ath12k_hw_wcn7850_rx_desc_get_mpdu_frame_ctl,
diff --git a/drivers/net/wireless/ath/ath12k/hal.h b/drivers/net/wireless/ath/ath12k/hal.h
index 0d4fa12ea622..88c0d022f5f6 100644
--- a/drivers/net/wireless/ath/ath12k/hal.h
+++ b/drivers/net/wireless/ath/ath12k/hal.h
@@ -1064,6 +1064,7 @@  struct hal_ops {
 	bool (*rx_desc_mac_addr2_valid)(struct hal_rx_desc *desc);
 	u8* (*rx_desc_mpdu_start_addr2)(struct hal_rx_desc *desc);
 	bool (*rx_desc_is_mcbc)(struct hal_rx_desc *desc);
+	bool (*rx_desc_is_da_mcbc)(struct hal_rx_desc *desc);
 	void (*rx_desc_get_dot11_hdr)(struct hal_rx_desc *desc,
 				      struct ieee80211_hdr *hdr);
 	u16 (*rx_desc_get_mpdu_frame_ctl)(struct hal_rx_desc *desc);