[Cake] NET_XMIT_CN question

[Dave Taht]

We decreased the size of the queue, regardless of what flow we dropped
from... so why do we not need to decrease the queue size when using
NET_XMIT_CN? Where is that done by the caller? I have grepped and
googled...

As best as I recall the semantics of NET_XMIT_CN changed over time, it used to
only work on enqueue on a tail drop queue, where it would push back on
the stack to not, actually, drop the packet, but to halve the window
and requeue.

Staring at the code now, I don't see how it could be right... here OR
in the existing fq_codel code in the mainline.

        /* Return Congestion Notification only if we dropped a packet
         * from this flow.
         */
        if (fq_codel_drop(sch) == idx)
                return NET_XMIT_CN;

        /* As we dropped a packet, better let upper stack know this */
        qdisc_tree_decrease_qlen(sch, 1);
        return NET_XMIT_SUCCESS;


(I am not sure NET_XMIT_CN works right when there is a hash collision,
for example, although the updated doc does seem to indicate that,

... still, then, where does (an equivalent for)
qdisc_tree_decrease_qlen kick in?)

So I ripped it out. Otherwise... the code does look correct, now that
I've reviewed it further.
But does putting it back in have any measurable effect?

...

Also I note there was a bunch of commits to sch_fq_codel mainline since july,
notably this one.

c0afd9ce4d6a646fb6433536f95a418bb348fab1


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


On Fri, Nov 27, 2015 at 3:33 PM, Kevin Darbyshire-Bryant
<kevin at darbyshire-bryant.me.uk> wrote:
> I've been pondering this commit and wonder if we're throwing out baby
> with bathwater:
>
> https://github.com/dtaht/sch_cake/commit/90e792115f10fa2a31327208302e66c0ab3f2251
>
> The original code, to paraphrase in something resembling English went:
>
> 1. I've just enqueud a packet & Oh sh*t we're over our q_disc buffer
> limit, we'd better do something about it, hmmm
> 2. Whilst we're over this limit, go look through our flows (cake_drop)
> and kill something, if that something was in the same flow as the thing
> we just enqueued then set a flag 'cos these guys are part of the
> problem!  Keep track of how many things we killed.
> 3 Now we're below the limit and finished killing packets, return back to
> our caller - if we killed stuff in the same flow the caller is sending,
> then return 'NET_XMIT_CN' (we're congested), else just say 'happy as Larry'
>
>  *//* NET_XMIT_CN is special. It does not guarantee that this packet is
> lost. It/*
> */* indicates that the device will soon be dropping packets, or already
> drops/*
> */* some packets of the same priority; prompting us to send less
> aggressively. *//*
>
> What I do agree looked odd in the original code was the 'dropped -
> same_flow':  We haven't dropped any more or fewer packets just 'cos we
> found one or more in 'the same flow'.
>
> -        qdisc_tree_decrease_qlen(sch, dropped - same_flow);  <--????????
> +        qdisc_tree_decrease_qlen(sch, dropped);
>
> Incidentally, a search of 'NET_XMIT_CN' turned up
> http://www.bufferbloat.net/issues/398.pdf
>
>
> _______________________________________________
> Cake mailing list
> Cake at lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake
>