From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [patch net-next v6 00/11] net: sched: allow qdiscs to share filter block instances Date: Sat, 6 Jan 2018 10:48:25 +0100 Message-ID: <20180106094825.GB2099@nanopsycho> References: <20180105230929.5645-1-jiri@resnulli.us> <20180106080728.GA2099@nanopsycho> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, davem@davemloft.net, jhs@mojatatu.com, xiyou.wangcong@gmail.com, mlxsw@mellanox.com, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com, michael.chan@broadcom.com, ganeshgr@chelsio.com, saeedm@mellanox.com, matanb@mellanox.com, leonro@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com, simon.horman@netronome.com, pieter.jansenvanvuuren@netronome.com, john.hurley@netronome.com, alexander.h.duyck@intel.com, ogerlitz@mellanox.com, john.fastabend@gmail.com, daniel@iogearbox.net To: David Ahern Return-path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:40581 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752672AbeAFJs2 (ORCPT ); Sat, 6 Jan 2018 04:48:28 -0500 Received: by mail-wm0-f68.google.com with SMTP id f206so6322344wmf.5 for ; Sat, 06 Jan 2018 01:48:27 -0800 (PST) Content-Disposition: inline In-Reply-To: <20180106080728.GA2099@nanopsycho> Sender: netdev-owner@vger.kernel.org List-ID: 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 >>> >>> 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?