--- Begin Message ---
> On 2 Jul 2018, at 18:04, Pete Heist <[email protected]> wrote:
>
>
>
>> On Jul 2, 2018, at 6:14 PM, Toke Høiland-Jørgensen <[email protected]> wrote:
>>
>> 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?
>
> Awesome Toke! It looks like from Kevin’s email that it works for him, but it
> didn’t work for me the first time around. This may have to do with how I
> added the patch as I’m still not that familiar with OpenWRT’s build system
> (first kernel patch I tried). I wasn’t sure if it should go into generic or
> platform, for one, so I tried generic…is that right?
>
> sysadmin@rey:~/src/openwrt/build_dir/target-mips_24kc_musl/linux-ar71xx_generic/linux-4.9.109/patches/generic$
> cat 010-gen_stats_fix.patch
> --- a/net/core/gen_stats.c
> +++ b/net/core/gen_stats.c
> @@ -77,8 +77,12 @@ gnet_stats_start_copy_compat(struct sk_b
> 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;
> }
>
generic is where it should go, but what I did was:
make package/iproute2/clean
make package/kmod-sched-cake/clean - both to be *really* sure they get
rebuilt, shouldn’t be required but!!!
make target/linux/{clean,prepare} (this gets the kernel build tree in a ready
to build state with all the openwrt patches)
edit build/target_mips34*/linux*/linux*/net/core/genstat.c directly
make
flash the image - test :-)
I assume tha patch has been tried on previously unbroken arch’s and they remain
unb0rked ?
Cheers,
Kevin D-B
012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A
signature.asc
Description: Message signed with OpenPGP
--- End Message ---
_______________________________________________
Cake mailing list
[email protected]
https://lists.bufferbloat.net/listinfo/cake
