Re: [PATCH net-next RFC v5 4/4] net/sched: act_blockcast: Introduce blockcast tc action

[Jamal Hadi Salim <jhs@mojatatu.com>]

On Wed, Dec 6, 2023 at 2:55 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> 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.

Ok, Lets go!

cheers,
jamal

>
> >
> >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
> >>
> >