Re: [Cerowrt-devel] [Make-wifi-fast] wifi airtime fairness patches could use eyeballs and testing

["Toke Høiland-Jørgensen" <toke@toke.dk>]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]

Toke Høiland-Jørgensen <toke@toke.dk> writes:

> Dave Taht <dave.taht@gmail.com> writes:
>
>> On Wed, Aug 10, 2016 at 11:50 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>>> Toke Høiland-Jørgensen <toke@toke.dk> writes:
>>>
>>>> On 10 August 2016 21:35:40 CEST, Dave Taht <dave.taht@gmail.com> wrote:
>>>>>Wow, that *is* weird. It is good to see the tcp window changing on
>>>>>this set of data (it wasn't before), and CWRs, but... hmmm... SCIENCE.
>>>>>
>>>>>Enabling ecn on both sides will rule out some potential bugs.
>>>>
>>>> Yeah, couldn't get ecn to work on the host I was using as the other endpoint on
>>>> that test. Will try with another box that's not on quite as ancient a kernel.
>>>> Was also planning to disable codel (by setting a very high target) to try to
>>>> narrow down the problem.
>>>
>>> OK, digging some more on this:
>>>
>>> I am seeing *no* drops by CoDel, and no backlog in the mac80211 softq
>>> layer either (or at most one or two packets when polling with a 1 sec
>>> interval). This is with one as well as with two flows.
>>
>> But there are tons of drops evident from the captures.
>
> Well the CoDel drop counter stays completely flat. There are cwnd
> reductions, yes, but are they drops? My thought was that they were
> retransmissions caused by OOO packets?

Here's the patch to expose the CoDel drop counter per flow, BTW. Can be
dropped into a LEDE build.

-Toke


[-- Attachment #2: 341-mac80211-Keep-CoDel-stats-per-txq-export-them-in-deb.patch --]
[-- Type: text/x-diff, Size: 4640 bytes --]

From d5a40556aa16fe0f452fd2ee975d7adce6f54837 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= <toke@toke.dk>
Date: Wed, 20 Jul 2016 16:06:49 +0200
Subject: [PATCH] mac80211: Keep CoDel stats per txq, export them in debugfs.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently the 'aqm' stats in mac80211 only keeps overlimit drop stats,
not CoDel stats. This moves the CoDel stats into the txqi structure and
adds the drop and mark counts to the debug output.

Cc: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 net/mac80211/debugfs.c     | 12 ++++++++----
 net/mac80211/ieee80211_i.h |  2 +-
 net/mac80211/tx.c          |  4 ++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 27e6fb9..69bf2e5 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -137,18 +137,20 @@ static int aqm_open(struct inode *inode, struct file *file)
 	len += scnprintf(info->buf + len,
 			 info->size - len,
 			 "* vif\n"
-			 "ifname addr ac backlog-bytes backlog-packets flows overlimit collisions tx-bytes tx-packets\n");
+			 "ifname addr ac backlog-bytes backlog-packets flows drops marks overlimit collisions tx-bytes tx-packets\n");
 
 	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
 		txqi = to_txq_info(sdata->vif.txq);
 		len += scnprintf(info->buf + len, info->size - len,
-				 "%s %pM %u %u %u %u %u %u %u %u\n",
+				 "%s %pM %u %u %u %u %u %u %u %u %u %u\n",
 				 sdata->name,
 				 sdata->vif.addr,
 				 txqi->txq.ac,
 				 txqi->tin.backlog_bytes,
 				 txqi->tin.backlog_packets,
 				 txqi->tin.flows,
+				 txqi->cstats.drop_count,
+				 txqi->cstats.ecn_mark,
 				 txqi->tin.overlimit,
 				 txqi->tin.collisions,
 				 txqi->tin.tx_bytes,
@@ -158,14 +160,14 @@ static int aqm_open(struct inode *inode, struct file *file)
 	len += scnprintf(info->buf + len,
 			 info->size - len,
 			 "* sta\n"
-			 "ifname addr tid ac backlog-bytes backlog-packets flows overlimit collisions tx-bytes tx-packets\n");
+			 "ifname addr tid ac backlog-bytes backlog-packets flows drops marks overlimit collisions tx-bytes tx-packets\n");
 
 	list_for_each_entry_rcu(sta, &local->sta_list, list) {
 		sdata = sta->sdata;
 		for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
 			txqi = to_txq_info(sta->sta.txq[i]);
 			len += scnprintf(info->buf + len, info->size - len,
-					 "%s %pM %d %d %u %u %u %u %u %u %u\n",
+					 "%s %pM %d %d %u %u %u %u %u %u %u %u %u\n",
 					 sdata->name,
 					 sta->sta.addr,
 					 txqi->txq.tid,
@@ -173,6 +175,8 @@ static int aqm_open(struct inode *inode, struct file *file)
 					 txqi->tin.backlog_bytes,
 					 txqi->tin.backlog_packets,
 					 txqi->tin.flows,
+					 txqi->cstats.drop_count,
+					 txqi->cstats.ecn_mark,
 					 txqi->tin.overlimit,
 					 txqi->tin.collisions,
 					 txqi->tin.tx_bytes,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c9f8c80..9f11b13 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -812,6 +812,7 @@ struct txq_info {
 	struct fq_tin tin;
 	struct fq_flow def_flow;
 	struct codel_vars def_cvars;
+	struct codel_stats cstats;
 	unsigned long flags;
 
 	/* keep last! */
@@ -1106,7 +1107,6 @@ struct ieee80211_local {
 	struct fq fq;
 	struct codel_vars *cvars;
 	struct codel_params cparams;
-	struct codel_stats cstats;
 
 	const struct ieee80211_ops *ops;
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 682011e..201167d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1339,7 +1339,7 @@ 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;
+	cstats = &txqi->cstats;
 
 	if (flow == &txqi->def_flow)
 		cvars = &txqi->def_cvars;
@@ -1399,6 +1399,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
 	fq_tin_init(&txqi->tin);
 	fq_flow_init(&txqi->def_flow);
 	codel_vars_init(&txqi->def_cvars);
+	codel_stats_init(&txqi->cstats);
 
 	txqi->txq.vif = &sdata->vif;
 
@@ -1437,7 +1438,6 @@ int ieee80211_txq_setup_flows(struct ieee80211_local *local)
 		return ret;
 
 	codel_params_init(&local->cparams);
-	codel_stats_init(&local->cstats);
 	local->cparams.interval = MS2TIME(100);
 	local->cparams.target = MS2TIME(20);
 	local->cparams.ecn = true;
-- 
2.9.0