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