netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jakub Kicinski <kubakici@wp.pl>, David Ahern <dsahern@gmail.com>,
	netdev@vger.kernel.org, davem@davemloft.net,
	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
Subject: Re: [patch net-next v4 00/10] net: sched: allow qdiscs to share filter block instances
Date: Thu, 4 Jan 2018 15:06:53 +0100	[thread overview]
Message-ID: <20180104140653.GF2213@nanopsycho> (raw)
In-Reply-To: <c2b9c7e0-4ac8-16f6-f4ac-8f5349560585@mojatatu.com>

Thu, Jan 04, 2018 at 02:43:08PM CET, jhs@mojatatu.com wrote:
>On 18-01-04 08:05 AM, Jiri Pirko wrote:
>> Thu, Jan 04, 2018 at 01:55:05PM CET, jhs@mojatatu.com wrote:
>
>> > 
>> > $ tc filter add block 1 protocol ip priority 10 flower ...
>> > classid 1:2
>> > 
>> > Looks good - things will work fine for packets showing
>> > up on egress of enps7 which match the flower rule
>> > and classid 1:2 is selected to queue the packet on.
>> > 
>> > Things will not _work fine_ for packets showing up on
>> > egress of ensp8. There is no classid 1:2 on egress of
>> > enps8. The prio qdisc is a bad example because it
>> > has a default queue (i think 10:2) in this case. Other
>> > qdiscs(off top of my head DRR) will just drop the packet.
>> 
>> I'm very well aware of this. The plan is to support this and resolve
>> the lack of classid withing the qdisc according to the qdisc type.
>
>If i understood you correctly,  that is still iffy.
>An admin will see packets for the same filter on one device going
>to an agreed-to queue but on another going to a default queue.

Yeah, it is. That needs a bit more thoughts and discussion. That was the
main purpose I did not address it in this patchset and allowed only
ingress and clsact qidsc.


>
>> However, currently the classfull qdiscs are not supported for
>> block sharing - only ingress and clsact qdiscs are supported.
>> 
>
>Essentially anything that doesnt have queues associated with it..
>(and ignore the tcf_result).
>
>> > 
>> > I think this is resolvable - but it will take more to the
>> > patches than the current set you posted Jiri.
>> > A simple solution is to say sharing only works for ingress
>> > (but that sounds very lame).
>> 
>> That is the current limitation of the patchset as you can see. The
>> sharing works only for ingress and clsact qdisc. So works for both
>> ingress and egress (clsact).
>> 
>
>One option is to name all egress queues the same way on all devices
>f.e in the two examples i provided call root qdiscs 1:0.
>
>I am not sure how cleanly you get this to work with egress
>My current thinking involves some brain somersault...
>I will think some more about it...

clsact egress works the same as ingress qdisc - please check the code.


>
>cheers,
>jamal

  reply	other threads:[~2018-01-04 14:06 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-23 15:54 [patch net-next v4 00/10] net: sched: allow qdiscs to share filter block instances Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 01/10] net: sched: introduce support for multiple filter chain pointers registration Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 02/10] net: sched: avoid usage of tp->q in tcf_classify Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 03/10] net: sched: introduce block mechanism to handle netif_keep_dst calls Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 04/10] net: sched: remove classid and q fields from tcf_proto Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 05/10] net: sched: keep track of offloaded filters and check tc offload feature Jiri Pirko
2017-12-24  2:20   ` Jakub Kicinski
2017-12-24  7:52     ` Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 06/10] net: sched: allow ingress and clsact qdiscs to share filter blocks Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 07/10] mlxsw: spectrum_acl: Reshuffle code around mlxsw_sp_acl_ruleset_create/destroy Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 08/10] mlxsw: spectrum_acl: Don't store netdev and ingress for ruleset unbind Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 09/10] mlxsw: spectrum_acl: Implement TC block sharing Jiri Pirko
2017-12-23 15:54 ` [patch net-next v4 10/10] mlxsw: spectrum_acl: Pass mlxsw_sp_port down to ruleset bind/unbind ops Jiri Pirko
2017-12-23 16:06 ` [patch iproute2] tc: implement filter block sharing to ingress and clsact qdiscs Jiri Pirko
2017-12-24  1:54 ` [patch net-next v4 00/10] net: sched: allow qdiscs to share filter block instances David Ahern
2017-12-24  7:19   ` Jiri Pirko
2017-12-24 16:25     ` David Ahern
2017-12-25 10:23       ` Jiri Pirko
2018-01-02 19:49         ` Jiri Pirko
2018-01-03  2:07           ` David Ahern
2018-01-03  9:40             ` Jiri Pirko
2018-01-03 15:57               ` David Ahern
2018-01-03 17:22                 ` Jiri Pirko
2018-01-03 23:51                   ` Jakub Kicinski
2018-01-04  6:57                     ` Jiri Pirko
2018-01-04  7:06                       ` Jakub Kicinski
2018-01-04 10:12                         ` Jiri Pirko
2018-01-04 12:41                           ` Jamal Hadi Salim
2018-01-04 13:00                             ` Jiri Pirko
2018-01-04 13:30                               ` Jamal Hadi Salim
2018-01-04 14:02                                 ` Jiri Pirko
2018-01-04 15:45                             ` David Miller
2018-01-04 12:55                           ` Jamal Hadi Salim
2018-01-04 13:05                             ` Jiri Pirko
2018-01-04 13:43                               ` Jamal Hadi Salim
2018-01-04 14:06                                 ` Jiri Pirko [this message]
2018-01-04 15:42                                   ` Jamal Hadi Salim
2018-01-04 15:33                           ` David Miller
2018-01-04 15:51                             ` Jiri Pirko
2018-01-05 10:38                               ` 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=20180104140653.GF2213@nanopsycho \
    --to=jiri@resnulli.us \
    --cc=alexander.h.duyck@intel.com \
    --cc=andrew@lunn.ch \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=ganeshgr@chelsio.com \
    --cc=idosch@mellanox.com \
    --cc=jhs@mojatatu.com \
    --cc=john.fastabend@gmail.com \
    --cc=john.hurley@netronome.com \
    --cc=kubakici@wp.pl \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).