Re: [PATCH net-next v1 2/5] net: sched: Introduce helpers for qevent blocks

[Petr Machata <petrm@mellanox.com>]

Cong Wang <xiyou.wangcong@gmail.com> writes:

> On Fri, Jun 26, 2020 at 3:46 PM Petr Machata <petrm@mellanox.com> wrote:
>> The function tcf_qevent_handle() should be invoked when qdisc hits the
>> "interesting event" corresponding to a block. This function releases root
>> lock for the duration of executing the attached filters, to allow packets
>> generated through user actions (notably mirred) to be reinserted to the
>> same qdisc tree.
>
> I read this again, another question here is: why is tcf_qevent_handle()
> special here? We call tcf_classify() under root lock in many other places
> too, for example htb_enqueue(), which of course includes act_mirred
> execution, so why isn't it a problem there?
>
> People added MIRRED_RECURSION_LIMIT for this kinda recursion,
> but never released that root lock.

Yes, I realized later that the qdiscs that use tcf_classify() for
classification have this exact problem as well. My intention was to fix
it by dropping the lock. Since the classification is the first step of
enqueing it should not really lead to races, so hopefully this will be
OK. I don't have any code as of yet.

The recursion limit makes sense for clsact, which is handled out of the
root lock.