> On 2 Jul 2018, at 17:14, Toke Høiland-Jørgensen wrote: > > Pete Heist writes: > >>> On Jul 2, 2018, at 2:03 PM, Toke Høiland-Jørgensen wrote: >>> >>> Pete Heist 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