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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a62e5912-8476-00aa-a099-b2f63da2c563@gmail.com>
Date:   Fri, 5 Jan 2018 20:57:21 -0700
From:   David Ahern <dsahern@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, jhs@...atatu.com, xiyou.wangcong@...il.com,
        mlxsw@...lanox.com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        michael.chan@...adcom.com, ganeshgr@...lsio.com,
        saeedm@...lanox.com, matanb@...lanox.com, leonro@...lanox.com,
        idosch@...lanox.com, jakub.kicinski@...ronome.com,
        simon.horman@...ronome.com, pieter.jansenvanvuuren@...ronome.com,
        john.hurley@...ronome.com, alexander.h.duyck@...el.com,
        ogerlitz@...lanox.com, john.fastabend@...il.com,
        daniel@...earbox.net
Subject: Re: [patch net-next v6 00/11] net: sched: allow qdiscs to share
 filter block instances

On 1/5/18 4:09 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...lanox.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

And it modifies the shared block:
$  tc filter show block 42
filter pref 1 flower chain 0
filter pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  dst_ip 192.168.100.2
  not_in_hw
	action order 1: gact action drop
	 random type none pass val 0
	 index 2 ref 1 bind 1

filter pref 1 flower chain 0 handle 0x2
  eth_type ipv4
  dst_ip 192.168.101.2
  not_in_hw
	action order 1: gact action drop
	 random type none pass val 0
	 index 3 ref 1 bind 1

filter pref 25 flower chain 0
filter pref 25 flower chain 0 handle 0x1
  eth_type ipv4
  dst_ip 192.168.0.0/16
  not_in_hw
	action order 1: gact action drop
	 random type none pass val 0
	 index 1 ref 1 bind 1

Notice the header does not give the 'filter block N protocol' part. I
don't get that using the device either (tc filter show dev eth2 ingress).

Something else I noticed is that I do not get an error message if I pass
an invalid block id:

$ tc filter show block 22
$ echo $?
0
$  tc qdisc show | grep block
qdisc ingress ffff: dev eth2 parent ffff:fff1 block 42