Thread (28 messages) 28 messages, 5 authors, 2023-12-06

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

From: Jiri Pirko <jiri@resnulli.us>
Date: 2023-11-23 16:51:15

Thu, Nov 23, 2023 at 05:20:27PM CET, hadi@mojatatu.com wrote:
On Thu, Nov 23, 2023 at 10:17 AM Jiri Pirko [off-list ref] wrote:
quoted
Thu, Nov 23, 2023 at 03:38:35PM CET, jhs@mojatatu.com wrote:
quoted
On Thu, Nov 23, 2023 at 9:04 AM Jiri Pirko [off-list ref] wrote:
quoted
Thu, Nov 23, 2023 at 02:37:13PM CET, jhs@mojatatu.com wrote:
quoted
On Thu, Nov 23, 2023 at 3:51 AM Jiri Pirko [off-list ref] wrote:
quoted
Fri, Nov 10, 2023 at 10:46:18PM CET, victor@mojatatu.com wrote:
quoted
This action takes advantage of the presence of tc block ports set in the
datapath and multicasts a packet to ports on a block. By default, it will
broadcast the packet to a block, that is send to all members of the block except
the port in which the packet arrived on. However, the user may specify
the option "tx_type all", which will send the packet to all members of the
block indiscriminately.

Example usage:
   $ tc qdisc add dev ens7 ingress_block 22
   $ tc qdisc add dev ens8 ingress_block 22

Now we can add a filter to broadcast packets to ports on ingress block id 22:
$ tc filter add block 22 protocol ip pref 25 \
 flower dst_ip 192.168.0.0/16 action blockcast blockid 22
Name the arg "block" so it is consistent with "filter add block". Make
sure this is aligned netlink-wise as well.

quoted
Or if we wish to send to all ports in the block:
$ tc filter add block 22 protocol ip pref 25 \
 flower dst_ip 192.168.0.0/16 action blockcast blockid 22 tx_type all
I read the discussion the the previous version again. I suggested this
to be part of mirred. Why exactly that was not addressed?
I am the one who pushed back (in that discussion). Actions should be
small and specific. Like i had said in that earlier discussion it was
a mistake to make mirred do both mirror and redirect - they should
For mirror and redirect, I agree. For redirect and redirect, does not
make much sense. It's just confusing for the user.
Blockcast only emulates the mirror part. I agree redirect doesnt make
any sense because once you redirect the packet is gone.
How is it mirror? It is redirect to multiple, isn't it?
mirror has been used (so far in mirred action and i believe in the
industry in general) to mean  "send a copy of the packet" - meaning
you can send to many ports and even when you are done sending to all
those ports the packet is still in the pipeline and you can continue
to execute other action on it. Whereas redirect means the packet is
stolen from the pipeline i.e if you redirect to a port the packet is
not available to redirect to the next port or for any other action
after that.
You could argue a loose interpretation of redirect to a block to mean
"mirror to all ports on the block but on the last port redirect".
it's stolen from the pipeline, right? That would be redirect from my
perspective.

quoted
quoted
quoted
quoted
have been two actions. So i feel like adding a block to mirred is
adding more knobs. We are also going to add dev->group as a way to
select what devices to mirror to. Should that be in mirred as well?
I need more details.
You set any port you want to be mirrored to using ip link, example:
ip link set dev $DEV1 group 2
ip link set dev $DEV2 group 2
That does not looks correct at all. Do tc stuff in tc, no?
We could certainly annotate the dev group via tc but it seems odd ....

cheers,
jamal
quoted
quoted
...

Then you can blockcast:
tc filter add devx protocol ip pref 25 \
 flower dst_ip 192.168.0.0/16 action blockcast group 2
"blockcasting" to something that is not a block anymore. Not nice.
quoted
cheers,
jamal
quoted
quoted
cheers,
jamal
quoted
Instead of:
$ tc filter add block 22 protocol ip pref 25 \
  flower dst_ip 192.168.0.0/16 action blockcast blockid 22
You'd have:
$ tc filter add block 22 protocol ip pref 25 \
  flower dst_ip 192.168.0.0/16 action mirred egress redirect block 22

I don't see why we need special action for this.

Regarding "tx_type all":
Do you expect to have another "tx_type"? Seems to me a bit odd. Why not
to have this as "no_src_skip" or some other similar arg, without value
acting as a bool (flag) on netlink level.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help