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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZV9b0HrM5WespGMW@nanopsycho>
Date: Thu, 23 Nov 2023 15:04:00 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: Victor Nogueira <victor@...atatu.com>, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	xiyou.wangcong@...il.com, mleitner@...hat.com, vladbu@...dia.com,
	paulb@...dia.com, pctammela@...atatu.com, netdev@...r.kernel.org,
	kernel@...atatu.com
Subject: Re: [PATCH net-next RFC v5 4/4] net/sched: act_blockcast: Introduce
 blockcast tc action

Thu, Nov 23, 2023 at 02:37:13PM CET, jhs@...atatu.com wrote:
>On Thu, Nov 23, 2023 at 3:51 AM Jiri Pirko <jiri@...nulli.us> wrote:
>>
>> Fri, Nov 10, 2023 at 10:46:18PM CET, victor@...atatu.com wrote:
>> >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.
>>
>>
>> >
>> >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.


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


>
>cheers,
>jamal
>
>> 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.
>>
>>