> On Oct 17, 2019, at 11:44, Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Kan Yan <kyan@xxxxxxxxxx> writes: > >> 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); >> } > > Ah, bugger; I was afraid we'd run into this. A quick grep indicates that > it's only ath10k and iwl that do this, though, so it's probably > manageable to just fix this. I think the simplest solution is just to > restore the field after clearing, no? > >> 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(). > > Hmm, no strong opinion about this; but yeah, since we have a dedicated > function for this use I guess there's no harm in adding it there :) > Silly question, is this Overhead guaranteed to be 38 Bytes for all eternity? Otherwise a variable or a preprocessor constant might be more future proof? Best Regards Sebastian > -Toke > > _______________________________________________ > Make-wifi-fast mailing list > Make-wifi-fast@xxxxxxxxxxxxxxxxxxxxx > https://lists.bufferbloat.net/listinfo/make-wifi-fast