| Message ID | 20220316150857.2442916-2-tobias@waldekranz.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ec7328b59176227216c461601c6bd0e922232a9b |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: bridge: Multiple Spanning Trees | expand |
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().
On Mon, Jan 09, 2023 at 12:02:36PM +0200, Vladimir Oltean wrote: > 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(). OK, thanks for confirming. Will send a patch later this week if Tobias won't take care of it by then. First patch will probably be [1] to make sure we dump the correct MST state to user space. It will also make it easier to show the problem and validate the fix. [1] diff --git a/net/bridge/br.c b/net/bridge/br.c index 4f5098d33a46..f02a1ad589de 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) case BR_BOOLOPT_MCAST_VLAN_SNOOPING: return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); case BR_BOOLOPT_MST_ENABLE: - return br_opt_get(br, BROPT_MST_ENABLED); + return br_mst_is_enabled(br); default: /* shouldn't be called with unsupported options */ WARN_ON(1); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 75aff9bbf17e..7f0475f62d45 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1827,7 +1827,7 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) /* br_mst.c */ #ifdef CONFIG_BRIDGE_VLAN_FILTERING DECLARE_STATIC_KEY_FALSE(br_mst_used); -static inline bool br_mst_is_enabled(struct net_bridge *br) +static inline bool br_mst_is_enabled(const struct net_bridge *br) { return static_branch_unlikely(&br_mst_used) && br_opt_get(br, BROPT_MST_ENABLED); @@ -1845,7 +1845,7 @@ int br_mst_fill_info(struct sk_buff *skb, int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr, struct netlink_ext_ack *extack); #else -static inline bool br_mst_is_enabled(struct net_bridge *br) +static inline bool br_mst_is_enabled(const struct net_bridge *br) { return false; }
On 09/01/2023 13:43, Ido Schimmel wrote: > On Mon, Jan 09, 2023 at 12:02:36PM +0200, Vladimir Oltean wrote: >> 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(). > > OK, thanks for confirming. Will send a patch later this week if Tobias > won't take care of it by then. First patch will probably be [1] to make > sure we dump the correct MST state to user space. It will also make it > easier to show the problem and validate the fix. > > [1] > diff --git a/net/bridge/br.c b/net/bridge/br.c > index 4f5098d33a46..f02a1ad589de 100644 > --- a/net/bridge/br.c > +++ b/net/bridge/br.c > @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) > case BR_BOOLOPT_MCAST_VLAN_SNOOPING: > return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); > case BR_BOOLOPT_MST_ENABLE: > - return br_opt_get(br, BROPT_MST_ENABLED); > + return br_mst_is_enabled(br); > default: > /* shouldn't be called with unsupported options */ > WARN_ON(1); > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 75aff9bbf17e..7f0475f62d45 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -1827,7 +1827,7 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) > /* br_mst.c */ > #ifdef CONFIG_BRIDGE_VLAN_FILTERING > DECLARE_STATIC_KEY_FALSE(br_mst_used); > -static inline bool br_mst_is_enabled(struct net_bridge *br) > +static inline bool br_mst_is_enabled(const struct net_bridge *br) > { > return static_branch_unlikely(&br_mst_used) && > br_opt_get(br, BROPT_MST_ENABLED); > @@ -1845,7 +1845,7 @@ int br_mst_fill_info(struct sk_buff *skb, > int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr, > struct netlink_ext_ack *extack); > #else > -static inline bool br_mst_is_enabled(struct net_bridge *br) > +static inline bool br_mst_is_enabled(const struct net_bridge *br) > { > return false; > } Ack, good catch. This should've been _inc/_dec indeed. Thanks, Nik
On Mon, Jan 09, 2023 at 01:43:46PM +0200, Ido Schimmel wrote: > OK, thanks for confirming. Will send a patch later this week if Tobias > won't take care of it by then. First patch will probably be [1] to make > sure we dump the correct MST state to user space. It will also make it > easier to show the problem and validate the fix. > > [1] > diff --git a/net/bridge/br.c b/net/bridge/br.c > index 4f5098d33a46..f02a1ad589de 100644 > --- a/net/bridge/br.c > +++ b/net/bridge/br.c > @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) > case BR_BOOLOPT_MCAST_VLAN_SNOOPING: > return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); > case BR_BOOLOPT_MST_ENABLE: > - return br_opt_get(br, BROPT_MST_ENABLED); > + return br_mst_is_enabled(br); Well, this did report the correct MST state despite the incorrect static branch state, no? The users of br_mst_is_enabled(br) are broken, not those of br_opt_get(br, BROPT_MST_ENABLED). Anyway, I see there's a br_mst_is_enabled() and also a br_mst_enabled()?! One is used in the fast path and the other in the slow path. They should probably be merged, I guess. They both exist probably because somebody thought that the "if (!netif_is_bridge_master(dev))" test is redundant in the fast path. > default: > /* shouldn't be called with unsupported options */ > WARN_ON(1); > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 75aff9bbf17e..7f0475f62d45 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -1827,7 +1827,7 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) > /* br_mst.c */ > #ifdef CONFIG_BRIDGE_VLAN_FILTERING > DECLARE_STATIC_KEY_FALSE(br_mst_used); > -static inline bool br_mst_is_enabled(struct net_bridge *br) > +static inline bool br_mst_is_enabled(const struct net_bridge *br) > { > return static_branch_unlikely(&br_mst_used) && > br_opt_get(br, BROPT_MST_ENABLED); > @@ -1845,7 +1845,7 @@ int br_mst_fill_info(struct sk_buff *skb, > int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr, > struct netlink_ext_ack *extack); > #else > -static inline bool br_mst_is_enabled(struct net_bridge *br) > +static inline bool br_mst_is_enabled(const struct net_bridge *br) > { > return false; > }
On Mon, Jan 09, 2023 at 01:56:53PM +0200, Vladimir Oltean wrote: > On Mon, Jan 09, 2023 at 01:43:46PM +0200, Ido Schimmel wrote: > > OK, thanks for confirming. Will send a patch later this week if Tobias > > won't take care of it by then. First patch will probably be [1] to make > > sure we dump the correct MST state to user space. It will also make it > > easier to show the problem and validate the fix. > > > > [1] > > diff --git a/net/bridge/br.c b/net/bridge/br.c > > index 4f5098d33a46..f02a1ad589de 100644 > > --- a/net/bridge/br.c > > +++ b/net/bridge/br.c > > @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) > > case BR_BOOLOPT_MCAST_VLAN_SNOOPING: > > return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); > > case BR_BOOLOPT_MST_ENABLE: > > - return br_opt_get(br, BROPT_MST_ENABLED); > > + return br_mst_is_enabled(br); > > Well, this did report the correct MST state despite the incorrect static > branch state, no? The users of br_mst_is_enabled(br) are broken, not > those of br_opt_get(br, BROPT_MST_ENABLED). I should have said "actual"/"effective" instead of "correct". IMO, it's better to use the same conditional in the both the data and control paths to eliminate discrepancies. Without the patch, a user will see that MST is supposedly enabled when it is actually disabled in the data path. > > Anyway, I see there's a br_mst_is_enabled() and also a br_mst_enabled()?! > One is used in the fast path and the other in the slow path. They should > probably be merged, I guess. They both exist probably because somebody > thought that the "if (!netif_is_bridge_master(dev))" test is redundant > in the fast path. The single user of br_mst_enabled() (DSA) is not affected by the bug (only the SW data path is), so I suggest making this consolidation in net-next after the bug is fixed. OK? > > > default: > > /* shouldn't be called with unsupported options */ > > WARN_ON(1); > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > > index 75aff9bbf17e..7f0475f62d45 100644 > > --- a/net/bridge/br_private.h > > +++ b/net/bridge/br_private.h > > @@ -1827,7 +1827,7 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) > > /* br_mst.c */ > > #ifdef CONFIG_BRIDGE_VLAN_FILTERING > > DECLARE_STATIC_KEY_FALSE(br_mst_used); > > -static inline bool br_mst_is_enabled(struct net_bridge *br) > > +static inline bool br_mst_is_enabled(const struct net_bridge *br) > > { > > return static_branch_unlikely(&br_mst_used) && > > br_opt_get(br, BROPT_MST_ENABLED); > > @@ -1845,7 +1845,7 @@ int br_mst_fill_info(struct sk_buff *skb, > > int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr, > > struct netlink_ext_ack *extack); > > #else > > -static inline bool br_mst_is_enabled(struct net_bridge *br) > > +static inline bool br_mst_is_enabled(const struct net_bridge *br) > > { > > return false; > > }
On Mon, Jan 09, 2023 at 02:20:02PM +0200, Ido Schimmel wrote: > On Mon, Jan 09, 2023 at 01:56:53PM +0200, Vladimir Oltean wrote: > > On Mon, Jan 09, 2023 at 01:43:46PM +0200, Ido Schimmel wrote: > > > OK, thanks for confirming. Will send a patch later this week if Tobias > > > won't take care of it by then. First patch will probably be [1] to make > > > sure we dump the correct MST state to user space. It will also make it > > > easier to show the problem and validate the fix. > > > > > > [1] > > > diff --git a/net/bridge/br.c b/net/bridge/br.c > > > index 4f5098d33a46..f02a1ad589de 100644 > > > --- a/net/bridge/br.c > > > +++ b/net/bridge/br.c > > > @@ -286,7 +286,7 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) > > > case BR_BOOLOPT_MCAST_VLAN_SNOOPING: > > > return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); > > > case BR_BOOLOPT_MST_ENABLE: > > > - return br_opt_get(br, BROPT_MST_ENABLED); > > > + return br_mst_is_enabled(br); > > > > Well, this did report the correct MST state despite the incorrect static > > branch state, no? The users of br_mst_is_enabled(br) are broken, not > > those of br_opt_get(br, BROPT_MST_ENABLED). > > I should have said "actual"/"effective" instead of "correct". IMO, it's > better to use the same conditional in the both the data and control > paths to eliminate discrepancies. Without the patch, a user will see > that MST is supposedly enabled when it is actually disabled in the data > path. The discussion is about to get philosophical, but I don't know if it's necessary to make a bug more widespread before fixing it.. The br_mst_used is an optimization to avoid calling br_opt_get() when surely MST is not enabled. There should be no discrepancy between using and not using it, if the static branch works correctly (not the case here). I would also expect that consolidation to be part of net-next though. > > Anyway, I see there's a br_mst_is_enabled() and also a br_mst_enabled()?! > > One is used in the fast path and the other in the slow path. They should > > probably be merged, I guess. They both exist probably because somebody > > thought that the "if (!netif_is_bridge_master(dev))" test is redundant > > in the fast path. > > The single user of br_mst_enabled() (DSA) is not affected by the bug > (only the SW data path is), so I suggest making this consolidation in > net-next after the bug is fixed. OK? Yes, net-next, sure.
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h index 2711c3522010..30a242195ced 100644 --- a/include/uapi/linux/if_bridge.h +++ b/include/uapi/linux/if_bridge.h @@ -759,6 +759,7 @@ struct br_mcast_stats { enum br_boolopt_id { BR_BOOLOPT_NO_LL_LEARN, BR_BOOLOPT_MCAST_VLAN_SNOOPING, + BR_BOOLOPT_MST_ENABLE, BR_BOOLOPT_MAX }; diff --git a/net/bridge/Makefile b/net/bridge/Makefile index 7fb9a021873b..24bd1c0a9a5a 100644 --- a/net/bridge/Makefile +++ b/net/bridge/Makefile @@ -20,7 +20,7 @@ obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o br_multicast_eht.o -bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o br_vlan_tunnel.o br_vlan_options.o +bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o br_vlan_tunnel.o br_vlan_options.o br_mst.o bridge-$(CONFIG_NET_SWITCHDEV) += br_switchdev.o diff --git a/net/bridge/br.c b/net/bridge/br.c index b1dea3febeea..96e91d69a9a8 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -265,6 +265,9 @@ int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on, case BR_BOOLOPT_MCAST_VLAN_SNOOPING: err = br_multicast_toggle_vlan_snooping(br, on, extack); break; + case BR_BOOLOPT_MST_ENABLE: + err = br_mst_set_enabled(br, on, extack); + break; default: /* shouldn't be called with unsupported options */ WARN_ON(1); @@ -281,6 +284,8 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt) return br_opt_get(br, BROPT_NO_LL_LEARN); case BR_BOOLOPT_MCAST_VLAN_SNOOPING: return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED); + case BR_BOOLOPT_MST_ENABLE: + return br_opt_get(br, BROPT_MST_ENABLED); default: /* shouldn't be called with unsupported options */ WARN_ON(1); diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index e0c13fcc50ed..196417859c4a 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -78,13 +78,22 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb u16 vid = 0; u8 state; - if (!p || p->state == BR_STATE_DISABLED) + if (!p) goto drop; br = p->br; + + if (br_mst_is_enabled(br)) { + state = BR_STATE_FORWARDING; + } else { + if (p->state == BR_STATE_DISABLED) + goto drop; + + state = p->state; + } + brmctx = &p->br->multicast_ctx; pmctx = &p->multicast_ctx; - state = p->state; if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, &vid, &state, &vlan)) goto out; @@ -370,9 +379,13 @@ static rx_handler_result_t br_handle_frame(struct sk_buff **pskb) return RX_HANDLER_PASS; forward: + if (br_mst_is_enabled(p->br)) + goto defer_stp_filtering; + switch (p->state) { case BR_STATE_FORWARDING: case BR_STATE_LEARNING: +defer_stp_filtering: if (ether_addr_equal(p->br->dev->dev_addr, dest)) skb->pkt_type = PACKET_HOST; diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c new file mode 100644 index 000000000000..0f9f596f86bc --- /dev/null +++ b/net/bridge/br_mst.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Bridge Multiple Spanning Tree Support + * + * Authors: + * Tobias Waldekranz <tobias@waldekranz.com> + */ + +#include <linux/kernel.h> + +#include "br_private.h" + +DEFINE_STATIC_KEY_FALSE(br_mst_used); + +static void br_mst_vlan_set_state(struct net_bridge_port *p, struct net_bridge_vlan *v, + u8 state) +{ + struct net_bridge_vlan_group *vg = nbp_vlan_group(p); + + if (v->state == state) + return; + + br_vlan_set_state(v, state); + + if (v->vid == vg->pvid) + br_vlan_set_pvid_state(vg, state); +} + +int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state, + struct netlink_ext_ack *extack) +{ + struct net_bridge_vlan_group *vg; + struct net_bridge_vlan *v; + + vg = nbp_vlan_group(p); + if (!vg) + return 0; + + list_for_each_entry(v, &vg->vlan_list, vlist) { + if (v->brvlan->msti != msti) + continue; + + br_mst_vlan_set_state(p, v, state); + } + + return 0; +} + +void br_mst_vlan_init_state(struct net_bridge_vlan *v) +{ + /* VLANs always start out in MSTI 0 (CST) */ + v->msti = 0; + + if (br_vlan_is_master(v)) + v->state = BR_STATE_FORWARDING; + else + v->state = v->port->state; +} + +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; +} diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 48bc61ebc211..c2190c8841fb 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -178,6 +178,7 @@ enum { * @br_mcast_ctx: if MASTER flag set, this is the global vlan multicast context * @port_mcast_ctx: if MASTER flag unset, this is the per-port/vlan multicast * context + * @msti: if MASTER flag set, this holds the VLANs MST instance * @vlist: sorted list of VLAN entries * @rcu: used for entry destruction * @@ -210,6 +211,8 @@ struct net_bridge_vlan { struct net_bridge_mcast_port port_mcast_ctx; }; + u16 msti; + struct list_head vlist; struct rcu_head rcu; @@ -445,6 +448,7 @@ enum net_bridge_opts { BROPT_NO_LL_LEARN, BROPT_VLAN_BRIDGE_BINDING, BROPT_MCAST_VLAN_SNOOPING_ENABLED, + BROPT_MST_ENABLED, }; struct net_bridge { @@ -1765,6 +1769,39 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow) } #endif +/* br_mst.c */ +#ifdef CONFIG_BRIDGE_VLAN_FILTERING +DECLARE_STATIC_KEY_FALSE(br_mst_used); +static inline bool br_mst_is_enabled(struct net_bridge *br) +{ + return static_branch_unlikely(&br_mst_used) && + br_opt_get(br, BROPT_MST_ENABLED); +} + +int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state, + struct netlink_ext_ack *extack); +void br_mst_vlan_init_state(struct net_bridge_vlan *v); +int br_mst_set_enabled(struct net_bridge *br, bool on, + struct netlink_ext_ack *extack); +#else +static inline bool br_mst_is_enabled(struct net_bridge *br) +{ + return false; +} + +static inline int br_mst_set_state(struct net_bridge_port *p, u16 msti, + u8 state, struct netlink_ext_ack *extack) +{ + return -EOPNOTSUPP; +} + +static inline int br_mst_set_enabled(struct net_bridge *br, bool on, + struct netlink_ext_ack *extack) +{ + return -EOPNOTSUPP; +} +#endif + struct nf_br_ops { int (*br_dev_xmit_hook)(struct sk_buff *skb); }; diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c index 1d80f34a139c..7d27b2e6038f 100644 --- a/net/bridge/br_stp.c +++ b/net/bridge/br_stp.c @@ -43,6 +43,12 @@ void br_set_state(struct net_bridge_port *p, unsigned int state) return; p->state = state; + if (br_opt_get(p->br, BROPT_MST_ENABLED)) { + err = br_mst_set_state(p, 0, state, NULL); + if (err) + br_warn(p->br, "error setting MST state on port %u(%s)\n", + p->port_no, netdev_name(p->dev)); + } err = switchdev_port_attr_set(p->dev, &attr, NULL); if (err && err != -EOPNOTSUPP) br_warn(p->br, "error setting offload STP state on port %u(%s)\n", diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 7557e90b60e1..0f5e75ccac79 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -226,6 +226,24 @@ static void nbp_vlan_rcu_free(struct rcu_head *rcu) kfree(v); } +static void br_vlan_init_state(struct net_bridge_vlan *v) +{ + struct net_bridge *br; + + if (br_vlan_is_master(v)) + br = v->br; + else + br = v->port->br; + + if (br_opt_get(br, BROPT_MST_ENABLED)) { + br_mst_vlan_init_state(v); + return; + } + + v->state = BR_STATE_FORWARDING; + v->msti = 0; +} + /* This is the shared VLAN add function which works for both ports and bridge * devices. There are four possible calls to this function in terms of the * vlan entry type: @@ -322,7 +340,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags, } /* set the state before publishing */ - v->state = BR_STATE_FORWARDING; + br_vlan_init_state(v); err = rhashtable_lookup_insert_fast(&vg->vlan_hash, &v->vnode, br_vlan_rht_params); diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c index a6382973b3e7..09112b56e79c 100644 --- a/net/bridge/br_vlan_options.c +++ b/net/bridge/br_vlan_options.c @@ -99,6 +99,11 @@ static int br_vlan_modify_state(struct net_bridge_vlan_group *vg, return -EBUSY; } + if (br_opt_get(br, BROPT_MST_ENABLED)) { + NL_SET_ERR_MSG_MOD(extack, "Can't modify vlan state directly when MST is enabled"); + return -EBUSY; + } + if (v->state == state) return 0;