Re: [Cake] Cake on openwrt - falling behind

[Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>]

[-- Attachment #1: Type: text/plain, Size: 4663 bytes --]



> On 2 Jul 2018, at 17:14, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> 
> Pete Heist <pete@heistp.net> writes:
> 
>>> On Jul 2, 2018, at 2:03 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>>> 
>>> Pete Heist <pete@heistp.net> writes:
>>> 
>>>> But, um, I find it curious that tb[TCA_PAD] has valid looking values
>>>> in it, and if I just go:
>>>> 
>>>> tb[TCA_STATS2] = tb[TCA_PAD]
>>>> 
>>>> right after parse_rtattr is called, I start getting tin stats printed
>>>> that look valid. There should be zeroes or invalid values at
>>>> tb[TCA_PAD], as that’s just supposed to be padding, right?
>>> 
>>> Hmm, that's interesting. Sounds like you are on the right track. What
>>> are the numerical values of TCA_STATS2 and TCA_PAD in the kernel and
>>> iproute2, respectively?
>> 
>> I never would’ve guessed they could be different when compiled at once
>> :) but true, there are at least five different versions of rtnetlink.h
>> under build_dir based on their md5sums, and I see one belongs to
>> iproute2-full. In all of those, it looks in the source like
>> TCA_STATS2=7 and TCA_PAD=9.
> 
> Aha! I think I figured out what is going on:
> 
> The gen_stats facility will add an nlattr header at the beginning of the
> qdisc stats, which is the toplevel TLV that contains all stats (and that
> we put our stats inside). It stores a reference to this header, and when
> all the per-qdisc callbacks have finished adding their stats, it goes
> back and fixes up the length of the containing header.
> 
> The problem is that on architectures that need padding, the padding TLV
> is added *first*, which means that the nlattr pointer that is stored
> before the callbacks are performed points to the padding TLV and not the
> stats TLV. And so, when the header is fixed up, the result (from the
> parser's perspective) is just a very big padding TLV.
> 
> The options TLV is before the stats TLV, so the bug only occurs if the
> options happen to have a length that means the stats will need padding.
> Which is why messing with the number of options "fixes" the bug.
> 
> Could you try applying the patch below (to the kernel) and see if that
> resolves the issue, please?
> 
> -Toke
> 
> 
> @@ -77,8 +77,12 @@ gnet_stats_start_copy_compat(struct sk_buff *skb, int type, int tc_stats_type,
> 		d->lock = lock;
> 		spin_lock_bh(lock);
> 	}
> -	if (d->tail)
> -		return gnet_stats_copy(d, type, NULL, 0, padattr);
> +	if (d->tail) {
> +		int ret = gnet_stats_copy(d, type, NULL, 0, padattr);
> +		if (!ret)
> +			d->tail = ((struct nlattr *)skb_tail_pointer(skb)) - 1;
> +		return ret;
> +	}
> 
> 	return 0;
> }

I do believe Sir you have cracked it.   Without my u32 hack on a MIPS34kc arch that previously required my hack…..

tc -s qdisc show dev ifb4pppoa-wan
qdisc cake 800b: root refcnt 2 bandwidth 2027Kbit diffserv3 dual-dsthost nat ingress split-gso rtt 100.0ms raw overhead 0
tca_stats 2011972452 tca_stats2 2011971788 tca_xstats 0
calling print_tcstats_attr()
print_tcstats_attr()
got stats2
 Sent 144 bytes 3 pkt (dropped 0, overlimits 0 requeues 0)
xstats 2145834940 tca_stats_app 2011971792
 backlog 0b 0p requeues 0
got xstats 2011971792 tca_stats 2011972452 tca_stats2 2011971788 tca_xstats 0
 memory used: 2464b of 4Mb
 capacity estimate: 2027Kbit
 min/max network layer size:           48 /      48
 min/max overhead-adjusted size:       48 /      48
 average network hdr offset:            0

                   Bulk  Best Effort        Voice
  thresh      126680bit     2027Kbit    506744bit
  target        143.4ms        9.0ms       35.9ms
  interval      286.8ms      104.0ms      130.9ms
  pk_delay          0us         17us          0us
  av_delay          0us          0us          0us
  sp_delay          0us          0us          0us
  backlog            0b           0b           0b
  pkts                0            3            0
  bytes               0          144            0
  way_inds            0            0            0
  way_miss            0            1            0
  way_cols            0            0            0
  drops               0            0            0
  marks               0            0            0
  ack_drop            0            0            0
  sp_flows            0            1            0
  bk_flows            0            0            0
  un_flows            0            0            0
  max_len             0           48            0
  quantum           300          300          300


Cheers,

Kevin D-B

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


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]