[Cake] cake review comments

Dave Taht dave.taht at gmail.com
Tue Aug 18 15:53:41 PDT 2015


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


More information about the Cake mailing list