RE: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Jean Pierre TOSONI]

> -Message d'origine-
> De : Felix Fietkau [mailto:n...@nbd.name]
> Envoyé : jeudi 15 février 2018 15:46
> À : Jean Pierre TOSONI; Kalle Valo
> Cc : linux-wireless@vger.kernel.org; ath9k-de...@qca.qualcomm.com
> Objet : Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received
> during channel setup
> 
> On 2018-02-14 17:26, Jean Pierre TOSONI wrote:
> > ath9k returns a wrong RSSI value for frames received in a 30ms
> time
> > window after a channel change. The correct value is typically 10dB
> > below the returned value.
> >
> > This was found with a Atheros AR9300 Rev:3 chip (WLE350NX /
> JWX6083
> > cards), during offchannel scans.
> >
> > Mark the signal value as invalid in this case.
> >
> > Signed-off-by: Jean Pierre TOSONI <jp.tos...@acksys.fr>
> It seems that this will potentially cause a lot of scan results with
> RSSI information missing, which I think may be worse than a 10 db
> fluctuation.

Hmmm... My client is moving linearly from AP to AP and if it finds
 that the previous AP is "better" than the current or next one,
 havoc will occur on throughput! In this case I prefer to wait for
 the next scan round than use a wrong value.

Also, note that my patch sets the invalid flag but it is currently
 Ignored by mac80211 (I submitted another patch about that).

Actually the problem shows up only on passive channels, because
(1) on active channels the wrong beacon report will be soon
overwritten by the probe response report
(2) mac80211 waits only for 108ms on passive channels, if it waited
for 133ms we would have the opportunity to obtain a 2nd correct
beacon report (well, depending on the beacon interval of course)

> Actually, it could also be that the hardware is reporting accurate
> RSSI values (they're reported as SNR relative to the noise floor)
> and this is just a blip caused by the fact that the hw is updating
> its internal noise floor value based on running measurements.
> 
> Does the same fluctuation also happen if you set update = false for
> calls to ath9k_hw_start_nfcal?

Thanks for the hint, I will have a look at that 

> 
> - Felix

Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Felix Fietkau]

On 2018-02-14 17:26, Jean Pierre TOSONI wrote:
> ath9k returns a wrong RSSI value for frames received in a 30ms time
> window after a channel change. The correct value is typically 10dB
> below the returned value.
> 
> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX / JWX6083
> cards), during offchannel scans.
> 
> Mark the signal value as invalid in this case.
> 
> Signed-off-by: Jean Pierre TOSONI 
It seems that this will potentially cause a lot of scan results with
RSSI information missing, which I think may be worse than a 10 db
fluctuation.
Actually, it could also be that the hardware is reporting accurate RSSI
values (they're reported as SNR relative to the noise floor) and this is
just a blip caused by the fact that the hw is updating its internal
noise floor value based on running measurements.

Does the same fluctuation also happen if you set update = false for
calls to ath9k_hw_start_nfcal?

- Felix

Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[James Cameron]

On Thu, Feb 15, 2018 at 08:52:53AM +, Jean Pierre TOSONI wrote:
> > -Message d'origine-
> > De : qu...@laptop.org [mailto:qu...@laptop.org]
> > Envoyé : jeudi 15 février 2018 08:21
> > À : Kalle Valo
> > Cc : Jean Pierre TOSONI; linux-wireless@vger.kernel.org; ath9k-
> > de...@qca.qualcomm.com
> > Objet : Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received
> > during channel setup
> > 
> > On Thu, Feb 15, 2018 at 07:51:28AM +0200, Kalle Valo wrote:
> > > James Cameron <qu...@laptop.org> writes:
> > >
> > >> On Wed, Feb 14, 2018 at 04:26:42PM +, Jean Pierre TOSONI
> > wrote:
> > >>> ath9k returns a wrong RSSI value for frames received
> > >>> in a 30ms time window after a channel change. The
> > >>> correct value is typically 10dB below the returned value.
> > >>
> > >> How was your correct value determined?
> > >>
> 
> 1) test setup:
> Connecting the AP through coax and attenuators, then making 500 passive scans 
> off-channel, then drawing an histogram of the beacon signals found by the 
> chip. The off-channel period is 108 ms. The probability of being in the 30 ms 
> window is 28%. The histogram shows 2 spikes, one large with the expected 
> value, one small at around +10dB above.
> 
> 2) value determination
> Adjust the delay (CONFIG_HZ=250) by trial and error. 25ms was not enough to 
> completely absorb the +10dB spike in the histogram, while 30ms was enough.
> 
> Do you think of a better approach?

No, I think your approach is fine.  I was curious.  Thanks for explaining.

> Maybe the guys at Qualcomm know the correct value?

Yes, that seems likely.

> > >>> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX /
> > >>> JWX6083 cards), during offchannel scans.
> > >>>
> > >>> Mark the signal value as invalid in this case.
> > >>
> > >> Why not adjust by 10dB?
> 
> I considered that also. But, 
> 1) during how much time should I do this adjustment? Around 30 ms after 
> channel switch?

Yes.  If RSSI is so critical for your application, you'll do what you
can to get a real RSSI rather than drop it.

> 2) The histogram shows a scattering of the measures in a +/- 3dB range around 
> the mean value.

Perhaps a sampling error by the device.

> So I could not decide for sure if it needed -9dB, -10dB or -11dB?
> 
> > >>
> > >> Speculating: in a typical card, RSSI is calculated by firmware
> > from
> > >> readings of ADCs attached to the receiver.  Firmware may average
> > >> several readings.  Firmware may apply other offsets or
> > calibrations,
> > >> based on frequency and temperature.  This sounds like a firmware
> > >> problem.
> > >
> > > ath9k does not have firmware, only ath9k_htc has it.
> > 
> > Heh.  s/firmware/silicon implementation/g
> 
> Oh well, if it's silicon problem, then it's a hardware problem, and
> I am right to correct it that way, since there is no other way :-)

Yes, if it can be reproduced by every ath9k.

-- 
James Cameron
http://quozl.netrek.org/

RE: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Jean Pierre TOSONI]

> -Message d'origine-
> De : qu...@laptop.org [mailto:qu...@laptop.org]
> Envoyé : jeudi 15 février 2018 08:21
> À : Kalle Valo
> Cc : Jean Pierre TOSONI; linux-wireless@vger.kernel.org; ath9k-
> de...@qca.qualcomm.com
> Objet : Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received
> during channel setup
> 
> On Thu, Feb 15, 2018 at 07:51:28AM +0200, Kalle Valo wrote:
> > James Cameron <qu...@laptop.org> writes:
> >
> >> On Wed, Feb 14, 2018 at 04:26:42PM +, Jean Pierre TOSONI
> wrote:
> >>> ath9k returns a wrong RSSI value for frames received
> >>> in a 30ms time window after a channel change. The
> >>> correct value is typically 10dB below the returned value.
> >>
> >> How was your correct value determined?
> >>

1) test setup:
Connecting the AP through coax and attenuators, then making 500 passive scans 
off-channel, then drawing an histogram of the beacon signals found by the chip. 
The off-channel period is 108 ms. The probability of being in the 30 ms window 
is 28%. The histogram shows 2 spikes, one large with the expected value, one 
small at around +10dB above.

2) value determination
Adjust the delay (CONFIG_HZ=250) by trial and error. 25ms was not enough to 
completely absorb the +10dB spike in the histogram, while 30ms was enough.

Do you think of a better approach? Maybe the guys at Qualcomm know the correct 
value? 

> >>> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX /
> >>> JWX6083 cards), during offchannel scans.
> >>>
> >>> Mark the signal value as invalid in this case.
> >>
> >> Why not adjust by 10dB?

I considered that also. But, 
1) during how much time should I do this adjustment? Around 30 ms after channel 
switch?
2) The histogram shows a scattering of the measures in a +/- 3dB range around 
the mean value.
So I could not decide for sure if it needed -9dB, -10dB or -11dB?

> >>
> >> Speculating: in a typical card, RSSI is calculated by firmware
> from
> >> readings of ADCs attached to the receiver.  Firmware may average
> >> several readings.  Firmware may apply other offsets or
> calibrations,
> >> based on frequency and temperature.  This sounds like a firmware
> >> problem.
> >
> > ath9k does not have firmware, only ath9k_htc has it.
> 
> Heh.  s/firmware/silicon implementation/g

Oh well, if it's silicon problem, then it's a hardware problem, and
I am right to correct it that way, since there is no other way :-)

> 
> --
> James Cameron
> http://quozl.netrek.org/

RE: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Jean Pierre TOSONI]

> -Message d'origine-
> De : steve.deros...@gmail.com [mailto:steve.deros...@gmail.com] De
> la part de Steve deRosier
> Envoyé : mercredi 14 février 2018 19:16
> À : Jean Pierre TOSONI
> Cc : Kalle Valo; linux-wireless@vger.kernel.org; ath9k-
> de...@qca.qualcomm.com
> Objet : Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received
> during channel setup
> 
> On Wed, Feb 14, 2018 at 8:26 AM, Jean Pierre TOSONI
> <jp.tos...@acksys.fr> wrote:
> > ath9k returns a wrong RSSI value for frames received in a 30ms
> time
> > window after a channel change. The correct value is typically 10dB
> > below the returned value.
> >
> > This was found with a Atheros AR9300 Rev:3 chip (WLE350NX /
> JWX6083
> > cards), during offchannel scans.
> >
> 
> Is this true for ALL ath9k chips or only for the specific device? If
> it's not generally true, we shouldn't be adding code that applies to
> all. Perhaps a quirk or keyed to a specific chip id or DT entry or
> other way to guard it?

You are right.
On my hardware I could only try these two PCIe cards which use the same chip.
But I cannot be sure it applies to other ath9k. Should I check the PCI product 
ID?

> 
> - Steve

Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[James Cameron]

On Thu, Feb 15, 2018 at 07:51:28AM +0200, Kalle Valo wrote:
> James Cameron  writes:
> 
> > On Wed, Feb 14, 2018 at 04:26:42PM +, Jean Pierre TOSONI wrote:
> >> ath9k returns a wrong RSSI value for frames received in a 30ms time
> >> window after a channel change. The correct value is typically 10dB
> >> below the returned value.
> >
> > How was your correct value determined?
> >
> >> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX / JWX6083
> >> cards), during offchannel scans.
> >> 
> >> Mark the signal value as invalid in this case.
> >
> > Why not adjust by 10dB?
> >
> > Speculating: in a typical card, RSSI is calculated by firmware from
> > readings of ADCs attached to the receiver.  Firmware may average
> > several readings.  Firmware may apply other offsets or calibrations,
> > based on frequency and temperature.  This sounds like a firmware
> > problem.
> 
> ath9k does not have firmware, only ath9k_htc has it.

Heh.  s/firmware/silicon implementation/g

-- 
James Cameron
http://quozl.netrek.org/

Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Kalle Valo]

James Cameron  writes:

> On Wed, Feb 14, 2018 at 04:26:42PM +, Jean Pierre TOSONI wrote:
>> ath9k returns a wrong RSSI value for frames received in a 30ms time
>> window after a channel change. The correct value is typically 10dB
>> below the returned value.
>
> How was your correct value determined?
>
>> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX / JWX6083
>> cards), during offchannel scans.
>> 
>> Mark the signal value as invalid in this case.
>
> Why not adjust by 10dB?
>
> Speculating: in a typical card, RSSI is calculated by firmware from
> readings of ADCs attached to the receiver.  Firmware may average
> several readings.  Firmware may apply other offsets or calibrations,
> based on frequency and temperature.  This sounds like a firmware
> problem.

ath9k does not have firmware, only ath9k_htc has it.

-- 
Kalle Valo

Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[James Cameron]

On Wed, Feb 14, 2018 at 04:26:42PM +, Jean Pierre TOSONI wrote:
> ath9k returns a wrong RSSI value for frames received in a 30ms time
> window after a channel change. The correct value is typically 10dB
> below the returned value.

How was your correct value determined?

> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX / JWX6083
> cards), during offchannel scans.
> 
> Mark the signal value as invalid in this case.

Why not adjust by 10dB?

Speculating: in a typical card, RSSI is calculated by firmware from
readings of ADCs attached to the receiver.  Firmware may average
several readings.  Firmware may apply other offsets or calibrations,
based on frequency and temperature.  This sounds like a firmware
problem.

> Signed-off-by: Jean Pierre TOSONI 
> [...]

-- 
James Cameron
http://quozl.netrek.org/

Re: [PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Steve deRosier]

On Wed, Feb 14, 2018 at 8:26 AM, Jean Pierre TOSONI  wrote:
> ath9k returns a wrong RSSI value for frames received in a 30ms time
> window after a channel change. The correct value is typically 10dB
> below the returned value.
>
> This was found with a Atheros AR9300 Rev:3 chip (WLE350NX / JWX6083
> cards), during offchannel scans.
>

Is this true for ALL ath9k chips or only for the specific device? If
it's not generally true, we shouldn't be adding code that applies to
all. Perhaps a quirk or keyed to a specific chip id or DT entry or
other way to guard it?

- Steve

[PATCH v2] ath9k: mark RSSI as invalid if frame received during channel setup

[Jean Pierre TOSONI]

ath9k returns a wrong RSSI value for frames received in a 30ms time
window after a channel change. The correct value is typically 10dB
below the returned value.

This was found with a Atheros AR9300 Rev:3 chip (WLE350NX / JWX6083
cards), during offchannel scans.

Mark the signal value as invalid in this case.

Signed-off-by: Jean Pierre TOSONI 
---
V2: replace LEDE patch with due kernel patch (with excuses to Kalle Valo)

 drivers/net/wireless/ath/ath9k/ani.c|1 +
 drivers/net/wireless/ath/ath9k/common.c |   10 ++
 drivers/net/wireless/ath/ath9k/hw.h |1 +
 3 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ani.c 
b/drivers/net/wireless/ath/ath9k/ani.c
index 5214dd7..a7e07ed 100644
--- a/drivers/net/wireless/ath/ath9k/ani.c
+++ b/drivers/net/wireless/ath/ath9k/ani.c
@@ -317,6 +317,7 @@ void ath9k_ani_reset(struct ath_hw *ah, bool is_scanning)
return;
 
BUG_ON(aniState == NULL);
+   ah->chan_stable_time = jiffies + (30*HZ)/1000; /* 30ms */
ah->stats.ast_ani_reset++;
 
ofdm_nil = max_t(int, ATH9K_ANI_OFDM_DEF_LEVEL,
diff --git a/drivers/net/wireless/ath/ath9k/common.c 
b/drivers/net/wireless/ath/ath9k/common.c
index 099f3d4..7e5f7a3 100644
--- a/drivers/net/wireless/ath/ath9k/common.c
+++ b/drivers/net/wireless/ath/ath9k/common.c
@@ -221,6 +221,16 @@ void ath9k_cmn_process_rssi(struct ath_common *common,
int i, j;
 
/*
+* RSSI is not available before 30ms after reset on ath9k.
+*/
+   if (time_before(jiffies, ah->chan_stable_time)) {
+   ath_dbg(common, ANY, "early frame rssi=%d\n",rx_stats->rs_rssi);
+   rxs->flag |= RX_FLAG_NO_SIGNAL_VAL;
+   return;
+   }
+   ah->chan_stable_time = jiffies; /* advance stable time to cop with 
jiffies wraparound */
+
+   /*
 * RSSI is not available for subframes in an A-MPDU.
 */
if (rx_stats->rs_moreaggr) {
diff --git a/drivers/net/wireless/ath/ath9k/hw.h 
b/drivers/net/wireless/ath/ath9k/hw.h
index 9804a24..ec07da9 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -809,6 +809,7 @@ struct ath_hw {
 
bool reset_power_on;
bool htc_reset_init;
+   unsigned long chan_stable_time;
 
enum nl80211_iftype opmode;
enum ath9k_power_mode power_mode;
-- 
1.7.2.5