From: Dave Taht <dave.taht@gmail.com>
To: cake@lists.bufferbloat.net
Subject: [Cake] cake review comments
Date: Tue, 18 Aug 2015 15:53:41 -0700 [thread overview]
Message-ID: <CAA93jw5gLea+Y1yoWmV5UZcXS=mDf+BxeOLMhWsMADgTTurgxQ@mail.gmail.com> (raw)
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=0; j < CAKE_MAX_CLASSES; j++) {
fqcd = &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 = (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..
--
Dave Täht
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
reply other threads:[~2015-08-18 22:53 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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='CAA93jw5gLea+Y1yoWmV5UZcXS=mDf+BxeOLMhWsMADgTTurgxQ@mail.gmail.com' \
--to=dave.taht@gmail.com \
--cc=cake@lists.bufferbloat.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