netdev - Re: [RFC net-next] net: dsa: add support for MC

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]

Date:   Thu, 20 Jun 2019 19:24:47 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vivien Didelot <vivien.didelot@...il.com>, netdev@...r.kernel.org,
        idosch@...lanox.com, Jiri Pirko <jiri@...nulli.us>
Cc:     linux@...linux.org.uk, andrew@...n.ch, davem@...emloft.net
Subject: Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute



On 6/20/2019 4:56 PM, Vivien Didelot wrote:
> This patch adds support for enabling or disabling the flooding of
> unknown multicast traffic on the CPU ports, depending on the value
> of the switchdev SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED attribute.
> 
> This allows the user to prevent the CPU to be flooded with a lot of
> undesirable traffic that the network stack needs to filter in software.
> 
> The bridge has multicast snooping enabled by default, hence CPU ports
> aren't bottlenecked with arbitrary network applications anymore.
> But this can be an issue in some scenarios such as pinging the bridge's
> IPv6 address. Setting /sys/class/net/br0/bridge/multicast_snooping to
> 0 would restore unknown multicast flooding and thus fix ICMPv6. As
> an alternative, enabling multicast_querier would program the bridge
> address into the switch.
>From what I can read from mlxsw, we should probably also implement the
SWITCHDEV_ATTR_ID_PORT_MROUTER attribute in order to be consistent.

Since the attribute MC_DISABLED is on the bridge master, we should also
iterate over the list of switch ports being a member of that bridge and
change their flooding attribute, taking into account whether
BR_MCAST_FLOOD is set on that particular port or not. Just paraphrasing
what mlxsw does here again...

Once you act on the user-facing ports, you might be able to leave the
CPU port flooding unconditionally, since it would only "flood" the CPU
port either because an user-facing port has BR_MCAST_FLOOD set, or
because this is known MC traffic that got programmed via the bridge's
MDB. Would that work?

On a higher level, I really wish we did not have to re-implement a lot
of identical or similar logic in each switch drivers and had a more
central model of what is behaviorally expected.

[snip]

> +int dsa_port_bridge_mc_disabled(const struct dsa_port *dp, bool mc_disabled,
> +				struct switchdev_trans *trans)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +	int port = dp->index;
> +
> +	if (switchdev_trans_ph_prepare(trans)) {
> +		if (!ds->ops->port_egress_floods)
> +			return -EOPNOTSUPP;
> +
> +		return 0;
> +	}
> +
> +	/* When multicast snooping is disabled,
> +	 * every multicast packet should be flooded to the CPU port.
> +         */

The comment alignment is a bit off.
-- 
Florian