Message ID | 1540033534-11211-4-git-send-email-rmanohar@codeaurora.org (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Johannes Berg |
Headers | show |
Series | Move TXQ scheduling and airtime fairness into mac80211 | expand |
Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > From: Toke Høiland-Jørgensen <toke@toke.dk> > > This adds airtime accounting and scheduling to the mac80211 TXQ > scheduler. A new callback, ieee80211_sta_register_airtime(), is added > that drivers can call to report airtime usage for stations. > > When airtime information is present, mac80211 will schedule TXQs > (through ieee80211_next_txq()) in a way that enforces airtime fairness > between active stations. This scheduling works the same way as the ath9k > in-driver airtime fairness scheduling. If no airtime usage is reported > by the driver, the scheduler will default to round-robin scheduling. > > For drivers that don't control TXQ scheduling in software, a new API > function, ieee80211_txq_may_transmit(), is added which the driver can use > to check if the TXQ is eligible for transmission, or should be throttled to > enforce fairness. Calls to this function must also be enclosed in > ieee80211_txq_schedule_{start,end}() calls to ensure proper locking. > > The API ieee80211_txq_may_transmit() also ensures that TXQ list will be > aligned aginst driver's own round-robin scheduler list. i.e it rotates > the TXQ list till it makes the requested node becomes the first entry > in TXQ list. Thus both the TXQ list and driver's list are in sync. > > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> > Signed-off-by: Rajkumar Manoharan <rmanohar@codeaurora.org> > --- > include/net/mac80211.h | 58 ++++++++++++++++++++++++++++++ > net/mac80211/cfg.c | 3 ++ > net/mac80211/debugfs.c | 3 ++ > net/mac80211/debugfs_sta.c | 50 ++++++++++++++++++++++++-- > net/mac80211/ieee80211_i.h | 2 ++ > net/mac80211/main.c | 4 +++ > net/mac80211/sta_info.c | 45 +++++++++++++++++++++-- > net/mac80211/sta_info.h | 13 +++++++ > net/mac80211/status.c | 6 ++++ > net/mac80211/tx.c | 90 +++++++++++++++++++++++++++++++++++++++++++--- > 10 files changed, 264 insertions(+), 10 deletions(-) > > diff --git a/include/net/mac80211.h b/include/net/mac80211.h > index 2f5c0fbd453c..0ced3adb09ac 100644 > --- a/include/net/mac80211.h > +++ b/include/net/mac80211.h > @@ -2334,6 +2334,8 @@ enum ieee80211_hw_flags { > * @tx_sk_pacing_shift: Pacing shift to set on TCP sockets when frames from > * them are encountered. The default should typically not be changed, > * unless the driver has good reasons for needing more buffers. > + * > + * @airtime_weight: Default airtime weight preferred by driver. > */ > struct ieee80211_hw { > struct ieee80211_conf conf; > @@ -2370,6 +2372,7 @@ struct ieee80211_hw { > const struct ieee80211_cipher_scheme *cipher_schemes; > u8 max_nan_de_entries; > u8 tx_sk_pacing_shift; > + u32 airtime_weight; > }; This doesn't make sense. Airtime weights can be set by userspace, so even if a driver sets another default it is not guaranteed to be honoured. So what's the point? > +bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw, > + struct ieee80211_txq *txq) > +{ > + struct ieee80211_local *local = hw_to_local(hw); > + struct txq_info *iter, *tmp, *txqi = to_txq_info(txq); > + struct sta_info *sta; > + u8 ac = txq->ac; > + > + lockdep_assert_held(&local->active_txq_lock[ac]); > + > + if (!txqi->txq.sta) > + goto out; > + > + if (list_empty(&txqi->schedule_order)) > + goto out; > + > + list_for_each_entry_safe(iter, tmp, &local->active_txqs[ac], > + schedule_order) { > + if (iter == txqi) > + break; > + > + if (!iter->txq.sta) { > + list_move_tail(&iter->schedule_order, > + &local->active_txqs[ac]); > + continue; > + } > + sta = container_of(iter->txq.sta, struct sta_info, sta); > + if (sta->airtime[ac].deficit < 0) > + sta->airtime[ac].deficit += sta->airtime_weight; > + list_move_tail(&iter->schedule_order, &local->active_txqs[ac]); > + } So since we're rotating the queue on every call to the function, I'm wondering if this actually succeeds in throttling the slow station's airtime usage enough to achieve fairness? So I'll ask again: Did you test the fairness performance, and how? Also, taking a step back: With this, we're doing lots of work to make sure that the hardware's round-robin scheduling queue lines up with mac80211; so if we do this anyway, why can't we just control the order directly from mac80211 (which is what we do with the other next_txq() API)? -Toke
On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote: > Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > >> From: Toke Høiland-Jørgensen <toke@toke.dk> [...] >> u8 max_nan_de_entries; >> u8 tx_sk_pacing_shift; >> + u32 airtime_weight; >> }; > > This doesn't make sense. Airtime weights can be set by userspace, so > even if a driver sets another default it is not guaranteed to be > honoured. So what's the point? > The reason for driver specific default is to avoid performance impact in ath10k when the user is using vanilla ath10k with default airtime. As I mentioned earlier, mac80211 default (256us) is too low for 11ac devices especially with driver is bursting aggregation. Yes. I do understand the user can change airtime at anytime but It must be noted that different airtime weight will result in different throughput. IMHO the defaults should not impact current benchmark. Otherwise it will be alarmed as regression later. isn't it? > So since we're rotating the queue on every call to the function, I'm > wondering if this actually succeeds in throttling the slow station's > airtime usage enough to achieve fairness? So I'll ask again: Did you > test the fairness performance, and how? > We are collecting data of mixed clients (11ac, 11n and legacy) keeping them at same distance and different distance. So that lower rate clients will interfere higher MCS rate station. Also configuring different airtime weight for each stations so that throttling low rate clients more should help higher performing(better MCS) clients. By allowing different airtime for each stations, the user can control guest network over primary network. Also It helped to improve performance of preferred station and algo. to control station is given to cloud or user application. As of now, We are testing with 4 11ac clients of same distance. And collecting the performance data in multiple iteration. Below are current data of station's performance (Mbps) against airtime weight. airtime station1(%airtime) station2 station3 station4 (Mbps) No ATF 182 168 166 169 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%) 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%) 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) STA1(11ac) STA2 (11n) STA3(11a) ========== ========== ========= No ATF 225 166 3.5 ATF (4ms) 234 151 3.5 > > Also, taking a step back: > > With this, we're doing lots of work to make sure that the hardware's > round-robin scheduling queue lines up with mac80211; so if we do this > anyway, why can't we just control the order directly from mac80211 > (which is what we do with the other next_txq() API)? > The only way to enforce mac80211 ordering in ath10k is to disable pull mode in firmware and always operates in push mode similar to ath9k. -Rajkumar
Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote: >> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >> >>> From: Toke Høiland-Jørgensen <toke@toke.dk> > [...] >>> u8 max_nan_de_entries; >>> u8 tx_sk_pacing_shift; >>> + u32 airtime_weight; >>> }; >> >> This doesn't make sense. Airtime weights can be set by userspace, so >> even if a driver sets another default it is not guaranteed to be >> honoured. So what's the point? >> > The reason for driver specific default is to avoid performance impact > in ath10k when the user is using vanilla ath10k with default airtime. > As I mentioned earlier, mac80211 default (256us) is too low for 11ac > devices especially with driver is bursting aggregation. > > Yes. I do understand the user can change airtime at anytime but It > must be noted that different airtime weight will result in different > throughput. IMHO the defaults should not impact current benchmark. > Otherwise it will be alarmed as regression later. isn't it? My point is that if the user has to know the implementation-specific limitations of each driver before setting a weight, then it's not a particularly friendly API. I think we should be able to do better than that... >> So since we're rotating the queue on every call to the function, I'm >> wondering if this actually succeeds in throttling the slow station's >> airtime usage enough to achieve fairness? So I'll ask again: Did you >> test the fairness performance, and how? >> > We are collecting data of mixed clients (11ac, 11n and legacy) keeping > them at same distance and different distance. So that lower rate > clients will interfere higher MCS rate station. Also configuring > different airtime weight for each stations so that throttling low rate > clients more should help higher performing(better MCS) clients. > > By allowing different airtime for each stations, the user can control > guest network over primary network. Also It helped to improve > performance of preferred station and algo. to control station is given > to cloud or user application. > > As of now, We are testing with 4 11ac clients of same distance. And > collecting the performance data in multiple iteration. Below are > current data of station's performance (Mbps) against airtime weight. > > airtime station1(%airtime) station2 station3 station4 > (Mbps) > > No ATF 182 168 166 169 > > 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%) > > 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%) > > 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) > > 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) So this looks like it's doing *something*, but not like it's succeeding in achieving the set percentages. Did you check if the actual airtime values (in debugfs) corresponds to the configured weights? > > STA1(11ac) STA2 (11n) STA3(11a) > ========== ========== ========= > > No ATF 225 166 3.5 > > ATF (4ms) 234 151 3.5 This also shows like ATF has no effect? >> Also, taking a step back: >> >> With this, we're doing lots of work to make sure that the hardware's >> round-robin scheduling queue lines up with mac80211; so if we do this >> anyway, why can't we just control the order directly from mac80211 >> (which is what we do with the other next_txq() API)? >> > The only way to enforce mac80211 ordering in ath10k is to disable pull > mode in firmware and always operates in push mode similar to ath9k. And I assume that is not too likely to happen, or? What is the benefit of pull mode at the firmware level? -Toke
On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote: > Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > >> Yes. I do understand the user can change airtime at anytime but It >> must be noted that different airtime weight will result in different >> throughput. IMHO the defaults should not impact current benchmark. >> Otherwise it will be alarmed as regression later. isn't it? > > My point is that if the user has to know the implementation-specific > limitations of each driver before setting a weight, then it's not a > particularly friendly API. I think we should be able to do better than > that... > Hmm.. I was trying to balance throughput vs CPU usage. Keeping lower threshold is causing more lock contention and impacting throughput. I thought of configuring default airtime_weight from hostapd for legacy, 11n and 11ac stations. >> airtime station1(%airtime) station2 station3 station4 >> (Mbps) >> >> No ATF 182 168 166 169 >> >> 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%) >> >> 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%) >> >> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) >> >> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) > > So this looks like it's doing *something*, but not like it's succeeding > in achieving the set percentages. Did you check if the actual airtime > values (in debugfs) corresponds to the configured weights? > No. Will check that. >> The only way to enforce mac80211 ordering in ath10k is to disable pull >> mode in firmware and always operates in push mode similar to ath9k. > > And I assume that is not too likely to happen, or? What is the benefit > of pull mode at the firmware level? > Pull mode mainly required to support MU-MIMO. IMHO both MUMIMO & ATF can not co-exist unless both are implemented at host driver. Fixing tx mode in ath10k needs firmware reload. By introducing modparam, pull-mode can be disabled. -Rajkumar
On 2018-10-28 15:01, Rajkumar Manoharan wrote: > On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote: >> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >> >>> >>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) >>> >>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) >> >> So this looks like it's doing *something*, but not like it's >> succeeding >> in achieving the set percentages. Did you check if the actual airtime >> values (in debugfs) corresponds to the configured weights? >> > No. Will check that. > Toke, From above results, different airtime for each station is reflecting on output performance. Unfortunately I don't see such tput difference, when the tx mode is fixed in push-only. Even low weight station is giving same performance. Are you also seeing the same behavior in your setup? Could you please share your results? Not sure why low weight station (26us) is consuming more airtime than higher airtime station. Below result is taken in push-only mode that means only next_txq() ordering is followed. cat /sys/kernel/debug/ieee80211/phy0/netdev\:wlan0/stations/*/airtime RX: 0 us TX: 980443 us Weight: 176 Deficit: VO: 256 us VI: 256 us BE: -91 us BK: 256 us RX: 0 us TX: 2008512 us Weight: 26 Deficit: VO: 238 us VI: 256 us BE: 24 us BK: 256 us RX: 0 us TX: 513287 us Weight: 26 Deficit: VO: 256 us VI: 256 us BE: 1 us BK: 256 us RX: 0 us TX: 576746 us Weight: 26 Deficit: VO: 256 us VI: 256 us BE: 10 us BK: 256 us -Rajkumar
在 2018-10-28 23:48,Toke Høiland-Jørgensen 写道: > Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > >> On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote: >>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >>> >>>> From: Toke Høiland-Jørgensen <toke@toke.dk> >> [...] >>>> u8 max_nan_de_entries; >>>> u8 tx_sk_pacing_shift; >>>> + u32 airtime_weight; >>>> }; >>> >>> This doesn't make sense. Airtime weights can be set by userspace, so >>> even if a driver sets another default it is not guaranteed to be >>> honoured. So what's the point? >>> >> The reason for driver specific default is to avoid performance impact >> in ath10k when the user is using vanilla ath10k with default airtime. >> As I mentioned earlier, mac80211 default (256us) is too low for 11ac >> devices especially with driver is bursting aggregation. >> >> Yes. I do understand the user can change airtime at anytime but It >> must be noted that different airtime weight will result in different >> throughput. IMHO the defaults should not impact current benchmark. >> Otherwise it will be alarmed as regression later. isn't it? > > My point is that if the user has to know the implementation-specific > limitations of each driver before setting a weight, then it's not a > particularly friendly API. I think we should be able to do better than > that... > >>> So since we're rotating the queue on every call to the function, I'm >>> wondering if this actually succeeds in throttling the slow station's >>> airtime usage enough to achieve fairness? So I'll ask again: Did you >>> test the fairness performance, and how? >>> >> We are collecting data of mixed clients (11ac, 11n and legacy) keeping >> them at same distance and different distance. So that lower rate >> clients will interfere higher MCS rate station. Also configuring >> different airtime weight for each stations so that throttling low rate >> clients more should help higher performing(better MCS) clients. >> >> By allowing different airtime for each stations, the user can control >> guest network over primary network. Also It helped to improve >> performance of preferred station and algo. to control station is given >> to cloud or user application. >> >> As of now, We are testing with 4 11ac clients of same distance. And >> collecting the performance data in multiple iteration. Below are >> current data of station's performance (Mbps) against airtime weight. >> >> airtime station1(%airtime) station2 station3 station4 >> (Mbps) >> >> No ATF 182 168 166 169 >> >> 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%) >> >> 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%) >> >> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) >> >> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) > > So this looks like it's doing *something*, but not like it's succeeding > in achieving the set percentages. Did you check if the actual airtime > values (in debugfs) corresponds to the configured weights? > >> >> STA1(11ac) STA2 (11n) STA3(11a) >> ========== ========== ========= >> >> No ATF 225 166 3.5 >> >> ATF (4ms) 234 151 3.5 > > This also shows like ATF has no effect? > >>> Also, taking a step back: >>> >>> With this, we're doing lots of work to make sure that the hardware's >>> round-robin scheduling queue lines up with mac80211; so if we do this >>> anyway, why can't we just control the order directly from mac80211 >>> (which is what we do with the other next_txq() API)? >>> Toke and Raj, How about treating any txqs before the txq that FW asked for in push-pull mode as pending txqs? Then we can dequeue and reorder them.And airtime weight need to be tuned to make sure it won't cost too much time. After that, check the txq FW wishes to fetch in txq_may_transmit. Is this way able to achieve fairness and line up with mac80211? Also, we may need to consider if FW supports in this way. >> The only way to enforce mac80211 ordering in ath10k is to disable pull >> mode in firmware and always operates in push mode similar to ath9k. > > And I assume that is not too likely to happen, or? What is the benefit > of pull mode at the firmware level? > > -Toke
Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > On 2018-10-28 15:01, Rajkumar Manoharan wrote: >> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote: >>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >>> >>>> >>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) >>>> >>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) >>> >>> So this looks like it's doing *something*, but not like it's >>> succeeding >>> in achieving the set percentages. Did you check if the actual airtime >>> values (in debugfs) corresponds to the configured weights? >>> >> No. Will check that. >> > Toke, > > From above results, different airtime for each station is reflecting on > output performance. Unfortunately I don't see such tput difference, when > the tx mode is fixed in push-only. Even low weight station is giving > same > performance. Are you also seeing the same behavior in your setup? Could > you please share your results? Sorry, I've been travelling this week; I'll be back in the office next week and can run some tests then. I may also have an idea for a different algorithm that will work better in pull mode, but need to see if it works at all first :) How do I force ath10k into push mode? -Toke
On 2018-11-02 03:30, Toke Høiland-Jørgensen wrote: > Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > >> On 2018-10-28 15:01, Rajkumar Manoharan wrote: >>> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote: >>>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >>>> >>>>> >>>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) >>>>> >>>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) >>>> >>>> So this looks like it's doing *something*, but not like it's >>>> succeeding >>>> in achieving the set percentages. Did you check if the actual >>>> airtime >>>> values (in debugfs) corresponds to the configured weights? >>>> >>> No. Will check that. >>> >> Toke, >> >> From above results, different airtime for each station is reflecting >> on >> output performance. Unfortunately I don't see such tput difference, >> when >> the tx mode is fixed in push-only. Even low weight station is giving >> same >> performance. Are you also seeing the same behavior in your setup? >> Could >> you please share your results? > > Sorry, I've been travelling this week; I'll be back in the office next > week and can run some tests then. I may also have an idea for a > different algorithm that will work better in pull mode, but need to see > if it works at all first :) > Wow... :) Meanwhile we did some more experiments with both modes. The experiment was done in open environment and fixed rate and UDP traffic ran for 60 seconds. Seems like push mode not honoring the configured weight. Always the airtime was almost same whereas in pull-mode airtime is changing based on configured weight. Hence I would like to know your results. sta1 sta2 sta3 sta4 pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us) 12s(256us) 12s(256us) 13s(256us) 12s(256us) 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms) push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us) 15s(256us) 12s(256us) 16s(256us) 12s(256us) 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms) > How do I force ath10k into push mode? > Attaching the change to fix push mode. -Rajkumar diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index c8dbbfa901af..1981e0ea8c7e 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -41,6 +41,7 @@ static bool uart_print; static bool skip_otp; static bool rawmode; +bool peer_flow_ctl = false; unsigned long ath10k_coredump_mask = BIT(ATH10K_FW_CRASH_DUMP_REGISTERS) | BIT(ATH10K_FW_CRASH_DUMP_CE_DATA); @@ -52,6 +53,7 @@ module_param(skip_otp, bool, 0644); module_param(rawmode, bool, 0644); module_param_named(coredump_mask, ath10k_coredump_mask, ulong, 0444); +module_param(peer_flow_ctl, bool, 0644); MODULE_PARM_DESC(debug_mask, "Debugging mask"); MODULE_PARM_DESC(uart_print, "Uart target debugging"); diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index e6f597d9f226..bf2e70411d90 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -1201,6 +1201,7 @@ static inline bool ath10k_peer_stats_enabled(struct ath10k *ar) } extern unsigned long ath10k_coredump_mask; +extern bool peer_flow_ctl; struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev, enum ath10k_bus bus, diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index d74b55ba9914..d5ad88b85b69 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -216,6 +216,9 @@ int ath10k_mac_ext_resource_config(struct ath10k *ar, u32 val) else platform_type = WMI_HOST_PLATFORM_HIGH_PERF; + if (!peer_flow_ctl) + platform_type = WMI_HOST_PLATFORM_LOW_PERF_NO_FETCH; + ret = ath10k_wmi_ext_resource_config(ar, platform_type, val); if (ret && ret != -EOPNOTSUPP) { diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h index f7badd079051..6958d4f58a21 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.h +++ b/drivers/net/wireless/ath/ath10k/wmi.h @@ -7128,6 +7128,7 @@ struct wmi_pno_scan_req { enum wmi_host_platform_type { WMI_HOST_PLATFORM_HIGH_PERF, WMI_HOST_PLATFORM_LOW_PERF, + WMI_HOST_PLATFORM_LOW_PERF_NO_FETCH, }; enum wmi_bss_survey_req_type {
Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > On 2018-11-02 03:30, Toke Høiland-Jørgensen wrote: >> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >> >>> On 2018-10-28 15:01, Rajkumar Manoharan wrote: >>>> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote: >>>>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >>>>> >>>>>> >>>>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%) >>>>>> >>>>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%) >>>>> >>>>> So this looks like it's doing *something*, but not like it's >>>>> succeeding >>>>> in achieving the set percentages. Did you check if the actual >>>>> airtime >>>>> values (in debugfs) corresponds to the configured weights? >>>>> >>>> No. Will check that. >>>> >>> Toke, >>> >>> From above results, different airtime for each station is reflecting >>> on >>> output performance. Unfortunately I don't see such tput difference, >>> when >>> the tx mode is fixed in push-only. Even low weight station is giving >>> same >>> performance. Are you also seeing the same behavior in your setup? >>> Could >>> you please share your results? >> >> Sorry, I've been travelling this week; I'll be back in the office next >> week and can run some tests then. I may also have an idea for a >> different algorithm that will work better in pull mode, but need to see >> if it works at all first :) >> > Wow... :) > > Meanwhile we did some more experiments with both modes. The experiment > was done in open environment and fixed rate and UDP traffic ran for 60 > seconds. > > Seems like push mode not honoring the configured weight. Always the > airtime was almost same whereas in pull-mode airtime is changing based > on configured weight. Hence I would like to know your results. Right, so I verified that the current version of the patch set still works with ath9k. However, the ath10k card I have doesn't seem to support peer stats, so I can't test ath10k. $ lspci | grep Qualcomm 03:00.0 Network controller: Qualcomm Atheros QCA986x/988x 802.11ac Wireless Network Adapter $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/chip_id 0x043202ff $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/wmi_services | grep PEER WMI_SERVICE_PEER_CACHING - WMI_SERVICE_PEER_STATS - Is there a way to force-enable airtime support, is this a hardware issue? > sta1 sta2 sta3 sta4 > pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us) > 12s(256us) 12s(256us) 13s(256us) 12s(256us) > 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms) > > push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us) > 15s(256us) 12s(256us) 16s(256us) 12s(256us) > 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms) Right, so the pull-mode results are encouraging! *Something* is happening, at least, even though the aggregate airtime doesn't quite match the ratios of the configured weights. Are you running the UDP generator on the AP itself, or on a separate device, BTW? If it's on the AP, the userspace socket can get throttled, which will interfere with results, so it's better to have it on a separate device (connected via ethernet). As for push-mode, could this be because of bad buffer management? I.e., because there's a lag between the time airtime is registered, and the time that airtime usage is reported, the driver just pushes a whole bunch of packets to the firmware when it gets the chance, which prevents the scheduler from throttling properly. This could also explain the better, but not quite perfect, results in pull mode, assuming that pull mode results in better firmware buffer management which reduces, but doesn't quite remove, the lag. If this is indeed the reason, the queue limit patches should hopefully be a solution. So guess we need to get those working as well :) -Toke
On 2018-11-07 06:53, Toke Høiland-Jørgensen wrote: > Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > >> Meanwhile we did some more experiments with both modes. The experiment >> was done in open environment and fixed rate and UDP traffic ran for 60 >> seconds. >> >> Seems like push mode not honoring the configured weight. Always the >> airtime was almost same whereas in pull-mode airtime is changing based >> on configured weight. Hence I would like to know your results. > > Right, so I verified that the current version of the patch set still > works with ath9k. However, the ath10k card I have doesn't seem to > support peer stats, so I can't test ath10k. > > $ lspci | grep Qualcomm > 03:00.0 Network controller: Qualcomm Atheros QCA986x/988x 802.11ac > Wireless Network Adapter > > $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/chip_id > 0x043202ff > > $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/wmi_services | grep PEER > WMI_SERVICE_PEER_CACHING - > WMI_SERVICE_PEER_STATS - > Oops... Yeah 988x firmware (10.2.4) does not have peer stats support. > > Is there a way to force-enable airtime support, is this a hardware > issue? > Unfortunately not. There is one more pending change that handles airtime report from HTT tx-compl. Again it depends firmware support. These experiments are taken with this f/w interface. Will post the change. >> sta1 sta2 sta3 sta4 >> pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us) >> 12s(256us) 12s(256us) 13s(256us) 12s(256us) >> 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms) >> >> push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us) >> 15s(256us) 12s(256us) 16s(256us) 12s(256us) >> 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms) > > Right, so the pull-mode results are encouraging! *Something* is > happening, at least, even though the aggregate airtime doesn't quite > match the ratios of the configured weights. > > Are you running the UDP generator on the AP itself, or on a separate > device, BTW? If it's on the AP, the userspace socket can get throttled, > which will interfere with results, so it's better to have it on a > separate device (connected via ethernet). > Traffic b/w wired client (via ethernet) and wireless clients. > As for push-mode, could this be because of bad buffer management? I.e., > because there's a lag between the time airtime is registered, and the > time that airtime usage is reported, the driver just pushes a whole > bunch of packets to the firmware when it gets the chance, which > prevents > the scheduler from throttling properly. This could also explain the > better, but not quite perfect, results in pull mode, assuming that pull > mode results in better firmware buffer management which reduces, but > doesn't quite remove, the lag. > Hmm... I agree that lag in reporting airtime may cause more data push to hw. Right now both tx and tx-compl are serialized by active_txq_lock. So once lock acquired by tx path, it will download all frames. i.e it is even true for ath9k driver. Hence I am wondering how it is working only with ath9k. In ath10k, The airtime always be reported in tx-completion. I dont see much lag from my experiments. > If this is indeed the reason, the queue limit patches should hopefully > be a solution. So guess we need to get those working as well :) > I would prefer to baseline the basic infra into upstream first and do enhancement on top of that. I request you to revisit maintaining per driver default. Otherwise there would be performance impact with 256us. :( -Rajkumar
Rajkumar Manoharan <rmanohar@codeaurora.org> writes: > On 2018-11-07 06:53, Toke Høiland-Jørgensen wrote: >> Rajkumar Manoharan <rmanohar@codeaurora.org> writes: >> >>> Meanwhile we did some more experiments with both modes. The experiment >>> was done in open environment and fixed rate and UDP traffic ran for 60 >>> seconds. >>> >>> Seems like push mode not honoring the configured weight. Always the >>> airtime was almost same whereas in pull-mode airtime is changing based >>> on configured weight. Hence I would like to know your results. >> >> Right, so I verified that the current version of the patch set still >> works with ath9k. However, the ath10k card I have doesn't seem to >> support peer stats, so I can't test ath10k. >> >> $ lspci | grep Qualcomm >> 03:00.0 Network controller: Qualcomm Atheros QCA986x/988x 802.11ac >> Wireless Network Adapter >> >> $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/chip_id >> 0x043202ff >> >> $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/wmi_services | grep PEER >> WMI_SERVICE_PEER_CACHING - >> WMI_SERVICE_PEER_STATS - >> > > Oops... Yeah 988x firmware (10.2.4) does not have peer stats support. > >> >> Is there a way to force-enable airtime support, is this a hardware >> issue? >> > Unfortunately not. There is one more pending change that handles > airtime report from HTT tx-compl. Again it depends firmware support. > These experiments are taken with this f/w interface. Will post the > change. Right, thought so. Too bad. Guess you are doing all the ath10k testing, then ;) >>> sta1 sta2 sta3 sta4 >>> pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us) >>> 12s(256us) 12s(256us) 13s(256us) 12s(256us) >>> 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms) >>> >>> push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us) >>> 15s(256us) 12s(256us) 16s(256us) 12s(256us) >>> 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms) >> >> Right, so the pull-mode results are encouraging! *Something* is >> happening, at least, even though the aggregate airtime doesn't quite >> match the ratios of the configured weights. >> >> Are you running the UDP generator on the AP itself, or on a separate >> device, BTW? If it's on the AP, the userspace socket can get throttled, >> which will interfere with results, so it's better to have it on a >> separate device (connected via ethernet). >> > Traffic b/w wired client (via ethernet) and wireless clients. Cool. >> As for push-mode, could this be because of bad buffer management? I.e., >> because there's a lag between the time airtime is registered, and the >> time that airtime usage is reported, the driver just pushes a whole >> bunch of packets to the firmware when it gets the chance, which >> prevents >> the scheduler from throttling properly. This could also explain the >> better, but not quite perfect, results in pull mode, assuming that pull >> mode results in better firmware buffer management which reduces, but >> doesn't quite remove, the lag. >> > Hmm... I agree that lag in reporting airtime may cause more data push > to hw. Right now both tx and tx-compl are serialized by > active_txq_lock. So once lock acquired by tx path, it will download > all frames. i.e it is even true for ath9k driver. Hence I am wondering > how it is working only with ath9k. If enough packets are dequeued at once that the TXQ empties and is not put back on the scheduling loop, the next_txq() loop is just going to loop through the remaining TXQs and immediately increase their quantum. In ath9k, there's a maximum of two outstanding aggregates below the TXQ level, but I think you mentioned that ath10k can queue thousands in firmware? Your patch removes the 'max 16 packets at a time' check before the call to ath10k_mac_tx_push_txq(); try adding that back and see if it helps? >> If this is indeed the reason, the queue limit patches should hopefully >> be a solution. So guess we need to get those working as well :) >> > I would prefer to baseline the basic infra into upstream first and do > enhancement on top of that. Sure, I'm fine with merging this first and building on top. > I request you to revisit maintaining per driver default. Otherwise > there would be performance impact with 256us. :( Hmm, how about we make it a driver-specified multiplier instead (which could be 4, 8 or 16 for ath10k)? That way it would still work even if userspace changes the weights. It would affect the minimum possible granularity, but that is probably acceptable; and we would be free to change the mechanism later, without affecting the userspace API. -Toke
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 2f5c0fbd453c..0ced3adb09ac 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -2334,6 +2334,8 @@ enum ieee80211_hw_flags { * @tx_sk_pacing_shift: Pacing shift to set on TCP sockets when frames from * them are encountered. The default should typically not be changed, * unless the driver has good reasons for needing more buffers. + * + * @airtime_weight: Default airtime weight preferred by driver. */ struct ieee80211_hw { struct ieee80211_conf conf; @@ -2370,6 +2372,7 @@ struct ieee80211_hw { const struct ieee80211_cipher_scheme *cipher_schemes; u8 max_nan_de_entries; u8 tx_sk_pacing_shift; + u32 airtime_weight; }; static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw, @@ -5350,6 +5353,34 @@ void ieee80211_sta_block_awake(struct ieee80211_hw *hw, void ieee80211_send_eosp_nullfunc(struct ieee80211_sta *pubsta, int tid); /** + * ieee80211_sta_register_airtime - register airtime usage for a sta/tid + * + * Register airtime usage for a given sta on a given tid. The driver can call + * this function to notify mac80211 that a station used a certain amount of + * airtime. This information will be used by the TXQ scheduler to schedule + * stations in a way that ensures airtime fairness. + * + * The reported airtime should as a minimum include all time that is spent + * transmitting to the remote station, including overhead and padding, but not + * including time spent waiting for a TXOP. If the time is not reported by the + * hardware it can in some cases be calculated from the rate and known frame + * composition. When possible, the time should include any failed transmission + * attempts. + * + * The driver can either call this function synchronously for every packet or + * aggregate, or asynchronously as airtime usage information becomes available. + * TX and RX airtime can be reported together, or separately by setting one of + * them to 0. + * + * @pubsta: the station + * @tid: the TID to register airtime for + * @tx_airtime: airtime used during TX (in usec) + * @rx_airtime: airtime used during RX (in usec) + */ +void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid, + u32 tx_airtime, u32 rx_airtime); + +/** * ieee80211_iter_keys - iterate keys programmed into the device * @hw: pointer obtained from ieee80211_alloc_hw() * @vif: virtual interface to iterate, may be %NULL for all @@ -6107,6 +6138,33 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw, void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac); /** + * ieee80211_txq_may_transmit - check whether TXQ is allowed to transmit + * + * This function is used to check whether given txq is allowed to transmit by + * the airtime scheduler, and can be used by drivers to access the airtime + * fairness accounting without going using the scheduling order enfored by + * next_txq(). + * + * Returns %true if the airtime scheduler thinks the TXQ should be allowed to + * transmit, and %false if it should be throttled. This function can also have + * the side effect of rotating the TXQ in the scheduler rotation, which will + * eventually bring the deficit to positive and allow the station to transmit + * again. + * + * The API ieee80211_txq_may_transmit() also ensures that TXQ list will be + * aligned aginst driver's own round-robin scheduler list. i.e it rotates + * the TXQ list till it makes the requested node becomes the first entry + * in TXQ list. Thus both the TXQ list and driver's list are in sync. If this + * function returns %true, the driver is expected to schedule packets + * for transmission, and then return the TXQ through ieee80211_return_txq(). + * + * @hw: pointer as obtained from ieee80211_alloc_hw() + * @txq: pointer obtained from station or virtual interface + */ +bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw, + struct ieee80211_txq *txq); + +/** * ieee80211_txq_get_depth - get pending frame/byte count of given txq * * The values are not guaranteed to be coherent with regard to each other, i.e. diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 914aef7e7afd..337ac2b57e59 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -1388,6 +1388,9 @@ static int sta_apply_parameters(struct ieee80211_local *local, if (ieee80211_vif_is_mesh(&sdata->vif)) sta_apply_mesh_params(local, sta, params); + if (params->airtime_weight) + sta->airtime_weight = params->airtime_weight; + /* set the STA state after all sta info from usermode has been set */ if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) || set & BIT(NL80211_STA_FLAG_ASSOCIATED)) { diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c index 3fe541e358f3..81c5fec2eae7 100644 --- a/net/mac80211/debugfs.c +++ b/net/mac80211/debugfs.c @@ -383,6 +383,9 @@ void debugfs_hw_add(struct ieee80211_local *local) if (local->ops->wake_tx_queue) DEBUGFS_ADD_MODE(aqm, 0600); + debugfs_create_u16("airtime_flags", 0600, + phyd, &local->airtime_flags); + statsd = debugfs_create_dir("statistics", phyd); /* if the dir failed, don't put all the other things into the root! */ diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c index af5185a836e5..446908ab3f5d 100644 --- a/net/mac80211/debugfs_sta.c +++ b/net/mac80211/debugfs_sta.c @@ -181,9 +181,9 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf, txqi->tin.tx_bytes, txqi->tin.tx_packets, txqi->flags, - txqi->flags & (1<<IEEE80211_TXQ_STOP) ? "STOP" : "RUN", - txqi->flags & (1<<IEEE80211_TXQ_AMPDU) ? " AMPDU" : "", - txqi->flags & (1<<IEEE80211_TXQ_NO_AMSDU) ? " NO-AMSDU" : ""); + test_bit(IEEE80211_TXQ_STOP, &txqi->flags) ? "STOP" : "RUN", + test_bit(IEEE80211_TXQ_AMPDU, &txqi->flags) ? " AMPDU" : "", + test_bit(IEEE80211_TXQ_NO_AMSDU, &txqi->flags) ? " NO-AMSDU" : ""); } rcu_read_unlock(); @@ -195,6 +195,46 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf, } STA_OPS(aqm); +static ssize_t sta_airtime_read(struct file *file, char __user *userbuf, + size_t count, loff_t *ppos) +{ + struct sta_info *sta = file->private_data; + struct ieee80211_local *local = sta->sdata->local; + size_t bufsz = 200; + char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf; + u64 rx_airtime = 0, tx_airtime = 0; + s64 deficit[IEEE80211_NUM_ACS]; + ssize_t rv; + int ac; + + if (!buf) + return -ENOMEM; + + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { + spin_lock_bh(&local->active_txq_lock[ac]); + rx_airtime += sta->airtime[ac].rx_airtime; + tx_airtime += sta->airtime[ac].tx_airtime; + deficit[ac] = sta->airtime[ac].deficit; + spin_unlock_bh(&local->active_txq_lock[ac]); + } + + p += scnprintf(p, bufsz + buf - p, + "RX: %llu us\nTX: %llu us\nWeight: %u\n" + "Deficit: VO: %lld us VI: %lld us BE: %lld us BK: %lld us\n", + rx_airtime, + tx_airtime, + sta->airtime_weight, + deficit[0], + deficit[1], + deficit[2], + deficit[3]); + + rv = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf); + kfree(buf); + return rv; +} +STA_OPS(airtime); + static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { @@ -906,6 +946,10 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta) if (local->ops->wake_tx_queue) DEBUGFS_ADD(aqm); + if (wiphy_ext_feature_isset(local->hw.wiphy, + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS)) + DEBUGFS_ADD(airtime); + if (sizeof(sta->driver_buffered_tids) == sizeof(u32)) debugfs_create_x32("driver_buffered_tids", 0400, sta->debugfs_dir, diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 976531717902..7abc20c0e47c 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1136,6 +1136,8 @@ struct ieee80211_local { struct list_head active_txqs[IEEE80211_NUM_ACS]; u16 schedule_round[IEEE80211_NUM_ACS]; + u16 airtime_flags; + const struct ieee80211_ops *ops; /* diff --git a/net/mac80211/main.c b/net/mac80211/main.c index 3ea2369e0992..3d79f42042dd 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -667,6 +667,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len, INIT_LIST_HEAD(&local->active_txqs[i]); spin_lock_init(&local->active_txq_lock[i]); } + local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX; INIT_LIST_HEAD(&local->chanctx_list); mutex_init(&local->chanctx_mtx); @@ -1153,6 +1154,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw) if (!local->hw.max_nan_de_entries) local->hw.max_nan_de_entries = IEEE80211_MAX_NAN_INSTANCE_ID; + if (!local->hw.airtime_weight) + local->hw.airtime_weight = IEEE80211_DEFAULT_AIRTIME_WEIGHT; + result = ieee80211_wep_init(local); if (result < 0) wiphy_debug(local->hw.wiphy, "Failed to initialize wep: %d\n", diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index c2f5cb7df54f..5a6d05063341 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -90,7 +90,6 @@ static void __cleanup_single_sta(struct sta_info *sta) struct tid_ampdu_tx *tid_tx; struct ieee80211_sub_if_data *sdata = sta->sdata; struct ieee80211_local *local = sdata->local; - struct fq *fq = &local->fq; struct ps_data *ps; if (test_sta_flag(sta, WLAN_STA_PS_STA) || @@ -120,9 +119,7 @@ static void __cleanup_single_sta(struct sta_info *sta) txqi = to_txq_info(sta->sta.txq[i]); - spin_lock_bh(&fq->lock); ieee80211_txq_purge(local, txqi); - spin_unlock_bh(&fq->lock); } } @@ -387,9 +384,12 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, if (sta_prepare_rate_control(local, sta, gfp)) goto free_txq; + sta->airtime_weight = hw->airtime_weight; + for (i = 0; i < IEEE80211_NUM_ACS; i++) { skb_queue_head_init(&sta->ps_tx_buf[i]); skb_queue_head_init(&sta->tx_filtered[i]); + sta->airtime[i].deficit = sta->airtime_weight; } for (i = 0; i < IEEE80211_NUM_TIDS; i++) @@ -1826,6 +1826,28 @@ void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta, } EXPORT_SYMBOL(ieee80211_sta_set_buffered); +void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid, + u32 tx_airtime, u32 rx_airtime) +{ + struct sta_info *sta = container_of(pubsta, struct sta_info, sta); + struct ieee80211_local *local = sta->sdata->local; + struct txq_info *txqi; + u8 ac = ieee80211_ac_from_tid(tid); + u32 airtime = 0; + + if (sta->local->airtime_flags & AIRTIME_USE_TX) + airtime += tx_airtime; + if (sta->local->airtime_flags & AIRTIME_USE_RX) + airtime += rx_airtime; + + spin_lock_bh(&local->active_txq_lock[ac]); + sta->airtime[ac].tx_airtime += tx_airtime; + sta->airtime[ac].rx_airtime += rx_airtime; + sta->airtime[ac].deficit -= airtime; + spin_unlock_bh(&local->active_txq_lock[ac]); +} +EXPORT_SYMBOL(ieee80211_sta_register_airtime); + int sta_info_move_state(struct sta_info *sta, enum ieee80211_sta_state new_state) { @@ -2188,6 +2210,23 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo, sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED); } + if (!(sinfo->filled & BIT(NL80211_STA_INFO_RX_DURATION))) { + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) + sinfo->rx_duration += sta->airtime[ac].rx_airtime; + sinfo->filled |= BIT(NL80211_STA_INFO_RX_DURATION); + } + + if (!(sinfo->filled & BIT(NL80211_STA_INFO_TX_DURATION))) { + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) + sinfo->tx_duration += sta->airtime[ac].tx_airtime; + sinfo->filled |= BIT(NL80211_STA_INFO_TX_DURATION); + } + + if (!(sinfo->filled & BIT(NL80211_STA_INFO_AIRTIME_WEIGHT))) { + sinfo->airtime_weight = sta->airtime_weight; + sinfo->filled |= BIT(NL80211_STA_INFO_AIRTIME_WEIGHT); + } + sinfo->rx_dropped_misc = sta->rx_stats.dropped; if (sta->pcpu_rx_stats) { for_each_possible_cpu(cpu) { diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index 9a04327d71d1..b1b0fd6a2e21 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -127,6 +127,16 @@ enum ieee80211_agg_stop_reason { AGG_STOP_DESTROY_STA, }; +/* Debugfs flags to enable/disable use of RX/TX airtime in scheduler */ +#define AIRTIME_USE_TX BIT(0) +#define AIRTIME_USE_RX BIT(1) + +struct airtime_info { + u64 rx_airtime; + u64 tx_airtime; + s64 deficit; +}; + struct sta_info; /** @@ -563,6 +573,9 @@ struct sta_info { } tx_stats; u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1]; + struct airtime_info airtime[IEEE80211_NUM_ACS]; + u16 airtime_weight; + /* * Aggregation information, locked with lock. */ diff --git a/net/mac80211/status.c b/net/mac80211/status.c index 91d7c0cd1882..cd4582f80fea 100644 --- a/net/mac80211/status.c +++ b/net/mac80211/status.c @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw, ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data, acked, info->status.tx_time); + if (info->status.tx_time && + wiphy_ext_feature_isset(local->hw.wiphy, + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS)) + ieee80211_sta_register_airtime(&sta->sta, tid, + info->status.tx_time, 0); + if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) { if (info->flags & IEEE80211_TX_STAT_ACK) { if (sta->status_stats.lost_packets) diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 305965283506..dd2354188357 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1488,8 +1488,11 @@ void ieee80211_txq_purge(struct ieee80211_local *local, struct fq *fq = &local->fq; struct fq_tin *tin = &txqi->tin; + spin_lock_bh(&fq->lock); fq_tin_reset(fq, tin, fq_skb_free_func); ieee80211_purge_tx_queue(&local->hw, &txqi->frags); + spin_unlock_bh(&fq->lock); + spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]); list_del_init(&txqi->schedule_order); spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]); @@ -3638,11 +3641,28 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac) lockdep_assert_held(&local->active_txq_lock[ac]); + begin: txqi = list_first_entry_or_null(&local->active_txqs[ac], struct txq_info, schedule_order); + if (!txqi) + return NULL; + + if (txqi->txq.sta) { + struct sta_info *sta = container_of(txqi->txq.sta, + struct sta_info, sta); + + if (sta->airtime[txqi->txq.ac].deficit < 0) { + sta->airtime[txqi->txq.ac].deficit += + sta->airtime_weight; + list_move_tail(&txqi->schedule_order, + &local->active_txqs[txqi->txq.ac]); + goto begin; + } + } + - if (!txqi || txqi->schedule_round == local->schedule_round[ac]) + if (txqi->schedule_round == local->schedule_round[ac]) return NULL; list_del_init(&txqi->schedule_order); @@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw, lockdep_assert_held(&local->active_txq_lock[txq->ac]); if (list_empty(&txqi->schedule_order) && - (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) - list_add_tail(&txqi->schedule_order, - &local->active_txqs[txq->ac]); + (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) { + /* If airtime accounting is active, always enqueue STAs at the + * head of the list to ensure that they only get moved to the + * back by the airtime DRR scheduler once they have a negative + * deficit. A station that already has a negative deficit will + * get immediately moved to the back of the list on the next + * call to ieee80211_next_txq(). + */ + if (wiphy_ext_feature_isset(local->hw.wiphy, + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS) + && txqi->txq.sta) + list_add(&txqi->schedule_order, + &local->active_txqs[txq->ac]); + else + list_add_tail(&txqi->schedule_order, + &local->active_txqs[txq->ac]); + } } EXPORT_SYMBOL(ieee80211_return_txq); +bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw, + struct ieee80211_txq *txq) +{ + struct ieee80211_local *local = hw_to_local(hw); + struct txq_info *iter, *tmp, *txqi = to_txq_info(txq); + struct sta_info *sta; + u8 ac = txq->ac; + + lockdep_assert_held(&local->active_txq_lock[ac]); + + if (!txqi->txq.sta) + goto out; + + if (list_empty(&txqi->schedule_order)) + goto out; + + list_for_each_entry_safe(iter, tmp, &local->active_txqs[ac], + schedule_order) { + if (iter == txqi) + break; + + if (!iter->txq.sta) { + list_move_tail(&iter->schedule_order, + &local->active_txqs[ac]); + continue; + } + sta = container_of(iter->txq.sta, struct sta_info, sta); + if (sta->airtime[ac].deficit < 0) + sta->airtime[ac].deficit += sta->airtime_weight; + list_move_tail(&iter->schedule_order, &local->active_txqs[ac]); + } + + sta = container_of(txqi->txq.sta, struct sta_info, sta); + if (sta->airtime[ac].deficit >= 0) + goto out; + + sta->airtime[ac].deficit += sta->airtime_weight; + list_move_tail(&txqi->schedule_order, &local->active_txqs[ac]); + + return false; +out: + if (!list_empty(&txqi->schedule_order)) + list_del_init(&txqi->schedule_order); + + return true; +} +EXPORT_SYMBOL(ieee80211_txq_may_transmit); + void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac) { struct ieee80211_local *local = hw_to_local(hw);