Hi Toke,
Thanks for getting this done! I will give it a try in the next few
days. A few comments:
> The estimated airtime for each skb is stored in the tx_info, so we can
> subtract the same amount from the running total when the skb is freed or
> recycled.
Looks like ath10k driver zero out the info->status before calling
ieee80211_tx_status(...):
int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
const struct htt_tx_done *tx_done)
{
...
info = IEEE80211_SKB_CB(msdu);
memset(&info->status, 0, sizeof(info->status));
...
ieee80211_tx_status(htt->ar->hw, msdu);
}
We need either restore the info->status.tx_time_est or calling
ieee80211_sta_update_pending_airtime() in ath10k before tx_time_est
get erased.
> + if (local->airtime_flags & AIRTIME_USE_AQL) {
> + airtime = ieee80211_calc_expected_tx_airtime(hw, vif, txq->sta,
> + skb->len + 38);
I think it is better to put the "+ 38" that takes care of the header
overhead inside ieee80211_calc_expected_tx_airtime().
Kan
On Tue, Oct 15, 2019 at 10:19 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>
> From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
>
> The previous commit added the ability to throttle stations when they queue
> too much airtime in the hardware. This commit enables the functionality by
> calculating the expected airtime usage of each packet that is dequeued from
> the TXQs in mac80211, and accounting that as pending airtime.
>
> The estimated airtime for each skb is stored in the tx_info, so we can
> subtract the same amount from the running total when the skb is freed or
> recycled. The throttling mechanism relies on this accounting to be
> accurate (i.e., that we are not freeing skbs without subtracting any
> airtime they were accounted for), so we put the subtraction into
> ieee80211_report_used_skb(). As an optimisation, we also subtract the
> airtime on regular TX completion, zeroing out the value stored in the
> packet afterwards, to avoid having to do an expensive lookup of the
> station from the packet data on every packet.
>
> This patch does *not* include any mechanism to wake a throttled TXQ again,
> on the assumption that this will happen anyway as a side effect of whatever
> freed the skb (most commonly a TX completion).
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> ---
> net/mac80211/status.c | 38 ++++++++++++++++++++++++++++++++++++++
> net/mac80211/tx.c | 16 ++++++++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
> index ab8ba5835ca0..ce990a1e9043 100644
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -676,6 +676,33 @@ static void ieee80211_report_used_skb(struct ieee80211_local *local,
> if (dropped)
> acked = false;
>
> + if (info->status.tx_time_est) {
> + struct ieee80211_sub_if_data *sdata;
> + struct sta_info *sta = NULL;
> + u8 *qc, ac;
> + int tid;
> +
> + rcu_read_lock();
> +
> + sdata = ieee80211_sdata_from_skb(local, skb);
> + if (sdata)
> + sta = sta_info_get_bss(sdata, skb_mac_header(skb));
> +
> + if (ieee80211_is_data_qos(hdr->frame_control)) {
> + qc = ieee80211_get_qos_ctl(hdr);
> + tid = qc[0] & 0xf;
> + ac = ieee80211_ac_from_tid(tid);
> + } else {
> + ac = IEEE80211_AC_BE;
> + }
> +
> + ieee80211_sta_update_pending_airtime(local, sta, ac,
> + info->status.tx_time_est,
> + true);
> + rcu_read_unlock();
> +
> + }
> +
> if (info->flags & IEEE80211_TX_INTFL_MLME_CONN_TX) {
> struct ieee80211_sub_if_data *sdata;
>
> @@ -930,6 +957,17 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
> tid = qc[0] & 0xf;
> }
>
> + if (info->status.tx_time_est) {
> + /* Do this here to avoid the expensive lookup of the sta
> + * in ieee80211_report_used_skb().
> + */
> + ieee80211_sta_update_pending_airtime(local, sta,
> + ieee80211_ac_from_tid(tid),
> + info->status.tx_time_est,
> + true);
> + info->status.tx_time_est = 0;
> + }
> +
> if (!acked && ieee80211_is_back_req(fc)) {
> u16 control;
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 405f622b3fe0..b6b47171b340 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -3539,9 +3539,14 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> struct ieee80211_tx_data tx;
> ieee80211_tx_result r;
> struct ieee80211_vif *vif = txq->vif;
> + u8 ac = txq->ac;
> + u32 airtime;
>
> WARN_ON_ONCE(softirq_count() == 0);
>
> + if (!ieee80211_txq_airtime_check(hw, txq))
> + return NULL;
> +
> begin:
> spin_lock_bh(&fq->lock);
>
> @@ -3652,6 +3657,17 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> }
>
> IEEE80211_SKB_CB(skb)->control.vif = vif;
> +
> + if (local->airtime_flags & AIRTIME_USE_AQL) {
> + airtime = ieee80211_calc_expected_tx_airtime(hw, vif, txq->sta,
> + skb->len + 38);
> + if (airtime) {
> + info->control.tx_time_est = airtime;
> + ieee80211_sta_update_pending_airtime(local, tx.sta, ac,
> + airtime, false);
> + }
> + }
> +
> return skb;
>
> out:
>
