On Wed, Oct 14, 2015 at 5:11 AM, Jamal Hadi Salim <j...@mojatatu.com> wrote: > On 10/12/15 14:38, Cong Wang wrote: >> >> When the bottom qdisc decides to, for example, drop some packet, >> it calls qdisc_tree_decrease_qlen() to update the queue length >> for all its ancestors, we need to update the backlog too to >> keep the stats on root qdisc accurate. >> > > > There is more than one change in there (the codel change seems > out of place and i wasnt sure why it was needed).
I thought it is clear that when codel decides to drop some packets we don't know how many bytes it drops, we only know how many packets before my patch. For example, - qdisc_tree_decrease_qlen(sch, q->cstats.drop_count); + qdisc_tree_reduce_backlog(sch, q->cstats.drop_count, + q->cstats.drop_len); This clearly means I need some codel stats from codel to pass to qdisc_tree_reduce_backlog(), this is why the codel part is necessary. > Also it seems possible you are double-dipping in some cases; > i dont have time to scrutinize - but looking at codel_change() change > when the queue limit is exceeded you will end up affecting backlog from > both qdisc_qstats_backlog_dec() and your new > qdisc_tree_reduce_backlog() Nope, qdisc_qstats_backlog_dec() decreases the backlog of itself, qdisc_tree_reduce_backlog() decreases its upper qdiscs'. It is correct as it was. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
