2022-03-14 16:38:43

by Tobias Waldekranz

[permalink] [raw]
Subject: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode

Allow the user to switch from the current per-VLAN STP mode to an MST
mode.

Up to this point, per-VLAN STP states where always isolated from each
other. This is in contrast to the MSTP standard (802.1Q-2018, Clause
13.5), where VLANs are grouped into MST instances (MSTIs), and the
state is managed on a per-MSTI level, rather that at the per-VLAN
level.

Perhaps due to the prevalence of the standard, many switching ASICs
are built after the same model. Therefore, add a corresponding MST
mode to the bridge, which we can later add offloading support for in a
straight-forward way.

For now, all VLANs are fixed to MSTI 0, also called the Common
Spanning Tree (CST). That is, all VLANs will follow the port-global
state.

Upcoming changes will make this actually useful by allowing VLANs to
be mapped to arbitrary MSTIs and allow individual MSTI states to be
changed.

Signed-off-by: Tobias Waldekranz <[email protected]>
---
include/uapi/linux/if_bridge.h | 1 +
net/bridge/Makefile | 2 +-
net/bridge/br.c | 5 ++
net/bridge/br_input.c | 17 ++++++-
net/bridge/br_mst.c | 84 ++++++++++++++++++++++++++++++++++
net/bridge/br_private.h | 27 +++++++++++
net/bridge/br_stp.c | 3 ++
net/bridge/br_vlan.c | 20 +++++++-
net/bridge/br_vlan_options.c | 5 ++
9 files changed, 160 insertions(+), 4 deletions(-)
create mode 100644 net/bridge/br_mst.c

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..e1ec9d39c660
--- /dev/null
+++ b/net/bridge/br_mst.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Bridge Multiple Spanning Tree Support
+ *
+ * Authors:
+ * Tobias Waldekranz <[email protected]>
+ */
+
+#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);
+}
+
+void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state)
+{
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_vlan *v;
+
+ vg = nbp_vlan_group(p);
+ if (!vg)
+ return;
+
+ list_for_each_entry(v, &vg->vlan_list, vlist) {
+ if (v->brvlan->msti != msti)
+ continue;
+
+ br_mst_vlan_set_state(p, v, state);
+ }
+}
+
+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..35b47f6b449a 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,29 @@ 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);
+}
+
+void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state);
+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 void br_mst_set_state(struct net_bridge_port *p,
+ u16 msti, u8 state) {}
+#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..82a97a021a57 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -43,6 +43,9 @@ 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))
+ br_mst_set_state(p, 0, state);
+
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;

--
2.25.1


2022-03-14 16:39:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode

Hi Tobias,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Tobias-Waldekranz/net-bridge-Multiple-Spanning-Trees/20220314-175717
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git de29aff976d3216e7f3ab41fcd7af46fa8f7eab7
config: xtensa-randconfig-m031-20220313 (https://download.01.org/0day-ci/archive/20220314/[email protected]/config)
compiler: xtensa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/702c502efb27c12860bc55fc8d9b1bfd99466623
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tobias-Waldekranz/net-bridge-Multiple-Spanning-Trees/20220314-175717
git checkout 702c502efb27c12860bc55fc8d9b1bfd99466623
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash net/bridge/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

net/bridge/br.c: In function 'br_boolopt_toggle':
>> net/bridge/br.c:269:23: error: implicit declaration of function 'br_mst_set_enabled'; did you mean 'br_stp_set_enabled'? [-Werror=implicit-function-declaration]
269 | err = br_mst_set_enabled(br, on, extack);
| ^~~~~~~~~~~~~~~~~~
| br_stp_set_enabled
cc1: some warnings being treated as errors


vim +269 net/bridge/br.c

245
246 /* br_boolopt_toggle - change user-controlled boolean option
247 *
248 * @br: bridge device
249 * @opt: id of the option to change
250 * @on: new option value
251 * @extack: extack for error messages
252 *
253 * Changes the value of the respective boolean option to @on taking care of
254 * any internal option value mapping and configuration.
255 */
256 int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on,
257 struct netlink_ext_ack *extack)
258 {
259 int err = 0;
260
261 switch (opt) {
262 case BR_BOOLOPT_NO_LL_LEARN:
263 br_opt_toggle(br, BROPT_NO_LL_LEARN, on);
264 break;
265 case BR_BOOLOPT_MCAST_VLAN_SNOOPING:
266 err = br_multicast_toggle_vlan_snooping(br, on, extack);
267 break;
268 case BR_BOOLOPT_MST_ENABLE:
> 269 err = br_mst_set_enabled(br, on, extack);
270 break;
271 default:
272 /* shouldn't be called with unsupported options */
273 WARN_ON(1);
274 break;
275 }
276
277 return err;
278 }
279

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/[email protected]

2022-03-14 23:46:34

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode

Hi Tobias,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Tobias-Waldekranz/net-bridge-Multiple-Spanning-Trees/20220314-175717
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git de29aff976d3216e7f3ab41fcd7af46fa8f7eab7
config: hexagon-randconfig-r041-20220314 (https://download.01.org/0day-ci/archive/20220315/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3e4950d7fa78ac83f33bbf1658e2f49a73719236)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/702c502efb27c12860bc55fc8d9b1bfd99466623
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tobias-Waldekranz/net-bridge-Multiple-Spanning-Trees/20220314-175717
git checkout 702c502efb27c12860bc55fc8d9b1bfd99466623
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/bridge/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> net/bridge/br.c:269:9: error: implicit declaration of function 'br_mst_set_enabled' [-Werror,-Wimplicit-function-declaration]
err = br_mst_set_enabled(br, on, extack);
^
net/bridge/br.c:269:9: note: did you mean 'br_stp_set_enabled'?
net/bridge/br_private.h:1828:5: note: 'br_stp_set_enabled' declared here
int br_stp_set_enabled(struct net_bridge *br, unsigned long val,
^
1 error generated.


vim +/br_mst_set_enabled +269 net/bridge/br.c

245
246 /* br_boolopt_toggle - change user-controlled boolean option
247 *
248 * @br: bridge device
249 * @opt: id of the option to change
250 * @on: new option value
251 * @extack: extack for error messages
252 *
253 * Changes the value of the respective boolean option to @on taking care of
254 * any internal option value mapping and configuration.
255 */
256 int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on,
257 struct netlink_ext_ack *extack)
258 {
259 int err = 0;
260
261 switch (opt) {
262 case BR_BOOLOPT_NO_LL_LEARN:
263 br_opt_toggle(br, BROPT_NO_LL_LEARN, on);
264 break;
265 case BR_BOOLOPT_MCAST_VLAN_SNOOPING:
266 err = br_multicast_toggle_vlan_snooping(br, on, extack);
267 break;
268 case BR_BOOLOPT_MST_ENABLE:
> 269 err = br_mst_set_enabled(br, on, extack);
270 break;
271 default:
272 /* shouldn't be called with unsupported options */
273 WARN_ON(1);
274 break;
275 }
276
277 return err;
278 }
279

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/[email protected]

2022-03-16 14:44:22

by Nikolay Aleksandrov

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode

On 14/03/2022 11:52, Tobias Waldekranz wrote:
> Allow the user to switch from the current per-VLAN STP mode to an MST
> mode.
>
> Up to this point, per-VLAN STP states where always isolated from each
> other. This is in contrast to the MSTP standard (802.1Q-2018, Clause
> 13.5), where VLANs are grouped into MST instances (MSTIs), and the
> state is managed on a per-MSTI level, rather that at the per-VLAN
> level.
>
> Perhaps due to the prevalence of the standard, many switching ASICs
> are built after the same model. Therefore, add a corresponding MST
> mode to the bridge, which we can later add offloading support for in a
> straight-forward way.
>
> For now, all VLANs are fixed to MSTI 0, also called the Common
> Spanning Tree (CST). That is, all VLANs will follow the port-global
> state.
>
> Upcoming changes will make this actually useful by allowing VLANs to
> be mapped to arbitrary MSTIs and allow individual MSTI states to be
> changed.
>
> Signed-off-by: Tobias Waldekranz <[email protected]>
> ---
[snip]
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 48bc61ebc211..35b47f6b449a 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,29 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
> }
> #endif
>
> +/* br_mst.c */
> +#ifdef CONFIG_BRIDGE_VLAN_FILTERING

There is already such ifdef, you can embed all MST code inside it.

> +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);
> +}
> +
> +void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state);
> +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 void br_mst_set_state(struct net_bridge_port *p,
> + u16 msti, u8 state) {}
> +#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..82a97a021a57 100644
> --- a/net/bridge/br_stp.c
> +++ b/net/bridge/br_stp.c
> @@ -43,6 +43,9 @@ 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))
> + br_mst_set_state(p, 0, state);
> +
> 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;
>

2022-03-17 06:39:08

by Nikolay Aleksandrov

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode

On 14/03/2022 12:37, Nikolay Aleksandrov wrote:
> On 14/03/2022 11:52, Tobias Waldekranz wrote:
>> Allow the user to switch from the current per-VLAN STP mode to an MST
>> mode.
>>
>> Up to this point, per-VLAN STP states where always isolated from each
>> other. This is in contrast to the MSTP standard (802.1Q-2018, Clause
>> 13.5), where VLANs are grouped into MST instances (MSTIs), and the
>> state is managed on a per-MSTI level, rather that at the per-VLAN
>> level.
>>
>> Perhaps due to the prevalence of the standard, many switching ASICs
>> are built after the same model. Therefore, add a corresponding MST
>> mode to the bridge, which we can later add offloading support for in a
>> straight-forward way.
>>
>> For now, all VLANs are fixed to MSTI 0, also called the Common
>> Spanning Tree (CST). That is, all VLANs will follow the port-global
>> state.
>>
>> Upcoming changes will make this actually useful by allowing VLANs to
>> be mapped to arbitrary MSTIs and allow individual MSTI states to be
>> changed.
>>
>> Signed-off-by: Tobias Waldekranz <[email protected]>
>> ---
> [snip]
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 48bc61ebc211..35b47f6b449a 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,29 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
>> }
>> #endif
>>
>> +/* br_mst.c */
>> +#ifdef CONFIG_BRIDGE_VLAN_FILTERING
>
> There is already such ifdef, you can embed all MST code inside it.
>

My bad, I somehow confused the function placement. This is good. :)

Acked-by: Nikolay Aleksandrov <[email protected]>