[Cake] Dropping last_len from stats collection & display.

[Cake] Dropping last_len from stats collection & display.

[Kevin Darbyshire-Bryant]

Gentlemen,

Two pull requests https://github.com/dtaht/sch_cake/pull/26  & 
https://github.com/dtaht/tc-adv/pull/11

These drop the collection of last packet's length for stats purposes.

I've not dropped maxlen as I have personally found that *incredibly* 
useful in working out what overheads linux has already accounted for on 
a variety of connections.  It is also a useful indicator of GSO/GRO 
activity.

Not updating another variable (memory write) for each packet may help 
cake's CPU usage a little.

Done in a backwards compatible way for tc.

Please ACK/NACK as you see fit.

Kevin

Re: [Cake] Dropping last_len from stats collection & display.

[Jonathan Morton]

> On 1 Jun, 2016, at 14:40, Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote:
> 
> Not updating another variable (memory write) for each packet may help cake's CPU usage a little.

I’ll think about this one.

I don’t think it should gate the LEDE merge (nor should it require a stats version bump), since as long as the entry remains in the stats struct, it will simply remain zero if not written.

In future the space could be reused, in which case a version bump would be appropriate.

 - Jonathan Morton

Re: [Cake] Dropping last_len from stats collection & display.

[Kevin Darbyshire-Bryant]

On 01/06/16 15:00, Jonathan Morton wrote:
>
>> On 1 Jun, 2016, at 14:40, Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote:
>>
>> Not updating another variable (memory write) for each packet may help cake's CPU usage a little.
>
> I’ll think about this one.
>
> I don’t think it should gate the LEDE merge (nor should it require a stats version bump), since as long as the entry remains in the stats struct, it will simply remain zero if not written.
>
> In future the space could be reused, in which case a version bump would be appropriate.
>
>   - Jonathan Morton
>

Two points/suggestions/alternate perspectives:

Not bumping the version leads to a situation where tc constantly reports 
the last length as zero.  I would suggest that looks 
odd/oversight/wrong.  A version bump at least allows a later tc to know 
not to output a dead value.


I can see your point with regard to the structure size, how about 
re-adding as

u32 __attribute__((used)) last_skblen;  /* V3 and below */

to stop the unused variable warning that would otherwise result.

The other argument I keep having with myself is "none of this has been 
released...yet".  Aren't we still free to break this as we see fit :-)

Re: [Cake] Dropping last_len from stats collection & display.

[moeller0]

Hi Kevin,


> On Jun 1, 2016, at 13:40 , Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote:
> 
> Gentlemen,
> 
> Two pull requests https://github.com/dtaht/sch_cake/pull/26  & https://github.com/dtaht/tc-adv/pull/11
> 
> These drop the collection of last packet's length for stats purposes.
> 
> I've not dropped maxlen as I have personally found that *incredibly* useful in working out what overheads linux has already accounted for on a variety of connections.  It is also a useful indicator of GSO/GRO activity.

	I fully concur overhead deduction is hard any piece of information can help, also I note that fq_codel reports the same and I believe we all agree that Eric Dumazet knows what he is doing ;)

Best Regards
	Sebastian

> 
> Not updating another variable (memory write) for each packet may help cake's CPU usage a little.
> 
> Done in a backwards compatible way for tc.
> 
> Please ACK/NACK as you see fit.
> 
> Kevin
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake