From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-x230.google.com (mail-ob0-x230.google.com [IPv6:2607:f8b0:4003:c01::230]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id 36B6E21FA3D for ; Tue, 18 Aug 2015 15:53:42 -0700 (PDT) Received: by obkg7 with SMTP id g7so23035534obk.3 for ; Tue, 18 Aug 2015 15:53:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type :content-transfer-encoding; bh=AOZRY0Js4A2fErnhHLOqEFxCqPsySRGan8WQnEXgcZI=; b=nB7MkBuHjehQKouq/IecrPga1WfGGObrB9ra7Kw0q2oD81BXnjaFj/t3zEld0yAhOJ tMYc7PTK26qtL8y6kFwxUp2wCnKnhthcIwCyUEPJOJS/TJdPY6wIWRyHZxWtbMsqYYGL AYf6BFHSgN4Xhrnd8banJlaoq1XI3UAOlotsVovQQFgy81aLpwQtwORE2MveLsmA9B85 EkZuJ2l0k9/EuX65tKnpGFZLn3KvwsTR6t8YEMWttkciSs84E9FnsdA6PWVE7RM4q0La jT3HlKD/dpITgNKOCx7Wc/7TMNyXWPKNimcwDjAVHpPgdCr70zApRkGml+VIQHmcmZSf DcvQ== MIME-Version: 1.0 X-Received: by 10.60.96.35 with SMTP id dp3mr8433109oeb.47.1439938421988; Tue, 18 Aug 2015 15:53:41 -0700 (PDT) Received: by 10.202.108.12 with HTTP; Tue, 18 Aug 2015 15:53:41 -0700 (PDT) Date: Tue, 18 Aug 2015 15:53:41 -0700 Message-ID: From: Dave Taht To: cake@lists.bufferbloat.net Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Cake] cake review comments 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: Tue, 18 Aug 2015 22:54:05 -0000 I would like to see cake migrate into KernelStyle soon, and start passing the kernel checkpatch routine, etc. There is both under and excessive tabbing... These days I am still very much into keeping it out of tree until it is more baked... and we still have to deal with a lot of backporting issues like somehow adopting the new hash stuff in 4.2. there are other improvements that might improve readability, like, for example, "fqcd" is not really a helpful name as part of a struct type in places, and something more like "c" is just as informative as a variable name and shorter. I would like cake_drop to be evaluated with some less exaustive search. It also looks wrong for(j=3D0; j < CAKE_MAX_CLASSES; j++) { fqcd =3D &q->classes[j]; 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 this needs to get fixed properly (and I still want SQUASHING in cake - everything else is in here...) // FIXME: cow is not needed // if (!pskb_network_may_pull(skb, sizeof(struct iphdr))) if (unlikely(skb_cow_head(skb, sizeof(struct iphdr)))) return 0; There are places that should wrap at 78 columns throughout like if(unlikely((len * max(fqcd->flow_count, 1) > q->peel_threshold && skb_is_gso(skb)))) and I would like to test the smaller peel threshold at a variety of rates and flows q->peel_threshold =3D (q->rate_flags & CAKE_FLAG_ATM) ? 0 : min(65535U, q->rate_bps >> 12); it would be nice if a cake change bandwidth was more fastpathed than it is. At 10gig rates, it seems likely the default 1MB buffer for unlimited is possibly too small. It is not clear to me how or if sch->limit should be enforced. Yet another diffserv code point is being proposed for LE in the ietf, can't remember the pattern... After we get a few more rounds for correctness, some profiling would be good and a hard look at xmit_more related stuff and the new less lock full stuff.. --=20 Dave T=C3=A4ht worldwide bufferbloat report: http://www.dslreports.com/speedtest/results/bufferbloat And: What will it take to vastly improve wifi for everyone? https://plus.google.com/u/0/explore/makewififast