On Thu, 2021-01-07 at 14:08 +0100, Toke Høiland-Jørgensen wrote: > Ryder Lee <ryder.lee@xxxxxxxxxxxx> writes: > > > On Wed, 2021-01-06 at 16:41 +0100, Toke Høiland-Jørgensen wrote: > >> Felix Fietkau <nbd@xxxxxxxx> writes: > >> > >> > On 2021-01-06 11:51, Toke Høiland-Jørgensen wrote: > >> >> Ryder Lee <ryder.lee@xxxxxxxxxxxx> writes: > >> >> > >> >>> The selected txq should be scheduled unconditionally if > >> >>> NL80211_EXT_FEATURE_AIRTIME_FAIRNESS is not set by driver. > >> >>> > >> >>> Also put the sta to the end of the active_txqs list if > >> >>> deficit is negative then move on to the next txq. > >> >> > >> >> Why is this needed? If the feature is not set, no airtime should ever be > >> >> accounted to the station, and so sta->airtime[txqi->txq.ac].deficit will > >> >> always be 0 - so you're just adding another check that doesn't actually > >> >> change the behaviour, aren't you? > >> > > >> > I think it might make sense to keep airtime reporting even when airtime > >> > fairness is disabled at run time, so this patch makes sense to me. > >> > Instead of this patch, the right place to deal with this would probably > >> > be ieee80211_sta_register_airtime. > >> > >> When the fairness mechanism is user-disabled I agree it makes sense to > >> still keep the accounting; and in fact that's what > >> ieee80211_sta_register_airtime() already does when the accounting is > >> turned off by way of the airtime_flags field... So don't think anything > >> else is needed there either? > >> > >> -Toke > > > > Not sure I get this right. Are you talking about local->airtime_flags = > > AIRTIME_USE_TX | AIRTIME_USE_RX ? I think that's different and we still > > need to take NL80211_EXT_FEATURE_AIRTIME_FAIRNESS into account, right? > > I just meant that what Felix was asking for (a way *for the user* to > disable airtime fairness while still getting the airtime usage > accounted) is possible by setting those flags. The EXT_FEATURE flag is > meant as a way for the driver to signal to mac80211 that it supports > reporting airtime at all; so ideally it should be a flag that is only > set once. > > Going back and reading your initial response it seems like you may be > toggling the flag dynamically in the driver, though? Is this accurate? > And if so, why? Is it not enough for you to fiddle with the > USE_TX/USE_RX flags? :) > > -Toke Gotcha. We just set it once indeed. So the way you think is disable the EXT_FEATURE flag and clear AIRTIME_USE_TX through debugfs (DEBUGFS_ADD_MODE(airtime_flags, 0600)) in the meantime. Ryder
