Re: [PATCH net-next v2 2/3] net: dsa: mv88e6xxx: add support for bridge flags

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Tue, Feb 19, 2019 at 02:56:27PM -0500, Vivien Didelot wrote:
> Hi Russell,
> 
> On Tue, 19 Feb 2019 19:10:16 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > > True, let's stick with ops->port_egress_flood(ds, port, bool uc, bool mc).
> > > I do not think that it is necessary to add support for BR_BCAST_FLOOD yet,
> > > we can extend this routine later if we need to.
> > > 
> > > Your dsa_port_bridge_flags() core function can notify the understood
> > > features. This will allow us to scope the support of the bridge flags in
> > > the core, and preventing the drivers to do that themselves.
> > 
> > So, if we have ops->port_egress_flood, then we tell bridge that
> > we support BR_FLOOD | BR_MCAST_FLOOD, irrespective of whether the
> > bridge actually supports both?
> 
> I would say so yes. If a driver implements port_egress_flood(), this means
> its switch device supports both BR_FLOOD | BR_MCAST_FLOOD.
> 
> I have one concern though. The documentation of mcast_flood for bridge(8)
> says that this flag "controls whether a given port will *be flooded* with
> [unknown] multicast traffic". From this I understand allowing this port to
> *receive* frames with unknown destination addresses. But with mv88e6xxx, we
> program whether the port is allowed to egress a frame that has an unknown
> destination address. Otherwise, it will not go out this port.
> 
> Am I mistaken? If I understood correctly, is it safe to assume it is the
> same thing we are implementing here?

Please look at the net/bridge code to resolve questions such as this.
The relevant code is net/bridge/br_forward.c::br_flood():

void br_flood(struct net_bridge *br, struct sk_buff *skb,
              enum br_pkt_type pkt_type, bool local_rcv, bool local_orig)
{
...
        list_for_each_entry_rcu(p, &br->port_list, list) {
                /* Do not flood unicast traffic to ports that turn it off, nor
                 * other traffic if flood off, except for traffic we originate
                 */
                switch (pkt_type) {
                case BR_PKT_UNICAST:
                        if (!(p->flags & BR_FLOOD))
                                continue;
                        break;
                case BR_PKT_MULTICAST:
                        if (!(p->flags & BR_MCAST_FLOOD) && skb->dev != br->dev)                                continue;
                        break;
                case BR_PKT_BROADCAST:
                        if (!(p->flags & BR_BCAST_FLOOD) && skb->dev != br->dev)                                continue;
                        break;
                }
...
                prev = maybe_deliver(prev, p, skb, local_orig);
        }

So, BR_FLOOD, BR_MCAST_FLOOD and BR_BCAST_FLOOD control whether the
packet of type pkt_type being flooded on the bridge egresses from
port p, where p is each port attached to the bridge.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up