[PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Toke Høiland-Jørgensen]

Some ath9k chips can, seemingly at random, end up in a state which can
be described as "deaf". No or nearly no interrupts are generated anymore
for incoming packets. Existing links either break down after a while and
new links will not be established.

The circumstances leading to this "deafness" is still unclear, but some
particular chips (especially 2-stream 11n SoCs, but also others) can go
'deaf' when running AP or mesh (or both) after some time. It's probably
a hardware issue, and doing a channel scan to trigger a chip
reset (which one normally can't do on an AP interface) recovers the
hardware.

The only way the driver can detect this state, is by detecting if there
has been no RX activity for a while. In this case we can proactively
reset the chip (which only takes a small number of milliseconds, so
shouldn't interrupt things too much if it has been idle for several
seconds), which functions as a workaround.

OpenWrt, and various derivatives, have been carrying versions of this
workaround for years, that were never upstreamed. One version[0],
written by Felix Fietkau, used a simple counter and only reset if there
was precisely zero RX activity for a long period of time. This had the
problem that in some cases a small number of interrupts would appear
even if the device was otherwise not responsive. For this reason,
another version[1], written by Simon Wunderlich and Sven Eckelmann, used
a time-based approach to calculate the average number of RX interrupts
over a longer (four-second) interval, and reset the chip when seeing
less than one interrupt per second over this period. However, that
version relied on debugfs counters to keep track of the number of
interrupts, which means it didn't work at all if debugfs was not
enabled.

This patch unifies the two versions: it uses the same approach as Felix'
patch to count the number of RX handler invocations, but uses the same
time-based windowing approach as Simon and Sven's patch to still handle
the case where occasional interrupts appear but the device is otherwise
deaf.

Since this is based on ideas by all three people, but not actually
directly derived from any of the patches, I'm including Suggested-by
tags from Simon, Sven and Felix below, which should hopefully serve as
proper credit.

[0] https://patchwork.kernel.org/project/linux-wireless/patch/20170125163654.66431-3-nbd@nbd.name/
[1] https://patchwork.kernel.org/project/linux-wireless/patch/20161117083614.19188-2-sven.eckelmann@open-mesh.com/

Suggested-by: Simon Wunderlich <sw@simonwunderlich.de>
Suggested-by: Sven Eckelmann <se@simonwunderlich.de>
Suggested-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  2 ++
 drivers/net/wireless/ath/ath9k/debug.c |  1 +
 drivers/net/wireless/ath/ath9k/debug.h |  1 +
 drivers/net/wireless/ath/ath9k/link.c  | 33 +++++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath9k/main.c  |  1 +
 5 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 29ca65a732a66c9452b19ad184f513f92e4c8c2c..bcfc8df0efe5b2c5510230e7f59592d51df8365e 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -1018,6 +1018,8 @@ struct ath_softc {
 
 	u8 gtt_cnt;
 	u32 intrstatus;
+	u32 rx_active_check_time;
+	u32 rx_active_count;
 	u16 ps_flags; /* PS_* */
 	bool ps_enabled;
 	bool ps_idle;
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 51abc470125b3cc4e773f5272e2394c8f790a32e..a1e3bcf796f27fd58f5ab307a198927274f8f321 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -750,6 +750,7 @@ static int read_file_reset(struct seq_file *file, void *data)
 		[RESET_TYPE_CALIBRATION] = "Calibration error",
 		[RESET_TX_DMA_ERROR] = "Tx DMA stop error",
 		[RESET_RX_DMA_ERROR] = "Rx DMA stop error",
+		[RESET_TYPE_RX_INACTIVE] = "Rx path inactive",
 	};
 	int i;
 
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 389459c04d14a5215783b916519d05419b970c20..cb3e75969875afed282b09ff2458dd8ca286d5b1 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -53,6 +53,7 @@ enum ath_reset_type {
 	RESET_TYPE_CALIBRATION,
 	RESET_TX_DMA_ERROR,
 	RESET_RX_DMA_ERROR,
+	RESET_TYPE_RX_INACTIVE,
 	__RESET_TYPE_MAX
 };
 
diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c
index d1e5767aab3cbc2ee91018edadf89a223cf1d759..d078a59d7d3cdbe623b507d29cc974b801f2deaf 100644
--- a/drivers/net/wireless/ath/ath9k/link.c
+++ b/drivers/net/wireless/ath/ath9k/link.c
@@ -50,7 +50,36 @@ static bool ath_tx_complete_check(struct ath_softc *sc)
 		"tx hung, resetting the chip\n");
 	ath9k_queue_reset(sc, RESET_TYPE_TX_HANG);
 	return false;
+}
+
+#define RX_INACTIVE_CHECK_INTERVAL (4 * MSEC_PER_SEC)
+
+static bool ath_hw_rx_inactive_check(struct ath_softc *sc)
+{
+	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+	u32 interval, count;
+
+	interval = jiffies_to_msecs(jiffies - sc->rx_active_check_time);
+	count = sc->rx_active_count;
+
+	if (interval < RX_INACTIVE_CHECK_INTERVAL)
+		return true; /* too soon to check */
 
+	sc->rx_active_count = 0;
+	sc->rx_active_check_time = jiffies;
+
+	/* Need at least one interrupt per second, and we should only react if
+	 * we are within a factor two of the expected interval
+	 */
+	if (interval > RX_INACTIVE_CHECK_INTERVAL * 2 ||
+	    count >= interval / MSEC_PER_SEC)
+		return true;
+
+	ath_dbg(common, RESET,
+		"RX inactivity detected. Schedule chip reset\n");
+	ath9k_queue_reset(sc, RESET_TYPE_RX_INACTIVE);
+
+	return false;
 }
 
 void ath_hw_check_work(struct work_struct *work)
@@ -58,8 +87,8 @@ void ath_hw_check_work(struct work_struct *work)
 	struct ath_softc *sc = container_of(work, struct ath_softc,
 					    hw_check_work.work);
 
-	if (!ath_hw_check(sc) ||
-	    !ath_tx_complete_check(sc))
+	if (!ath_hw_check(sc) || !ath_tx_complete_check(sc) ||
+	    !ath_hw_rx_inactive_check(sc))
 		return;
 
 	ieee80211_queue_delayed_work(sc->hw, &sc->hw_check_work,
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index b92c89dad8deac7281e0165c9e1a566ba99ece65..998f717a1a86eee539058f6f4d9318dd459fc8dd 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -453,6 +453,7 @@ void ath9k_tasklet(struct tasklet_struct *t)
 			ath_rx_tasklet(sc, 0, true);
 
 		ath_rx_tasklet(sc, 0, false);
+		sc->rx_active_count++;
 	}
 
 	if (status & ATH9K_INT_TX) {

---
base-commit: c33f9c2728d0ccc7472e6239346c0fb3de556e0f
change-id: 20241105-ath9k-deaf-detection-0e26bb3243a6

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Sven Eckelmann]

Hi,

Thank you for submitting the patch.

On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
> Since this is based on ideas by all three people, but not actually
> directly derived from any of the patches, I'm including Suggested-by
> tags from Simon, Sven and Felix below, which should hopefully serve as
> proper credit.

At least for me, this is more than enough. Thanks.

I don't have the setup at the moment to test it again - maybe Issam can do 
this. One concern I would have (because I don't find the notes regarding this 
problem), is whether this check is now breaking because we count more things. 
In the past, rxlp/rxok was used for the check. And now I don't know whether 
the count for the other ones were still increasing.

* RXHP (rather sure that "high priority frame" wasn't increasing)
* RXEOL ("no RX descriptors available" - I would guess no, but I can't say for
  sure)
* RXORN ("FIFO overrun" I would guess no, but I can't say for sure)

Reviewed-by: Sven Eckelmann <se@simonwunderlich.de>

Kind regards,
	Sven

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Toke Høiland-Jørgensen]

Sven Eckelmann <se@simonwunderlich.de> writes:

> Hi,
>
> Thank you for submitting the patch.
>
> On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
>> Since this is based on ideas by all three people, but not actually
>> directly derived from any of the patches, I'm including Suggested-by
>> tags from Simon, Sven and Felix below, which should hopefully serve as
>> proper credit.
>
> At least for me, this is more than enough. Thanks.
>
> I don't have the setup at the moment to test it again - maybe Issam can do 
> this. One concern I would have (because I don't find the notes regarding this 
> problem), is whether this check is now breaking because we count more things. 
> In the past, rxlp/rxok was used for the check. And now I don't know whether 
> the count for the other ones were still increasing.
>
> * RXHP (rather sure that "high priority frame" wasn't increasing)
> * RXEOL ("no RX descriptors available" - I would guess no, but I can't say for
>   sure)
> * RXORN ("FIFO overrun" I would guess no, but I can't say for sure)
>
> Reviewed-by: Sven Eckelmann <se@simonwunderlich.de>

Great, thanks for the review! I'll let it sit in patchwork for a little
while to give people a chance to test it out before sending it over to
Kalle to be applied :)

-Toke

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

On Wednesday, November 6, 2024 3:12:59 PM CET Toke Høiland-Jørgensen wrote:
> Sven Eckelmann <se@simonwunderlich.de> writes:
> > Hi,
> > 
> > Thank you for submitting the patch.
> > 
> > On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
> >> Since this is based on ideas by all three people, but not actually
> >> directly derived from any of the patches, I'm including Suggested-by
> >> tags from Simon, Sven and Felix below, which should hopefully serve as
> >> proper credit.
> > 
> > At least for me, this is more than enough. Thanks.
> > 
> > I don't have the setup at the moment to test it again - maybe Issam can do
> > this. One concern I would have (because I don't find the notes regarding
> > this problem), is whether this check is now breaking because we count
> > more things. In the past, rxlp/rxok was used for the check. And now I
> > don't know whether the count for the other ones were still increasing.
> > 
> > * RXHP (rather sure that "high priority frame" wasn't increasing)
> > * RXEOL ("no RX descriptors available" - I would guess no, but I can't say
> > for> 
> >   sure)
> > 
> > * RXORN ("FIFO overrun" I would guess no, but I can't say for sure)
> > 
> > Reviewed-by: Sven Eckelmann <se@simonwunderlich.de>
> 
> Great, thanks for the review! I'll let it sit in patchwork for a little
> while to give people a chance to test it out before sending it over to
> Kalle to be applied :)
> 
> -Toke

Hi Toke,

this looks good to me in general. I'm not sure either about the particular RX 
interrupts. We can test this by putting the AP in a shield box and verify that 
the counters are actually increasing, and that should be good enough.

Acked-by: Simon Wunderlich <sw@simonwunderlich.de>

Thank you!
      Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Toke Høiland-Jørgensen]

Simon Wunderlich <sw@simonwunderlich.de> writes:

> On Wednesday, November 6, 2024 3:12:59 PM CET Toke Høiland-Jørgensen wrote:
>> Sven Eckelmann <se@simonwunderlich.de> writes:
>> > Hi,
>> > 
>> > Thank you for submitting the patch.
>> > 
>> > On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
>> >> Since this is based on ideas by all three people, but not actually
>> >> directly derived from any of the patches, I'm including Suggested-by
>> >> tags from Simon, Sven and Felix below, which should hopefully serve as
>> >> proper credit.
>> > 
>> > At least for me, this is more than enough. Thanks.
>> > 
>> > I don't have the setup at the moment to test it again - maybe Issam can do
>> > this. One concern I would have (because I don't find the notes regarding
>> > this problem), is whether this check is now breaking because we count
>> > more things. In the past, rxlp/rxok was used for the check. And now I
>> > don't know whether the count for the other ones were still increasing.
>> > 
>> > * RXHP (rather sure that "high priority frame" wasn't increasing)
>> > * RXEOL ("no RX descriptors available" - I would guess no, but I can't say
>> > for> 
>> >   sure)
>> > 
>> > * RXORN ("FIFO overrun" I would guess no, but I can't say for sure)
>> > 
>> > Reviewed-by: Sven Eckelmann <se@simonwunderlich.de>
>> 
>> Great, thanks for the review! I'll let it sit in patchwork for a little
>> while to give people a chance to test it out before sending it over to
>> Kalle to be applied :)
>> 
>> -Toke
>
> Hi Toke,
>
> this looks good to me in general. I'm not sure either about the particular RX 
> interrupts. We can test this by putting the AP in a shield box and verify that 
> the counters are actually increasing, and that should be good enough.
>
> Acked-by: Simon Wunderlich <sw@simonwunderlich.de>

Great, thanks! Would be awesome if you could test it out an report back! :)

-Toke

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Hamdi Issam]

On 11/6/24 17:03, Simon Wunderlich wrote:
> On Wednesday, November 6, 2024 3:12:59 PM CET Toke Høiland-Jørgensen wrote:
>> Sven Eckelmann <se@simonwunderlich.de> writes:
>>> Hi,
>>>
>>> Thank you for submitting the patch.
>>>
>>> On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
>>>> Since this is based on ideas by all three people, but not actually
>>>> directly derived from any of the patches, I'm including Suggested-by
>>>> tags from Simon, Sven and Felix below, which should hopefully serve as
>>>> proper credit.
>>> At least for me, this is more than enough. Thanks.
>>>
>>> I don't have the setup at the moment to test it again - maybe Issam can do
>>> this. One concern I would have (because I don't find the notes regarding
>>> this problem), is whether this check is now breaking because we count
>>> more things. In the past, rxlp/rxok was used for the check. And now I
>>> don't know whether the count for the other ones were still increasing.
>>>
>>> * RXHP (rather sure that "high priority frame" wasn't increasing)
>>> * RXEOL ("no RX descriptors available" - I would guess no, but I can't say
>>> for>
>>>    sure)
>>>
>>> * RXORN ("FIFO overrun" I would guess no, but I can't say for sure)
>>>
>>> Reviewed-by: Sven Eckelmann <se@simonwunderlich.de>
>> Great, thanks for the review! I'll let it sit in patchwork for a little
>> while to give people a chance to test it out before sending it over to
>> Kalle to be applied :)
>>
>> -Toke
> Hi Toke,
>
> this looks good to me in general. I'm not sure either about the particular RX
> interrupts. We can test this by putting the AP in a shield box and verify that
> the counters are actually increasing, and that should be good enough.
>
> Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
>
> Thank you!
>        Simon

Hi Toke,

I have tested this patch in mesh mode, and it functions as expected.

I conducted the test by placing one node inside a shield box and the 
other outside, then verified whether a reset occurred due to RX path 
inactivity.

Tested-by: Issam Hamdi <ih@simonwunderlich.de>

Regards,

Issam


-

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Toke Høiland-Jørgensen]

Hamdi Issam <ih@simonwunderlich.de> writes:

> On 11/6/24 17:03, Simon Wunderlich wrote:
>> On Wednesday, November 6, 2024 3:12:59 PM CET Toke Høiland-Jørgensen wrote:
>>> Sven Eckelmann <se@simonwunderlich.de> writes:
>>>> Hi,
>>>>
>>>> Thank you for submitting the patch.
>>>>
>>>> On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
>>>>> Since this is based on ideas by all three people, but not actually
>>>>> directly derived from any of the patches, I'm including Suggested-by
>>>>> tags from Simon, Sven and Felix below, which should hopefully serve as
>>>>> proper credit.
>>>> At least for me, this is more than enough. Thanks.
>>>>
>>>> I don't have the setup at the moment to test it again - maybe Issam can do
>>>> this. One concern I would have (because I don't find the notes regarding
>>>> this problem), is whether this check is now breaking because we count
>>>> more things. In the past, rxlp/rxok was used for the check. And now I
>>>> don't know whether the count for the other ones were still increasing.
>>>>
>>>> * RXHP (rather sure that "high priority frame" wasn't increasing)
>>>> * RXEOL ("no RX descriptors available" - I would guess no, but I can't say
>>>> for>
>>>>    sure)
>>>>
>>>> * RXORN ("FIFO overrun" I would guess no, but I can't say for sure)
>>>>
>>>> Reviewed-by: Sven Eckelmann <se@simonwunderlich.de>
>>> Great, thanks for the review! I'll let it sit in patchwork for a little
>>> while to give people a chance to test it out before sending it over to
>>> Kalle to be applied :)
>>>
>>> -Toke
>> Hi Toke,
>>
>> this looks good to me in general. I'm not sure either about the particular RX
>> interrupts. We can test this by putting the AP in a shield box and verify that
>> the counters are actually increasing, and that should be good enough.
>>
>> Acked-by: Simon Wunderlich <sw@simonwunderlich.de>
>>
>> Thank you!
>>        Simon
>
> Hi Toke,
>
> I have tested this patch in mesh mode, and it functions as expected.
>
> I conducted the test by placing one node inside a shield box and the 
> other outside, then verified whether a reset occurred due to RX path 
> inactivity.
>
> Tested-by: Issam Hamdi <ih@simonwunderlich.de>

Great, thanks for testing! :)

-Toke

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Sven Eckelmann]

On Wednesday, 6 November 2024 14:39:14 CET Sven Eckelmann wrote:
> On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
> > Since this is based on ideas by all three people, but not actually
> > directly derived from any of the patches, I'm including Suggested-by
> > tags from Simon, Sven and Felix below, which should hopefully serve as
> > proper credit.
> 
> At least for me, this is more than enough. Thanks.
> 
> I don't have the setup at the moment to test it again

I've prepared a new Freifunk Vogtland firmware with this patch and placed it 
on a node which "regularly" has this problem. It is a node in a residential 
neighbourhood with the usual amount of WiFi activity. It was known that this 
node requires ~2 resets in 10 days.

After having it running for 10 1/2 days, I could see that the node was still 
working correctly and the "Rx path inactive" counter showed 3 resets during 
that time period.

Tested-by: Sven Eckelmann <se@simonwunderlich.de>

Kind regards,
	Sven

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Toke Høiland-Jørgensen]

Sven Eckelmann <se@simonwunderlich.de> writes:

> On Wednesday, 6 November 2024 14:39:14 CET Sven Eckelmann wrote:
>> On Wednesday, 6 November 2024 13:41:44 CET Toke Høiland-Jørgensen wrote:
>> > Since this is based on ideas by all three people, but not actually
>> > directly derived from any of the patches, I'm including Suggested-by
>> > tags from Simon, Sven and Felix below, which should hopefully serve as
>> > proper credit.
>> 
>> At least for me, this is more than enough. Thanks.
>> 
>> I don't have the setup at the moment to test it again
>
> I've prepared a new Freifunk Vogtland firmware with this patch and placed it 
> on a node which "regularly" has this problem. It is a node in a residential 
> neighbourhood with the usual amount of WiFi activity. It was known that this 
> node requires ~2 resets in 10 days.
>
> After having it running for 10 1/2 days, I could see that the node was still 
> working correctly and the "Rx path inactive" counter showed 3 resets during 
> that time period.
>
> Tested-by: Sven Eckelmann <se@simonwunderlich.de>

Great, thanks for testing!

-Toke

Re: [PATCH] ath9k: Add RX inactivity detection and reset chip when it occurs

[Jeff Johnson]

On Wed, 06 Nov 2024 13:41:44 +0100, Toke Høiland-Jørgensen wrote:
> Some ath9k chips can, seemingly at random, end up in a state which can
> be described as "deaf". No or nearly no interrupts are generated anymore
> for incoming packets. Existing links either break down after a while and
> new links will not be established.
> 
> The circumstances leading to this "deafness" is still unclear, but some
> particular chips (especially 2-stream 11n SoCs, but also others) can go
> 'deaf' when running AP or mesh (or both) after some time. It's probably
> a hardware issue, and doing a channel scan to trigger a chip
> reset (which one normally can't do on an AP interface) recovers the
> hardware.
> 
> [...]

Applied, thanks!

[1/1] ath9k: Add RX inactivity detection and reset chip when it occurs
      commit: b5f871ab4913b2403a7cdcbcde16d39d0b071fb3

Best regards,
-- 
Jeff Johnson <quic_jjohnson@quicinc.com>