Re: Fwd: u32 ht filters

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

On Wed, Feb 7, 2018 at 11:38 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Feb 08, 2018 at 12:08:36AM CET, xiyou.wangcong@gmail.com wrote:
>>On Tue, Feb 6, 2018 at 11:01 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Wed, Feb 07, 2018 at 06:09:15AM CET, xiyou.wangcong@gmail.com wrote:
>>>>Hi, Jiri
>>>>
>>>>Your  commit 7fa9d974f3c2a016b9accb18f4ee2ed2a738585c
>>>>breaks the tc script by Paweł. Please find below for details.
>>>
>>> Did you do the bisection?
>>> The commit just uses block struct instead of q, but since they
>>> are in 1:1 relation, that should be equvivalent. So basically you still
>>> have per-qdisc hashtables for u32.
>>
>>Well, at least the following fixes the problem here. But I am not sure
>>if it is expected too for shared block among multiple qdiscs.
>
> For shared block, block->q is null.

According to this comment:
/* block_index not 0 means the shared block is requested */

and the code,

        if (!block) {
                block = tcf_block_create(net, q, extack);

block->q is set to q, and q is always non-NULL AFAIU.

Also, I don't know if it is intended, but block->q always points to
the parent qdisc rather than the qdisc attached to a class.


>>
>>
>>@@ -338,7 +330,7 @@ static struct hlist_head *tc_u_common_hash;
>>
>> static unsigned int tc_u_hash(const struct tcf_proto *tp)
>> {
>>-       return hash_ptr(tp->chain->block, U32_HASH_SHIFT);
>>+       return hash_ptr(tp->chain->block->q, U32_HASH_SHIFT);
>> }
>>
>> static struct tc_u_common *tc_u_common_find(const struct tcf_proto *tp)
>>@@ -348,7 +340,7 @@ static struct tc_u_common *tc_u_common_find(const
>>struct tcf_proto *tp)
>>
>>        h = tc_u_hash(tp);
>>        hlist_for_each_entry(tc, &tc_u_common_hash[h], hnode) {
>>-               if (tc->block == tp->chain->block)
>>+               if (tc->block->q == tp->chain->block->q)
>
> :O I don't get it. tc->block is pointer, tc->block->q is pointer. And
> they are different at the same time for non-shared block.

If you look into Pawel's script, a new block is created for each class
therefore a different tc_u_common is created which causes the
ht 9:22 can't be found.