Re: [PATCH v1 2/4] wifi: ath11k: enable airtime queue limits

Julius Bairaktaris <[email protected]> writes:

> ath11k does not advertise NL80211_EXT_FEATURE_AQL, and every user of AQL in
> mac80211 is gated on that bit: the airtime charge in
> ieee80211_tx_dequeue(), ieee80211_txq_airtime_check(),
> ieee80211_sta_update_pending_airtime(), and the per-station aql file in
> debugfs, which is not even created. AQL is not untuned on this driver, it
> is absent.
>
> That matters here more than it would elsewhere, because AQL is the only
> brake. ath11k uses ieee80211_handle_wake_tx_queue(), whose
> wake_tx_push_queue() drains a selected txq whole into the driver, and the
> check at the top of ieee80211_tx_dequeue() is what stops that drain.
> Without the feature bit nothing does, so mac80211 holds no backlog and
> every queued byte of a saturated download sits in the hardware TX rings,
> where there is no AQM and no flow separation.
>
> Measured on an IPQ8074 AP against one HE 160 MHz station at MCS 11, taken
> from that station's own aql file while it received 280 Mbit/s: its BE queue
> depth reaches 6356 us of the 12000 us default limit, and 2972 us when the
> limit is lowered to 500/1000 us. Without this patch the file does not exist
> and the limits are never consulted.

Did you test if this actually has any impact on latency under load for
the station in question?

> The rate the estimator needs is already there: ath11k keeps a per-station
> rate_info in arsta->last_txrate and passes it to ieee80211_tx_status_ext(),
> which stores it as tx_stats.last_rate_info, and that is what
> ieee80211_calc_expected_tx_airtime() reads. Where no rate is known yet the
> estimator returns zero, no airtime is charged, and AQL stays inert, so this
> cannot behave worse than the current code.
>
> The estimate is only as fresh as arsta->last_txrate, which is updated from
> the HTT PPDU stats when a descriptor is evicted from ar->ppdu_stats_info,
> so the rate behind it can lag a rate change by up to
> HTT_PPDU_DESC_MAX_DEPTH PPDUs.
>
> ieee80211_sta_update_pending_airtime() returns the airtime but does not
> reschedule the txq, and ath11k has no completion-side push, so a queue held
> back by AQL is re-poked only by the next enqueue or queue wake.

This sounds like it could leave to stuck queues? :/

-Toke