[Cake] [PATCH] q_cake: Update xstats format to use per-tin structure

2018-02-11 Thread Toke Høiland-Jørgensen
This updates tc to understand the updated cake xstats structure (which
splits out the tin stats in a separate structure the length of which is
included in the containing struct).

Old versions of the cake stats will no longer be understood by the
resulting version of tc.

Signed-off-by: Toke Høiland-Jørgensen 
---
 tc/q_cake.c | 103 ++--
 1 file changed, 51 insertions(+), 52 deletions(-)

diff --git a/tc/q_cake.c b/tc/q_cake.c
index 6987c4d..3ddc13c 100644
--- a/tc/q_cake.c
+++ b/tc/q_cake.c
@@ -555,6 +555,11 @@ static int cake_print_opt(struct qdisc_util *qu, FILE *f, 
struct rtattr *opt)
return 0;
 }
 
+#define FOR_EACH_TIN(xstats, tst, i)   \
+   for(tst = xstats->tin_stats, i = 0; \
+   i < xstats->tin_cnt;\
+   i++, tst = ((void *) xstats->tin_stats) + xstats->tin_stats_size * 
i)
+
 static int cake_print_xstats(struct qdisc_util *qu, FILE *f,
 struct rtattr *xstats)
 {
@@ -597,17 +602,15 @@ static int cake_print_xstats(struct qdisc_util *qu, FILE 
*f,
fprintf(f, " drop_next %s",
sprint_time(st->class_stats.drop_next, 
b1));
}
-   } else if (stnc->version >= 1 && stnc->version < 0xFF
-   && stnc->max_tins == TC_CAKE_MAX_TINS
-   && RTA_PAYLOAD(xstats) >= offsetof(struct 
tc_cake_xstats, capacity_estimate))
+   } else if (stnc->version > 0xFF
+   && RTA_PAYLOAD(xstats) >= (sizeof(struct tc_cake_xstats) +
+   stnc->tin_stats_size * stnc->tin_cnt))
{
+   struct tc_cake_tin_stats  *tst;
int i;
 
-   if(stnc->version >= 3)
-   fprintf(f, " memory used: %s of %s\n", 
sprint_size(stnc->memory_used, b1), sprint_size(stnc->memory_limit, b2));
-
-   if(stnc->version >= 2)
-   fprintf(f, " capacity estimate: %s\n", 
sprint_rate(stnc->capacity_estimate, b1));
+   fprintf(f, " memory used: %s of %s\n", 
sprint_size(stnc->memory_used, b1), sprint_size(stnc->memory_limit, b2));
+   fprintf(f, " capacity estimate: %s\n", 
sprint_rate(stnc->capacity_estimate, b1));
 
switch(stnc->tin_cnt) {
case 3:
@@ -630,97 +633,93 @@ static int cake_print_xstats(struct qdisc_util *qu, FILE 
*f,
};
 
fprintf(f, "  thresh  ");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12s", sprint_rate(stnc->threshold_rate[i], 
b1));
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12s", sprint_rate(tst->threshold_rate, 
b1));
fprintf(f, "\n");
 
fprintf(f, "  target  ");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12s", sprint_time(stnc->target_us[i], b1));
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12s", sprint_time(tst->target_us, b1));
fprintf(f, "\n");
 
fprintf(f, "  interval");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12s", sprint_time(stnc->interval_us[i], 
b1));
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12s", sprint_time(tst->interval_us, b1));
fprintf(f, "\n");
 
fprintf(f, "  pk_delay");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12s", sprint_time(stnc->peak_delay_us[i], 
b1));
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12s", sprint_time(tst->peak_delay_us, b1));
fprintf(f, "\n");
 
fprintf(f, "  av_delay");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12s", sprint_time(stnc->avge_delay_us[i], 
b1));
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12s", sprint_time(tst->avge_delay_us, b1));
fprintf(f, "\n");
 
fprintf(f, "  sp_delay");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12s", sprint_time(stnc->base_delay_us[i], 
b1));
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12s", sprint_time(tst->base_delay_us, b1));
fprintf(f, "\n");
 
fprintf(f, "  pkts");
-   for(i=0; i < stnc->tin_cnt; i++)
-   fprintf(f, "%12u", stnc->sent[i].packets);
+   FOR_EACH_TIN(stnc, tst, i)
+   fprintf(f, "%12u", tst->sent.packets);
fprintf(f, "\n");
 
fprintf(f, "  bytes   ");
-   for(i=0; i < stnc->tin_cnt; i++)

[Cake] [PATCH v3] Split tin stats to its own structure to decrease size of tc_cake_xstats

2018-02-11 Thread Toke Høiland-Jørgensen
This splits out the tin stats from tc_cake_xstats, which seems like the
least intrusive way of decreasing the size of the stats structure. This
way, we can send only the statistics corresponding to the actual number of
allocated tins, rather than having the xstats structure always be allocated
for the full number of tins.

Signed-off-by: Toke Høiland-Jørgensen 
---
Updated in v3:
- Keep the version field in place in the xstats structure, and set it to
  a value higher than 0xFF, which will make old versions of tc ignore
  it. This means old versions of tc simply won't print statistics,
  rather than print garbled statistics because the struct layout is
  wrong.
- Actually tested the code :)

 pkt_sched.h | 52 +---
 sch_cake.c  | 58 +++---
 2 files changed, 60 insertions(+), 50 deletions(-)

diff --git a/pkt_sched.h b/pkt_sched.h
index ed7c111..3a86d60 100644
--- a/pkt_sched.h
+++ b/pkt_sched.h
@@ -964,33 +964,39 @@ struct tc_cake_traffic_stats {
 };
 
 #define TC_CAKE_MAX_TINS (8)
-struct tc_cake_xstats {
-   __u16 version;  /* == 5, increments when struct extended */
-   __u8  max_tins; /* == TC_CAKE_MAX_TINS */
-   __u8  tin_cnt;  /* <= TC_CAKE_MAX_TINS */
+struct tc_cake_tin_stats {
+
+   __u32 threshold_rate;
+   __u32 target_us;
+   struct tc_cake_traffic_stats sent;
+   struct tc_cake_traffic_stats dropped;
+   struct tc_cake_traffic_stats ecn_marked;
+   struct tc_cake_traffic_stats backlog;
+   __u32 interval_us;
+   __u32 way_indirect_hits;
+   __u32 way_misses;
+   __u32 way_collisions;
+   __u32 peak_delay_us; /* ~= bulk flow delay */
+   __u32 avge_delay_us;
+   __u32 base_delay_us; /* ~= sparse flows delay */
+   __u16 sparse_flows;
+   __u16 bulk_flows;
+   __u16 unresponse_flows; /* v4 - was u32 last_len */
+   __u16 spare; /* v4 - split last_len */
+   __u32 max_skblen;
+   struct tc_cake_traffic_stats ack_drops; /* v5 */
+};
 
-   __u32 threshold_rate[TC_CAKE_MAX_TINS];
-   __u32 target_us[TC_CAKE_MAX_TINS];
-   struct tc_cake_traffic_stats sent[TC_CAKE_MAX_TINS];
-   struct tc_cake_traffic_stats dropped[TC_CAKE_MAX_TINS];
-   struct tc_cake_traffic_stats ecn_marked[TC_CAKE_MAX_TINS];
-   struct tc_cake_traffic_stats backlog[TC_CAKE_MAX_TINS];
-   __u32 interval_us[TC_CAKE_MAX_TINS];
-   __u32 way_indirect_hits[TC_CAKE_MAX_TINS];
-   __u32 way_misses[TC_CAKE_MAX_TINS];
-   __u32 way_collisions[TC_CAKE_MAX_TINS];
-   __u32 peak_delay_us[TC_CAKE_MAX_TINS]; /* ~= bulk flow delay */
-   __u32 avge_delay_us[TC_CAKE_MAX_TINS];
-   __u32 base_delay_us[TC_CAKE_MAX_TINS]; /* ~= sparse flows delay */
-   __u16 sparse_flows[TC_CAKE_MAX_TINS];
-   __u16 bulk_flows[TC_CAKE_MAX_TINS];
-   __u16 unresponse_flows[TC_CAKE_MAX_TINS]; /* v4 - was u32 last_len */
-   __u16 spare[TC_CAKE_MAX_TINS]; /* v4 - split last_len */
-   __u32 max_skblen[TC_CAKE_MAX_TINS];
+struct tc_cake_xstats {
+   __u16 version;
+   __u16 tin_stats_size; /* == sizeof(struct tc_cake_tin_stats) */
__u32 capacity_estimate;  /* version 2 */
__u32 memory_limit;   /* version 3 */
__u32 memory_used;/* version 3 */
-   struct tc_cake_traffic_stats ack_drops[TC_CAKE_MAX_TINS]; /* v5 */
+   __u8  tin_cnt;  /* <= TC_CAKE_MAX_TINS */
+
+   struct tc_cake_tin_stats tin_stats[0]; /* keep last */
 };
 
+
 #endif
diff --git a/sch_cake.c b/sch_cake.c
index 7f6ff8e..62b67e7 100644
--- a/sch_cake.c
+++ b/sch_cake.c
@@ -2478,51 +2478,55 @@ nla_put_failure:
 static int cake_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 {
struct cake_sched_data *q = qdisc_priv(sch);
-   struct tc_cake_xstats *st = kvzalloc(sizeof(*st), GFP_KERNEL);
+   struct tc_cake_xstats *st;
+   size_t size = sizeof(*st) + sizeof(struct tc_cake_tin_stats) * 
q->tin_cnt;
int i;
 
+   st = kvzalloc(size, GFP_KERNEL);
+
if (!st)
return -ENOMEM;
 
-   st->version = 5;
-   st->max_tins = TC_CAKE_MAX_TINS;
+   st->version = 0xFF + 1; /* old userspace code discards versions > 0xFF 
*/
+   st->tin_stats_size = sizeof(struct tc_cake_tin_stats);
st->tin_cnt = q->tin_cnt;
 
for (i = 0; i < q->tin_cnt; i++) {
struct cake_tin_data *b = >tins[q->tin_order[i]];
+   struct tc_cake_tin_stats *tstat = >tin_stats[i];
 
-   st->threshold_rate[i] = b->tin_rate_bps;
-   st->target_us[i]  = cobalt_time_to_us(b->cparams.target);
-   st->interval_us[i]= cobalt_time_to_us(b->cparams.interval);
+   tstat->threshold_rate = b->tin_rate_bps;
+   tstat->target_us  = cobalt_time_to_us(b->cparams.target);
+   tstat->interval_us= cobalt_time_to_us(b->cparams.interval);