All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bhagavathi Perumal S <bperumal@codeaurora.org>
To: ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org,
	Bhagavathi Perumal S <bperumal@codeaurora.org>
Subject: [PATCH] ath10k: Fix warning due to msdu limit error
Date: Mon, 19 Nov 2018 13:18:23 +0530	[thread overview]
Message-ID: <1542613703-9704-1-git-send-email-bperumal@codeaurora.org> (raw)

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(-)

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.
  *
-- 
1.9.1


WARNING: multiple messages have this Message-ID (diff)
From: Bhagavathi Perumal S <bperumal@codeaurora.org>
To: ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org,
	Bhagavathi Perumal S <bperumal@codeaurora.org>
Subject: [PATCH] ath10k: Fix warning due to msdu limit error
Date: Mon, 19 Nov 2018 13:18:23 +0530	[thread overview]
Message-ID: <1542613703-9704-1-git-send-email-bperumal@codeaurora.org> (raw)

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(-)

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.
  *
-- 
1.9.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

             reply	other threads:[~2018-11-19  7:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19  7:48 Bhagavathi Perumal S [this message]
2018-11-19  7:48 ` [PATCH] ath10k: Fix warning due to msdu limit error Bhagavathi Perumal S
2018-12-20 12:09 ` Kalle Valo
2018-12-20 12:09   ` Kalle Valo
2018-12-20 17:02 ` Kalle Valo
2018-12-20 17:02 ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1542613703-9704-1-git-send-email-bperumal@codeaurora.org \
    --to=bperumal@codeaurora.org \
    --cc=ath10k@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.