On 2019-03-17 12:32, Toke Høiland-Jørgensen wrote: > Felix Fietkau <nbd@xxxxxxxx> writes: > >> On 2019-03-16 23:28, Toke Høiland-Jørgensen wrote: >>> Felix Fietkau <nbd@xxxxxxxx> writes: >>> >>>> Performance improvement and preparation for adding airtime fairness >>>> support >>> >>> Great to see this! Do you have a plan for the airtime fairness part? >>> I.e., how to get the airtime information? >> Not yet. Still need to investigate what kind of information the hardware >> can provide. On a first glance it seems rather limited, so we may have >> to approximate based on tx status rates/retry and average packet size. > > OK, cool. A byte-based estimator can also be useful for preventing dumb > firmware from buffering too much. The Chromium guys did that for ath10k: > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/588190/13/drivers/net/wireless-4.2/ath/ath10k/mac.c#3826 Interesting, thanks. I can probably use some ideas from that. >>> The call to ieee80211_return_txq() is really meant to be unconditional. >>> The TXQ will only actually be scheduled if it still has packets queued. >>> I know it's slightly more expensive to have the check in mac80211, but >>> this is what makes it possible to change the implementation without >>> touching the drivers (such as in the RFC patch I sent earlier that >>> switches the scheduling algorithm)... >> I think this API needs to be extended to allow the driver to specify >> that it has buffered packets for a txq. Otherwise there's a small window >> where the driver has packets for a txq but mac80211 doesn't, and >> mac80211 won't schedule the queue in that case. >> I'll send a patch for this soon. > > Right, makes sense. As long as mac80211 is in control over how it will > react to that information (thus allowing to e.g., invert the logic if > needed), I have no objections to extending the API... :)I'm thinking of changing the code to make ieee80211_schedule_txq add the txq to the list, even if mac80211 does not have any frames buffered for it. I've looked at ath9k (the only user at the moment), and it seems to call the function in that way already: at PS wake or tx status time if it has frames in its internal retry queue. While it does not match the current documented behavior for that function, it nicely fits ath9k's currently unfulfilled expectations ;) - Felix
