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

[Kevin Darbyshire-Bryant]

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;




I'm not sure that cake behaves the same way:


                 u32 old_backlog = q->buffer_used;

                 while (q->buffer_used > q->buffer_limit) {
                         dropped++;
                         if (cake_drop(sch) == idx + (tin << 16))
                                 same_flow = 1;
                 }
                 b->drop_overlimit += dropped;
                 qdisc_tree_reduce_backlog(sch, dropped - same_flow, old_backlog - q->buffer_used);
                 return same_flow ? NET_XMIT_CN : NET_XMIT_SUCCESS;

It looks to me that we always report what was dropped in terms of length irrespective of whether it was from the same flow.  The number of packets *does* change, correctly.


>
>   - Jonathan Morton
>