Re: [Cake] GSO peel behaviour tweaks

[Dave Taht <dave.taht@gmail.com>]

I don't know what this used to look like but it is essentially wrong
in both (all?) versions.

-               q->peel_threshold = (q->rate_flags & CAKE_FLAG_ATM) ?
-                       0 : min(65535U, q->rate_bps >> 12);
+               q->peel_threshold = (q->rate_flags & CAKE_FLAG_ATM) ||
+                       q->rate_overhead ? 0 : min(65535U, q->rate_bps >> 12);

What we want to do is closer to:

A) start peeling once we start accruing or incurring delay in excess
of, say, 250usec.  At 1Mbit, this is basically peel always. At a gbit,
it's peel with roughly two 10 full-size packet offloads in play. There
are nuances vs a vs ack GRO stuff (served with a 300 quantum in
fq_codel), and in the 10-100Mbit range...

A1) So doing nothing at a rate unlimited is wrong
A2) Taking the current len * flows as a way to calculate it is wrong
A3) I don't know if this was ever "right". It doesn't need to be
perfect, but this is far from right...

While I am unfond of the rate estimator's overhead, it perhaps could
be used to calculate the peel threshold in a saner way...

B) always peel when we are trying to do accurate on-wire accounting.

As for the other patch...

In general random pointer lookups into memory (like the skb->gro
pointer) cost more than math as the other two params here are possibly
part of a local cache hit already... and I have no idea what the ratio
is between gso packets and how often you'd hit the comparison... but
see point A2 above...

-       if (unlikely((len * max_t(u32, b->bulk_flow_count, 1U) >
-                     q->peel_threshold && skb_is_gso(skb)))) {

+       if (unlikely(skb_is_gso(skb) &&
+               (len * max_t(u32, b->bulk_flow_count, 1U) >
+                     q->peel_threshold))) {





Dave Täht
Let's go make home routers and wifi faster! With better software!
https://www.gofundme.com/savewifi


On Tue, Nov 24, 2015 at 10:12 AM, Kevin Darbyshire-Bryant
<kevin@darbyshire-bryant.me.uk> wrote:
> I've just pushed 2 commits related to GSO peeling behaviour to master.
>
> 1st tweak is at worst benign and at best removes a multiply compare for
> every packet enqueued.  I'd like to think the optimiser in the compiler
> would have done what I've done explicitly (in essence check this is a
> gso packet 1st before thinking about peeling it) but when I checked on
> x86_64 there was a definite difference in produced code.
>
> 2nd tweak is *not* benign.  In essence this forces peeling if either ATM
> framing or packet overhead is specified.  Previously only ATM framing
> forced peeling.  I think this is more correct but unfortunately will be
> slower.
>
> Commits can be reverted - feel free :-)
>
>
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake
>