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

[Jarek Poplawski <jarkao2@o2.pl>]

On Thu, Nov 23, 2006 at 10:32:27AM +0100, Martin Devera wrote:
> >>>>>+{
> >>>>>+	struct htb_class *cl = (struct htb_class *)arg;
> >>>>>+
> >>>>>+	if (cl->un.leaf.q->q.qlen == 0)
> >>>>
> >>>>Probably "if (cl->prio_activity)" is needed here.
> >>>
> >>>No, the function is only called for active leaf classes, which
> >>>always have prio_activity != 0.
> >>
> >>So this was unnecessary in htb_graft? You should know better...
> >
> >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.
> 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...
> devik

I'm learning htb yet and was not sure whether leaf class
with non-zero qlen qdisc couldn't be deactivated for some
other reason. In such case the patch would change
"functionality", so my doubts.

Thanks very much for explanations to both authors of great
schedulers.

I'm very honored,

Jarek P.