Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute

Florian Fainelli
2019-06-21T02:24:53Z

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