From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net (mout.gmx.net [212.227.15.19]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mout.gmx.net", Issuer "TeleSec ServerPass DE-1" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id 0D4A621F95D for ; Sun, 4 Oct 2015 03:50:40 -0700 (PDT) Received: from hms-beagle.home.lan ([217.237.68.126]) by mail.gmx.com (mrgmx002) with ESMTPSA (Nemesis) id 0LuxG5-1aiFm01koL-0108BC; Sun, 04 Oct 2015 12:50:37 +0200 Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) From: Sebastian Moeller In-Reply-To: <56102F88.8070201@darbyshire-bryant.me.uk> Date: Sun, 4 Oct 2015 12:50:34 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <01DFDB5B-04E1-4DF9-B540-FE090111E3A6@gmx.de> References: <56102F88.8070201@darbyshire-bryant.me.uk> To: Kevin Darbyshire-Bryant X-Mailer: Apple Mail (2.1878.6) X-Provags-ID: V03:K0:GKSUTKfersLRw9ZPAan6tEEzP1ycRWPVl+IJfVWJEFTMY1RKD2k DGGha2wanVbsgydQ/tvVIFyOP3wKMdQiH8OwTbYzry/M93siDUAqUv7U3HO62BUvRVZI9Ww LpG2JSlHVoWWIfjKs3IfjdcbmzkEA+/Q2eBeNFXnmyDOlb7436zflfRPbwBRE+nUF4nATJN QKFItlFyvXuK816+AGBFw== X-UI-Out-Filterresults: notjunk:1;V01:K0:giC28ochxiw=:EGESoZ8eQwPEadyzPNgCEy OQ+FKshicQ7gvUC3RLRl+dg6wLEmce5VQq1o/52OBB2iQ1wFA+OR3aXGjmG7tmD9uLiPwGSLq 6iPA6uroNB2wJ7U5YyxLx8WyBa6iAN9V571Rt7xWh0lt7Imto2SDjTJIvbEkW2B4ti9W85si4 /KfbZQPXHHEyvJCzNfhPB9TFJm3ML8wmoMuQmUUOCtYONCFpp5jrMgLl1gwDnehty5e2SktCy Xyyerfo1IdJnjrmKb12jHOukalwD0fbC6CH/2SzgOJsXpkUtbHDLlLWixkaQyKg9vV7U+E3Pf sm6so+0X3fFSB2uNKmYbslExJFVdM147lhKD8cc00aQF5i1tJmWkrR6pA8h9FrQGdjDfGg5C8 qqhV7dJ9DVRT9X7jTvmZazdU5E0XO3hLtaQ5gj8nbGp080YToQT25SAYFVzlyEWZC48wrBiEL rLS1lub0CYdLsnzxNbBDtSN8yLzcxXKWViuScOH7oZXJIV4z0GYOSxP/sqIygAyJ4uP3oqbDE pj1YuDIvcD9FZW+OiOqMzZitKNOM2uKpZCV38GJ/mDnovrdswfNWF6aIrlIsS0weR0l1OrunZ W5MfFSO4/xaVQ+XDu64+ED1NyvPNfej3Ki6Bd0qRe7vmtfHXiwqGzogO2kl5sva0uMUhXCKGD r8TwAFqT5nIoBtQ0MznNIUPG6oj5uOYUbzfv+I+ZPjHw8X8Jd+FbcVTnWwqL/iqQOyRWIVcDJ 246Ipu+4Slcrf+ATUyNmK5PzIqqZrhCClLluKVTmWbuN35Wu/m/szcN6HbjVPTYoH3+LU6LkH J0BLsuD Cc: cake@lists.bufferbloat.net Subject: Re: [Cake] cake review comments cake_drop X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Oct 2015 10:51:03 -0000 Hi Kevin, On Oct 3, 2015, at 21:42 , Kevin Darbyshire-Bryant = wrote: > Hello All, >=20 > On 18th Aug (before I'd subscribed to the list), Dave posted a message > about reviewing cake > https://lists.bufferbloat.net/pipermail/cake/2015-August/000364.html >=20 > In it he mentioned a change to cake_drop: >=20 > " >=20 > I would like cake_drop to be evaluated with some less exaustive > search. It also looks wrong >=20 > for(j=3D0; j < CAKE_MAX_CLASSES; j++) { > fqcd =3D &q->classes[j]; >=20 > CAKE_MAX_CLASSES should actually be q->class_cnt here; I think. I am > unsure if other references to it are correct in face of changes, on > cleanup" >=20 >=20 > After making the change and nearly submitting a pull request I've > thought about this some more and to be blunt it makes me nervous. My > concern is what happens if cake is changed from say a diffserv8 config > to a single class(bin) config and for whatever reason 'cake_drop' is > called. with diffserv8 the fat flow is likely to be in a class(bin) > higher than the first....cake_drop would now only check the first = class > for the fat flow and possibly not find anything to drop.... I fear = what > happens next! I thought the following takes care of that (called from = cake_reconfigure() ): /* Discard leftover packets from a class no longer in use. */ static void cake_clear_class(struct Qdisc *sch, u16 class) { =20 struct cake_sched_data *q =3D qdisc_priv(sch); struct cake_fqcd_sched_data *fqcd =3D &q->classes[class]; =20 q->cur_class =3D class; for(q->cur_flow =3D 0; q->cur_flow < fqcd->flows_cnt; = q->cur_flow++) while(custom_dequeue(NULL, sch)) ; } I probably do no understand things fully enough, but to me this seems to = take care of your concern. Especially since cake_reconfigure will only = touch classes from q->class_cnt up to CAKE_MAX_CLASSES. Since I am not = very fluent in C I might have missed things=85 That means that = exchanging CAKE_MAX_CLASSES with q->class_cnt in cake_drop() should be = safe. The question left is then how costly is it to search empty = classes? I guess, cake_drop is the slow path and hence could afford to = search them all, then again cake_drop probably tests called most under = heavy traffic conditions, so even the slow path performance might be = noticeable to the user=85 Best Regards Sebastian >=20 > Kevin >=20 > _______________________________________________ > Cake mailing list > Cake@lists.bufferbloat.net > https://lists.bufferbloat.net/listinfo/cake