[Cake] Possible BUG - parent backlog incorrectly updated in case of NET_XMIT_CN

[Kevin Darbyshire-Bryant]

On 07/06/16 16:05, Kevin Darbyshire-Bryant wrote:
>
>
> On 07/06/16 15:50, Jonathan Morton wrote:
>>> On 7 Jun, 2016, at 14:20, Kevin Darbyshire-Bryant 
>>> <kevin at darbyshire-bryant.me.uk> wrote:
>>>
>>> I had a nose at CAKE but couldn't quit work out if a similar issue 
>>> is present but I suspect it is. Certainly if Eric can't get it right 
>>> "My prior attempt to fix the backlogs of parents failed." then it's 
>>> not an 'obvious to solve' problem :-)
>> It appears my code already handles it correctly.  This is most likely 
>> because it inherited the analogous handling from the old function 
>> called here.
>
> I was intrigued by how in the case of XMIT_CN he reports the backlog 
> without the packet just dropped 'cos the parent qdisc won't include 
> that length in its calculations because we reported we dropped the 
> packet.  Preventing double accounting.
>
> +    /* As we dropped packet(s), better let upper stack know this.
> +     * If we dropped a packet for this flow, return NET_XMIT_CN,
> +     * but in this case, our parents wont increase their backlogs.
> +     */
> +    prev_qlen -= sch->q.qlen;
> +    prev_backlog -= sch->qstats.backlog;
> +    if (ret == idx) {
> +        qdisc_tree_reduce_backlog(sch, prev_qlen - 1,
> +                      prev_backlog - qdisc_pkt_len(skb)); <<<<here
> +        return NET_XMIT_CN;
> +    }
> +    qdisc_tree_reduce_backlog(sch, prev_qlen, prev_backlog);
> +    return NET_XMIT_SUCCESS;

And now there's a version 2 of Eric's fix: 
http://marc.info/?l=linux-netdev&m=146507012302643&w=2

Does this change apply to cake?

Cheers,

Kevin