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

[Luis E. Garcia]

Nice work Kevin.

On Sat, Mar 10, 2018 at 09:56 Kevin Darbyshire-Bryant <
ke...@darbyshire-bryant.me.uk> wrote:

>
>
> > On 8 Mar 2018, at 11:21, Toke Høiland-Jørgensen  wrote:
> >
> >>
> >> Oh and curiously the bad values go away if you ask for json output
> >> it’s much better.  Which rather points at a ‘feature’ of the
> >> ‘print_string’ behaviour.
> >
> > Right. Well, the print_* functions are behind several levels of
> > pre-processor indirection, so not quite obvious what's going on here.
> > Don't really see why they should spit out garbage values, though.
> >
> >
> > Stephen, do you have any ideas?
> >
> > -Toke
>
> Right, cracked it and it’s horrible!
>
> print_uint is expanded thus:  Note the type of value uint64_t
>
>   void print_color_uint(enum output_type t, enum color_attr
> color, const char *key, const char *fmt, uint64_t value);
> static inline void print_uint  (enum output_type t,
> const char *key, const char *fmt, uint64_t value)
>  { print_color_uint( t, COLOR_NONE,
> key, fmt,  value); };
>
> So far so good.
>
> print_color_uint expands to:
>
>  void print_color_uint(enum output_type t, enum color_attr
> color, const char *key, const char *fmt, uint64_t value)
>  {
>   if (((t & PRINT_JSON || t & PRINT_ANY) && _jw))
> { if (!key) jsonw_uint(_jw, value);
>   else  jsonw_uint_field(_jw, key, value);
> }
>   else if ((!_jw && (t & PRINT_FP || t & PRINT_ANY)))
> { color_fprintf( (stdout) , color, fmt, value);
> }
>  };
>
> Again, no issue and we eventually call color_fprintf
>
> int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)
> {
> int ret = 0;
> va_list args;
>
> va_start(args, fmt);
>
> if (!color_is_enabled || attr == COLOR_NONE) {
> ret = vfprintf(fp, fmt, args);
> goto end;
> }
>
>
> Now, color_printf is a variable argument list function and as such is
> dependent upon being told the correct size of argument variables in the fmt
> variable.  And that’s our problem, we’re passing a 64bit integer but
> telling the format string that it’s ‘int’…which I’m guessing can be
> variable sizes depending on architecture, as can the endianness.
>
> If we instead do (in q_cake.c)
>
> #include 
>
> print_uint(PRINT_ANY, "min_transport_size", " min/max transport layer
> size: %10" PRIu64, stnc->min_trnlen);
>
> it works.  This needs sanity checking by a clever person.
>
> Cheers,
>
> Kevin D-B
>
> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
>
> ___
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake
>
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

> On 8 Mar 2018, at 11:21, Toke Høiland-Jørgensen  wrote:
> 
>> 
>> Oh and curiously the bad values go away if you ask for json output
>> it’s much better.  Which rather points at a ‘feature’ of the
>> ‘print_string’ behaviour.
> 
> Right. Well, the print_* functions are behind several levels of
> pre-processor indirection, so not quite obvious what's going on here.
> Don't really see why they should spit out garbage values, though.
> 
> 
> Stephen, do you have any ideas?
> 
> -Toke

Right, cracked it and it’s horrible!

print_uint is expanded thus:  Note the type of value uint64_t

  void print_color_uint(enum output_type t, enum color_attr color, 
const char *key, const char *fmt, uint64_t value);
static inline void print_uint  (enum output_type t,
const char *key, const char *fmt, uint64_t value)
 { print_color_uint( t, COLOR_NONE, 
   key, fmt,  value); };

So far so good.

print_color_uint expands to:

 void print_color_uint(enum output_type t, enum color_attr color, 
const char *key, const char *fmt, uint64_t value)
 {
  if (((t & PRINT_JSON || t & PRINT_ANY) && _jw))
{ if (!key) jsonw_uint(_jw, value);
  else  jsonw_uint_field(_jw, key, value);
}
  else if ((!_jw && (t & PRINT_FP || t & PRINT_ANY)))
{ color_fprintf( (stdout) , color, fmt, value);
}
 };

Again, no issue and we eventually call color_fprintf

int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)
{
int ret = 0;
va_list args;

va_start(args, fmt);

if (!color_is_enabled || attr == COLOR_NONE) {
ret = vfprintf(fp, fmt, args);
goto end;
}


Now, color_printf is a variable argument list function and as such is dependent 
upon being told the correct size of argument variables in the fmt variable.  
And that’s our problem, we’re passing a 64bit integer but telling the format 
string that it’s ‘int’…which I’m guessing can be variable sizes depending on 
architecture, as can the endianness.

If we instead do (in q_cake.c)

#include 

print_uint(PRINT_ANY, "min_transport_size", " min/max transport layer size: 
%10" PRIu64, stnc->min_trnlen);

it works.  This needs sanity checking by a clever person.

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A



signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Pete Heist]

> On Mar 7, 2018, at 12:26 PM, Dave Taht  wrote:
> 
> For those that haven't been tracking https://www.facebook.com/dtaht -
> 
> I bought an old 35' sailboat in the santa cruz, ca, harbor in December
> for 1 BTC. I figured living aboard would be cheaper than california
> rents, by far, and I'd be able to still work online a mile from shore,
> after adding solar panels, etc.
> 
> So far neither is true, and I've sunk more energy, time and money into
> making her shipshape than I'd ever imagined. Yes, it's a nice break
> from fixing the internet. Yes, yes, she's called the Bufferboat, at
> least tenatively - although it's federally registered as the Defiance.
> She sleeps six. If anyone wants a ride or be crew in the next year...
> let me know separately.
> 
> I'm increasingly unsure if the live-aboard-and-code idea is going to
> work out. I might need a bigger boat, and crew. ;)

Awesome! I just figured that was for a trip, not a home office. :) I don’t 
facebook, so thanks for the update. Fortune favors the crazy (I saw on an ad in 
a skyway in Heathrow, which makes it true).

I’m stateside (east cost) as well for a week or so, so won’t be bloat-ing. I 
got here just in time for the latest nor’easter, making it my second extended 
power outage and second thundersnow this season (0.07% of snowstorms have 
thundersnow, so yeah.) Spent last night burning tomato soup over an open indoor 
fire made with wood stolen from a closed suburban grocery store. Will go pay 
for that.

More importantly, will there be another effort to upstream Cake? If so, I could 
offer some help when I return if it’d be useful. I was in the middle of 
re-configuring my lab and servers when I left.
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Georgios Amanakis]

I am currently testing on my router, Archlinux with kernels
4.14.24/4.15.7, sch_cake from cobalt and tc-adv but it is behaving as
it should:

==8<==
qdisc cake 8001: dev ens4 root refcnt 2 bandwidth 12200Kbit besteffort
dual-dsthost wash ingress rtt 100.0ms noatm overhead 18mpu 64
 Sent 950929 bytes 4293 pkt (dropped 2, overlimits 641 requeues 0)
 backlog 0b 0p requeues 0
 memory used: 44528b of 4Mb
 capacity estimate: 12200Kbit
 min/max transport layer size: 28 /1500
 min/max overhead-adjusted size:   64 /1518
 average transport hdr offset: 14


qdisc cake 8002: dev ens3 root refcnt 2 bandwidth 2500Kbit besteffort
dual-srchost nat wash ack-filter rtt 100.0ms noatm overhead 18mpu 64
 Sent 254423 bytes 1712 pkt (dropped 0, overlimits 2158 requeues 0)
 backlog 0b 0p requeues 0
 memory used: 18Kb of 4Mb
 capacity estimate: 2500Kbit
 min/max transport layer size: 28 /1500
 min/max overhead-adjusted size:   64 /1518
 average transport hdr offset: 14
==8<==

George


On Thu, Mar 8, 2018 at 6:21 AM, Toke Høiland-Jørgensen  wrote:
> Kevin Darbyshire-Bryant  writes:
>
>>> On 8 Mar 2018, at 11:09, Kevin Darbyshire-Bryant 
>>>  wrote:
>>>
>>>
>>>
 On 8 Mar 2018, at 10:57, Toke Høiland-Jørgensen  wrote:

 Kevin Darbyshire-Bryant  writes:

> Archer c7 v2. master branch of openwrt

 Ah, great; I actually have one of those sitting on my desk that I could
 potentially reflash without breaking anything too important.

 In the meantime; do you get the same weird output on the
 dropped/overlimit/requeues fields if you install a different qdisc than
 cake?
>>>
>>> From previous email this morning:
>>>
 Definitely dubious and I’m no longer convinced it’s a cake only issue.  
 Looked at my AP which is running an older version of openwrt, so older 
 cake, older kernel etc etc and all qdiscs are returning odd impossibly 
 high values in a variety of fields.
>>>
>>>
>>>
>>> tc -s qdisc
>>> qdisc noqueue 0: dev lo root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc fq_codel 0: dev eth1 root refcnt 4485644 limit 4497776p flows 4535164 
>>> quantum 4536072 target 5.0ms interval 100.0ms memory_limit 4Mb ecn
>>> Sent 74281480509 bytes 91321865 pkts (dropped 0, overlimits 0)
>>>  maxpacket 1514 drop_overlimit 0 new_flow_count 7549 ecn_mark 0
>>>  new_flows_len 0 old_flows_len 0
>>> qdisc noqueue 0: dev br-lan root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev eth1.2 root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev br-wifi_guest root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev eth1.15 root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev wlan1 root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev wlan0 root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev wlan1-1 root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>> qdisc noqueue 0: dev wlan0-1 root refcnt 4485644
>>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>>>
>>> And that’s on an openwrt system from commit 
>>> f5b4f5f8e33624f27af9fb3f86e09084181c08ed
>>> Author: Alif M. Ahmad 
>>> Date:   Sun Feb 25 03:18:41 2018 +
>>>
>>> So actually this problem has been around a little while, pre recent cake 
>>> changes.
>>
>> Oh and curiously the bad values go away if you ask for json output
>> it’s much better.  Which rather points at a ‘feature’ of the
>> ‘print_string’ behaviour.
>
> Right. Well, the print_* functions are behind several levels of
> pre-processor indirection, so not quite obvious what's going on here.
> Don't really see why they should spit out garbage values, though.
>
>
> Stephen, do you have any ideas?
>
> -Toke
>
>
>>
>> tc -s -j qdisc
>> [{
>> "kind": "noqueue",
>> "handle": "0:",
>> "dev": "lo",
>> "root": true,
>> "refcnt": 2,
>> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> },{
>> "kind": "fq_codel",
>> "handle": "0:",
>> "dev": "eth1",
>> "root": true,
>> "refcnt": 2,
>> "options": {
>> "limit": 10240,
>> "flows": 1024,
>> "quantum": 1514,
>> "target": 4999,
>> "interval": 9,
>> "memory_limit": 4194304,
>> "ecn": true
>> } Sent 74283705614 bytes 91330210 pkts (dropped 0, overlimits 0)   
>> maxpacket 1514 drop_overlimit 0 new_flow_count 7549 ecn_mark 0
>>   new_flows_len 0 old_flows_len 0
>> },{
>> "kind": "noqueue",
>> "handle": "0:",
>>   

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

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 8 Mar 2018, at 11:09, Kevin Darbyshire-Bryant 
>>  wrote:
>> 
>> 
>> 
>>> On 8 Mar 2018, at 10:57, Toke Høiland-Jørgensen  wrote:
>>> 
>>> Kevin Darbyshire-Bryant  writes:
>>> 
 Archer c7 v2. master branch of openwrt
>>> 
>>> Ah, great; I actually have one of those sitting on my desk that I could
>>> potentially reflash without breaking anything too important.
>>> 
>>> In the meantime; do you get the same weird output on the
>>> dropped/overlimit/requeues fields if you install a different qdisc than
>>> cake?
>> 
>> From previous email this morning:
>> 
>>> Definitely dubious and I’m no longer convinced it’s a cake only issue.  
>>> Looked at my AP which is running an older version of openwrt, so older 
>>> cake, older kernel etc etc and all qdiscs are returning odd impossibly high 
>>> values in a variety of fields.
>> 
>> 
>> 
>> tc -s qdisc
>> qdisc noqueue 0: dev lo root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc fq_codel 0: dev eth1 root refcnt 4485644 limit 4497776p flows 4535164 
>> quantum 4536072 target 5.0ms interval 100.0ms memory_limit 4Mb ecn
>> Sent 74281480509 bytes 91321865 pkts (dropped 0, overlimits 0)
>>  maxpacket 1514 drop_overlimit 0 new_flow_count 7549 ecn_mark 0
>>  new_flows_len 0 old_flows_len 0
>> qdisc noqueue 0: dev br-lan root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev eth1.2 root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev br-wifi_guest root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev eth1.15 root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev wlan1 root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev wlan0 root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev wlan1-1 root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> qdisc noqueue 0: dev wlan0-1 root refcnt 4485644
>> Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
>> 
>> And that’s on an openwrt system from commit 
>> f5b4f5f8e33624f27af9fb3f86e09084181c08ed
>> Author: Alif M. Ahmad 
>> Date:   Sun Feb 25 03:18:41 2018 +
>> 
>> So actually this problem has been around a little while, pre recent cake 
>> changes.
>
> Oh and curiously the bad values go away if you ask for json output
> it’s much better.  Which rather points at a ‘feature’ of the
> ‘print_string’ behaviour.

Right. Well, the print_* functions are behind several levels of
pre-processor indirection, so not quite obvious what's going on here.
Don't really see why they should spit out garbage values, though.


Stephen, do you have any ideas?

-Toke


>
> tc -s -j qdisc
> [{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "lo",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "fq_codel",
> "handle": "0:",
> "dev": "eth1",
> "root": true,
> "refcnt": 2,
> "options": {
> "limit": 10240,
> "flows": 1024,
> "quantum": 1514,
> "target": 4999,
> "interval": 9,
> "memory_limit": 4194304,
> "ecn": true
> } Sent 74283705614 bytes 91330210 pkts (dropped 0, overlimits 0)   
> maxpacket 1514 drop_overlimit 0 new_flow_count 7549 ecn_mark 0
>   new_flows_len 0 old_flows_len 0
> },{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "br-lan",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "eth1.2",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "br-wifi_guest",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "eth1.15",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "wlan1",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "noqueue",
> "handle": "0:",
> "dev": "wlan0",
> "root": true,
> "refcnt": 2,
> "options": {} Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> },{
> "kind": "noqueue",
>

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

[Kevin Darbyshire-Bryant]

> On 8 Mar 2018, at 10:57, Toke Høiland-Jørgensen  wrote:
> 
> Kevin Darbyshire-Bryant  writes:
> 
>> Archer c7 v2. master branch of openwrt
> 
> Ah, great; I actually have one of those sitting on my desk that I could
> potentially reflash without breaking anything too important.
> 
> In the meantime; do you get the same weird output on the
> dropped/overlimit/requeues fields if you install a different qdisc than
> cake?

From previous email this morning:

> Definitely dubious and I’m no longer convinced it’s a cake only issue.  
> Looked at my AP which is running an older version of openwrt, so older cake, 
> older kernel etc etc and all qdiscs are returning odd impossibly high values 
> in a variety of fields.



tc -s qdisc
qdisc noqueue 0: dev lo root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc fq_codel 0: dev eth1 root refcnt 4485644 limit 4497776p flows 4535164 
quantum 4536072 target 5.0ms interval 100.0ms memory_limit 4Mb ecn
 Sent 74281480509 bytes 91321865 pkts (dropped 0, overlimits 0)
  maxpacket 1514 drop_overlimit 0 new_flow_count 7549 ecn_mark 0
  new_flows_len 0 old_flows_len 0
qdisc noqueue 0: dev br-lan root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev eth1.2 root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev br-wifi_guest root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev eth1.15 root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev wlan1 root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev wlan0 root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev wlan1-1 root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
qdisc noqueue 0: dev wlan0-1 root refcnt 4485644
 Sent 0 bytes 0 pkts (dropped 0, overlimits 0)

And that’s on an openwrt system from commit 
f5b4f5f8e33624f27af9fb3f86e09084181c08ed
Author: Alif M. Ahmad 
Date:   Sun Feb 25 03:18:41 2018 +

So actually this problem has been around a little while, pre recent cake 
changes.




signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

> Archer c7 v2. master branch of openwrt

Ah, great; I actually have one of those sitting on my desk that I could
potentially reflash without breaking anything too important.

In the meantime; do you get the same weird output on the
dropped/overlimit/requeues fields if you install a different qdisc than
cake?

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

> On 8 Mar 2018, at 00:49, Jonathan Morton  wrote:
> 
>> On 7 Mar, 2018, at 11:34 pm, Toke Høiland-Jørgensen  wrote:
>> 
>> Ah, totally missed that those were wrong as well. The values for
>> dropped, overlimits and requeues are not printed by the cake-specific
>> code at all, so there is something else dubious going on.
> 
> Could they be pointer values?
> 
> - Jonathan Morton

Definitely dubious and I’m no longer convinced it’s a cake only issue.  Looked 
at my AP which is running an older version of openwrt, so older cake, older 
kernel etc etc and all qdiscs are returning odd impossibly high values in a 
variety of fields.

So, at this stage I’m going to say it’s an anomaly in my build system until 
someone can reproduce it.  I’ve a couple of local commits in my tree that I’ve 
got my eye on….

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A



signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 7 Mar, 2018, at 11:34 pm, Toke Høiland-Jørgensen  wrote:
> 
> Ah, totally missed that those were wrong as well. The values for
> dropped, overlimits and requeues are not printed by the cake-specific
> code at all, so there is something else dubious going on.

Could they be pointer values?

 - Jonathan Morton
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 7 Mar, 2018, at 8:27 pm, Kevin Darbyshire-Bryant 
>  wrote:
> 
>> Are you using the latest kernel code? Those values were added in an
>> incompatible manner, so if you don't have userspace and kernel code in
>> sync they'll be garbage, basically…
> 
> I’m using latest commit on kernel space sch_cake cobalt branch and identical 
> user space tc/q_cake.c to tc-adv repo.  It’s not just those new values, 
> overhead, dropped, overlimits, requeues are all long standing parameters.
> 
> All the values, including the new, worked just before the big json conversion.

Supporting this, all five of the new fields are u16 or smaller, so are 
incapable of representing (or being misinterpreted as) values in the millions.

I haven't looked at the converted code yet.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

> On 7 Mar 2018, at 18:27, Kevin Darbyshire-Bryant 
>  wrote:
> 
> 
> 
>> On 7 Mar 2018, at 12:59, Toke Høiland-Jørgensen  wrote:
>> 
>> 
>>> qdisc cake 8004: root refcnt 4486780 bandwidth 19900Kbit diffserv3 
>>> dual-srchost nat rtt 100.0ms ptm overhead 4502152
>>> Sent 42615751 bytes 4491092 pkt (dropped 4491108, overlimits 4491124 
>>> requeues 4491152)
>>> backlog 0b 4491256p requeues 4491272
>>> memory used: 78592b of 4Mb
>>> capacity estimate: 19900Kbit
>>> min/max transport layer size:4537916 / 4537972
>>> min/max overhead-adjusted size:  4538000 / 4537972
>>> average transport hdr offset:4538072
>>> 
>>> I’m pondering if it’s an endian issue?
>> 
>> Are you using the latest kernel code? Those values were added in an
>> incompatible manner, so if you don't have userspace and kernel code in
>> sync they'll be garbage, basically…
> 
> I’m using latest commit on kernel space sch_cake cobalt branch and identical 
> user space tc/q_cake.c to tc-adv repo.  It’s not just those new values, 
> overhead, dropped, overlimits, requeues are all long standing parameters.
> 
> All the values, including the new, worked just before the big json conversion.

Aha, more clue - the json output ‘tc -j’ looks better (possibly correct though 
I’m dubious of the rate value)

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A



signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Dave Taht]

On Wed, Mar 7, 2018 at 7:52 AM, Toke Høiland-Jørgensen  wrote:
> Dave Taht  writes:
>
>> So it sounds like all the individual pieces are sync'd again?
>>
>> 4.16-rc4 is out, so there are several weeks left to make the net-next
>> window. I note that I don't feel strongly (never had) that I should be
>> the one to make the net-next submission, just so long as it now gets
>> through checkpatch (?)... so since the code is freshest in several
>> other minds than mine now, can someone volunteer to submit it to
>> net-next?
>>
>> Otherwise I'll try to clear some time to do it next week.
>
> Well, I was expecting you to still be at sea ;)

For those that haven't been tracking https://www.facebook.com/dtaht -

I bought an old 35' sailboat in the santa cruz, ca, harbor in December
for 1 BTC. I figured living aboard would be cheaper than california
rents, by far, and I'd be able to still work online a mile from shore,
after adding solar panels, etc.

So far neither is true, and I've sunk more energy, time and money into
making her shipshape than I'd ever imagined. Yes, it's a nice break
from fixing the internet. Yes, yes, she's called the Bufferboat, at
least tenatively - although it's federally registered as the Defiance.
She sleeps six. If anyone wants a ride or be crew in the next year...
let me know separately.

I'm increasingly unsure if the live-aboard-and-code idea is going to
work out. I might need a bigger boat, and crew. ;)

> So was going to submit the same(ish) patch to both netdev and openwrt;
> just want to make sure there is nothing more that needs to go in...

thx! All the space I had in my head for cake got eaten by the 1972
westerbeke manual.

>
> -Toke



-- 

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Dave Taht  writes:

> So it sounds like all the individual pieces are sync'd again?
>
> 4.16-rc4 is out, so there are several weeks left to make the net-next
> window. I note that I don't feel strongly (never had) that I should be
> the one to make the net-next submission, just so long as it now gets
> through checkpatch (?)... so since the code is freshest in several
> other minds than mine now, can someone volunteer to submit it to
> net-next?
>
> Otherwise I'll try to clear some time to do it next week.

Well, I was expecting you to still be at sea ;)

So was going to submit the same(ish) patch to both netdev and openwrt;
just want to make sure there is nothing more that needs to go in...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Dave Taht]

So it sounds like all the individual pieces are sync'd again?

4.16-rc4 is out, so there are several weeks left to make the net-next
window. I note that I don't feel strongly (never had) that I should be
the one to make the net-next submission, just so long as it now gets
through checkpatch (?)... so since the code is freshest in several
other minds than mine now, can someone volunteer to submit it to
net-next?

Otherwise I'll try to clear some time to do it next week.
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Sebastian Moeller  writes:

>> diff --git a/tc/q_cake.c b/tc/q_cake.c
>> index e21552e8..95301b41 100644
>> --- a/tc/q_cake.c
>> +++ b/tc/q_cake.c
>> @@ -243,12 +243,22 @@ static int cake_parse_opt(struct qdisc_util *qu, int 
>> argc, char **argv,
>>/* Typical VDSL2 framing schemes, both over PTM */
>>/* PTM has 64b/65b coding which absorbs some bandwidth */
>>} else if (strcmp(*argv, "pppoe-ptm") == 0) {
>> +   /* 2B PPP + 6B PPPoE + 6B dest MAC + 6B src MAC
>> +* + 2B ethertype + 4B Frame Check Sequence
>> +* + 1B Start of Frame (S) + 1B End of Frame (Ck)
>> +* + 2B TC-CRC (PTM-FCS) = 30B
>> +*/
>>atm = 2;
>> -   overhead += 27;
>> +   overhead += 30;
>>overhead_set = true;
>>} else if (strcmp(*argv, "bridged-ptm") == 0) {
>> +   /* 6B dest MAC + 6B src MAC + 2B ethertype
>> +* + 4B Frame Check Sequence
>> +* + 1B Start of Frame (S) + 1B End of Frame (Ck)
>> +* + 2B TC-CRC (PTM-FCS) = 22B
>> +*/
>>atm = 2;
>> -   overhead += 19;
>> +   overhead += 22;
>>overhead_set = true;
>> 
>> 
>> 
>> 
>> I assume 30 and 22 are the correct values? Could someone confirm this? :)
>
>   As I made that change all I can confirm that at the current time
>   I am convinced that 30 and 22 are the correct values. I did look
>   into the ITU standard documents for VDSL and to the best of my
>   knowledge these agree.

Awesome. Pushed that change to the tc-adv repo. Which means that the
cake-specific bits of both repos are now identical; but the tc-adv repo
is up-to-date with upstream iproute2-next.

I'll see if I can cook up a patch for openwrt...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

> I don’t the column alignment can be correct because the print lines don’t 
> include a leading space, so columns can run into each other.
>
> fprintf(f, "%12u", tst->unresponse_flows);
> v
> fprintf(f, " %12u", tst->unresponse_flows);
>
> The header lines are probably wrong.  Should be as per
> https://github.com/ldir-EDB0/iproute2-cake-next/commit/172c116722e2902bdc69824271303dac7249f379

Ah, right; ported that change over to the tc-adv repo as well. Which
brings us down to the following diff:

diff --git a/tc/q_cake.c b/tc/q_cake.c
index e21552e8..95301b41 100644
--- a/tc/q_cake.c
+++ b/tc/q_cake.c
@@ -243,12 +243,22 @@ static int cake_parse_opt(struct qdisc_util *qu, int 
argc, char **argv,
/* Typical VDSL2 framing schemes, both over PTM */
/* PTM has 64b/65b coding which absorbs some bandwidth */
} else if (strcmp(*argv, "pppoe-ptm") == 0) {
+   /* 2B PPP + 6B PPPoE + 6B dest MAC + 6B src MAC
+* + 2B ethertype + 4B Frame Check Sequence
+* + 1B Start of Frame (S) + 1B End of Frame (Ck)
+* + 2B TC-CRC (PTM-FCS) = 30B
+*/
atm = 2;
-   overhead += 27;
+   overhead += 30;
overhead_set = true;
} else if (strcmp(*argv, "bridged-ptm") == 0) {
+   /* 6B dest MAC + 6B src MAC + 2B ethertype
+* + 4B Frame Check Sequence
+* + 1B Start of Frame (S) + 1B End of Frame (Ck)
+* + 2B TC-CRC (PTM-FCS) = 22B
+*/
atm = 2;
-   overhead += 19;
+   overhead += 22;
overhead_set = true;
 



I assume 30 and 22 are the correct values? Could someone confirm this? :)

> qdisc cake 8004: root refcnt 4486780 bandwidth 19900Kbit diffserv3 
> dual-srchost nat rtt 100.0ms ptm overhead 4502152
>  Sent 42615751 bytes 4491092 pkt (dropped 4491108, overlimits 4491124 
> requeues 4491152)
>  backlog 0b 4491256p requeues 4491272
>  memory used: 78592b of 4Mb
>  capacity estimate: 19900Kbit
>  min/max transport layer size:4537916 / 4537972
>  min/max overhead-adjusted size:  4538000 / 4537972
>  average transport hdr offset:4538072
>
> I’m pondering if it’s an endian issue?

Are you using the latest kernel code? Those values were added in an
incompatible manner, so if you don't have userspace and kernel code in
sync they'll be garbage, basically...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

I don’t the column alignment can be correct because the print lines don’t 
include a leading space, so columns can run into each other.

fprintf(f, "%12u", tst->unresponse_flows);
v
fprintf(f, " %12u", tst->unresponse_flows);

The header lines are probably wrong.  Should be as per 
https://github.com/ldir-EDB0/iproute2-cake-next/commit/172c116722e2902bdc69824271303dac7249f379


I no longer think the ‘total overhead’ & ‘hard head len’ keywords are needed 
because they’re no longer output as part of the invocation line.  At one point 
there was a requirement to be able to cut’n’paste the output back in as a 
command line input.

All of this is fixed/consolidate into one place in the repo I pointed to make 
sure it wasn’t lost, with the JSON changes put on top.  All the cake commits 
are ‘on the top’ and easy to see.  The only issue I’m seeing since the JSON is 
impossibly high values for some of the stats.

qdisc cake 8004: root refcnt 4486780 bandwidth 19900Kbit diffserv3 dual-srchost 
nat rtt 100.0ms ptm overhead 4502152
 Sent 42615751 bytes 4491092 pkt (dropped 4491108, overlimits 4491124 requeues 
4491152)
 backlog 0b 4491256p requeues 4491272
 memory used: 78592b of 4Mb
 capacity estimate: 19900Kbit
 min/max transport layer size:4537916 / 4537972
 min/max overhead-adjusted size:  4538000 / 4537972
 average transport hdr offset:4538072

I’m pondering if it’s an endian issue?



> On 7 Mar 2018, at 11:28, Toke Høiland-Jørgensen  wrote:
> 
> Kevin Darbyshire-Bryant  writes:
> 
>> There were some useful stats column re-alignment changes as well,
>> wonder if you got those?
> 
> Probably not, as that code is not longer directly diff'able,
> unfortunately... The json-related changes were fairly intrusive...
> 
> Eyeballing the diff, however, I *think* that the column-alignment is the
> same...
> 
> Ah, great, but some of the calculations also changed :/
> 
> Could someone go through the patch below and check which is correct
> (the + lines or the - lines), please? + is tc-adv, - is
> iproute2-cake-next...
> 
> -Toke
> 
> 
> @@ -137,8 +139,8 @@ static int cake_parse_opt(struct qdisc_util *qu, int 
> argc, char **argv,
>interval = 100;
>target   =   5;
>} else if (strcmp(*argv, "interplanetary") == 0) {
> -   interval = 36U;
> -   target   =   5000;
> +   interval = 10;
> +   target   =   5000;
> 
>} else if (strcmp(*argv, "besteffort") == 0) {
>diffserv = 1;
> @@ -241,22 +243,12 @@ static int cake_parse_opt(struct qdisc_util *qu, int 
> argc, char **argv,
>/* Typical VDSL2 framing schemes, both over PTM */
>/* PTM has 64b/65b coding which absorbs some bandwidth */
>} else if (strcmp(*argv, "pppoe-ptm") == 0) {

> -   /* 2B PPP + 6B PPPoE + 6B dest MAC + 6B src MAC
> -* + 2B ethertype + 4B Frame Check Sequence
> -* + 1B Start of Frame (S) + 1B End of Frame (Ck)
> -* + 2B TC-CRC (PTM-FCS) = 30B
> -*/
>atm = 2;
> -   overhead += 30;
> +   overhead += 27;
the - is correct
>overhead_set = true;
>} else if (strcmp(*argv, "bridged-ptm") == 0) {
> -   /* 6B dest MAC + 6B src MAC + 2B ethertype
> -* + 4B Frame Check Sequence
> -* + 1B Start of Frame (S) + 1B End of Frame (Ck)
> -* + 2B TC-CRC (PTM-FCS) = 22B
> -*/
>atm = 2;
> -   overhead += 22;
> +   overhead += 19;
>overhead_set = true;
> 
the - is correct
>} else if (strcmp(*argv, "via-ethernet") == 0) {
> @@ -269,26 +261,8 @@ static int cake_parse_opt(struct qdisc_util *qu, int 
> argc, char **argv,
> * that automatically, and is thus ignored.
> *
> * It would be deleted entirely, but it appears in the
> -* stats output when the automatic compensation is
> -* active.
> -*/
> -
> -   } else if (strcmp(*argv, "total_overhead") == 0) {
> -   /*
> -* This is the overhead cake accounts for; added here 
> so
> -* that cake's "tc -s qdisc" output can be directly
> -* pasted into the tc command to instantate a new 
> cake..
> +* stats output when the automatic compensation is 
> active.
> */
> -   NEXT_ARG();
> -
> -

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

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 7 Mar 2018, at 10:31, Toke Høiland-Jørgensen  wrote:
>> 
>>> 
>> 
>> Please don't put something different into LEDE than what we're working
>> on upstreaming. It is difficult enough to keep track of the different
>> versions as they are. The tc-adv repo is already rebased on the upstream
>> iproute2-next, so if there is anything else that needs to be changed, it
>> should be changed in that repo and not in a fork...
>> 
>> -Toke
>
> Received & understood.  Not sending to lede… probably a good idea
> anyway as the main stats (not tin stats) are broken for me.  I wonder
> if this is because q_cake has to be imported to lede as a patch
> applied on top of whatever iproute2 version they’re using as the base?

As far as I can tell, the iproute2 version in openwrt is currently
4.15.0; which means that once we get this sorted out, it *should* be
straight forward to create a new patch that applies cleanly. I can do
that while preparing the upstream submission; but want to make sure we
have everything included without any version divergence first...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

> There were some useful stats column re-alignment changes as well,
> wonder if you got those?

Probably not, as that code is not longer directly diff'able,
unfortunately... The json-related changes were fairly intrusive...

Eyeballing the diff, however, I *think* that the column-alignment is the
same...

Ah, great, but some of the calculations also changed :/

Could someone go through the patch below and check which is correct
(the + lines or the - lines), please? + is tc-adv, - is
iproute2-cake-next...

-Toke


@@ -137,8 +139,8 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, 
char **argv,
interval = 100;
target   =   5;
} else if (strcmp(*argv, "interplanetary") == 0) {
-   interval = 36U;
-   target   =   5000;
+   interval = 10;
+   target   =   5000;
 
} else if (strcmp(*argv, "besteffort") == 0) {
diffserv = 1;
@@ -241,22 +243,12 @@ static int cake_parse_opt(struct qdisc_util *qu, int 
argc, char **argv,
/* Typical VDSL2 framing schemes, both over PTM */
/* PTM has 64b/65b coding which absorbs some bandwidth */
} else if (strcmp(*argv, "pppoe-ptm") == 0) {
-   /* 2B PPP + 6B PPPoE + 6B dest MAC + 6B src MAC
-* + 2B ethertype + 4B Frame Check Sequence
-* + 1B Start of Frame (S) + 1B End of Frame (Ck)
-* + 2B TC-CRC (PTM-FCS) = 30B
-*/
atm = 2;
-   overhead += 30;
+   overhead += 27;
overhead_set = true;
} else if (strcmp(*argv, "bridged-ptm") == 0) {
-   /* 6B dest MAC + 6B src MAC + 2B ethertype
-* + 4B Frame Check Sequence
-* + 1B Start of Frame (S) + 1B End of Frame (Ck)
-* + 2B TC-CRC (PTM-FCS) = 22B
-*/
atm = 2;
-   overhead += 22;
+   overhead += 19;
overhead_set = true;
 
} else if (strcmp(*argv, "via-ethernet") == 0) {
@@ -269,26 +261,8 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, 
char **argv,
 * that automatically, and is thus ignored.
 *
 * It would be deleted entirely, but it appears in the
-* stats output when the automatic compensation is
-* active.
-*/
-
-   } else if (strcmp(*argv, "total_overhead") == 0) {
-   /*
-* This is the overhead cake accounts for; added here so
-* that cake's "tc -s qdisc" output can be directly
-* pasted into the tc command to instantate a new cake..
+* stats output when the automatic compensation is 
active.
 */
-   NEXT_ARG();
-
-   } else if (strcmp(*argv, "hard_header_len") == 0) {
-   /*
-* This is the overhead the kernel automatically
-* accounted for; added here so that cake's "tc -s
-* qdisc" output can be directly pasted into the tc
-* command to instantiate a new cake..
-*/
-   NEXT_ARG();
 
} else if (strcmp(*argv, "ethernet") == 0) {
/* ethernet pre-amble & interframe gap & FCS
@@ -376,7 +350,7 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, 
char **argv,
addattr_l(n, 1024, TCA_CAKE_OVERHEAD, , 
sizeof(overhead));
if (overhead_override) {
unsigned zero = 0;
-   addattr_l(n, 1024, TCA_CAKE_ETHERNET, , sizeof(zero));
+   addattr_l(n, 1024, TCA_CAKE_RAW, , sizeof(zero));
}
if (mpu > 0)
addattr_l(n, 1024, TCA_CAKE_MPU, , sizeof(mpu));
@@ -532,9 +508,8 @@ static int cake_print_opt(struct qdisc_util *qu, FILE *f, 
struct rtattr *opt)
RTA_PAYLOAD(tb[TCA_CAKE_ACK_FILTER]) >= sizeof(__u32)) {
ack_filter = rta_getattr_u32(tb[TCA_CAKE_ACK_FILTER]);
}
-   if (tb[TCA_CAKE_ETHERNET] &&
-   RTA_PAYLOAD(tb[TCA_CAKE_ETHERNET]) >= sizeof(__u32)) {
-   ethernet = rta_getattr_u32(tb[TCA_CAKE_ETHERNET]);
+   if (tb[TCA_CAKE_RAW]) {
+   raw = 1;
}
if (tb[TCA_CAKE_RTT] &&
RTA_PAYLOAD(tb[TCA_CAKE_RTT]) >= sizeof(__u32)) {

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

[Sebastian Moeller]

Hi Kevin,




> On Mar 7, 2018, at 12:07, Kevin Darbyshire-Bryant 
>  wrote:
> 
> 
> 
>> On 7 Mar 2018, at 10:31, Toke Høiland-Jørgensen  wrote:
>> 
>>> 
>> 
>> Please don't put something different into LEDE than what we're working
>> on upstreaming. It is difficult enough to keep track of the different
>> versions as they are. The tc-adv repo is already rebased on the upstream
>> iproute2-next, so if there is anything else that needs to be changed, it
>> should be changed in that repo and not in a fork...
>> 
>> -Toke
> 
> Received & understood.  Not sending to lede… probably a good idea anyway as 
> the main stats (not tin stats) are broken for me.  I wonder if this is 
> because q_cake has to be imported to lede as a patch applied on top of 
> whatever iproute2 version they’re using as the base?
> 


Will you update the sch_cake ad tc patches (from tc-adv) for lede? That would 
be most excellent, as currently the overhead accountin of the cake in 
lede/openwrt is, let's say, interesting ;) (and since I try to support people 
in the forums I would very much like to not having to explain this especially 
since Jonathan really seems ti have squashed that (but to fully confirm that, I 
really want to get feedback from diverse group of lede cake-eaters ;)))

Best Regards
Sebastian


> 
> 
> Cheers,
> 
> Kevin D-B
> 
> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
> 
> ___
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

There were some useful stats column re-alignment changes as well, wonder if you 
got those?

> On 7 Mar 2018, at 10:36, Toke Høiland-Jørgensen  wrote:
> 
> Toke Høiland-Jørgensen  writes:
> 
>> Kevin Darbyshire-Bryant  writes:
>> 
>>> Hi All,
>>> 
>>> 
>>> 
 On 7 Mar 2018, at 08:50, Toke Høiland-Jørgensen  wrote:
 
 Jonathan Morton  writes:
 
>> On 6 Mar, 2018, at 11:06 pm, Toke Høiland-Jørgensen  wrote:
>> 
>> ...on the iproute2 side the only
>> thing missing before we can attempt an upstream submission is an update
>> of the man page, as far as I can tell. Any volunteers to do that? :)
> 
> I could look into that tomorrow.
 
 Awesome! Is there anything else you want to add on the kernel side
 before we submit?
>>> 
>>> I think there was mention of the repo that Dave started on the last
>>> round of getting cake in upstream iproute2. There were a few man page
>>> tweaks & output formatting tweaks which ideally shouldn’t be lost.
>>> I’ve forked that fork on github here
>>> https://github.com/ldir-EDB0/iproute2-cake-next/tree/cake rebased onto
>>> release tag v4.15.0 and pulled in the the recent changes (overhead
>>> handling paradigm, xstats & the JSON output).
>>> 
>>> In theory I guess there should be no/little difference to tc-adv/master now 
>>> but….
>>> 
>>> Anyway, it’s there if it’s of any interest… it’ll be the basis of what
>>> gets sent into LEDE a bit later today.
>> 
>> Please don't put something different into LEDE than what we're working
>> on upstreaming. It is difficult enough to keep track of the different
>> versions as they are. The tc-adv repo is already rebased on the upstream
>> iproute2-next, so if there is anything else that needs to be changed, it
>> should be changed in that repo and not in a fork...
> 
> I pulled in the man page changes from Dave's iproute2-next fork to the
> tc-adv repo now...
> 
> -Toke


Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A



signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

> On 7 Mar 2018, at 10:31, Toke Høiland-Jørgensen  wrote:
> 
>> 
> 
> Please don't put something different into LEDE than what we're working
> on upstreaming. It is difficult enough to keep track of the different
> versions as they are. The tc-adv repo is already rebased on the upstream
> iproute2-next, so if there is anything else that needs to be changed, it
> should be changed in that repo and not in a fork...
> 
> -Toke

Received & understood.  Not sending to lede… probably a good idea anyway as the 
main stats (not tin stats) are broken for me.  I wonder if this is because 
q_cake has to be imported to lede as a patch applied on top of whatever 
iproute2 version they’re using as the base?



Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A



signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen  writes:

> Kevin Darbyshire-Bryant  writes:
>
>> Hi All,
>>
>>
>>
>>> On 7 Mar 2018, at 08:50, Toke Høiland-Jørgensen  wrote:
>>> 
>>> Jonathan Morton  writes:
>>> 
> On 6 Mar, 2018, at 11:06 pm, Toke Høiland-Jørgensen  wrote:
> 
> ...on the iproute2 side the only
> thing missing before we can attempt an upstream submission is an update
> of the man page, as far as I can tell. Any volunteers to do that? :)
 
 I could look into that tomorrow.
>>> 
>>> Awesome! Is there anything else you want to add on the kernel side
>>> before we submit?
>>
>> I think there was mention of the repo that Dave started on the last
>> round of getting cake in upstream iproute2. There were a few man page
>> tweaks & output formatting tweaks which ideally shouldn’t be lost.
>> I’ve forked that fork on github here
>> https://github.com/ldir-EDB0/iproute2-cake-next/tree/cake rebased onto
>> release tag v4.15.0 and pulled in the the recent changes (overhead
>> handling paradigm, xstats & the JSON output).
>>
>> In theory I guess there should be no/little difference to tc-adv/master now 
>> but….
>>
>> Anyway, it’s there if it’s of any interest… it’ll be the basis of what
>> gets sent into LEDE a bit later today.
>
> Please don't put something different into LEDE than what we're working
> on upstreaming. It is difficult enough to keep track of the different
> versions as they are. The tc-adv repo is already rebased on the upstream
> iproute2-next, so if there is anything else that needs to be changed, it
> should be changed in that repo and not in a fork...

I pulled in the man page changes from Dave's iproute2-next fork to the
tc-adv repo now...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

> Hi All,
>
>
>
>> On 7 Mar 2018, at 08:50, Toke Høiland-Jørgensen  wrote:
>> 
>> Jonathan Morton  writes:
>> 
 On 6 Mar, 2018, at 11:06 pm, Toke Høiland-Jørgensen  wrote:
 
 ...on the iproute2 side the only
 thing missing before we can attempt an upstream submission is an update
 of the man page, as far as I can tell. Any volunteers to do that? :)
>>> 
>>> I could look into that tomorrow.
>> 
>> Awesome! Is there anything else you want to add on the kernel side
>> before we submit?
>
> I think there was mention of the repo that Dave started on the last
> round of getting cake in upstream iproute2. There were a few man page
> tweaks & output formatting tweaks which ideally shouldn’t be lost.
> I’ve forked that fork on github here
> https://github.com/ldir-EDB0/iproute2-cake-next/tree/cake rebased onto
> release tag v4.15.0 and pulled in the the recent changes (overhead
> handling paradigm, xstats & the JSON output).
>
> In theory I guess there should be no/little difference to tc-adv/master now 
> but….
>
> Anyway, it’s there if it’s of any interest… it’ll be the basis of what
> gets sent into LEDE a bit later today.

Please don't put something different into LEDE than what we're working
on upstreaming. It is difficult enough to keep track of the different
versions as they are. The tc-adv repo is already rebased on the upstream
iproute2-next, so if there is anything else that needs to be changed, it
should be changed in that repo and not in a fork...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Kevin Darbyshire-Bryant]

Hi All,



> On 7 Mar 2018, at 08:50, Toke Høiland-Jørgensen  wrote:
> 
> Jonathan Morton  writes:
> 
>>> On 6 Mar, 2018, at 11:06 pm, Toke Høiland-Jørgensen  wrote:
>>> 
>>> ...on the iproute2 side the only
>>> thing missing before we can attempt an upstream submission is an update
>>> of the man page, as far as I can tell. Any volunteers to do that? :)
>> 
>> I could look into that tomorrow.
> 
> Awesome! Is there anything else you want to add on the kernel side
> before we submit?

I think there was mention of the repo that Dave started on the last round of 
getting cake in upstream iproute2.  There were a few man page tweaks & output 
formatting tweaks which ideally shouldn’t be lost.  I’ve forked that fork on 
github here https://github.com/ldir-EDB0/iproute2-cake-next/tree/cake  rebased 
onto release tag v4.15.0 and pulled in the the recent changes (overhead 
handling paradigm, xstats & the JSON output).

In theory I guess there should be no/little difference to tc-adv/master now 
but….

Anyway, it’s there if it’s of any interest… it’ll be the basis of what gets 
sent into LEDE a bit later today.


Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A



signature.asc
Description: Message signed with OpenPGP
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Jonathan Morton  writes:

>> On 6 Mar, 2018, at 11:06 pm, Toke Høiland-Jørgensen  wrote:
>> 
>> ...on the iproute2 side the only
>> thing missing before we can attempt an upstream submission is an update
>> of the man page, as far as I can tell. Any volunteers to do that? :)
>
> I could look into that tomorrow.

Awesome! Is there anything else you want to add on the kernel side
before we submit?

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 6 Mar, 2018, at 11:06 pm, Toke Høiland-Jørgensen  wrote:
> 
> ...on the iproute2 side the only
> thing missing before we can attempt an upstream submission is an update
> of the man page, as far as I can tell. Any volunteers to do that? :)

I could look into that tomorrow.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen  writes:

> Stephen Hemminger  writes:
>
>> On Sun, 11 Feb 2018 18:26:18 +0100
>> Toke Høiland-Jørgensen  wrote:
>>
>>> 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 
>>
>> Please consider using new json printing routines in latest iproute2.
>
> Yeah, was going to look into that...

Right, so I merged the upstream iproute2-next repository into the master
branch and added JSON support to the Cake qdisc code.

I also removed a bunch of old compatibility code along the way, as well
as the fq_pie code and any changes to the repo that were not in upstream
and were not cake-related. So the delta between the tc-adv repo and
upstream is now only cake.

This was quite a bit of rearranging, so please do check if I broke
something along the way.

Other than that, though, this means that on the iproute2 side the only
thing missing before we can attempt an upstream submission is an update
of the man page, as far as I can tell. Any volunteers to do that? :)

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Stephen Hemminger  writes:

> On Sun, 11 Feb 2018 18:26:18 +0100
> Toke Høiland-Jørgensen  wrote:
>
>> 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 
>
> Please consider using new json printing routines in latest iproute2.

Yeah, was going to look into that...

> I am not going to accept any new code upstream that doesn't support
> JSON output as an option.

Excellent policy! :)

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Stephen Hemminger]

On Sun, 11 Feb 2018 18:26:18 +0100
Toke Høiland-Jørgensen  wrote:

> 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 

Please consider using new json printing routines in latest iproute2.
I am not going to accept any new code upstream that doesn't support JSON output
as an option.
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Sebastian Moeller]

Hi Jonathan,



> On Mar 6, 2018, at 14:08, Jonathan Morton  wrote:
> 
>> On 6 Mar, 2018, at 2:10 pm, Sebastian Moeller  wrote:
>> 
>> Question: am I right to assume that the _adj values are what you pass in as 
>> the true packet size into cake's calculation?
> 
> Yes, exactly.  They include ATM, PTM and MPU factors when relevant.

Excellent, this is basically the information we would also see when tc stab is 
used, and being the actually input in the (maybe indirect) transfer time 
calculations is the best number IMHO for sanity checking... 


> 
>> I believe that before the xstats and Jonathan's recent updates 
>> iproute2-cake-next  was more up to date, but I am probably wrong...
> 
> I understand it's based on a newer version of iproute2, so would be easier to 
> merge upstream, but its Cake support was exactly equivalent.  The latest 
> batch of changes should be straightforward to port forward to it.

Again, that sounds great, in your opinion now with the overhead/size 
accounting put on new footing and Toke's xstats work are we closer to being 
mergeable upstream? Any other blockers pending improvements?


Best Regards
Sebastian

> 
> - Jonathan Morton
> 

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 6 Mar, 2018, at 2:10 pm, Sebastian Moeller  wrote:
> 
> Question: am I right to assume that the _adj values are what you pass in as 
> the true packet size into cake's calculation?

Yes, exactly.  They include ATM, PTM and MPU factors when relevant.

> I believe that before the xstats and Jonathan's recent updates 
> iproute2-cake-next  was more up to date, but I am probably wrong...

I understand it's based on a newer version of iproute2, so would be easier to 
merge upstream, but its Cake support was exactly equivalent.  The latest batch 
of changes should be straightforward to port forward to it.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Sebastian Moeller]

Hi Jonathan,



> On Mar 6, 2018, at 12:46, Jonathan Morton  wrote:
> 
>> On 6 Mar, 2018, at 1:17 pm, Toke Høiland-Jørgensen  wrote:
>> 
>>> So far it looks like everything is working fine and behaving as it
>>> should (on a plain Ethernet interface).  The one wrinkle is that it
>>> takes a few thousand packets for the avg_off value to converge on the
>>> true value, but that should easily be tolerable in most cases.
>> 
>> So, erm, what do the new stats mean? ;)
> 
> avg_off: the average observed offset of the transport header in packets.  
> This should converge to 14 on Ethernet.
> 
> max/min_tran: tracking the transport-layer size of packets; max_tran = 
> max_len-avg_off if GSO is off.
> 
> max/min_adj: tracking the overhead-adjusted size of packets; these should be 
> the adjusted sizes of the corresponding transport-layer sizes, so can be used 
> to check the calculations.
> 
> It's probably feasible to lay these out better in the output, so they take up 
> fewer lines on screen.

Great this matches what I deduced from the numbers. I note that by replacing a 
cake instance with itself (so not changing the configuration) will reset the 
counters.
Question: am I right to assume that the _adj values are what you pass in as the 
true packet size into cake's calculation?
I really like these outputs so far (running flent I saw:

qdisc cake 8003: dev eth0 root refcnt 6 unlimited diffserv3 triple-isolate rtt 
100.0ms noatm overhead 34 mpu 64 
 Sent 269164303 bytes 611190 pkt (dropped 0, overlimits 0 requeues 2) 
 backlog 0b 0p requeues 2 
 memory used: 10880b of 15140Kb
 capacity estimate: 0bit
 Bulk   Best Effort  Voice
  thresh  0bit0bit0bit
  target 5.0ms   5.0ms   5.0ms
  interval 100.0ms 100.0ms 100.0ms
  pk_delay14us11us 8us
  av_delay 2us 2us 1us
  sp_delay 0us 0us 0us
  pkts   61400  395326  154464
  bytes   17617151   18325055268296600
  way_inds   0   0   0
  way_miss   3  89  11
  way_cols   0   0   0
  drops  0   0   0
  marks  0   0   0
  ack_drop   0   0   0
  sp_flows   2  13   7
  bk_flows   0   0   0
  un_flows   0   0   0
  max_len 301230283012
  max_tran1492
  max_adj 1526
  min_tran  28
  min_adj   64
  avg_off   14


Which is pretty much what I expected (speedtest ran over an effective MTU1492 
PPPoE link*), but I still want to make more in-depth tests. Now we just need to 
carry this nto lede to get more testers. Plus it might make sense to merge 
tc-adv with iproute2-cake-next so that we have one master repository for the tc 
changes as well. I believe that before the xstats and Jonathan's recent updates 
iproute2-cake-next  was more up to date, but I am probably wrong...

Best Regards
Sebastian

*) Interestingly takling stats later revealed:
qdisc cake 8003: dev eth0 root refcnt 6 unlimited diffserv3 triple-isolate rtt 
100.0ms noatm overhead 34 mpu 64 
 Sent 743833445 bytes 1965042 pkt (dropped 0, overlimits 0 requeues 6) 
 backlog 0b 0p requeues 6 
 memory used: 16346b of 15140Kb
 capacity estimate: 0bit
 Bulk   Best Effort  Voice
  thresh  0bit0bit0bit
  target 5.0ms   5.0ms   5.0ms
  interval 100.0ms 100.0ms 100.0ms
  pk_delay 8us12us 9us
  av_delay 1us 3us 3us
  sp_delay 1us 1us 1us
  pkts  167869 1382188  414985
  bytes   49022886   511662935   183147624
  way_inds   0 341   0
  way_miss   61137  20
  way_cols   0   0   0
  drops  0   0   0
  marks  0   0   0
  ack_drop   0   0   0
  sp_flows   1   1   0
  bk_flows   0   0   1
  un_flows   0   0   0
  max_len 301230283012
  max_tran1500
  max_adj 1534
  min_tran  28
  min_adj   64
  avg_off   14

Son between this sample and the speedtest the host exchanged MTU1500 packets 
with the local network...

> 
> - Jonathan Morton
> 

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 6 Mar, 2018, at 1:17 pm, Toke Høiland-Jørgensen  wrote:
> 
>> So far it looks like everything is working fine and behaving as it
>> should (on a plain Ethernet interface).  The one wrinkle is that it
>> takes a few thousand packets for the avg_off value to converge on the
>> true value, but that should easily be tolerable in most cases.
> 
> So, erm, what do the new stats mean? ;)

avg_off: the average observed offset of the transport header in packets.  This 
should converge to 14 on Ethernet.

max/min_tran: tracking the transport-layer size of packets; max_tran = 
max_len-avg_off if GSO is off.

max/min_adj: tracking the overhead-adjusted size of packets; these should be 
the adjusted sizes of the corresponding transport-layer sizes, so can be used 
to check the calculations.

It's probably feasible to lay these out better in the output, so they take up 
fewer lines on screen.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Jonathan Morton  writes:

>> On 1 Mar, 2018, at 1:06 pm, Sebastian Moeller  wrote:
>> 
>> BTW, my testing so far with the latest tc-adv did not result in any crashes, 
>> but I also did not research whether the overhead account behaves like 
>> expected...
>
> I've just added the relevant stats output.  This breaks the stats
> struct again (better to do that just after breaking it once, I
> suppose), but I left in some padding to reduce the need for that in
> future.  These stats are kept globally, so are only reported as if for
> one tin.

Ah yes, the new layout makes it somewhat difficult to add new global
stats. I suppose one could store an offset in the struct as well, but
that would probably be overdoing it, methinks. A few bytes of padding
should work fine; we probably shouldn't keep adding stats forever anyway
(famous last words) :)

> So far it looks like everything is working fine and behaving as it
> should (on a plain Ethernet interface).  The one wrinkle is that it
> takes a few thousand packets for the avg_off value to converge on the
> true value, but that should easily be tolerable in most cases.

So, erm, what do the new stats mean? ;)

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 1 Mar, 2018, at 1:06 pm, Sebastian Moeller  wrote:
> 
> BTW, my testing so far with the latest tc-adv did not result in any crashes, 
> but I also did not research whether the overhead account behaves like 
> expected...

I've just added the relevant stats output.  This breaks the stats struct again 
(better to do that just after breaking it once, I suppose), but I left in some 
padding to reduce the need for that in future.  These stats are kept globally, 
so are only reported as if for one tin.

So far it looks like everything is working fine and behaving as it should (on a 
plain Ethernet interface).  The one wrinkle is that it takes a few thousand 
packets for the avg_off value to converge on the true value, but that should 
easily be tolerable in most cases.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen  writes:

> Jonathan Morton  writes:
>
>>> On 11 Feb, 2018, at 7:26 pm, Toke Høiland-Jørgensen  wrote:
>>> 
>>> 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.
>>
>> I think you could make a pull request for these patches now.  I can
>> then add new stats to the new code instead of duplicating work.
>
> I think I actually have write access to the repos, so I'll just push the
> commits. Sometime tonight; the code is on my laptop :)

And totally forgot about it, of course. Sorry about that, pushed now :)

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Sebastian Moeller]

Dear All, 

I am a bit confused now which tc repository is the one to track.

I believe Jonathan made changes to https://github.com/dtaht/tc-adv

while https://github.com/dtaht/iproute2-cake-next seems overall more recent 
(modulo Jonathan's changes)

So which one is it?

BTW, my testing so far with the latest tc-adv did not result in any crashes, 
but I also did not research whether the overhead account behaves like 
expected...

Best Regards
Sebastian


> On Mar 1, 2018, at 12:02, Jonathan Morton  wrote:
> 
>> On 11 Feb, 2018, at 7:26 pm, Toke Høiland-Jørgensen  wrote:
>> 
>> 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.
> 
> I think you could make a pull request for these patches now.  I can then add 
> new stats to the new code instead of duplicating work.
> 
> - Jonathan Morton
> 
> ___
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[Jonathan Morton]

> On 11 Feb, 2018, at 7:26 pm, Toke Høiland-Jørgensen  wrote:
> 
> 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.

I think you could make a pull request for these patches now.  I can then add 
new stats to the new code instead of duplicating work.

 - Jonathan Morton

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

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

[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++)