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

On Fri, 2015-07-17 at 17:27 -0700, Alex Gartrell 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?

HTB + fq_codel ?

Make sure you applied Cong fix

commit c0afd9ce4d6a646fb6433536f95a418bb348fab1
Author: WANG Cong [off-list ref]
Date:   Tue Jul 14 11:21:58 2015 -0700

    fq_codel: fix return value of fq_codel_drop()
    
    The ->drop() is supposed to return the number of bytes it dropped,
    however fq_codel_drop() returns the index of the flow where it drops
    a packet from.
    
    Fix this by introducing a helper to wrap fq_codel_drop().