From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pb0-x22a.google.com (mail-pb0-x22a.google.com [IPv6:2607:f8b0:400e:c01::22a]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id A347521F151 for ; Sun, 2 Jun 2013 14:15:58 -0700 (PDT) Received: by mail-pb0-f42.google.com with SMTP id uo1so4750331pbc.29 for ; Sun, 02 Jun 2013 14:15:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-type:x-mailer:content-transfer-encoding:mime-version; bh=i6fJ6kpx2r13kekIxVGkNldes2G7pLMK6fMPyGTc/iE=; b=nBasnnlh3tFgT/LNxt4lPZZcYJwzhx4k+jsccsO1tDRV63Fvvoy1n8dk3Ut0Mjfqk6 f8wQf/V0OfcrJaaOARX4NBNEJnU9Hzsy21PW/SQeMCDbEP8lRsXew2vZaOlCnJn+wn8n P2mbDUzc3Q+HxuKaLag3te6LlN0yHasfdcE6U9QTJ6ufTWEpgS9uxmt/i7FLYBe2a3lD LW52tWoMxU4dXfT3ow/9z7JadwYEr0OtI66eXwrKKwseKA1arjCpy2VuVQlRdSGEGqXk qBub3dF6MsPZGF8+nLw3RJ+yvzGVPq54Xs1044LgVnDWJOjr46aC9jtThPhUKYxns7dG tCVw== X-Received: by 10.68.219.42 with SMTP id pl10mr21598530pbc.24.1370207758058; Sun, 02 Jun 2013 14:15:58 -0700 (PDT) Received: from [172.29.161.17] ([172.29.161.17]) by mx.google.com with ESMTPSA id dc3sm17740569pbc.9.2013.06.02.14.15.56 for (version=SSLv3 cipher=RC4-SHA bits=128/128); Sun, 02 Jun 2013 14:15:57 -0700 (PDT) Message-ID: <1370207755.24311.81.camel@edumazet-glaptop> From: Eric Dumazet To: Jesper Dangaard Brouer Date: Sun, 02 Jun 2013 14:15:55 -0700 In-Reply-To: <20130529151330.22c5c89e@redhat.com> References: <20130529151330.22c5c89e@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Cc: Toke =?ISO-8859-1?Q?H=F8iland-J=F8rgensen?= , Mike Frysinger , Jiri Benc , Jiri Pirko , netdev@vger.kernel.org, Patrick McHardy , Steven Barth , bloat@lists.bufferbloat.net, David Miller , Jussi Kivilinna , Felix Fietkau , Michal Soltys Subject: Re: [Bloat] tc linklayer ADSL calc broken after commit 56b765b79 (htb: improved accuracy at high rates) X-BeenThere: bloat@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: General list for discussing Bufferbloat List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Jun 2013 21:15:58 -0000 On Wed, 2013-05-29 at 15:13 +0200, Jesper Dangaard Brouer wrote: > I recently discovered that the (traffic control) tc linklayer > calculations for ATM/ADSL have been broken by: > commit 56b765b79 (htb: improved accuracy at high rates). > > Thus, people shaping on ADSL links, using e.g.: > tc class add ... htb rate X ceil Y linklayer atm overhead 10 > It seems the "overhead 10" was never reported back by "tc -s class show dev ... " Also, the "linklayer atm" changes the data[] part, and this one is not matched in qdisc_get_rtab() So two different rate specifications, but sharing same struct tc_ratespec could be shared... Oh well. It seems following fix would be needed anyway ? [PATCH] net_sched: qdisc_get_rtab() must check data[] array qdisc_get_rtab() should check not only the keys in struct tc_ratespec, but also the full data[] array. "tc ... linklayer atm " only perturbs values in the 256 slots array. Signed-off-by: Eric Dumazet --- net/sched/sch_api.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index 2b935e7..281c1bd 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -291,17 +291,18 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r, struct nlattr *ta { struct qdisc_rate_table *rtab; + if (tab == NULL || r->rate == 0 || r->cell_log == 0 || + nla_len(tab) != TC_RTAB_SIZE) + return NULL; + for (rtab = qdisc_rtab_list; rtab; rtab = rtab->next) { - if (memcmp(&rtab->rate, r, sizeof(struct tc_ratespec)) == 0) { + if (!memcmp(&rtab->rate, r, sizeof(struct tc_ratespec)) && + !memcmp(&rtab->data, nla_data(tab), 1024)) { rtab->refcnt++; return rtab; } } - if (tab == NULL || r->rate == 0 || r->cell_log == 0 || - nla_len(tab) != TC_RTAB_SIZE) - return NULL; - rtab = kmalloc(sizeof(*rtab), GFP_KERNEL); if (rtab) { rtab->rate = *r;