netdev - Re: [patch net-next 00/34] 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: <20171013062629.GB1952@nanopsycho.orion>
Date:   Fri, 13 Oct 2017 08:26:29 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, 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,
        jeffrey.t.kirsher@...el.com, saeedm@...lanox.com,
        matanb@...lanox.com, leonro@...lanox.com, idosch@...lanox.com,
        jakub.kicinski@...ronome.com, ast@...nel.org, daniel@...earbox.net,
        simon.horman@...ronome.com, pieter.jansenvanvuuren@...ronome.com,
        john.hurley@...ronome.com, edumazet@...gle.com,
        alexander.h.duyck@...el.com, john.fastabend@...il.com,
        willemb@...gle.com
Subject: Re: [patch net-next 00/34] net: sched: allow qdiscs to share filter
 block instances

Thu, Oct 12, 2017 at 11:37:30PM CEST, dsahern@...il.com wrote:
>On 10/12/17 11:17 AM, Jiri Pirko wrote:
>> 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 
>> 
>> Now we can add filter to any of qdiscs sharing the same block:
>> 
>> $ tc filter add dev ens7 parent ffff: protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>> 
>> 
>> We will see the same output if we list filters for ens7 and ens8, including stats:
>> 
>> $ tc -s filter show dev ens7 ingress
>> filter protocol ip pref 25 flower chain 0 
>> filter protocol ip 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 installed 39 sec used 2 sec
>>         Action statistics:
>>         Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0) 
>>         backlog 0b 0p requeues 0 
>> 
>> $ tc -s filter show dev ens8 ingress
>> filter protocol ip pref 25 flower chain 0 
>> filter protocol ip 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 installed 40 sec used 3 sec
>>         Action statistics:
>>         Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0) 
>>         backlog 0b 0p requeues 0
>
>This seems like really odd semantics to me ... a filter added to one
>device shows up on another.

Why is it odd? They share the same block, so it is natural that rule
added to one shows in list of rules for all devices that share the same
block.


>
>Why not make the shared block a standalone object that is configured
>through its own set of commands and then referenced by both devices?

I was thinking about that for a long time. That would require entirely
new set of netlink api and internal kernel handling just for this. Lots
of duplications. The reason is, the current API is strictly build around
ifindex. But the new API would not solve anything. As a user, I still
want so see shared rules in individial device listing, because they
would get processed for the device. So I believe that the proposed
behaviour is correct.