On Mon, Apr 01, 2024 at 08:11:08PM -0400, Joseph Huang wrote: > When a port turns into an mrouter port, enable multicast flooding > on that port even if multicast flooding is disabled by user config. This > is necessary so that in a distributed system, the multicast packets > can be forwarded to the Querier when the multicast source is attached > to a Non-Querier bridge. > > Consider the following scenario: > > +--------------------+ > | | > | Snooping | +------------+ > | Bridge 1 |----| Listener 1 | > | (Querier) | +------------+ > | | > +--------------------+ > | > | > +--------------------+ > | | mrouter | | > +-----------+ | +---------+ | > | MC Source |----| Snooping | > +-----------| | Bridge 2 | > | (Non-Querier) | > +--------------------+ > > In this scenario, Listener 1 will never receive multicast traffic > from MC Source if multicast flooding is disabled on the mrouter port on > Snooping Bridge 2. > > Signed-off-by: Joseph Huang <Joseph.Huang@xxxxxxxxxx> ... > @@ -6849,11 +6864,28 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port, > > if (flags.mask & BR_MCAST_FLOOD) { > bool multicast = !!(flags.val & BR_MCAST_FLOOD); > + struct mv88e6xxx_bridge *mv_bridge; > + struct mv88e6xxx_port *p; > + bool mrouter; > > - err = chip->info->ops->port_set_mcast_flood(chip, port, > - multicast); > - if (err) > - goto out; > + mv_bridge = mv88e6xxx_bridge_by_port(chip, port); > + if (!mv_bridge) > + return -EINVAL; I think that mv88e6xxx_reg_unlock(chip) is needed here. So perhaps (completely untested!): if (!mv_bridge) { err = -EINVAL; goto out; } Flagged by Smatch > + > + p = &chip->ports[port]; > + mrouter = !!(mv_bridge->mrouter_ports & BIT(port)); > + > + if (!mrouter) { > + err = chip->info->ops->port_set_mcast_flood(chip, port, > + multicast); > + if (err) > + goto out; > + } > + > + if (multicast) > + p->flags |= MV88E6XXX_PORT_FLAG_MC_FLOOD; > + else > + p->flags &= ~MV88E6XXX_PORT_FLAG_MC_FLOOD; > } > > if (flags.mask & BR_BCAST_FLOOD) { > @@ -6883,6 +6915,51 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port, > return err; > } > > +static int mv88e6xxx_port_mrouter(struct dsa_switch *ds, int port, > + bool mrouter, > + struct netlink_ext_ack *extack) > +{ > + struct mv88e6xxx_chip *chip = ds->priv; > + struct mv88e6xxx_bridge *mv_bridge; > + struct mv88e6xxx_port *p; > + bool old_mrouter; > + bool mc_flood; > + int err; > + > + if (!chip->info->ops->port_set_mcast_flood) > + return -EOPNOTSUPP; > + > + mv_bridge = mv88e6xxx_bridge_by_port(chip, port); > + if (!mv_bridge) > + return -EINVAL; > + > + old_mrouter = !!(mv_bridge->mrouter_ports & BIT(port)); > + if (mrouter == old_mrouter) > + return 0; > + > + p = &chip->ports[port]; > + mc_flood = !!(p->flags & MV88E6XXX_PORT_FLAG_MC_FLOOD); > + > + mv88e6xxx_reg_lock(chip); > + > + if (!mc_flood) { > + err = chip->info->ops->port_set_mcast_flood(chip, port, > + mrouter); > + if (err) > + goto out; > + } > + > + if (mrouter) > + mv_bridge->mrouter_ports |= BIT(port); > + else > + mv_bridge->mrouter_ports &= ~BIT(port); > + > +out: > + mv88e6xxx_reg_unlock(chip); If mc_flood is true then err is uninitialised here. Flagged by clang-17 W=1 build, and Smatch. > + > + return err; > +} > + > static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds, > struct dsa_lag lag, > struct netdev_lag_upper_info *info, ...