Re: [PATCH] mac80211: Keep CoDel stats per txq, export them in debugfs.

[Toke Høiland-Jørgensen]

Johannes Berg  writes:

>> @@ -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");
>
> It seems to me that you have to change the buffer length to take this
> into account?

Haven't run into issues with running out of buffer space in my testing.
But yeah, guess that could become an issue.

>>  list_for_each_entry_rcu(sdata, >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,
>
> Why is it this way anyway? It'd seem nicer to move the content of this
> into the per-netdev subdirectories, and then it becomes a lot simpler
> code too (yes, at the expense of some userspace, but still)

Yeah, makes sense. Can do a larger reorg moving things into the
per-netdev and per-station directories instead.

>>   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,
>
> Do you really want to add these in the middle? Seems that if you add
> them at the end, you at least have *some* way of keeping this working
> with older versions?

Well I though they should be logically grouped with overlimits, and was
counting on no one actually parsing these yet. Guess if the information
is moved that becomes moot.


Will re-send; thanks for the feedback :)

-Toke
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: Keep CoDel stats per txq, export them in debugfs.

[Johannes Berg]

> @@ -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");

It seems to me that you have to change the buffer length to take this
into account?
 
>   list_for_each_entry_rcu(sdata, >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,

Why is it this way anyway? It'd seem nicer to move the content of this
into the per-netdev subdirectories, and then it becomes a lot simpler
code too (yes, at the expense of some userspace, but still)

>    sdata->vif.addr,

This is also kinda pointless since it's easy to get elsewhere.

>    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,

Do you really want to add these in the middle? Seems that if you add
them at the end, you at least have *some* way of keeping this working
with older versions?

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mac80211: Keep CoDel stats per txq, export them in debugfs.

[Toke Høiland-Jørgensen]

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 
Signed-off-by: Toke Høiland-Jørgensen 
---
 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, >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, >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 = >cparams;
-   cstats = >cstats;
+   cstats = >cstats;
 
if (flow == >def_flow)
cvars = >def_cvars;
@@ -1399,6 +1399,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data 
*sdata,
fq_tin_init(>tin);
fq_flow_init(>def_flow);
codel_vars_init(>def_cvars);
+   codel_stats_init(>cstats);