Subject: Re: [NET_SCHED 06/06]: Fix endless loops (part 4): HTB

From: Patrick McHardy <[email-masked]>
Date: Thu, 23 Nov 2006 10:48:40 +0100

Martin Devera wrote:
>> In htb_graft it was necessary because the class could be
>> either active (qlen > 0) or inactive (qlen == 0). But since
>> qdisc_tree_decrease_qlen does nothing in case of a decrease
>> of zero the callback is only called for active classes and
>> the check becomes unnecessary.
>>
> 
> Patrick is right here. prio_activity for leaf is set in htb's enqueue
> routines. The qlen callback is called when len changed and because
> that zero test in htb's callback it is sure the subclass was nonempty
> before.

BTW, I plan to remove the check for empty child qdiscs in
htb_dequeue_tree and the check for prio_activity after the
calls to qdisc_tree_decrease_len in htb_change_class as a
follow-up patch in 2.6.20, for now I kept them to be safe.

> The only problem might be if some subqdisc "creates" new packets,
> then htb might deactivate non-active class (leading to BUG()).
> IIRC no qdisc does it...

netem can duplicate packets, but it enqueues them at the root,
so it shouldn't be a problem. It might be preferable to do
something similar to qdisc_tree_decrease_qlen in that case to
make sure the packet really ends up in the netem qdisc, but for
now I just wanted to fix the endless loops.