[RFC PATCH v2] wifi: ath12k: service all RX rings in an interrupt group
Tamizh Raja
tamizh.raja at oss.qualcomm.com
Tue Oct 27 02:50:00 PDT 2026
On Sun, Oct 25, 2026 at 11:50 PM John Macias <dencity7 at gmail.com> wrote:
>
> 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.
>
Does any current upstream ring-mask configuration trigger this issue,
or is it only present in downstream trees?
> 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 at 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++) {
whether two passes are always sufficient in all kind of scenarios?
> + 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);
This will have the performance impact in case of srng->lock taking
multiple times in a single handling.
> + 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
>
>
If current upstream configurations don't hit this issue, is this
change needed, or can we avoid the group ring-mask configuration
instead?
--
- Tamizh.
More information about the ath12k
mailing list