netdev - Re: [patch net-next v4 00/10] 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: <20180104140206.GE2213@nanopsycho>
Date:   Thu, 4 Jan 2018 15:02:06 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Jakub Kicinski <kubakici@...pl>, David Ahern <dsahern@...il.com>,
        netdev@...r.kernel.org, davem@...emloft.net,
        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, 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 v4 00/10] net: sched: allow qdiscs to share
 filter block instances

Thu, Jan 04, 2018 at 02:30:54PM CET, jhs@...atatu.com wrote:
>On 18-01-04 08:00 AM, Jiri Pirko wrote:
>> Thu, Jan 04, 2018 at 01:41:54PM CET, jhs@...atatu.com wrote:
>
>
>> > 
>> > And that a simple "tc qdisc add dev ens7 ingress" should
>> > either not be able to create a block (or we can say creates
>> > block id 0 - owned by the netdev - for consistency).
>> 
>> This allocates a new block - as it needs it internally anyway - and tc
>> core will assign unused block id. You can see this id in the list then.
>> Later you can use this block id for other created qdiscs.
>> 
>> Block ids are per-ns.
>> 
>
>Hrm. I was thinking more that there is a block that cannot be shared
>and is only owned by the netdev i.e current behavior. But what you
>describe should work
>(and let the user decide if they want to share).

Yeah. This is how it is implemented right now. Works fine. And provides
user possibility to set the share with other qdisc while the filters are
already there. It is actually quite nice.

>
>> > 
>> > Maybe what we need is another knob to control this new functionality?
>> > Either a kernel config option or an extra parameter when creating
>> > the qdisc or a system wide boolean per netdev (which of course
>> > contradicts the "less is more" principle). If this knob was set
>> > then you reject addition of filters via a port/qdisc which is sharing.
>> 
>> I don't like this knob idea. It just adds confusion for no good reason
>> what so ever.
>
>I dont like it either but i see it as a knob to choose between
>improved usability/manageability (which new interface provides)
>vs least suprise - which is to keep the old syntax around.
>If i was an admin and turned on this knob - I am prepared to deal
>with fallout of some user script which tries to add filters via
>the device instead of the block.

This smells awfully like "break_my_system" knob described by DaveM
during our route offloading discussions :)

I would just go without this knob for now in a way I suggested earlier
in my reply to DavidA. I would leave the functionality there as it is in
the current patchset, leaving all original interfaces to work and on top
of that, I will add "filter add block" and "filter list block" handles.
Sounds fine?


>
>cheers,
>jamal