Re: [Cake] [PATCH 1/3] pkt_sched.h: add support for sch_cake API

[Dave Taht <dave@taht.net>]

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Tue, 21 Nov 2017 16:09:40 -0800
> Dave Taht <dave.taht@gmail.com> wrote:
>
>>  
>> +/* CAKE */
>> +enum {
>> +	TCA_CAKE_UNSPEC,
>> +	TCA_CAKE_BASE_RATE,
>> +	TCA_CAKE_DIFFSERV_MODE,
>> +	TCA_CAKE_ATM,
>> +	TCA_CAKE_FLOW_MODE,
>> +	TCA_CAKE_OVERHEAD,
>> +	TCA_CAKE_RTT,
>> +	TCA_CAKE_TARGET,
>> +	TCA_CAKE_AUTORATE,
>> +	TCA_CAKE_MEMORY,
>> +	TCA_CAKE_NAT,
>> +	TCA_CAKE_ETHERNET,
>> +	TCA_CAKE_WASH,
>> +	TCA_CAKE_MPU,
>> +	TCA_CAKE_INGRESS,
>> +	TCA_CAKE_ACK_FILTER,
>> +	__TCA_CAKE_MAX
>> +};
>> +#define TCA_CAKE_MAX	(__TCA_CAKE_MAX - 1)
>> +
>> +struct tc_cake_traffic_stats {
>> +	__u32 packets;
>> +	__u32 link_ms;
>> +	__u64 bytes;
>> +};
>> +
>> +#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 */
>> +
>> +	__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];
>> +	__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 */
>> +};
>> +
>
> Rather than passing one big structure (like xstats) through netlink, it is more typical
> to break it into separate TLV items.

This is an API in its 5th revision over four years. If ever there were a
time to break it and do something saner, it would be now, of course.

> Also, when I see multiple arrays of same size. It seems the API should
> be:
> 	struct tc_cake_tin_stats {
> 		__u32 threshold_rate;
> 		__u32 target_us;
> 		struct tc_cake_traffic_stats sent;
> ...
>
> What if you want to change number of TINS, the ABI shouldn't have to change.

It is hard to imagine ever wanting more than 8 tins. Using up 3, sanely,
has proven hard.

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