From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x242.google.com (mail-wm0-x242.google.com [IPv6:2a00:1450:400c:c09::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 2F1793BA8E for ; Mon, 2 Jul 2018 13:04:54 -0400 (EDT) Received: by mail-wm0-x242.google.com with SMTP id v25-v6so8826164wmc.0 for ; Mon, 02 Jul 2018 10:04:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=heistp.net; s=google; h=from:message-id:mime-version:subject:date:in-reply-to:cc:to :references; bh=i5TwoCneEEVLZScchzB+n6Zzwbbntfmq8Cy88mdsouo=; b=RzYEHB/uMaaajAF2ky98QbAV41e7O5T2jDQRUNxtknp/M6s2/Jwl16uctWNuMQjdfg 67YGcUrztI7eC7GFeKf2JSkXicMfgHQOkcuqfxuqjr5jy/AuWDXXCjLbaK2bMKrGiHQH Nyhhcn0usDjdMnF5LPo5RAQqxbNPWbQVcpR8tTy6HW1y86aD0lPvTD1RXtrSYYLAzuSF XM2g502ytKOUZZkcYQDL185MQTYPKicvHllh2zLvawAbDB33uYgH78S3dQdxhv7PZ2K1 zYrZhGpVoXc508UcZ4ZupQRB3aq+M4wn9JOzZHKL82dod/hvET9Egma3LhfSV14XACaL 4+AA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=i5TwoCneEEVLZScchzB+n6Zzwbbntfmq8Cy88mdsouo=; b=PDIFDaujscoAblSLCN+pPco7jTsvi+ksfg1VmLueWEc/yAFi9YaPG8EQkCYrLna6YK SAOFc/yb3hrgZuYk48hYG0Lx+0irWdDS0CnLaFKO7PBKC4eoaDCTMCVcuk38iMmi6e7I r6MgqdE9PHMZM23pX42oEDYpt9PB34oLCNUTtCl2bpL2/XoYjfEb3tGbOBl6v95eBhUR 506yQW/QpV6DcuhXA7PtbHiE6kCH71SbJtPhIJKF5FtxfQo7exW7SpYxSqd2hqzMQwx6 HuPdx2J5SpQkj5WPdxFIRiS0rF0fiTi+J7YPV8XElM6XFJdj4+VPIchP6YlAQxWiI9jw dy0Q== X-Gm-Message-State: APt69E3Otu5Or3wQxFzQ5+/NG6IuFUXK9Jp+oF6wmZbtkJpZX/S5jnTh qzMIk6KxcsWzry8MTVB1XYnEfw== X-Google-Smtp-Source: AAOMgpe0H92rj6nQ5MdIiAapK1JVpOfc0gBUQ1oh6uWEpJjvGY0fvLnQLiy9p9a4vIIXV39QSFwddA== X-Received: by 2002:a1c:27c3:: with SMTP id n186-v6mr8488360wmn.10.1530551093107; Mon, 02 Jul 2018 10:04:53 -0700 (PDT) Received: from [10.72.0.151] (h-1169.lbcfree.net. [185.193.85.130]) by smtp.gmail.com with ESMTPSA id r140-v6sm5770369wmd.27.2018.07.02.10.04.51 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Jul 2018 10:04:52 -0700 (PDT) From: Pete Heist Message-Id: <8815D90E-DEAB-4211-B4B4-7058178DEA47@heistp.net> Content-Type: multipart/alternative; boundary="Apple-Mail=_0A47EDAD-E060-4C2D-9EAF-D71ECFE714B3" Mime-Version: 1.0 (Mac OS X Mail 11.4 \(3445.8.2\)) Date: Mon, 2 Jul 2018 19:04:51 +0200 In-Reply-To: <87fu11ipir.fsf@toke.dk> Cc: Kevin Darbyshire-Bryant , Cake List To: =?utf-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= References: <6DF9A5E0-EFD5-4519-9889-BC0A7B9BD48E@darbyshire-bryant.me.uk> <1A8BA286-6B31-4581-86C9-6855AC28C245@heistp.net> <673EAD3F-AB09-4B90-88BB-5DCE0BD65534@heistp.net> <6FE8D434-01BE-41A1-BD6B-EFFD67AC8784@heistp.net> <94C9790F-E9BC-4D59-9845-17C305E4B910@darbyshire-bryant.me.uk> <17AF79A0-0213-44E3-95B9-62795A644A47@heistp.net> <87lgatj13k.fsf@toke.dk> <87fu11ipir.fsf@toke.dk> X-Mailer: Apple Mail (2.3445.8.2) Subject: Re: [Cake] Cake on openwrt - falling behind X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2018 17:04:55 -0000 --Apple-Mail=_0A47EDAD-E060-4C2D-9EAF-D71ECFE714B3 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Jul 2, 2018, at 6:14 PM, Toke H=C3=B8iland-J=C3=B8rgensen = wrote: >=20 > Aha! I think I figured out what is going on: >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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=E2=80=99s email that it works for = him, but it didn=E2=80=99t work for me the first time around. This may = have to do with how I added the patch as I=E2=80=99m still not that = familiar with OpenWRT=E2=80=99s build system (first kernel patch I = tried). I wasn=E2=80=99t sure if it should go into generic or platform, = for one, so I tried generic=E2=80=A6is that right? = sysadmin@rey:~/src/openwrt/build_dir/target-mips_24kc_musl/linux-ar71xx_ge= neric/linux-4.9.109/patches/generic$ cat 010-gen_stats_fix.patch=20 --- 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 =3D lock; spin_lock_bh(lock); } - if (d->tail) - return gnet_stats_copy(d, type, NULL, 0, padattr); + if (d->tail) { + int ret =3D gnet_stats_copy(d, type, NULL, 0, padattr); + if (!ret) + d->tail =3D ((struct nlattr = *)skb_tail_pointer(skb)) - 1; + return ret; + } =20 return 0; } --Apple-Mail=_0A47EDAD-E060-4C2D-9EAF-D71ECFE714B3 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On Jul 2, 2018, at 6:14 PM, Toke H=C3=B8iland-J=C3=B8rgensen = <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=E2=80=99s = email that it works for him, but it didn=E2=80=99t work for me the first = time around. This may have to do with how I added the patch as I=E2=80=99m= still not that familiar with OpenWRT=E2=80=99s build system (first = kernel patch I tried). I wasn=E2=80=99t sure if it should go into = generic or platform, for one, so I tried generic=E2=80=A6is that = right?

sysadmin@rey:~/src/openwrt/build_dir/target-mips_24kc_musl/linu= x-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 =3D lock;
 = spin_lock_bh(lock);
  }
- = if (d->tail)
- return gnet_stats_copy(d, = type, NULL, 0, padattr);
+ if (d->tail) {
+ = int ret =3D gnet_stats_copy(d, type, NULL, 0, padattr);
+ = if (!ret)
+ d->tail =3D = ((struct nlattr *)skb_tail_pointer(skb)) - 1;
+ = return ret;
+ }
 
 = return 0;
 }

= --Apple-Mail=_0A47EDAD-E060-4C2D-9EAF-D71ECFE714B3--