[Cake] Re: [PATCH net] net/sched: Mark qdisc for deletion if graft cannot delete

[Jamal Hadi Salim <jhs@mojatatu.com>]

On Thu, Mar 12, 2026 at 7:51 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 12 Mar 2026 16:36:48 -0400 Jamal Hadi Salim wrote:
> > > > Two of the several (I think 4!) patches we had took a similar path. I
> > > > am trying to remember at least one variant was bad for performance and
> > > > the other was unstable. Let's see if we can revive it and take a
> > > > closer look. BTW - none were pretty, it was maybe half the lines of
> > > > code but touched many things.
> > >
> > > FWIW / of course, we have to apply similar change to all(?) callers of
> > > __tcf_qdisc_find in cls_api. So LOC-wise it may end up also pretty long.
> > > And it's not going to help the already spaghetti-looking locking. But
> > > even if it's more LoC I quite like the idea of containing the poopy
> > > code to where problems originate which is the lockless filter handling.
> > > Fingers crossed..
> >
> > Something like attached.
> > Unfortunately after running it for a few hours it reproduced.
> > The action code path (entered by virtue of filter code path execution)
> > releases the rtnl when attempting to load an action module. A parallel
> > qdisc operation waiting for the lock then grabs it and we hit the same
> > issue...
> >
> > So now we have to be more invasive and start coordinating the action
> > code etc, which is not appealing. Thoughts?
>
> I see. Doesn't seem entirely crazy to let tcf_proto_lookup_ops()
> return -EAGAIN without actually loading the module, and have it's
> call path (of which there are only 2?) do the module loading once
> all the locks are released. The call paths handle the EAGAIN and
> retry already they just assume tcf_proto_lookup_ops() has loaded
> the module so they don't have to.

I might not be entirely following what you are saying. The core issue
is not module loading perse. We can avoid module loading in
tcf_proto_lookup_ops while the rtnl lock is released and the qdisc
refcnt is greater than 1 (which is what the patch i sent attempted).
The new quark is when we have actions associated with the filter.

I will send you a much lengthier description of the issue in an hour
or two  to get us on the same page.

cheers,
jamal