All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Jiri Pirko <jiri@resnulli.us>
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
Subject: Re: [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances
Date: Wed, 13 Dec 2017 11:28:59 -0700	[thread overview]
Message-ID: <3d3f57ae-23b7-6389-7276-3019f57dce01@gmail.com> (raw)
In-Reply-To: <20171213173948.GK2031@nanopsycho>

On 12/13/17 10:39 AM, Jiri Pirko wrote:
> Wed, Dec 13, 2017 at 06:18:04PM CET, dsahern@gmail.com wrote:
>> On 12/13/17 10:07 AM, Jiri Pirko wrote:
>>> Wed, Dec 13, 2017 at 05:54:35PM CET, dsahern@gmail.com wrote:
>>>> On 12/13/17 8:10 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
>>>>>
>>>>> 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 to any of qdiscs sharing the same block:
>>>>>
>>>>> $ tc filter add dev ens7 ingress protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>>>>
>>>> I still say this is very odd user semantic - making changes to device M
>>>> and the changes magically affect device N. Operating on the shared block
>>>> as a separate object makes it is much more direct and clear.
>>>
>>> I plan to do it as a follow-up patch. But this is how things are done
>>> now and have to continue to work.
>>
>> Why is that? You are introducing the notion of a shared block with this
>> patch set. What is the legacy "how things are done now" you are
>> referring to?
> 
> Well, the filter add/del should just work no matter if the block behind is
> shared or not.

My argument is that modifying a shared block instance via a dev should
not be allowed. Those changes should only be allowed via the shared
block. So if a user puts adds a shared block to the device and then
attempts to add a filter via the device it should not be allowed.

  reply	other threads:[~2017-12-13 18:29 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13 15:10 [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 01/10] net: sched: introduce support for multiple filter chain pointers registration Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 02/10] net: sched: avoid usage of tp->q in tcf_classify Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 03/10] net: sched: introduce block mechanism to handle netif_keep_dst calls Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 04/10] net: sched: remove classid and q fields from tcf_proto Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 05/10] net: sched: keep track of offloaded filters and check tc offload feature Jiri Pirko
2017-12-14  1:05   ` Jakub Kicinski
2017-12-14  9:47     ` Jiri Pirko
2017-12-14 13:10       ` Jiri Pirko
2017-12-14 18:49         ` Jakub Kicinski
2017-12-15  9:09           ` Jiri Pirko
2017-12-14 19:22   ` Jakub Kicinski
2017-12-15  9:09     ` Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 06/10] net: sched: allow ingress and clsact qdiscs to share filter blocks Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 07/10] mlxsw: spectrum_acl: Reshuffle code around mlxsw_sp_acl_ruleset_create/destroy Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 08/10] mlxsw: spectrum_acl: Don't store netdev and ingress for ruleset unbind Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 09/10] mlxsw: spectrum_acl: Implement TC block sharing Jiri Pirko
2017-12-13 15:10 ` [patch net-next v3 10/10] mlxsw: spectrum_acl: Pass mlxsw_sp_port down to ruleset bind/unbind ops Jiri Pirko
2017-12-13 15:13 ` [patch iproute2] tc: implement filter block sharing to ingress and clsact qdiscs Jiri Pirko
2017-12-16 18:12   ` Stephen Hemminger
2017-12-17 16:05     ` Jiri Pirko
2017-12-13 16:54 ` [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances David Ahern
2017-12-13 17:07   ` Jiri Pirko
2017-12-13 17:18     ` David Ahern
2017-12-13 17:39       ` Jiri Pirko
2017-12-13 18:28         ` David Ahern [this message]
2017-12-13 18:42           ` Jiri Pirko
2017-12-14  0:46             ` Jakub Kicinski
2017-12-15 17:08               ` David Ahern
2017-12-15 17:10                 ` Jiri Pirko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3d3f57ae-23b7-6389-7276-3019f57dce01@gmail.com \
    --to=dsahern@gmail.com \
    --cc=alexander.h.duyck@intel.com \
    --cc=andrew@lunn.ch \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=ganeshgr@chelsio.com \
    --cc=idosch@mellanox.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=john.hurley@netronome.com \
    --cc=leonro@mellanox.com \
    --cc=matanb@mellanox.com \
    --cc=michael.chan@broadcom.com \
    --cc=mlxsw@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=ogerlitz@mellanox.com \
    --cc=pieter.jansenvanvuuren@netronome.com \
    --cc=saeedm@mellanox.com \
    --cc=simon.horman@netronome.com \
    --cc=vivien.didelot@savoirfairelinux.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.