Linux Ethernet Bridging - Jan 2023 - [Bridge] [PATCH v5 net-next 01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode

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

Hi Ido,

On Mon, Jan 09, 2023 at 10:05:53AM +0200, Ido Schimmel
wrote:
> > +	if (on)
> > +		static_branch_enable(&br_mst_used);
> > +	else
> > +		static_branch_disable(&br_mst_used);
> 
> 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?
Sounds about right. FWIW, br_switchdev_tx_fwd_offload does use
static_branch_inc() / static_branch_dec().