Re: [Cake] Cake on openwrt - falling behind

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

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



> On 2 Jul 2018, at 18:04, Pete Heist <pete@heistp.net> wrote:
> 
> 
> 
>> On Jul 2, 2018, at 6:14 PM, Toke Høiland-Jørgensen <toke@toke.dk> 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


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