Re: [patch net-next v6 00/11] net: sched: allow qdiscs to share filter block instances

[Jiri Pirko <jiri@resnulli.us>]

Sat, Jan 06, 2018 at 09:07:28AM CET, jiri@resnulli.us wrote:
>Sat, Jan 06, 2018 at 04:57:21AM CET, dsahern@gmail.com wrote:
>>On 1/5/18 4:09 PM, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>> 
>>> Currently the filters added to qdiscs are independent. So for example if you
>>> have 2 netdevices and you create ingress qdisc on both and you want to add
>>> identical filter rules both, you need to add them twice. This patchset
>>> makes this easier and mainly saves resources allowing to share all filters
>>> within a qdisc - I call it a "filter block". Also this helps to save
>>> resources when we do offload to hw for example to expensive TCAM.
>>> 
>>> So back to the example. First, we create 2 qdiscs. Both will share
>>> block number 22. "22" is just an identification. If we don't pass any
>>> block number, a new one will be generated by kernel:
>>> 
>>> $ tc qdisc add dev ens7 ingress block 22
>>>                                 ^^^^^^^^
>>> $ tc qdisc add dev ens8 ingress block 22
>>>                                 ^^^^^^^^
>>> 
>>> Now if we list the qdiscs, we will see the block index in the output:
>>> 
>>> $ tc qdisc
>>> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
>>> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>>> 
>>> 
>>> To make is more visual, the situation looks like this:
>>> 
>>>    ens7 ingress qdisc                 ens7 ingress qdisc
>>>           |                                  |
>>>           |                                  |
>>>           +---------->  block 22  <----------+
>>> 
>>> Unlimited number of qdiscs may share the same block.
>>> 
>>> Now we can add filter using the block index:
>>> 
>>> $ tc filter add block 22 protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>>> 
>>> 
>>> Note we cannot use the qdisc for filter manipulations for shared blocks:
>>> 
>>> $ tc filter add dev ens8 ingress protocol ip pref 1 flower dst_ip 192.168.100.2 action drop
>>> Error: Cannot work with shared block, please use block index.
>>> 
>>> 
>>> We will see the same output if we list filters for ingress qdisc of
>>> ens7 and ens8, also for the block 22:
>>> 
>>> $ tc filter show block 22
>>> filter block 22 protocol ip pref 25 flower chain 0
>>> filter block 22 protocol ip pref 25 flower chain 0 handle 0x1
>>> ...
>>> 
>>> $ tc filter show dev ens7 ingress
>>> filter block 22 protocol ip pref 25 flower chain 0
>>> filter block 22 protocol ip pref 25 flower chain 0 handle 0x1
>>> ...
>>> 
>>> $ tc filter show dev ens8 ingress
>>> filter block 22 protocol ip pref 25 flower chain 0
>>> filter block 22 protocol ip pref 25 flower chain 0 handle 0x1
>>> ...
>>
>>I like the API and output shown here, but I am not getting that with the
>>patches.
>>
>>In this example, I am using 42 for the block id:
>>
>>$ tc qdisc show dev eth2
>>qdisc mq 0: root
>>qdisc pfifo_fast 0: parent :2 bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1
>>1 1 1
>>qdisc pfifo_fast 0: parent :1 bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1
>>1 1 1
>>qdisc ingress ffff: parent ffff:fff1 block 42
>>
>>It allows me to add a filter using the device:
>>$ tc filter add dev eth2 ingress protocol ip pref 1 flower dst_ip
>>192.168.101.2 action drop
>>$  echo $?
>>0
>
>Yes, because the block is not shared yet. You have it only for one
>qdisc. As long as you have that, the "filter add dev" api still works.
>It stops working when you add another qdisc to that block.

Or, do you think it should work like:

$ tc qdisc add dev ens8 ingress
$ tc qdisc
qdisc ingress ffff: dev ens8 parent ffff:fff1

$ tc qdisc add dev ens7 ingress block 22
$ tc qdisc
qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
qdisc ingress ffff: dev ens8 parent ffff:fff1

And the only shareable block is the one which I spefify block index for?
And for that, I have to always use block index handle for filter
add/del/get?