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]

Message-ID: <20190623064838.GA13466@splinter>
Date:   Sun, 23 Jun 2019 06:48:41 +0000
From:   Ido Schimmel <idosch@...lanox.com>
To:     Florian Fainelli <f.fainelli@...il.com>
CC:     Vivien Didelot <vivien.didelot@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...nulli.us>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute

On Thu, Jun 20, 2019 at 07:24:47PM -0700, Florian Fainelli wrote:
> 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...

When multicast snooping is enabled, unregistered multicast traffic
should be flooded to mrouter ports only. Otherwise, it should be flooded
to all ports.

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

Well, that model is the bridge driver... But I agree that we can
probably simplify the interface towards drivers and move more code up
the stack.

For example, two things mlxsw is doing when multicast snooping is
enabled:

1. Writing MDB entries to the device. When multicast snooping is
disabled, MDB entries are ignored by the bridge driver. Can we agree to
have the bridge driver generate SWITCHDEV_OBJ_ID_PORT_MDB add / delete
for all MDB entries when multicast snooping is toggled?

2. Flooding unregistered multicast traffic only to mrouter ports. The
bridge driver can iterate over the bridge members and toggle
BR_MCAST_FLOOD accordingly. It will not actually change this value. Only
emulate this change towards drivers.

I will try to come up with a more detailed list later this week. I
really want to start moving more logic out of drivers.