[PATCH] ath10k: strip qos data bit always

[PATCH] ath10k: strip qos data bit always

[Michal Kazior]

NativeWifi tx mode expects QoS Data frames to be
delivered as Data frames with QoS part (e.g. tid)
being delievered out-of-band in fw tx command.

The QoS bit wasn't stripped before submitting to
firmware.

Stripping fixes two known problems:

 * qca6174 IOT with some APs, e.g.
   Cisco AIR-AP 1252 (which would crash after
   ath10k association). Some ath9k APs would
   crash as well.

 * sniffing own tx frames via radiotap because,
   e.g. wireshark was seeing QoS bit set but
   since QoS Control was stripped in ath10k it
   would parse beginning of LLC/SNAP

>From debugability point of view this removes the
ability to distinguish QoS from non-QoS frames
when sniffing own tx via radiotap. On the other
hand frames can be now parsed correctly without
special software modification.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 1138f4f..625e2cb 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2463,16 +2463,15 @@ static void ath10k_tx_h_nwifi(struct ieee80211_hw *hw, struct sk_buff *skb)
 		skb->data, (void *)qos_ctl - (void *)skb->data);
 	skb_pull(skb, IEEE80211_QOS_CTL_LEN);
 
-	/* Fw/Hw generates a corrupted QoS Control Field for QoS NullFunc
-	 * frames. Powersave is handled by the fw/hw so QoS NyllFunc frames are
-	 * used only for CQM purposes (e.g. hostapd station keepalive ping) so
-	 * it is safe to downgrade to NullFunc.
+	/* Some firmware revisions don't handle sending QoS NullFunc well.
+	 * These frames are mainly used for CQM purposes so it doesn't really
+	 * matter whether QoS NullFunc or NullFunc are sent.
 	 */
 	hdr = (void *)skb->data;
-	if (ieee80211_is_qos_nullfunc(hdr->frame_control)) {
-		hdr->frame_control &= ~__cpu_to_le16(IEEE80211_STYPE_QOS_DATA);
+	if (ieee80211_is_qos_nullfunc(hdr->frame_control))
 		cb->htt.tid = HTT_DATA_TX_EXT_TID_NON_QOS_MCAST_BCAST;
-	}
+
+	hdr->frame_control &= ~__cpu_to_le16(IEEE80211_STYPE_QOS_DATA);
 }
 
 static void ath10k_tx_h_add_p2p_noa_ie(struct ath10k *ar,
-- 
1.8.5.3

Re: [PATCH] ath10k: strip qos data bit always

[Johannes Berg]

On Wed, 2015-03-11 at 14:25 +0100, Michal Kazior wrote:
> NativeWifi tx mode expects QoS Data frames to be
> delivered as Data frames with QoS part (e.g. tid)
> being delievered out-of-band in fw tx command.
> 
> The QoS bit wasn't stripped before submitting to
> firmware.
> 
> Stripping fixes two known problems:
> 
>  * qca6174 IOT with some APs, e.g.
>    Cisco AIR-AP 1252 (which would crash after
>    ath10k association). Some ath9k APs would
>    crash as well.

It would probably be interesting to figure out why and fix that - this
is clearly a major issue.

johannes

Re: [PATCH] ath10k: strip qos data bit always

[Michal Kazior]

On 11 March 2015 at 17:22, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2015-03-11 at 14:25 +0100, Michal Kazior wrote:
>> NativeWifi tx mode expects QoS Data frames to be
>> delivered as Data frames with QoS part (e.g. tid)
>> being delievered out-of-band in fw tx command.
>>
>> The QoS bit wasn't stripped before submitting to
>> firmware.
>>
>> Stripping fixes two known problems:
>>
>>  * qca6174 IOT with some APs, e.g.
>>    Cisco AIR-AP 1252 (which would crash after
>>    ath10k association). Some ath9k APs would
>>    crash as well.
>
> It would probably be interesting to figure out why and fix that - this
> is clearly a major issue.

Good point. The patch was originally just a small fix for sniffing but
it happened to fix the IOT problem that we started seeing with the new
chip fw as well.

I believe that if 11n was disabled on APs the problem did not
reproduce. I think ath9k was crashing somewhere along the BA session
teardown (as per kernel call trace). The ath9k codebase from what I
know was rather old (some custom 3.4 fork) so perhaps it was a race of
some sort that has been fixed long since.

I'll try to get some sniff logs and take a look at this more closely.


Michał

Re: [PATCH] ath10k: strip qos data bit always

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> NativeWifi tx mode expects QoS Data frames to be
> delivered as Data frames with QoS part (e.g. tid)
> being delievered out-of-band in fw tx command.
>
> The QoS bit wasn't stripped before submitting to
> firmware.
>
> Stripping fixes two known problems:
>
>  * qca6174 IOT with some APs, e.g.
>    Cisco AIR-AP 1252 (which would crash after
>    ath10k association). Some ath9k APs would
>    crash as well.
>
>  * sniffing own tx frames via radiotap because,
>    e.g. wireshark was seeing QoS bit set but
>    since QoS Control was stripped in ath10k it
>    would parse beginning of LLC/SNAP
>
>>From debugability point of view this removes the
> ability to distinguish QoS from non-QoS frames
> when sniffing own tx via radiotap. On the other
> hand frames can be now parsed correctly without
> special software modification.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Thanks, applied.

-- 
Kalle Valo