From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [patch net-next v4 00/10] net: sched: allow qdiscs to share filter block instances Date: Wed, 3 Jan 2018 23:06:58 -0800 Message-ID: <20180103230658.595eac7d@cakuba.netronome.com> References: <20171224071956.GA1883@nanopsycho> <780a80d0-9384-ae34-4cab-3070b004b64e@gmail.com> <20171225102346.GB1885@nanopsycho> <20180102194944.GG2051@nanopsycho.orion> <20180103094025.GA2067@nanopsycho.orion> <20180103172209.GD2067@nanopsycho.orion> <20180103155152.7e94a295@cakuba.netronome.com> <20180104065702.GH2067@nanopsycho.orion> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: David Ahern , 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, 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: Jiri Pirko Return-path: Received: from mx3.wp.pl ([212.77.101.9]:26763 "EHLO mx3.wp.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750896AbeADHHM (ORCPT ); Thu, 4 Jan 2018 02:07:12 -0500 In-Reply-To: <20180104065702.GH2067@nanopsycho.orion> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 4 Jan 2018 07:57:02 +0100, Jiri Pirko wrote: > Thu, Jan 04, 2018 at 12:51:52AM CET, kubakici@wp.pl wrote: > >On Wed, 3 Jan 2018 18:22:09 +0100, Jiri Pirko wrote: > >> However I don't agree about breaking the existing filter add and show > >> and also imposibility to make not-shared block shared in the runtime > >> before defining it first. > > > >FWIW I would agree with David that allowing add on a shared block > >modify filters on another interface can break existing users. (No > >opinion on dump and lifetime). > > I don't think that David is saying that, but why do you think it would > break existing users? Perhaps I worded is too strongly as "breaking existing users", but it certainly introduces surprising side effects. David put it into words very well: On Tue, 2 Jan 2018 19:07:36 -0700, David Ahern wrote: > The disagreement is in how they should be managed. I think my last > response concisely captures my concerns -- the principle of least surprise. > > So with the initial commands above, all is fine. Then someone is > debugging a problem or wants to add another filter to ens8, so they run: > > $ tc filter add dev ens8 ingress protocol ip pref 25 flower dst_ip > 192.168.1.0/16 action drop > > Then traffic flows through ens7 break and some other user is struggling > to understand what just happened. That the new filter magically appears > on ens7 when the user operated on ens8 is a surprise. Nothing about that > last command acknowledges that it is changing a shared resource. > > Consider the commands being run by different people, and a time span > between. Allowing the shared block to be configured by any device using > the block is just setting up users for errors and confusion. > > > forcing user to explicitly create some block entity and then to attach > > it to qdisc instances. I don't really see good reason for it. Could you > > please clear this up for me? > > It forces the user to acknowledge it is changing a resource that may be > shared by more than one device. > > $ tc filter add dev ens8 ingress protocol ip pref 25 flower dst_ip > 192.168.1.0/16 action drop > Error: This qdisc is a shared block. Use the block API to configure.