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: <20190623070949.GB13466@splinter>
Date:   Sun, 23 Jun 2019 07:09:52 +0000
From:   Ido Schimmel <idosch@...lanox.com>
To:     Vivien Didelot <vivien.didelot@...il.com>
CC:     Florian Fainelli <f.fainelli@...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 Fri, Jun 21, 2019 at 05:29:52PM -0400, Vivien Didelot wrote:
> On Thu, 20 Jun 2019 19:24:47 -0700, Florian Fainelli <f.fainelli@...il.com> 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...
> 
> Ouch, doesn't sound like what a driver should be doing :-(
> 
> Ido, I cannot find documentation for multicast_snooping or MC_DISABLED
> and the naming isn't clear. Can this be considered as an equivalent
> of mcast_flood but targeting the bridge device itself, describing
> whether the bridge is interested or not in unknown multicast traffic?

Not really, you're only looking at one aspect of this, but there is
more. For example, when multicast snooping is disabled, the MDB is not
considered.

> > 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?
> 
> You may want the machine or network connected behind a bridge port
> to be flooded with unknown multicast traffic, without having the
> CPU conduit clogged up with this traffic. So these are two distinct
> settings for me.
> 
> The only scenario I can think of needing the CPU to be flooded is if
> there's a non-DSA port in the bridge maybe. But IMHO this should be
> handled by the bridge, offloading or not the appropriate attribute.
> 
> > 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.
> 
> I couldn't agree more, ethernet switch drivers should only offload
> the notified bridge configuration, not noodling their own logic...

Please see my comment to Florian. The driver is doing what needs to be
done in order to comply with the behavior of the bridge driver. As I
wrote to Florian, we can probably move some logic up the stack and
simplify drivers.

> Russell, Ido, Florian, so far I understand that a multicast-unaware
> bridge must flood unknown traffic everywhere (CPU included);
> and a multicast-aware bridge must only flood its ports if their
> mcast_flood is on, and known traffic targeting the bridge must be
> offloaded accordingly. Do you guys agree with this?

When multicast snooping is enabled unregistered multicast traffic should
only be flooded to mrouter ports.