Cake - FQ_codel the next generation
 help / color / mirror / Atom feed
* [Cake] cake review comments
@ 2015-08-18 22:53 Dave Taht
  0 siblings, 0 replies; only message in thread
From: Dave Taht @ 2015-08-18 22:53 UTC (permalink / raw)
  To: cake

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2015-08-18 22:53 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-18 22:53 [Cake] cake review comments Dave Taht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox