On Sat, Sep 10, 2016 at 3:33 PM, Toke Høiland-Jørgensen wrote: > CoDel can be too aggressive if a station sends at a very low rate, > leading to starvation. This gets worse the more stations are present, as > each station gets more bursty the longer the round-robin scheduling > between stations takes. > > This adds dynamic adjustment of CoDel parameters per station. It uses > the rate selection information to estimate throughput and sets more > lenient CoDel parameters if the estimated throughput is below a > threshold. To not change parameters too often, a hysteresis of two > seconds is added. > ​Where is this 2 second constant coming from? I'd expect it should be of order the maximum RTT (or a small constant factor of that, which for intercontinental connections should be 200-300ms. More interestingly, maybe the adjustment should be related to the # of active stations. Basically, I'm pushing back about an arbitrary number apparently picked out of thin air... ;-). - Jim ​ > > A new callback is added that drivers can use to notify mac80211 about > changes in expected throughput, so the same adjustment can be made for > cards that implement rate control in firmware. Drivers that don't use > this will just get the default parameters. > > The threshold used and the CoDel parameters set for slow stations are > chosen to err on the side of caution. I.e. rather be too lenient than > too aggressive. A better algorithm can then be added later. > > Cc: Michal Kazior > Cc: Felix Fietkau > Signed-off-by: Toke Høiland-Jørgensen > --- > include/net/mac80211.h | 17 +++++++++++++++++ > net/mac80211/rate.c | 2 ++ > net/mac80211/sta_info.c | 35 +++++++++++++++++++++++++++++++++++ > net/mac80211/sta_info.h | 12 ++++++++++++ > net/mac80211/tx.c | 9 ++++++++- > 5 files changed, 74 insertions(+), 1 deletion(-) > > diff --git a/include/net/mac80211.h b/include/net/mac80211.h > index cca510a..6e0cf85 100644 > --- a/include/net/mac80211.h > +++ b/include/net/mac80211.h > @@ -4089,6 +4089,23 @@ void ieee80211_get_tx_rates(struct ieee80211_vif > *vif, > int max_rates); > > /** > + * ieee80211_sta_set_expected_throughput - set the expected throughput > for a > + * station > + * > + * Call this function to notify mac80211 about a change in expected > throughput > + * to a station. A driver for a device that does rate control in firmware > can > + * call this function when the expected throughput estimate towards a > station > + * changes. The information is used to tune the CoDel AQM applied to > traffic > + * going towards that station (which can otherwise be too aggressive and > cause > + * slow stations to starve). > + * > + * @sta: the station to set throughput for. > + * @thr: the current expected throughput in kbps. > + */ > +void ieee80211_sta_set_expected_throughput(struct ieee80211_sta *pubsta, > + u32 thr); > + > +/** > * ieee80211_tx_status - transmit status callback > * > * Call this function for all transmitted frames after they have been > diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c > index 206698b..5370f5c 100644 > --- a/net/mac80211/rate.c > +++ b/net/mac80211/rate.c > @@ -890,6 +890,8 @@ int rate_control_set_rates(struct ieee80211_hw *hw, > > drv_sta_rate_tbl_update(hw_to_local(hw), sta->sdata, pubsta); > > + sta_update_codel_params(sta, sta_get_expected_throughput(sta)); > + > return 0; > } > EXPORT_SYMBOL(rate_control_set_rates); > diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c > index 19f14c9..6deda4a 100644 > --- a/net/mac80211/sta_info.c > +++ b/net/mac80211/sta_info.c > @@ -20,6 +20,7 @@ > #include > #include > > +#include > #include > #include "ieee80211_i.h" > #include "driver-ops.h" > @@ -419,6 +420,8 @@ struct sta_info *sta_info_alloc(struct > ieee80211_sub_if_data *sdata, > > sta->sta.max_rc_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_BA; > > + sta_update_codel_params(sta, 0); > + > sta_dbg(sdata, "Allocated STA %pM\n", sta->sta.addr); > > return sta; > @@ -2306,6 +2309,15 @@ u32 sta_get_expected_throughput(struct sta_info > *sta) > return thr; > } > > +void ieee80211_sta_set_expected_throughput(struct ieee80211_sta *pubsta, > + u32 thr) > +{ > + struct sta_info *sta = container_of(pubsta, struct sta_info, sta); > + > + sta_update_codel_params(sta, thr); > +} > +EXPORT_SYMBOL(ieee80211_sta_set_expected_throughput); > + > unsigned long ieee80211_sta_last_active(struct sta_info *sta) > { > struct ieee80211_sta_rx_stats *stats = sta_get_last_rx_stats(sta); > @@ -2314,3 +2326,26 @@ unsigned long ieee80211_sta_last_active(struct > sta_info *sta) > return stats->last_rx; > return sta->status_stats.last_ack; > } > + > +void sta_update_codel_params(struct sta_info *sta, u32 thr) > +{ > + u64 now = ktime_get_ns(); > + > + if (!sta->sdata->local->ops->wake_tx_queue) > + return; > + > + if (now - sta->cparams.update_time < STA_CPARAMS_HYSTERESIS) > + return; > + > + if (thr && thr < STA_SLOW_THRESHOLD) { > + sta->cparams.params.target = MS2TIME(50); > + sta->cparams.params.interval = MS2TIME(300); > + sta->cparams.params.ecn = false; > + } else { > + sta->cparams.params.target = MS2TIME(20); > + sta->cparams.params.interval = MS2TIME(100); > + sta->cparams.params.ecn = true; > + } > + sta->cparams.params.ce_threshold = CODEL_DISABLED_THRESHOLD; > + sta->cparams.update_time = now; > +} > diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h > index 0556be3..ad088ff 100644 > --- a/net/mac80211/sta_info.h > +++ b/net/mac80211/sta_info.h > @@ -384,6 +384,14 @@ struct ieee80211_sta_rx_stats { > u64 msdu[IEEE80211_NUM_TIDS + 1]; > }; > > +#define STA_CPARAMS_HYSTERESIS (2L * NSEC_PER_SEC) > +#define STA_SLOW_THRESHOLD 8000 /* 8 Mbps */ > + > +struct sta_codel_params { > + struct codel_params params; > + u64 update_time; > +}; > + > /** > * struct sta_info - STA information > * > @@ -437,6 +445,7 @@ struct ieee80211_sta_rx_stats { > * @known_smps_mode: the smps_mode the client thinks we are in. Relevant > for > * AP only. > * @cipher_scheme: optional cipher scheme for this station > + * @cparams: CoDel parameters for this station. > * @reserved_tid: reserved TID (if any, otherwise > IEEE80211_TID_UNRESERVED) > * @fast_tx: TX fastpath information > * @fast_rx: RX fastpath information > @@ -540,6 +549,8 @@ struct sta_info { > enum ieee80211_smps_mode known_smps_mode; > const struct ieee80211_cipher_scheme *cipher_scheme; > > + struct sta_codel_params cparams; > + > u8 reserved_tid; > > struct cfg80211_chan_def tdls_chandef; > @@ -713,6 +724,7 @@ void sta_set_rate_info_tx(struct sta_info *sta, > void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo); > > u32 sta_get_expected_throughput(struct sta_info *sta); > +void sta_update_codel_params(struct sta_info *sta, u32 thr); > > void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata, > unsigned long exp_time); > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c > index efc38e7..ec60ac1 100644 > --- a/net/mac80211/tx.c > +++ b/net/mac80211/tx.c > @@ -1342,9 +1342,16 @@ static struct sk_buff *fq_tin_dequeue_func(struct > fq *fq, > > local = container_of(fq, struct ieee80211_local, fq); > txqi = container_of(tin, struct txq_info, tin); > - cparams = &local->cparams; > cstats = &local->cstats; > > + if (txqi->txq.sta) { > + struct sta_info *sta = container_of(txqi->txq.sta, > + struct sta_info, sta); > + cparams = &sta->cparams.params; > + } else { > + cparams = &local->cparams; > + } > + > if (flow == &txqi->def_flow) > cvars = &txqi->def_cvars; > else > -- > 2.9.3 > > base-commit: abc3750c31320f36e230f88b235a90e0a35f7032 > _______________________________________________ > Make-wifi-fast mailing list > Make-wifi-fast@lists.bufferbloat.net > https://lists.bufferbloat.net/listinfo/make-wifi-fast >