Re: Incomplete fix for recent bug in tc / hfsc

[Cong Wang <xiyou.wangcong@gmail.com>]

On Sun, Jun 29, 2025 at 10:29:44AM -0400, Jamal Hadi Salim wrote:
> > On "What do you think the root cause is here?"
> >
> > I believe the root cause is that qdiscs like hfsc and qfq are dropping
> > all packets in enqueue (mostly in relation to peek()) and that result
> > is not being reflected in the return code returned to its parent
> > qdisc.
> > So, in the example you described in this thread, drr is oblivious to
> > the fact that the child qdisc dropped its packet because the call to
> > its child enqueue returned NET_XMIT_SUCCESS. This causes drr to
> > activate a class that shouldn't have been activated at all.
> >
> > You can argue that drr (and other similar qdiscs) may detect this by
> > checking the call to qlen_notify (as the drr patch was
> > doing), but that seems really counter-intuitive. Imagine writing a new
> > qdisc and having to check for that every time you call a child's
> > enqueue. Sure  your patch solves this, but it also seems like it's not
> > fixing the underlying issue (which is drr activating the class in the
> > first place). Your patch is simply removing all the classes from their
> > active lists when you delete them. And your patch may seem ok for now,
> > but I am worried it might break something else in the future that we
> > are not seeing.
> >
> > And do note: All of the examples of the hierarchy I have seen so far,
> > that put us in this situation, are nonsensical
> >
> 
> At this point my thinking is to apply your patch and then we discuss a
> longer term solution. Cong?

I agree. If Lion's patch works, it is certainly much better as a bug fix
for both -net and -stable.

Also for all of those ->qlen_notify() craziness, I think we need to
rethink about the architecture, _maybe_ there are better architectural
solutions.

Thanks!