All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Marcelo Ricardo Leitner <mleitner@redhat.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
	Jamal Hadi Salim <hadi@mojatatu.com>,
	Victor Nogueira <victor@mojatatu.com>,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, xiyou.wangcong@gmail.com, vladbu@nvidia.com,
	paulb@nvidia.com, pctammela@mojatatu.com, netdev@vger.kernel.org,
	kernel@mojatatu.com
Subject: Re: [PATCH net-next RFC v5 4/4] net/sched: act_blockcast: Introduce blockcast tc action
Date: Wed, 6 Dec 2023 08:55:55 +0100	[thread overview]
Message-ID: <ZXApC8od2deGjKYi@nanopsycho> (raw)
In-Reply-To: <CALnP8Zavd8N=9n42sbeKqE-mMKXHsFtmCHKOuG7sZEN5Z8m7kw@mail.gmail.com>

Tue, Dec 05, 2023 at 11:12:23PM CET, mleitner@redhat.com wrote:
>On Tue, Dec 05, 2023 at 10:27:31AM -0500, Jamal Hadi Salim wrote:
>> On Tue, Dec 5, 2023 at 9:52 AM Marcelo Ricardo Leitner
>> <mleitner@redhat.com> wrote:
>> >
>> > On Tue, Dec 05, 2023 at 09:41:02AM +0100, Jiri Pirko wrote:
>> > > Mon, Dec 04, 2023 at 09:10:18PM CET, jhs@mojatatu.com wrote:
>> > > >On Mon, Dec 4, 2023 at 4:49 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> > > >>
>> > > >> Fri, Dec 01, 2023 at 07:45:47PM CET, jhs@mojatatu.com wrote:
>> > ...
>> > > >> >Ok, so we are moving forward with mirred "mirror" option only for this then...
>> > > >>
>> > > >> Could you remind me why mirror and not redirect? Does the packet
>> > > >> continue through the stack?
>> > > >
>> > > >For mirror it is _a copy_ of the packet so it continues up the stack
>> > > >and you can have other actions follow it (including multiple mirrors
>> > > >after the first mirror). For redirect the packet is TC_ACT_CONSUMED -
>> > > >so removed from the stack processing (and cant be sent to more ports).
>> > > >That is how mirred has always worked and i believe thats how most
>> > > >hardware works as well.
>> > > >So sending to multiple ports has to be mirroring semantics (most
>> > > >hardware assumes the same semantics).
>> > >
>> > > You assume cloning (sending to multiple ports) means mirror,
>> > > that is I believe a mistake. Look at it from the perspective of
>> > > replacing device by target for each action. Currently we have:
>> > >
>> > > 1) mirred mirror TARGET_DEVICE
>> > >    Clones, sends to TARGET_DEVICE and continues up the stack
>> > > 2) mirred redirect TARGET_DEVICE
>> > >    Sends to TARGET_DEVICE, nothing is sent up the stack
>> > >
>> > > For block target, there should be exacly the same semantics:
>> > >
>> > > 1) mirred mirror TARGET_BLOCK
>> > >    Clones (multiple times, for each block member), sends to TARGET_BLOCK
>> > >    and continues up the stack
>> > > 2) mirred redirect TARGET_BLOCK
>> > >    Clones (multiple times, for each block member - 1), sends to
>> > >    TARGET_BLOCK, nothing is sent up the stack
>> >
>> > This makes sense to me as well. When I first read Jamal's email I
>> > didn't spot any confusion, but now I see there can be some. I think he
>> > meant pretty much the same thing, referencing cascading other outputs
>> > after blockcast (and not the inner outputs, lets say), but that's just
>> > my interpretation. :)
>>
>> In my (shall i say long experience) I have never seen the prescribed
>> behavior of redirect meaning mirror to (all - last one) then redirect
>> on last one.. Jiri, does spectrum work like this?
>> Neither in s/w nor in h/w. From h/w - example, the nvidia CX6 you have
>> to give explicit mirror, mirror, mirror, redirect. IOW, i dont think
>> the hardware can be told "here's a list of ports, please mirror to all
>> of them and for the last one steal the packet and redirect".
>
>Precisely. I/(we?) were talking about tc sw/user expectations, not how
>to offload it.
>
>From a tc user perspective, the user should still be able to do this:
>1) mirred mirror TARGET_BLOCK
>2) mirred redirect TARGET_BLOCK
>regardless of how the implementation actually works. Because ovs and
>other users will rely on this semantic.

Exactly. Forget about hw for now.


>
>As for the actual implementation, as you said, it will have to somehow
>unpack that into "[mirror, mirror, ...,] <mirror/redirect>", depending
>on what the user requested, as I doubt there will be hw support for
>outputting to multiple ports in one action.
>
>> Having said that i am not opposed to it - it will just make the code
>> slightly more complex and i am sure slightly slower in the datapath.
>>
>> cheers,
>> jamal
>>
>

  reply	other threads:[~2023-12-06  7:55 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-10 21:46 [PATCH net-next RFC v5 0/4] net/sched: Introduce tc block ports tracking and use Victor Nogueira
2023-11-10 21:46 ` [PATCH net-next RFC v5 1/4] net/sched: act_mirred: Separate mirror and redirect into two distinct functions Victor Nogueira
2023-11-11  0:13   ` kernel test robot
2023-11-11  0:13   ` kernel test robot
2023-11-23  6:58   ` Jiri Pirko
2023-11-10 21:46 ` [PATCH net-next RFC v5 2/4] net/sched: Introduce tc block netdev tracking infra Victor Nogueira
2023-11-10 21:46 ` [PATCH net-next RFC v5 3/4] net/sched: cls_api: Expose tc block to the datapath Victor Nogueira
2023-11-10 21:46 ` [PATCH net-next RFC v5 4/4] net/sched: act_blockcast: Introduce blockcast tc action Victor Nogueira
2023-11-23  8:51   ` Jiri Pirko
2023-11-23 13:37     ` Jamal Hadi Salim
2023-11-23 14:04       ` Jiri Pirko
2023-11-23 14:38         ` Jamal Hadi Salim
2023-11-23 15:17           ` Jiri Pirko
2023-11-23 16:20             ` Jamal Hadi Salim
2023-11-23 16:51               ` Jiri Pirko
2023-11-23 16:21             ` Jamal Hadi Salim
2023-11-23 16:52               ` Jiri Pirko
2023-11-27 15:50                 ` Jamal Hadi Salim
2023-11-27 18:52                   ` Marcelo Ricardo Leitner
2023-12-01 18:45                     ` Jamal Hadi Salim
2023-12-04  9:49                       ` Jiri Pirko
2023-12-04 20:10                         ` Jamal Hadi Salim
2023-12-05  8:41                           ` Jiri Pirko
2023-12-05 14:51                             ` Marcelo Ricardo Leitner
2023-12-05 15:27                               ` Jamal Hadi Salim
2023-12-05 22:12                                 ` Marcelo Ricardo Leitner
2023-12-06  7:55                                   ` Jiri Pirko [this message]
2023-12-06 15:09                                     ` Jamal Hadi Salim
2023-11-23 14:29       ` Marcelo Ricardo Leitner
2023-11-23 15:18         ` 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=ZXApC8od2deGjKYi@nanopsycho \
    --to=jiri@resnulli.us \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hadi@mojatatu.com \
    --cc=jhs@mojatatu.com \
    --cc=kernel@mojatatu.com \
    --cc=kuba@kernel.org \
    --cc=mleitner@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=paulb@nvidia.com \
    --cc=pctammela@mojatatu.com \
    --cc=victor@mojatatu.com \
    --cc=vladbu@nvidia.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.