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