Re: [RGC PATCH v3.10] net: sched: validate that class is found in qdisc_tree_decrease_qlen

On Fri, Jul 17, 2015 at 5:27 PM, Alex Gartrell [off-list ref] wrote:

On Fri, Jul 17, 2015 at 5:10 PM, Cong Wang [off-list ref] wrote:

quoted

Hmm, but in htb_delete() we do reset the leaf qdisc before removing the
class from ha

        if (!cl->level) {
                qlen = cl->un.leaf.q->q.qlen;
                qdisc_reset(cl->un.leaf.q);
                qdisc_tree_decrease_qlen(cl->un.leaf.q, qlen);
        }

therefore, the leaf fq_codel qdisc is supposed to have 0 skb after that,
that is, the second fq_codel_reset() is supposed to return NULL immediately?
Why is qdidsc_tree_decrease_qlen() called in the second fq_codel_reset()?

Or there is a race condition between ->delete() and ->put()? In which new
skb can be enqueued?

This makes the most sense to me, but I have ~32 hours of experience
with this subsystem :)

Taking that for granted, it seems like it'd be appropriate to note the
invariant in the code i've changed with a WARN_ON and to skip it, and
then to otherwise find a way to close the hole.  Do you agree?

Your patch _does_ make sense for me, I was just trying to check
if there is any better way to fix it. :)

I was thinking about locking the qdisc tree before ->get(), but it is not
easy and may not worth the effort either. So, unless others have a
better idea here, I think your patch is fine, just please update the
changelog and rebase it against latest -net tree.

Thanks!