On Tue, Oct 10, 2023 at 09:41:11AM +0200, Jiri Pirko wrote:

Mon, Oct 09, 2023 at 10:54:07PM CEST, mleitner@redhat.com wrote:

quoted

On Sat, Oct 07, 2023 at 07:20:52PM +0200, Jiri Pirko wrote:

quoted

Sat, Oct 07, 2023 at 04:09:15PM CEST, jhs@mojatatu.com wrote:

quoted

On Sat, Oct 7, 2023 at 8:43 AM Jiri Pirko [off-list ref] wrote:

...

quoted

quoted

quoted

My primary point is, this should be mirred redirect to block instead of
what we currently have only for dev. That's it.

Agreed (and such a feature should be added regardless of this action).
The tc block provides a simple abstraction, but do you think it is
enough? Alternative is to use a list of ports given to mirred: it
allows us to group ports from different tc blocks or even just a
subset of what is in a tc block - but it will require a lot more code
to express such functionality.

Again, you attach filter to either dev or block. If you extend mirred
redirect to accept the same 2 types of target, I think it would be best.

The difference here between filter and action here is that you don't
really have an option for filters: you either attach it to either dev
or block, or you create an entire new class of objects, say,
"blockfilter", all while retaining the same filters, parameters, etc.
I'm not aware of a single filter that behaves differently over a block
than a netdev.

Why do you talk about different behaviour? Nobody suggested that. I have

Because if mirred gets updated to support blocks, that's what will
happen. It will have options that only make sense for netdev or for
blocks.

no idea what you mean by "blockfilter".

Well, it's described above. It was just an example.

quoted

But for actions, there is the option, and despite the fact that both

Which option?

To create a new action.

quoted

"output packets", the semantics are not that close. It actually
helps with parameter parsing, documentation (man pages), testing (as
use and test cases can be more easily tracked) and perhaps more
importantly: if I don't want this feature, I can disable the new
module.

Later someone will say "hey why not have a hash_dst_selector", so it
can implement a load balancer through the block output? And mirred,
once a simple use case (with an already complex implementation),
becomes a partial implementation of bonding then. :)

In short, I'm not sure if having the user to fiddle through a maze of
options that only work in mode A or B or work differently is better
than having more specialized actions (which can and should reuse code).

Sure, you can have "blockmirredredirect" that would only to the
redirection for block target. No problem. I don't see why it can't be
just a case of mirred redirect, but if people want that separate, why
not.

My problem with the action "blockcast" is that it somehow works with
configuration of an entity the filter is attached to:
blockX->filterY->blockcastZ

Z goes all the way down to blockX to figure out where to redirect the
packet. If that is not odd, then I don't know what else is.

Maybe fried eggs with chocolate. :-D
Just joking..

Has other consequences, like what happens if the filter is not attached
to block, but dev directly? What happens is blockcast action is shared
among filter? Etc.

Good points!

Configuration should be action property. That is my point.

Makes sense. Whoever is adding such filter, already knows the block,
and can add the parameter to the action as well. Making it automatic
is not helping out that much in the end.

  Marcelo