ath10k: Fix warning due to msdu limit error
diff mbox series

Message ID 1542613703-9704-1-git-send-email-bperumal@codeaurora.org
State New
Headers show
Series
  • ath10k: Fix warning due to msdu limit error
Related show

Commit Message

Bhagavathi Perumal S Nov. 19, 2018, 7:48 a.m. UTC
Some hardwares variants(QCA99x0) are limiting msdu deaggregation with
some threshold value(default limit in QCA99x0 is 64 msdus), it was introduced to
avoid excessive MSDU-deaggregation in error cases. When number of sub frames
exceeds the limit, target hardware will send all msdus starting from present
msdu in RAW format as a single msdu packet and it will be indicated with
error status bit "RX_MSDU_END_INFO0_MSDU_LIMIT_ERR" set in rx descriptor.
This msdu frame is a partial raw MSDU and does't have first msdu and ieee80211
header. It caused below warning message.

[  320.151332] ------------[ cut here ]------------
[  320.155006] WARNING: CPU: 0 PID: 3 at drivers/net/wireless/ath/ath10k/htt_rx.c:1188

In our issue case, MSDU limit error happened due to FCS error and generated
this warning message.

This fixes the warning by handling the MSDU limit error. If msdu limit error
happens, driver adds first MSDU's ieee80211 header and sets A-MSDU present bit
in QOS header so that upper layer processes this frame if it is valid or drop it
if FCS error set. And removed the warning message, hence partial msdus without
first msdu is expected in msdu limit error cases.

Tested on QCA9984, Firmware 10.4-3.6-00104

Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c  | 41 ++++++++++++++++++++++++++++---
 drivers/net/wireless/ath/ath10k/hw.c      |  7 ++++++
 drivers/net/wireless/ath/ath10k/hw.h      | 10 ++++++++
 drivers/net/wireless/ath/ath10k/rx_desc.h |  7 ++++++
 4 files changed, 61 insertions(+), 4 deletions(-)

Comments

Kalle Valo Dec. 20, 2018, 12:09 p.m. UTC | #1
Bhagavathi Perumal S <bperumal@codeaurora.org> writes:

> Some hardwares variants(QCA99x0) are limiting msdu deaggregation with
> some threshold value(default limit in QCA99x0 is 64 msdus), it was introduced to
> avoid excessive MSDU-deaggregation in error cases. When number of sub frames
> exceeds the limit, target hardware will send all msdus starting from present
> msdu in RAW format as a single msdu packet and it will be indicated with
> error status bit "RX_MSDU_END_INFO0_MSDU_LIMIT_ERR" set in rx descriptor.
> This msdu frame is a partial raw MSDU and does't have first msdu and ieee80211
> header. It caused below warning message.
>
> [  320.151332] ------------[ cut here ]------------
> [  320.155006] WARNING: CPU: 0 PID: 3 at drivers/net/wireless/ath/ath10k/htt_rx.c:1188
>
> In our issue case, MSDU limit error happened due to FCS error and generated
> this warning message.
>
> This fixes the warning by handling the MSDU limit error. If msdu limit error
> happens, driver adds first MSDU's ieee80211 header and sets A-MSDU present bit
> in QOS header so that upper layer processes this frame if it is valid or drop it
> if FCS error set. And removed the warning message, hence partial msdus without
> first msdu is expected in msdu limit error cases.
>
> Tested on QCA9984, Firmware 10.4-3.6-00104
>
> Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org>

This introduced a new checkpatch warning:

drivers/net/wireless/ath/ath10k/rx_desc.h:681: please, no space before tabs

I fixed that in the pending branch.
Kalle Valo Dec. 20, 2018, 5:02 p.m. UTC | #2
Bhagavathi Perumal S <bperumal@codeaurora.org> wrote:

> Some hardwares variants (QCA99x0) are limiting msdu deaggregation with
> some threshold value(default limit in QCA99x0 is 64 msdus), it was introduced to
> avoid excessive MSDU-deaggregation in error cases. When number of sub frames
> exceeds the limit, target hardware will send all msdus starting from present
> msdu in RAW format as a single msdu packet and it will be indicated with
> error status bit "RX_MSDU_END_INFO0_MSDU_LIMIT_ERR" set in rx descriptor.
> This msdu frame is a partial raw MSDU and does't have first msdu and ieee80211
> header. It caused below warning message.
> 
> [  320.151332] ------------[ cut here ]------------
> [  320.155006] WARNING: CPU: 0 PID: 3 at drivers/net/wireless/ath/ath10k/htt_rx.c:1188
> 
> In our issue case, MSDU limit error happened due to FCS error and generated
> this warning message.
> 
> This fixes the warning by handling the MSDU limit error. If msdu limit error
> happens, driver adds first MSDU's ieee80211 header and sets A-MSDU present bit
> in QOS header so that upper layer processes this frame if it is valid or drop it
> if FCS error set. And removed the warning message, hence partial msdus without
> first msdu is expected in msdu limit error cases.
> 
> Tested on QCA9984, Firmware 10.4-3.6-00104
> 
> Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

a2864772f33a ath10k: fix warning due to msdu limit error

Patch
diff mbox series

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 984b045..a42f4d9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1159,7 +1159,8 @@  static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar,
 					struct sk_buff *msdu,
 					struct ieee80211_rx_status *status,
 					enum htt_rx_mpdu_encrypt_type enctype,
-					bool is_decrypted)
+					bool is_decrypted,
+					const u8 first_hdr[64])
 {
 	struct ieee80211_hdr *hdr;
 	struct htt_rx_desc *rxd;
@@ -1167,6 +1168,9 @@  static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar,
 	size_t crypto_len;
 	bool is_first;
 	bool is_last;
+	bool msdu_limit_err;
+	int bytes_aligned = ar->hw_params.decap_align_bytes;
+	u8 *qos;
 
 	rxd = (void *)msdu->data - sizeof(*rxd);
 	is_first = !!(rxd->msdu_end.common.info0 &
@@ -1184,16 +1188,45 @@  static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar,
 	 * [FCS] <-- at end, needs to be trimmed
 	 */
 
+	/* Some hardwares(QCA99x0 variants) limit number of msdus in a-msdu when
+	 * deaggregate, so that unwanted MSDU-deaggregation is avoided for
+	 * error packets. If limit exceeds, hw sends all remaining MSDUs as
+	 * a single last MSDU with this msdu limit error set.
+	 */
+	msdu_limit_err = ath10k_rx_desc_msdu_limit_error(&ar->hw_params, rxd);
+
+	/* If MSDU limit error happens, then don't warn on, the partial raw MSDU
+	 * without first MSDU is expected in that case, and handled later here.
+	 */
 	/* This probably shouldn't happen but warn just in case */
-	if (WARN_ON_ONCE(!is_first))
+	if (WARN_ON_ONCE(!is_first && !msdu_limit_err))
 		return;
 
 	/* This probably shouldn't happen but warn just in case */
-	if (WARN_ON_ONCE(!(is_first && is_last)))
+	if (WARN_ON_ONCE(!(is_first && is_last) && !msdu_limit_err))
 		return;
 
 	skb_trim(msdu, msdu->len - FCS_LEN);
 
+	/* Push original 80211 header */
+	if (unlikely(msdu_limit_err)) {
+		hdr = (struct ieee80211_hdr *)first_hdr;
+		hdr_len = ieee80211_hdrlen(hdr->frame_control);
+		crypto_len = ath10k_htt_rx_crypto_param_len(ar, enctype);
+
+		if (ieee80211_is_data_qos(hdr->frame_control)) {
+			qos = ieee80211_get_qos_ctl(hdr);
+			qos[0] |= IEEE80211_QOS_CTL_A_MSDU_PRESENT;
+		}
+
+		if (crypto_len)
+			memcpy(skb_push(msdu, crypto_len),
+			       (void *)hdr + round_up(hdr_len, bytes_aligned),
+			       crypto_len);
+
+		memcpy(skb_push(msdu, hdr_len), hdr, hdr_len);
+	}
+
 	/* In most cases this will be true for sniffed frames. It makes sense
 	 * to deliver them as-is without stripping the crypto param. This is
 	 * necessary for software based decryption.
@@ -1467,7 +1500,7 @@  static void ath10k_htt_rx_h_undecap(struct ath10k *ar,
 	switch (decap) {
 	case RX_MSDU_DECAP_RAW:
 		ath10k_htt_rx_h_undecap_raw(ar, msdu, status, enctype,
-					    is_decrypted);
+					    is_decrypted, first_hdr);
 		break;
 	case RX_MSDU_DECAP_NATIVE_WIFI:
 		ath10k_htt_rx_h_undecap_nwifi(ar, msdu, status, first_hdr,
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index af8ae81..61ecf93 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -1119,8 +1119,15 @@  static int ath10k_qca99x0_rx_desc_get_l3_pad_bytes(struct htt_rx_desc *rxd)
 		  RX_MSDU_END_INFO1_L3_HDR_PAD);
 }
 
+static bool ath10k_qca99x0_rx_desc_msdu_limit_error(struct htt_rx_desc *rxd)
+{
+	return !!(rxd->msdu_end.common.info0 &
+		  __cpu_to_le32(RX_MSDU_END_INFO0_MSDU_LIMIT_ERR));
+}
+
 const struct ath10k_hw_ops qca99x0_ops = {
 	.rx_desc_get_l3_pad_bytes = ath10k_qca99x0_rx_desc_get_l3_pad_bytes,
+	.rx_desc_get_msdu_limit_error = ath10k_qca99x0_rx_desc_msdu_limit_error,
 };
 
 const struct ath10k_hw_ops qca6174_ops = {
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 1b5da27..e50a8dc5 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -624,6 +624,7 @@  struct ath10k_hw_ops {
 	int (*rx_desc_get_l3_pad_bytes)(struct htt_rx_desc *rxd);
 	void (*set_coverage_class)(struct ath10k *ar, s16 value);
 	int (*enable_pll_clk)(struct ath10k *ar);
+	bool (*rx_desc_get_msdu_limit_error)(struct htt_rx_desc *rxd);
 };
 
 extern const struct ath10k_hw_ops qca988x_ops;
@@ -642,6 +643,15 @@  struct ath10k_hw_ops {
 	return 0;
 }
 
+static inline bool
+ath10k_rx_desc_msdu_limit_error(struct ath10k_hw_params *hw,
+				struct htt_rx_desc *rxd)
+{
+	if (hw->hw_ops->rx_desc_get_msdu_limit_error)
+		return hw->hw_ops->rx_desc_get_msdu_limit_error(rxd);
+	return false;
+}
+
 /* Target specific defines for MAIN firmware */
 #define TARGET_NUM_VDEVS			8
 #define TARGET_NUM_PEER_AST			2
diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
index 310674d..b626cc2 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
@@ -572,6 +572,7 @@  struct rx_msdu_start {
 #define RX_MSDU_END_INFO0_REPORTED_MPDU_LENGTH_LSB  0
 #define RX_MSDU_END_INFO0_FIRST_MSDU                BIT(14)
 #define RX_MSDU_END_INFO0_LAST_MSDU                 BIT(15)
+#define RX_MSDU_END_INFO0_MSDU_LIMIT_ERR            BIT(18)
 #define RX_MSDU_END_INFO0_PRE_DELIM_ERR             BIT(30)
 #define RX_MSDU_END_INFO0_RESERVED_3B               BIT(31)
 
@@ -676,6 +677,12 @@  struct rx_msdu_end {
  *		Indicates the last MSDU of the A-MSDU.  MPDU end status is
  *		only valid when last_msdu is set.
  *
+ *msdu_limit_error
+ * 		Indicates that the MSDU threshold was exceeded and thus
+ *		all the rest of the MSDUs will not be scattered and
+ *		will not be decapsulated but will be received in RAW format
+ *		as a single MSDU buffer.
+ *
  *reserved_3a
  *		Reserved: HW should fill with zero.  FW should ignore.
  *