> From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> > > Some devices have deep buffers in firmware and/or hardware which prevents > the FQ structure in mac80211 from effectively limiting bufferbloat on the > link. For Ethernet devices we have BQL to limit the lower-level queues, but > this cannot be applied to mac80211 because transmit rates can vary wildly > between packets depending on which station we are transmitting it to. > > To overcome this, we can use airtime-based queue limiting (AQL), where we > estimate the transmission time for each packet before dequeueing it, and > use that to limit the amount of data in-flight to the hardware. This idea > was originally implemented as part of the out-of-tree airtime fairness > patch to ath10k[0] in chromiumos. > > This patch ports that idea over to mac80211. The basic idea is simple > enough: Whenever we dequeue a packet from the TXQs and send it to the > driver, we estimate its airtime usage, based on the last recorded TX rate > of the station that packet is destined for. We keep a running per-AC total > of airtime queued for the whole device, and when that total climbs above 8 > ms' worth of data (corresponding to two maximum-sized aggregates), we > simply throttle the queues until it drops down again. > > 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(). > > 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). > > The throttling mechanism only kicks in if the queued airtime total goes > above the limit. Since mac80211 calculates the time based on the reported > last_tx_time from the driver, the whole throttling mechanism only kicks in > for drivers that actually report this value. With the exception of > multicast, where we always calculate an estimated tx time on the assumption > that multicast is transmitted at the lowest (6 Mbps) rate. > > The throttling added in this patch is in addition to any throttling already > performed by the airtime fairness mechanism, and in principle the two > mechanisms are orthogonal (and currently also uses two different sources of > airtime). In the future, we could amend this, using the airtime estimates > calculated by this mechanism as a fallback input to the airtime fairness > scheduler, to enable airtime fairness even on drivers that don't have a > hardware source of airtime usage for each station. > > [0] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/588190/13/drivers/net/wireless-4.2/ath/ath10k/mac.c#3845 > > Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> > --- > net/mac80211/debugfs.c | 24 ++++++++++++++++++++++++ > net/mac80211/ieee80211_i.h | 7 +++++++ > net/mac80211/status.c | 22 ++++++++++++++++++++++ > net/mac80211/tx.c | 38 +++++++++++++++++++++++++++++++++++++- > 4 files changed, 90 insertions(+), 1 deletion(-) Hi Toke, Thx a lot for working on this. Few comments inline. Regards, Lorenzo > > diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c > index 568b3b276931..c846c6e7f3e3 100644 > --- a/net/mac80211/debugfs.c > +++ b/net/mac80211/debugfs.c > @@ -148,6 +148,29 @@ static const struct file_operations aqm_ops = { > .llseek = default_llseek, > }; > [...] > @@ -3581,8 +3591,19 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw, > tx.skb = skb; > tx.sdata = vif_to_sdata(info->control.vif); > > - if (txq->sta) > + pktlen = skb->len + 38; > + if (txq->sta) { > tx.sta = container_of(txq->sta, struct sta_info, sta); > + if (tx.sta->last_tx_bitrate) { > + airtime = (pktlen * 8 * 1000 * > + tx.sta->last_tx_bitrate_reciprocal) >> IEEE80211_RECIPROCAL_SHIFT; > + airtime += IEEE80211_AIRTIME_OVERHEAD; Here we are not taking into account aggregation burst size (it is done in a rough way in chromeos implementation) and tx retries. I have not carried out any tests so far but I think IEEE80211_AIRTIME_OVERHEAD will led to a significant airtime overestimation. Do you think this can be improved? (..I agree this is not a perfect world, but .. :)) Moreover, can this approach be affected by some interrupt coalescing implemented by the chipset? > + } > + } else { > + airtime = (pktlen * 8 * 1000 * > + IEEE80211_AIRTIME_MINRATE_RECIPROCAL) >> IEEE80211_RECIPROCAL_SHIFT; > + airtime += IEEE80211_AIRTIME_OVERHEAD; > + } > > /* > * The key can be removed while the packet was queued, so need to call > @@ -3659,6 +3680,15 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw, > } > > IEEE80211_SKB_CB(skb)->control.vif = vif; > + > + if (airtime) { > + info->control.tx_time_est = airtime; > + > + spin_lock_bh(&local->active_txq_lock[ac]); > + local->airtime_queued[ac] += airtime; > + spin_unlock_bh(&local->active_txq_lock[ac]); > + } > + > return skb; > > out: > @@ -3676,6 +3706,9 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac) > > spin_lock_bh(&local->active_txq_lock[ac]); > > + if (local->airtime_queued[ac] > IEEE80211_AIRTIME_QUEUE_LIMIT) > + goto out; > + > begin: > txqi = list_first_entry_or_null(&local->active_txqs[ac], > struct txq_info, > @@ -3753,6 +3786,9 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw, > > spin_lock_bh(&local->active_txq_lock[ac]); > > + if (local->airtime_queued[ac] > IEEE80211_AIRTIME_QUEUE_LIMIT) > + goto out; > + > if (!txqi->txq.sta) > goto out; > >
Attachment:
signature.asc
Description: PGP signature
