[RFC PATCH v2] wifi: ath12k: service all RX rings in an interrupt group

[RFC PATCH v2] wifi: ath12k: service all RX rings in an interrupt group

[John Macias]

ath12k_wifi7_dp_service_srng() services only the highest REO destination
ring selected by ring_mask->rx[grp_id]. If a group selects more than one
ring, the other rings in that group are never reaped, and the flows
hashed to them stall.

Every in-tree ring mask selects one REO ring per group today, so this is
latent upstream. It is hit by the QCN6432 ring mask from the vendor's
downstream tree, which puts REO rings 2 and 3 in one group. On a
GL-BE3600 using that mask, 7 of 16 single-flow TCP downloads stalled or
failed, and ring 2 reaped no packets at all.

Walking every ring with the full budget is not enough: under load the
first ring can use the whole budget on every poll and starve the others.
With that approach ring 2 reaped under 1% of ring 3's packets, and the
flows on it ran at 0.2-4.4 Mb/s while the others ran at 100-360 Mb/s.

Instead, give each ring in the group an equal share of the budget first,
then hand any budget left over back to the rings in a second pass. A
group with a single ring still gets the whole budget in one pass, so
current hardware behaves as before.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1

Assisted-by: LLM
Signed-off-by: John Macias <dencity7@gmail.com>
---
Changes in v2:
- Share the budget between the rings of a group. v1 walked the rings
  with the full budget, which starved the lower ring under load (see
  below).
- Rebased on ath-next.
- Added a Tested-on tag, full name in the sign-off and an Assisted-by
  tag, as Jeff asked.
- Link to v1: https://lore.kernel.org/ath12k/CA+KzPsr13czsH=k_UWJqj48C4Vsb-diQ-tiCh7W4XW31E8WzHg@mail.gmail.com/

This is still an RFC because I have no WCN hardware, so the WCN side is
untested. Since every in-tree mask has one ring per group, the change
should be a no-op there, but I would be grateful if someone with a
WCN7850 could test it and reply with a Tested-on tag.

Testing. Both radios ran OpenWrt (Linux 6.18, backports 7.2), and only
the ath12k_wifi7 module was swapped between runs. Rebuilding the
unmodified sources reproduced the installed modules byte for byte, so
each test module differed only in this handler. The QCN9274 build also
carries an unrelated local REO fix, as installed. The QCN6432 is a
GL-BE3600 using the downstream QCN6432 support (not upstream yet), as a
5 GHz client of a GL-BE9300 with QCN9274. iperf3 over that link:

                              before    v1           v2
  16 single TCP flows done     9/16     16/16        16/16
  slowest of 8 TCP streams     stall    0.2-4.4      70-83 Mb/s
  ring 2 vs ring 3 packets     0 vs -   3k-8k vs     about equal
                                        0.7M-1M
  QCN9274 RX, 8 TCP streams    1012     1048         1034-1042 Mb/s
  UDP loss, 8 x 50 Mb/s        -        <= 0.02%     0.00%

No warnings or errors appeared in the kernel log on either side. The
QCN9274 groups are single-ring, so that tag shows no regression rather
than the fix itself. The hardware runs used a variant that differs only
in cosmetics (left-- was inside max(), and the comment sat inside the
loop). The patch as sent builds without warnings with W=1 on x86_64.

This work was assisted by an LLM (Claude), which helped analyse the
handler, write the test scripts and draft this patch. I reviewed the
change and the test results, and take responsibility for them.

 drivers/net/wireless/ath/ath12k/wifi7/dp.c | 31 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/wifi7/dp.c b/drivers/net/wireless/ath/ath12k/wifi7/dp.c
index 397da016b..67a347362 100644
--- a/drivers/net/wireless/ath/ath12k/wifi7/dp.c
+++ b/drivers/net/wireless/ath/ath12k/wifi7/dp.c
@@ -27,6 +27,7 @@ static int ath12k_wifi7_dp_service_srng(struct ath12k_dp *dp,
 	int tot_work_done = 0;
 	enum dp_monitor_mode monitor_mode;
 	u8 ring_mask;
+	int rx_rings, pass, quota;
 
 	if (dp->hw_params->ring_mask->tx[grp_id]) {
 		i = fls(dp->hw_params->ring_mask->tx[grp_id]) - 1;
@@ -50,13 +51,29 @@ static int ath12k_wifi7_dp_service_srng(struct ath12k_dp *dp,
 			goto done;
 	}
 
-	if (dp->hw_params->ring_mask->rx[grp_id]) {
-		i = fls(dp->hw_params->ring_mask->rx[grp_id]) - 1;
-		work_done = ath12k_wifi7_dp_rx_process(dp, i, napi, budget);
-		budget -= work_done;
-		tot_work_done += work_done;
-		if (budget <= 0)
-			goto done;
+	/* A group may select more than one REO ring. Give each ring an equal
+	 * share of the budget first, so a busy ring cannot starve the others,
+	 * then hand any budget left over back to the rings in a second pass.
+	 */
+	ring_mask = dp->hw_params->ring_mask->rx[grp_id];
+	rx_rings = hweight8(ring_mask);
+	for (pass = 0; ring_mask && pass < 2; pass++) {
+		u8 mask = ring_mask;
+		int left = rx_rings;
+
+		while (mask) {
+			i = fls(mask) - 1;
+			mask &= ~BIT(i);
+			quota = pass ? budget : max(budget / left, 1);
+			left--;
+			work_done = ath12k_wifi7_dp_rx_process(dp, i, napi, quota);
+			budget -= work_done;
+			tot_work_done += work_done;
+			if (budget <= 0)
+				goto done;
+		}
+		if (rx_rings == 1)
+			break;
 	}
 
 	if (dp->hw_params->ring_mask->rx_mon_status[grp_id]) {

base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
-- 
2.47.3