[PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[Karthikeyan Periyasamy]

This reverts commit 55884c045d31a29cf69db8332d1064a1b61dd159.

When Ath10k is in AP mode and an unassociated STA sends a VHT action frame
(Operating Mode Notification for the NSS change) periodically to AP this causes
ath10k to call ath10k_station_assoc() which sends WMI_PEER_ASSOC_CMDID during
NSS update. Over the time (with a certain client it can happen within 15 mins
when there are over 500 of these VHT action frames) continuous calls of
WMI_PEER_ASSOC_CMDID cause firmware to assert due to resource exhaust.

To my knowledge setting WMI_PEER_NSS peer param itself enough to handle NSS
updates and no need to call ath10k_station_assoc(). So revert the original
commit from 2014 as it's unclear why the change was really needed.
Now the firmware assert doesn't happen anymore.

Issue observed in QCA9984 platform with firmware version:10.4-3.5.3-00053.
This Change tested in QCA9984 with firmware version: 10.4-3.5.3-00053 and
QCA988x platform with firmware version: 10.2.4-1.0-00036.

Signed-off-by: Karthikeyan Periyasamy <periyasa@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/mac.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index ebb3f1b..800a86e 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -6028,9 +6028,8 @@ static void ath10k_sta_rc_update_wk(struct work_struct *wk)
 				    sta->addr, smps, err);
 	}
 
-	if (changed & IEEE80211_RC_SUPP_RATES_CHANGED ||
-	    changed & IEEE80211_RC_NSS_CHANGED) {
-		ath10k_dbg(ar, ATH10K_DBG_MAC, "mac update sta %pM supp rates/nss\n",
+	if (changed & IEEE80211_RC_SUPP_RATES_CHANGED) {
+		ath10k_dbg(ar, ATH10K_DBG_MAC, "mac update sta %pM supp rates\n",
 			   sta->addr);
 
 		err = ath10k_station_assoc(ar, arvif->vif, sta, true);
-- 
1.9.1

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[Adrian Chadd]

hi!

On 25 February 2018 at 22:16, Karthikeyan Periyasamy
<periyasa@codeaurora.org> wrote:
> This reverts commit 55884c045d31a29cf69db8332d1064a1b61dd159.
>
> When Ath10k is in AP mode and an unassociated STA sends a VHT action frame
> (Operating Mode Notification for the NSS change) periodically to AP this causes
> ath10k to call ath10k_station_assoc() which sends WMI_PEER_ASSOC_CMDID during
> NSS update. Over the time (with a certain client it can happen within 15 mins
> when there are over 500 of these VHT action frames) continuous calls of
> WMI_PEER_ASSOC_CMDID cause firmware to assert due to resource exhaust.
>
> To my knowledge setting WMI_PEER_NSS peer param itself enough to handle NSS
> updates and no need to call ath10k_station_assoc(). So revert the original
> commit from 2014 as it's unclear why the change was really needed.
> Now the firmware assert doesn't happen anymore.
>
> Issue observed in QCA9984 platform with firmware version:10.4-3.5.3-00053.
> This Change tested in QCA9984 with firmware version: 10.4-3.5.3-00053 and
> QCA988x platform with firmware version: 10.2.4-1.0-00036.

Did you test this on any of the other major firmware variants? I
wonder if it snuck in because of some firmware quirk in something way
before dakota/cascade and 10.4 were a thing.

Eg, Peregrine? Rome? Maybe even earlier Beeliner, just to double check?

Thanks,



-adrian

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[periyasa]

On 2018-02-26 14:15, Adrian Chadd wrote:
> hi!
> 
> On 25 February 2018 at 22:16, Karthikeyan Periyasamy
> <periyasa@codeaurora.org> wrote:
>> This reverts commit 55884c045d31a29cf69db8332d1064a1b61dd159.
>> 
>> When Ath10k is in AP mode and an unassociated STA sends a VHT action 
>> frame
>> (Operating Mode Notification for the NSS change) periodically to AP 
>> this causes
>> ath10k to call ath10k_station_assoc() which sends WMI_PEER_ASSOC_CMDID 
>> during
>> NSS update. Over the time (with a certain client it can happen within 
>> 15 mins
>> when there are over 500 of these VHT action frames) continuous calls 
>> of
>> WMI_PEER_ASSOC_CMDID cause firmware to assert due to resource exhaust.
>> 
>> To my knowledge setting WMI_PEER_NSS peer param itself enough to 
>> handle NSS
>> updates and no need to call ath10k_station_assoc(). So revert the 
>> original
>> commit from 2014 as it's unclear why the change was really needed.
>> Now the firmware assert doesn't happen anymore.
>> 
>> Issue observed in QCA9984 platform with firmware 
>> version:10.4-3.5.3-00053.
>> This Change tested in QCA9984 with firmware version: 10.4-3.5.3-00053 
>> and
>> QCA988x platform with firmware version: 10.2.4-1.0-00036.
> 
> Did you test this on any of the other major firmware variants? I
> wonder if it snuck in because of some firmware quirk in something way
> before dakota/cascade and 10.4 were a thing.
> 
> Eg, Peregrine? Rome? Maybe even earlier Beeliner, just to double check?
> 
Yes. I tested this on peregrine, Beeliner, Dakota and Cascade. This code 
was introduced before Rome.

Thanks,
Karthikeyan.

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[Ben Greear]

On 02/25/2018 10:16 PM, Karthikeyan Periyasamy wrote:
> This reverts commit 55884c045d31a29cf69db8332d1064a1b61dd159.
>
> When Ath10k is in AP mode and an unassociated STA sends a VHT action frame
> (Operating Mode Notification for the NSS change) periodically to AP this causes
> ath10k to call ath10k_station_assoc() which sends WMI_PEER_ASSOC_CMDID during
> NSS update. Over the time (with a certain client it can happen within 15 mins
> when there are over 500 of these VHT action frames) continuous calls of
> WMI_PEER_ASSOC_CMDID cause firmware to assert due to resource exhaust.

Can you share exactly which resource the firmware ran out of?  It would seem to
be a FW bug if it is leaking, so maybe it can be fixed as well...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[periyasa]

Hi,


> Can you share exactly which resource the firmware ran out of?  It would 
> seem to
> be a FW bug if it is leaking, so maybe it can be fixed as well...
> 

Firmware have total user_id = 528 (512 clients + 16 VAPs). Each user_id 
is allocated to peer when Firmware receive the WMI_PEER_ASSOC_CMDID 
request from host driver. Firmware free the user_id in peer delete 
operation.

Regards,
Karthikeyan P.

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[Adrian Chadd]

hi,

so it's going to eventually leak? Can we fix the firmware bug too? :)


-a


On 26 February 2018 at 09:06,  <periyasa@codeaurora.org> wrote:
> Hi,
>
>
>> Can you share exactly which resource the firmware ran out of?  It would
>> seem to
>> be a FW bug if it is leaking, so maybe it can be fixed as well...
>>
>
> Firmware have total user_id = 528 (512 clients + 16 VAPs). Each user_id is
> allocated to peer when Firmware receive the WMI_PEER_ASSOC_CMDID request
> from host driver. Firmware free the user_id in peer delete operation.
>
> Regards,
> Karthikeyan P.
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[periyasa]

Hi,

This is not a bug, Firmware treats every WMI_PEER_ASSOC_CMDID as a new 
peer assoc. so driver should not give WMI_PEER_ASSOC_CMDID for the 
already associated peer.
For the NSS update, wmi_peer_set_param is enough to update the new NSS 
value.

Regards,
Karthikeyan P.

On 2018-02-27 00:51, Adrian Chadd wrote:
> hi,
> 
> so it's going to eventually leak? Can we fix the firmware bug too? :)
> 
> 
> -a
> 
> 
> On 26 February 2018 at 09:06,  <periyasa@codeaurora.org> wrote:
>> Hi,
>> 
>> 
>>> Can you share exactly which resource the firmware ran out of?  It 
>>> would
>>> seem to
>>> be a FW bug if it is leaking, so maybe it can be fixed as well...
>>> 
>> 
>> Firmware have total user_id = 528 (512 clients + 16 VAPs). Each 
>> user_id is
>> allocated to peer when Firmware receive the WMI_PEER_ASSOC_CMDID 
>> request
>> from host driver. Firmware free the user_id in peer delete operation.
>> 
>> Regards,
>> Karthikeyan P.
>> 
>> _______________________________________________
>> ath10k mailing list
>> ath10k@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] Revert "ath10k: send (re)assoc peer command when NSS changed"

[Peter Oh]

> This reverts commit 55884c045d31a29cf69db8332d1064a1b61dd159.
>
> When Ath10k is in AP mode and an unassociated STA sends a VHT action frame
> (Operating Mode Notification for the NSS change) periodically to AP this causes
> ath10k to call ath10k_station_assoc() which sends WMI_PEER_ASSOC_CMDID during
> NSS update. Over the time (with a certain client it can happen within 15 mins
> when there are over 500 of these VHT action frames) continuous calls of
> WMI_PEER_ASSOC_CMDID cause firmware to assert due to resource exhaust.
Can you include the FW assert message in this commit?

Thanks,
Peter

fixed bit rate 9Mbps set as 24Mbps in g mode(legacy)

[KAVITA MATHUR]

Hi,

I have configured AP in g mode and tested legacy rates.All basic rates and supported
rates(1,2,5.5,11,6,9,12,18,24,36,48,54) are set except 9Mbps.

When fixed bit rate 9Mbps is set, it gets set as 24 Mbps. This issue has been observed
in backports-4.4.2-1 as well as latest git version of ath10k driver.


Is anybody aware of this issue? How can I fix this rate 9Mbps?


Thanks & Regards,
Kavita

Re: fixed bit rate 9Mbps set as 24Mbps in g mode(legacy)

[Ben Greear]

On 02/27/2018 12:49 AM, KAVITA MATHUR wrote:
> Hi,
>
> I have configured AP in g mode and tested legacy rates.All basic rates and supported
> rates(1,2,5.5,11,6,9,12,18,24,36,48,54) are set except 9Mbps.
>
> When fixed bit rate 9Mbps is set, it gets set as 24 Mbps. This issue has been observed
> in backports-4.4.2-1 as well as latest git version of ath10k driver.
>
>
> Is anybody aware of this issue? How can I fix this rate 9Mbps?

You should specify which firmware/chipset you are using, since this might
easily be a firmware issue.

And, how are you determining that 'it gets set to 24Mbps'.  Is that the encoding
you see in a wifi sniff, or is the driver being set to 24Mbps improperly when
you use iw (or whatever) to try configuring the rate?

Recent upstream mac80211 has broken the ability to set a single tx rate as well,
last I checked.

Thanks,
Ben

>
>
> Thanks & Regards,
> Kavita
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe backports" in
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: fixed bit rate 9Mbps set as 24Mbps in g mode(legacy)

[Ben Greear]

On 02/27/2018 08:03 PM, KAVITA MATHUR wrote:
>
>
>
> On Tue, 27 Feb 2018 08:41:50 -0800, Ben Greear wrote
>> On 02/27/2018 12:49 AM, KAVITA MATHUR wrote:
>>> Hi,
>>>
>>> I have configured AP in g mode and tested legacy rates.All basic rates and supported
>>> rates(1,2,5.5,11,6,9,12,18,24,36,48,54) are set except 9Mbps.
>>>
>>> When fixed bit rate 9Mbps is set, it gets set as 24 Mbps. This issue has been observed
>>> in backports-4.4.2-1 as well as latest git version of ath10k driver.
>>>
>>>
>>> Is anybody aware of this issue? How can I fix this rate 9Mbps?
>>
>> You should specify which firmware/chipset you are using, since this might
>> easily be a firmware issue.
>
>
> Chipset : QCA988x
> ath10k version : git 16 oct 2017(v4.14-rc2-1-24-gc6db471)
> firmware version : firmware-5.bin_10.2.4.70.66
> Kernel version : 3.12
>
>> And, how are you determining that 'it gets set to 24Mbps'.  Is that the
>> encoding you see in a wifi sniff, or is the driver being set to 24Mbps
>> improperly when you use iw (or whatever) to try configuring the rate?
>
> I have used command "iw wlan0 station dump" to see set data rate as well as I have
> sniffer. Measured data throughput is also as per 24Mbps.

I use more recent (and hacked) kernels/drivers, and my own wave-1 firmware.  I did have
a bug in some code I wrote because the 48Mbps rate code is '0x0', and some of
my code was ignoring it thinking it was not properly set.  I did see correct
on-air 48Mbps encoding rates though.

Possibly you have similar problems with rate-code 0x0.

Have you tried doing a capture with an ath9k or some other non-ath10k driver
to see if it is really 24Mbps on air?

>
>
>
>> Recent upstream mac80211 has broken the ability to set a single tx rate as
>> well, last I checked.
>
> Other rates are set properly except 9Mbps.By default it sets as 6Mbps, when data
> transfer starts, then it set to the mentioned fixed data rate.

Setting the tx rate through normal API will not effect management frames, and at the beginning of
the connection, perhaps no data frames have been sent/received yet, so you would
see 6Mbps reported.

Thanks,
Ben

>
> Thanks & Regards,
> Kavita
>> Thanks,
>> Ben
>>
>>>
>>>
>>> Thanks & Regards,
>>> Kavita
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe backports" in
>>>
>>
>> --
>> Ben Greear <greearb@candelatech.com>
>> Candela Technologies Inc  http://www.candelatech.com
>
>
> Thanks & Regards,
> कविता माथुर Kavita Mathur
> वरिष्ठ अनुसंधान अभियंता  Senior Research Engineer
> सी-डॉट                      C-DOT
> इलैक्ट्रॉनिक्स सिटी फेज़ I         Electronics City Phase I
> होसूर रोड, बेंगलूरु               Hosur Road, Bengaluru – 560100
> फोन  Ph 080-28529896
> Disclaimer:
> ----------
> This email and any files transmitted with it
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com