Cake - FQ_codel the next generation
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@toke.dk>
To: Pete Heist <pete@heistp.net>
Cc: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>,
	Cake List <cake@lists.bufferbloat.net>
Subject: Re: [Cake] Cake on openwrt - falling behind
Date: Mon, 02 Jul 2018 21:31:29 +0200	[thread overview]
Message-ID: <874lhhigdq.fsf@toke.dk> (raw)
In-Reply-To: <AD1BBAC6-ED50-470B-ADF0-5FFC96160B82@heistp.net>

Pete Heist <pete@heistp.net> writes:

>> On Jul 2, 2018, at 7:04 PM, Pete Heist <pete@heistp.net> wrote:
>> 
>> 
>> 
>>> On Jul 2, 2018, at 6:14 PM, Toke Høiland-Jørgensen <toke@toke.dk <mailto: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?
>
> Ok, I got it to work after re-flashing with tftp. :) It looks like the
> OM2P is not always successfully performing sysupgrades, perhaps due to
> its limited memory (64M), but I’m not sure.

Great, thanks for testing, both of you!

> I still have my debugging in place and do still have one question. The
> pointer in TCA_STATS2 is now valid, but there is still a pointer value
> in TCA_PAD, which is pointing to a place 32 bits before TCA_STATS2. Is
> that expected?

Yes, that is expected. The PAD is there to align the subsequent STATS
TLV, so it is being parsed but is unused.

This is the offending spot from Kevin's pcap files:

Working: The header starts at byte 0x17c with a TLV of type 7 (TCA_STATS2)
of length 0x268. No padding TLV needed, so stats work:

00000170: 0000 0000 0008 000a 0000 0000 0268 0007  .............h..
00000180: 0234 0004 0008 0002 0025 f4cc 0008 0003  .4.......%......

Broken: The header starts at byte 0x180 with a PAD TLV (type 9) followed
by the TCA_STATS2 TLV. Both start out with 4-byte lengths (just the
header), but because the PAD TLV is the one being extended, that gets a
new length of 0x29c, which means it now contains the TCA_STATS2 TLV as
the first nested TLV. What should have happened was that the TCA_PAD TLV
should have stayed at 4 bytes, thus aligning the payload of the
TCA_STATS2 TLV at 0x188 bytes, and the TCA_STATS2 TLV should have been
length 0x298. Which is what happens after the patch, and why TCA_PAD is
still set by the parser.

00000180: 029c 0009 0004 0007 0260 0004 000c 0002  .........`......


-Toke

  reply	other threads:[~2018-07-02 19:31 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <mailman.381.1530347349.3512.cake@lists.bufferbloat.net>
2018-06-30 16:37 ` Georgios Amanakis
2018-06-30 17:26   ` Pete Heist
2018-06-30 18:09     ` Georgios Amanakis
2018-06-30 18:55       ` Kevin Darbyshire-Bryant
2018-06-30 19:57         ` Pete Heist
2018-06-30 20:58           ` Georgios Amanakis
2018-06-30 21:37             ` Pete Heist
2018-06-30 22:43               ` Pete Heist
2018-06-30 23:20                 ` Pete Heist
     [not found]                   ` <CACvFP_gkdAPKSEO7j9+cMPqTa-fJYd8XFEEBD6ZzLuVvaNwsvg@mail.gmail.com>
2018-07-01  2:37                     ` [Cake] Fwd: " Georgios Amanakis
2018-07-01  7:18                       ` [Cake] " Pete Heist
2018-07-01 13:48                         ` Pete Heist
2018-07-01 14:02                           ` Dave Taht
2018-07-01 15:30                             ` Pete Heist
2018-07-01 14:38                           ` Kevin Darbyshire-Bryant
2018-07-01 16:54                             ` Pete Heist
2018-07-01 19:41                               ` Kevin Darbyshire-Bryant
2018-07-02 10:19                                 ` Pete Heist
2018-07-02 11:38                                   ` Pete Heist
2018-07-02 11:59                                     ` Kevin Darbyshire-Bryant
2018-07-02 14:01                                       ` Pete Heist
2018-07-02 12:03                                     ` Toke Høiland-Jørgensen
2018-07-02 14:51                                       ` Pete Heist
2018-07-02 16:14                                         ` Toke Høiland-Jørgensen
2018-07-02 16:59                                           ` Kevin Darbyshire-Bryant
2018-07-02 17:04                                           ` Pete Heist
2018-07-02 17:12                                             ` Kevin Darbyshire-Bryant
2018-07-02 18:24                                             ` Pete Heist
2018-07-02 19:31                                               ` Toke Høiland-Jørgensen [this message]
2018-07-02 20:09                                                 ` Pete Heist
2018-07-02 20:11                                                   ` Toke Høiland-Jørgensen
2018-07-02 20:46                                                     ` Pete Heist
     [not found]                                           ` <mailman.407.1530550780.3512.cake@lists.bufferbloat.net>
2018-07-02 17:50                                             ` Kevin Darbyshire-Bryant
2018-07-02 19:33                                               ` Toke Høiland-Jørgensen
2018-07-02 19:36                                                 ` Kevin Darbyshire-Bryant
2018-07-02 19:39                                                   ` Toke Høiland-Jørgensen
2018-07-02 20:03                                                     ` [Cake] cake at 60gbit Dave Taht
2018-07-02 20:09                                                       ` Toke Høiland-Jørgensen
2018-07-02 21:16                                                         ` Pete Heist
2018-07-02 21:35                                                           ` Toke Høiland-Jørgensen
2018-07-02 22:07                                                             ` Georgios Amanakis
2018-07-02 22:12                                                               ` Dave Taht
2018-07-02 23:48                                                                 ` Georgios Amanakis
2018-07-02 22:23                                                               ` Toke Høiland-Jørgensen
2018-07-03  7:35                                                                 ` Pete Heist
2018-07-03  9:18                                                                 ` Jonathan Morton
2018-07-03  9:57                                                                   ` Pete Heist
2018-07-03 10:27                                                                   ` Toke Høiland-Jørgensen
2018-07-03 10:41                                                                     ` Pete Heist
2018-07-05 22:31                                                                     ` Toke Høiland-Jørgensen
2018-07-05 23:48                                                                       ` Georgios Amanakis
2018-07-06  1:21                                                                         ` Dave Taht
2018-07-06  2:55                                                                           ` George Amanakis
2018-07-06  3:06                                                                             ` George Amanakis
2018-07-06  9:22                                                                               ` Toke Høiland-Jørgensen
2018-07-06  9:21                                                                           ` Toke Høiland-Jørgensen
2018-07-06  8:55                                                                       ` Pete Heist
2018-07-06  9:29                                                                         ` Toke Høiland-Jørgensen
2018-07-06 10:00                                                                           ` Pete Heist
2018-07-06 10:46                                                                             ` Toke Høiland-Jørgensen
2018-07-06 11:33                                                                               ` Toke Høiland-Jørgensen
2018-07-06 11:43                                                                                 ` Jonathan Morton
2018-07-06 11:48                                                                                   ` Toke Høiland-Jørgensen
2018-07-06 11:58                                                                                 ` Pete Heist
2018-07-06 12:04                                                                                   ` Toke Høiland-Jørgensen
2018-07-02 18:39                                           ` [Cake] Cake on openwrt - falling behind Dave Taht
2018-07-02 19:11                                             ` Kevin Darbyshire-Bryant
2018-07-02 19:23                                               ` Toke Høiland-Jørgensen
2018-07-02 19:27                                                 ` Dave Taht
2018-07-02 19:38                                                   ` Toke Høiland-Jørgensen
2018-07-02 20:05                                                     ` Toke Høiland-Jørgensen
2018-07-02 19:31                                             ` Pete Heist
     [not found]                               ` <mailman.397.1530474091.3512.cake@lists.bufferbloat.net>
2018-07-01 23:55                                 ` Dave Taht
2018-07-02  0:05                                   ` Dave Taht
     [not found]                           ` <mailman.392.1530455913.3512.cake@lists.bufferbloat.net>
2018-07-01 15:17                             ` Jonathan Morton
     [not found]       ` <mailman.384.1530384918.3512.cake@lists.bufferbloat.net>
2018-07-01  9:46         ` Magnus Olsson
2018-07-01 12:34           ` Kevin Darbyshire-Bryant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.bufferbloat.net/postorius/lists/cake.lists.bufferbloat.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874lhhigdq.fsf@toke.dk \
    --to=toke@toke.dk \
    --cc=cake@lists.bufferbloat.net \
    --cc=kevin@darbyshire-bryant.me.uk \
    --cc=pete@heistp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox