Re: [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode

Ido Schimmel
2023-01-09T08:06:47Z

On Wed, Mar 16, 2022 at 04:08:43PM +0100, Tobias Waldekranz wrote:
> +DEFINE_STATIC_KEY_FALSE(br_mst_used);

[...]

> +int br_mst_set_enabled(struct net_bridge *br, bool on,
> + struct netlink_ext_ack *extack)
> +{
> + struct net_bridge_vlan_group *vg;
> + struct net_bridge_port *p;
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + vg = nbp_vlan_group(p);
> +
> + if (!vg->num_vlans)
> + continue;
> +
> + NL_SET_ERR_MSG(extack,
> + "MST mode can't be changed while VLANs exist");
> + return -EBUSY;
> + }
> +
> + if (br_opt_get(br, BROPT_MST_ENABLED) == on)
> + return 0;
> +
> + if (on)
> + static_branch_enable(&br_mst_used);
> + else
> + static_branch_disable(&br_mst_used);
> +
> + br_opt_toggle(br, BROPT_MST_ENABLED, on);
> + return 0;
> +}

Hi,

I'm not actually using MST, but I ran into this code and was wondering
if the static key usage is correct. The static key is global (not
per-bridge), so what happens when two bridges have MST enabled and then
it is disabled on one? I believe it would be disabled for both. If so,
maybe use static_branch_inc() / static_branch_dec() instead?

Thanks