linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees
@ 2022-03-14  9:52 Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
                   ` (13 more replies)
  0 siblings, 14 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

The bridge has had per-VLAN STP support for a while now, since:

https://lore.kernel.org/netdev/20200124114022.10883-1-nikolay@cumulusnetworks.com/

The current implementation has some problems:

- The mapping from VLAN to STP state is fixed as 1:1, i.e. each VLAN
  is managed independently. This is awkward from an MSTP (802.1Q-2018,
  Clause 13.5) point of view, where the model is that multiple VLANs
  are grouped into MST instances.

  Because of the way that the standard is written, presumably, this is
  also reflected in hardware implementations. It is not uncommon for a
  switch to support the full 4k range of VIDs, but that the pool of
  MST instances is much smaller. Some examples:

  Marvell LinkStreet (mv88e6xxx): 4k VLANs, but only 64 MSTIs
  Marvell Prestera: 4k VLANs, but only 128 MSTIs
  Microchip SparX-5i: 4k VLANs, but only 128 MSTIs

- By default, the feature is enabled, and there is no way to disable
  it. This makes it hard to add offloading in a backwards compatible
  way, since any underlying switchdevs have no way to refuse the
  function if the hardware does not support it

- The port-global STP state has precedence over per-VLAN states. In
  MSTP, as far as I understand it, all VLANs will use the common
  spanning tree (CST) by default - through traffic engineering you can
  then optimize your network to group subsets of VLANs to use
  different trees (MSTI). To my understanding, the way this is
  typically managed in silicon is roughly:

  Incoming packet:
  .----.----.--------------.----.-------------
  | DA | SA | 802.1Q VID=X | ET | Payload ...
  '----'----'--------------'----'-------------
                        |
                        '->|\     .----------------------------.
                           | +--> | VID | Members | ... | MSTI |
                   PVID -->|/     |-----|---------|-----|------|
                                  |   1 | 0001001 | ... |    0 |
                                  |   2 | 0001010 | ... |   10 |
                                  |   3 | 0001100 | ... |   10 |
                                  '----------------------------'
                                                             |
                               .-----------------------------'
                               |  .------------------------.
                               '->| MSTI | Fwding | Lrning |
                                  |------|--------|--------|
                                  |    0 | 111110 | 111110 |
                                  |   10 | 110111 | 110111 |
                                  '------------------------'

  What this is trying to show is that the STP state (whether MSTP is
  used, or ye olde STP) is always accessed via the VLAN table. If STP
  is running, all MSTI pointers in that table will reference the same
  index in the STP stable - if MSTP is running, some VLANs may point
  to other trees (like in this example).

  The fact that in the Linux bridge, the global state (think: index 0
  in most hardware implementations) is supposed to override the
  per-VLAN state, is very awkward to offload. In effect, this means
  that when the global state changes to blocking, drivers will have to
  iterate over all MSTIs in use, and alter them all to match. This
  also means that you have to cache whether the hardware state is
  currently tracking the global state or the per-VLAN state. In the
  first case, you also have to cache the per-VLAN state so that you
  can restore it if the global state transitions back to forwarding.

This series adds a new mst_enable bridge setting (as suggested by Nik)
that can only be changed when no VLANs are configured on the
bridge. Enabling this mode has the following effect:

- The port-global STP state is used to represent the CST (Common
  Spanning Tree) (1/14)

- Ingress STP filtering is deferred until the frame's VLAN has been
  resolved (1/14)

- The preexisting per-VLAN states can no longer be controlled directly
  (1/14). They are instead placed under the MST module's control,
  which is managed using a new netlink interface (described in 3/14)

- VLANs can br mapped to MSTIs in an arbitrary M:N fashion, using a
  new global VLAN option (2/14)

Switchdev notifications are added so that a driver can track:
- MST enabled state
- VID to MSTI mappings
- MST port states

An offloading implementation is this provided for mv88e6xxx.

A proposal for the corresponding iproute2 interface is available here:

https://github.com/wkz/iproute2/tree/mst

v2 -> v3:
  Bridge:
  - Use new boolopt API to enable/disable the MST mode (Nik)
  - Mark br_mst_vlan_set_state as static (Vladimir)
  - Avoid updates/notifications on repeated VLAN to MSTI mapping
    configurations (Vladimir)
  - Configure MSTI states via the existing RTM_GET/SETLINK interface
    (Roopa)
  - Refactor switchdev replay logic (Vladimir)
  - Send switchdev notifications when enabling/disabling MST
    (Vladimir)
  DSA:
  - Align VLAN MSTI callback with existing APIs (Vladimir)
  - Only flush entries in the affected VLANs when changing an MST
    state (Vladimir)
  - Refuse offloading, unless all required ops are implemented
    (Vladimir)
  mv88e6xxx:
  - Always keep the driver's MST state in sync with hardware
    (Vladimir)
  - Fix SID leaks (Vladimir)
  - Only flush entries in the affected VLANs when changing an MST
    state (Vladimir)

v1 (RFC) -> v2:
  - Add a separate MST mode that is distinct from the exiting per-VLAN
    state functionality
  - Control MSTI states explicitly, rather than via an associated VLAN

Tobias Waldekranz (14):
  net: bridge: mst: Multiple Spanning Tree (MST) mode
  net: bridge: mst: Allow changing a VLAN's MSTI
  net: bridge: mst: Support setting and reporting MST port states
  net: bridge: mst: Notify switchdev drivers of MST mode changes
  net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations
  net: bridge: mst: Notify switchdev drivers of MST state changes
  net: bridge: mst: Add helper to map an MSTI to a VID set
  net: bridge: mst: Add helper to check if MST is enabled
  net: dsa: Validate hardware support for MST
  net: dsa: Pass VLAN MSTI migration notifications to driver
  net: dsa: Handle MST state changes
  net: dsa: mv88e6xxx: Disentangle STU from VTU
  net: dsa: mv88e6xxx: Export STU as devlink region
  net: dsa: mv88e6xxx: MST Offloading

 drivers/net/dsa/mv88e6xxx/chip.c        | 305 ++++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/chip.h        |  38 +++
 drivers/net/dsa/mv88e6xxx/devlink.c     |  94 +++++++
 drivers/net/dsa/mv88e6xxx/global1.h     |  10 +
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 311 ++++++++++++++----------
 include/linux/if_bridge.h               |  11 +
 include/net/dsa.h                       |   6 +
 include/net/switchdev.h                 |  16 ++
 include/uapi/linux/if_bridge.h          |  19 ++
 include/uapi/linux/rtnetlink.h          |   1 +
 net/bridge/Makefile                     |   2 +-
 net/bridge/br.c                         |   5 +
 net/bridge/br_input.c                   |  17 +-
 net/bridge/br_mst.c                     | 310 +++++++++++++++++++++++
 net/bridge/br_netlink.c                 |  32 ++-
 net/bridge/br_private.h                 |  43 ++++
 net/bridge/br_stp.c                     |   3 +
 net/bridge/br_switchdev.c               |  46 ++++
 net/bridge/br_vlan.c                    |  20 +-
 net/bridge/br_vlan_options.c            |  20 ++
 net/dsa/dsa_priv.h                      |   6 +
 net/dsa/port.c                          |  99 +++++++-
 net/dsa/slave.c                         |  18 ++
 23 files changed, 1281 insertions(+), 151 deletions(-)
 create mode 100644 net/bridge/br_mst.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 10:37   ` Nikolay Aleksandrov
                     ` (2 more replies)
  2022-03-14  9:52 ` [PATCH v3 net-next 02/14] net: bridge: mst: Allow changing a VLAN's MSTI Tobias Waldekranz
                   ` (12 subsequent siblings)
  13 siblings, 3 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

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 <tobias@waldekranz.com>
---
 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		<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);
+}
+
+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


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 02/14] net: bridge: mst: Allow changing a VLAN's MSTI
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 10:45   ` Nikolay Aleksandrov
  2022-03-14  9:52 ` [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states Tobias Waldekranz
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Allow a VLAN to move out of the CST (MSTI 0), to an independent tree.

The user manages the VID to MSTI mappings via a global VLAN
setting. The proposed iproute2 interface would be:

    bridge vlan global set dev br0 vid <VID> msti <MSTI>

Changing the state in non-zero MSTIs is still not supported, but will
be addressed in upcoming changes.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/uapi/linux/if_bridge.h |  1 +
 net/bridge/br_mst.c            | 42 ++++++++++++++++++++++++++++++++++
 net/bridge/br_private.h        |  1 +
 net/bridge/br_vlan_options.c   | 15 ++++++++++++
 4 files changed, 59 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 30a242195ced..f60244b747ae 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -564,6 +564,7 @@ enum {
 	BRIDGE_VLANDB_GOPTS_MCAST_QUERIER,
 	BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS,
 	BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE,
+	BRIDGE_VLANDB_GOPTS_MSTI,
 	__BRIDGE_VLANDB_GOPTS_MAX
 };
 #define BRIDGE_VLANDB_GOPTS_MAX (__BRIDGE_VLANDB_GOPTS_MAX - 1)
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index e1ec9d39c660..78ef5fea4d2b 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -43,6 +43,48 @@ void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state)
 	}
 }
 
+static void br_mst_vlan_sync_state(struct net_bridge_vlan *pv, u16 msti)
+{
+	struct net_bridge_vlan_group *vg = nbp_vlan_group(pv->port);
+	struct net_bridge_vlan *v;
+
+	list_for_each_entry(v, &vg->vlan_list, vlist) {
+		/* If this port already has a defined state in this
+		 * MSTI (through some other VLAN membership), inherit
+		 * it.
+		 */
+		if (v != pv && v->brvlan->msti == msti) {
+			br_mst_vlan_set_state(pv->port, pv, v->state);
+			return;
+		}
+	}
+
+	/* Otherwise, start out in a new MSTI with all ports disabled. */
+	return br_mst_vlan_set_state(pv->port, pv, BR_STATE_DISABLED);
+}
+
+int br_mst_vlan_set_msti(struct net_bridge_vlan *mv, u16 msti)
+{
+	struct net_bridge_vlan_group *vg;
+	struct net_bridge_vlan *pv;
+	struct net_bridge_port *p;
+
+	if (mv->msti == msti)
+		return 0;
+
+	mv->msti = msti;
+
+	list_for_each_entry(p, &mv->br->port_list, list) {
+		vg = nbp_vlan_group(p);
+
+		pv = br_vlan_find(vg, mv->vid);
+		if (pv)
+			br_mst_vlan_sync_state(pv, msti);
+	}
+
+	return 0;
+}
+
 void br_mst_vlan_init_state(struct net_bridge_vlan *v)
 {
 	/* VLANs always start out in MSTI 0 (CST) */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 35b47f6b449a..b907d389b63a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1779,6 +1779,7 @@ static inline bool br_mst_is_enabled(struct net_bridge *br)
 }
 
 void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state);
+int br_mst_vlan_set_msti(struct net_bridge_vlan *v, u16 msti);
 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);
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 09112b56e79c..a2724d03278c 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -296,6 +296,7 @@ bool br_vlan_global_opts_can_enter_range(const struct net_bridge_vlan *v_curr,
 					 const struct net_bridge_vlan *r_end)
 {
 	return v_curr->vid - r_end->vid == 1 &&
+		v_curr->msti == r_end->msti &&
 	       ((v_curr->priv_flags ^ r_end->priv_flags) &
 		BR_VLFLAG_GLOBAL_MCAST_ENABLED) == 0 &&
 		br_multicast_ctx_options_equal(&v_curr->br_mcast_ctx,
@@ -384,6 +385,9 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 #endif
 #endif
 
+	if (nla_put_u16(skb, BRIDGE_VLANDB_GOPTS_MSTI, v_opts->msti))
+		goto out_err;
+
 	nla_nest_end(skb, nest);
 
 	return true;
@@ -415,6 +419,7 @@ static size_t rtnl_vlan_global_opts_nlmsg_size(const struct net_bridge_vlan *v)
 		+ nla_total_size(0) /* BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS */
 		+ br_rports_size(&v->br_mcast_ctx) /* BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS */
 #endif
+		+ nla_total_size(sizeof(u16)) /* BRIDGE_VLANDB_GOPTS_MSTI */
 		+ nla_total_size(sizeof(u16)); /* BRIDGE_VLANDB_GOPTS_RANGE */
 }
 
@@ -564,6 +569,15 @@ static int br_vlan_process_global_one_opts(const struct net_bridge *br,
 	}
 #endif
 #endif
+	if (tb[BRIDGE_VLANDB_GOPTS_MSTI]) {
+		u16 msti;
+
+		msti = nla_get_u16(tb[BRIDGE_VLANDB_GOPTS_MSTI]);
+		err = br_mst_vlan_set_msti(v, msti);
+		if (err)
+			return err;
+		*changed = true;
+	}
 
 	return 0;
 }
@@ -583,6 +597,7 @@ static const struct nla_policy br_vlan_db_gpol[BRIDGE_VLANDB_GOPTS_MAX + 1] = {
 	[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_INTVL]	= { .type = NLA_U64 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_INTVL]	= { .type = NLA_U64 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL] = { .type = NLA_U64 },
+	[BRIDGE_VLANDB_GOPTS_MSTI] = NLA_POLICY_MAX(NLA_U16, VLAN_N_VID - 1),
 };
 
 int br_vlan_rtm_process_global_options(struct net_device *dev,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 02/14] net: bridge: mst: Allow changing a VLAN's MSTI Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 10:37   ` Nikolay Aleksandrov
  2022-03-14 14:58   ` Vladimir Oltean
  2022-03-14  9:52 ` [PATCH v3 net-next 04/14] net: bridge: mst: Notify switchdev drivers of MST mode changes Tobias Waldekranz
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Make it possible to change the port state in a given MSTI by extending
the bridge port netlink interface (RTM_SETLINK on PF_BRIDGE).The
proposed iproute2 interface would be:

    bridge mst set dev <PORT> msti <MSTI> state <STATE>

Current states in all applicable MSTIs can also be dumped via a
corresponding RTM_GETLINK. The proposed iproute interface looks like
this:

$ bridge mst
port              msti
vb1               0
		    state forwarding
		  100
		    state disabled
vb2               0
		    state forwarding
		  100
		    state forwarding

The preexisting per-VLAN states are still valid in the MST
mode (although they are read-only), and can be queried as usual if one
is interested in knowing a particular VLAN's state without having to
care about the VID to MSTI mapping (in this example VLAN 20 and 30 are
bound to MSTI 100):

$ bridge -d vlan
port              vlan-id
vb1               10
		    state forwarding mcast_router 1
		  20
		    state disabled mcast_router 1
		  30
		    state disabled mcast_router 1
		  40
		    state forwarding mcast_router 1
vb2               10
		    state forwarding mcast_router 1
		  20
		    state forwarding mcast_router 1
		  30
		    state forwarding mcast_router 1
		  40
		    state forwarding mcast_router 1

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/uapi/linux/if_bridge.h |  17 ++++++
 include/uapi/linux/rtnetlink.h |   1 +
 net/bridge/br_mst.c            | 105 +++++++++++++++++++++++++++++++++
 net/bridge/br_netlink.c        |  32 +++++++++-
 net/bridge/br_private.h        |  15 +++++
 5 files changed, 169 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index f60244b747ae..879dfaef8da0 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -122,6 +122,7 @@ enum {
 	IFLA_BRIDGE_VLAN_TUNNEL_INFO,
 	IFLA_BRIDGE_MRP,
 	IFLA_BRIDGE_CFM,
+	IFLA_BRIDGE_MST,
 	__IFLA_BRIDGE_MAX,
 };
 #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
@@ -453,6 +454,21 @@ enum {
 
 #define IFLA_BRIDGE_CFM_CC_PEER_STATUS_MAX (__IFLA_BRIDGE_CFM_CC_PEER_STATUS_MAX - 1)
 
+enum {
+	IFLA_BRIDGE_MST_UNSPEC,
+	IFLA_BRIDGE_MST_ENTRY,
+	__IFLA_BRIDGE_MST_MAX,
+};
+#define IFLA_BRIDGE_MST_MAX (__IFLA_BRIDGE_MST_MAX - 1)
+
+enum {
+	IFLA_BRIDGE_MST_ENTRY_UNSPEC,
+	IFLA_BRIDGE_MST_ENTRY_MSTI,
+	IFLA_BRIDGE_MST_ENTRY_STATE,
+	__IFLA_BRIDGE_MST_ENTRY_MAX,
+};
+#define IFLA_BRIDGE_MST_ENTRY_MAX (__IFLA_BRIDGE_MST_ENTRY_MAX - 1)
+
 struct bridge_stp_xstats {
 	__u64 transition_blk;
 	__u64 transition_fwd;
@@ -786,4 +802,5 @@ enum {
 	__BRIDGE_QUERIER_MAX
 };
 #define BRIDGE_QUERIER_MAX (__BRIDGE_QUERIER_MAX - 1)
+
 #endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 51530aade46e..83849a37db5b 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -817,6 +817,7 @@ enum {
 #define RTEXT_FILTER_MRP	(1 << 4)
 #define RTEXT_FILTER_CFM_CONFIG	(1 << 5)
 #define RTEXT_FILTER_CFM_STATUS	(1 << 6)
+#define RTEXT_FILTER_MST	(1 << 7)
 
 /* End of information exported to user level */
 
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index 78ef5fea4d2b..df65aa7701c1 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -124,3 +124,108 @@ int br_mst_set_enabled(struct net_bridge *br, bool on,
 	br_opt_toggle(br, BROPT_MST_ENABLED, on);
 	return 0;
 }
+
+int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg)
+{
+	struct net_bridge_vlan *v;
+	struct nlattr *nest;
+	unsigned long *seen;
+	int err = 0;
+
+	seen = bitmap_zalloc(VLAN_N_VID, 0);
+	if (!seen)
+		return -ENOMEM;
+
+	list_for_each_entry(v, &vg->vlan_list, vlist) {
+		if (test_bit(v->brvlan->msti, seen))
+			continue;
+
+		nest = nla_nest_start_noflag(skb, IFLA_BRIDGE_MST_ENTRY);
+		if (!nest ||
+		    nla_put_u16(skb, IFLA_BRIDGE_MST_ENTRY_MSTI, v->brvlan->msti) ||
+		    nla_put_u8(skb, IFLA_BRIDGE_MST_ENTRY_STATE, v->state)) {
+			err = -EMSGSIZE;
+			break;
+		}
+		nla_nest_end(skb, nest);
+
+		set_bit(v->brvlan->msti, seen);
+	}
+
+	kfree(seen);
+	return err;
+}
+
+static const struct nla_policy br_mst_nl_policy[IFLA_BRIDGE_MST_ENTRY_MAX + 1] = {
+	[IFLA_BRIDGE_MST_ENTRY_MSTI] = NLA_POLICY_RANGE(NLA_U16,
+						   1, /* 0 reserved for CST */
+						   VLAN_N_VID - 1),
+	[IFLA_BRIDGE_MST_ENTRY_STATE] = NLA_POLICY_RANGE(NLA_U8,
+						    BR_STATE_DISABLED,
+						    BR_STATE_BLOCKING),
+};
+
+static int br_mst_parse_one(struct net_bridge_port *p,
+			    const struct nlattr *attr,
+			    struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[IFLA_BRIDGE_MST_ENTRY_MAX + 1];
+	u16 msti;
+	u8 state;
+	int err;
+
+	err = nla_parse_nested(tb, IFLA_BRIDGE_MST_ENTRY_MAX, attr,
+			       br_mst_nl_policy, extack);
+	if (err)
+		return err;
+
+	if (!tb[IFLA_BRIDGE_MST_ENTRY_MSTI]) {
+		NL_SET_ERR_MSG_MOD(extack, "MSTI not specified");
+		return -EINVAL;
+	}
+
+	if (!tb[IFLA_BRIDGE_MST_ENTRY_STATE]) {
+		NL_SET_ERR_MSG_MOD(extack, "State not specified");
+		return -EINVAL;
+	}
+
+	msti = nla_get_u16(tb[IFLA_BRIDGE_MST_ENTRY_MSTI]);
+	state = nla_get_u8(tb[IFLA_BRIDGE_MST_ENTRY_STATE]);
+
+	br_mst_set_state(p, msti, state);
+	return 0;
+}
+
+int br_mst_parse(struct net_bridge_port *p, struct nlattr *mst_attr,
+		 struct netlink_ext_ack *extack)
+{
+	struct nlattr *attr;
+	int err, msts = 0;
+	int rem;
+
+	if (!br_opt_get(p->br, BROPT_MST_ENABLED)) {
+		NL_SET_ERR_MSG_MOD(extack, "Can't modify MST state when MST is disabled");
+		return -EBUSY;
+	}
+
+	nla_for_each_nested(attr, mst_attr, rem) {
+		switch (nla_type(attr)) {
+		case IFLA_BRIDGE_MST_ENTRY:
+			err = br_mst_parse_one(p, attr, extack);
+			break;
+		default:
+			continue;
+		}
+
+		msts++;
+		if (err)
+			break;
+	}
+
+	if (!msts) {
+		NL_SET_ERR_MSG_MOD(extack, "Found no MST entries to process");
+		err = -EINVAL;
+	}
+
+	return err;
+}
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 7d4432ca9a20..d2b4550f30d6 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -485,7 +485,8 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 			   RTEXT_FILTER_BRVLAN_COMPRESSED |
 			   RTEXT_FILTER_MRP |
 			   RTEXT_FILTER_CFM_CONFIG |
-			   RTEXT_FILTER_CFM_STATUS)) {
+			   RTEXT_FILTER_CFM_STATUS |
+			   RTEXT_FILTER_MST)) {
 		af = nla_nest_start_noflag(skb, IFLA_AF_SPEC);
 		if (!af)
 			goto nla_put_failure;
@@ -564,7 +565,28 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 		nla_nest_end(skb, cfm_nest);
 	}
 
+	if ((filter_mask & RTEXT_FILTER_MST) &&
+	    br_opt_get(br, BROPT_MST_ENABLED) && port) {
+		struct net_bridge_vlan_group *vg = nbp_vlan_group(port);
+		struct nlattr *mst_nest;
+		int err;
+
+		if (!vg || !vg->num_vlans)
+			goto done;
+
+		mst_nest = nla_nest_start(skb, IFLA_BRIDGE_MST);
+		if (!mst_nest)
+			goto nla_put_failure;
+
+		err = br_mst_fill_info(skb, vg);
+		if (err)
+			goto nla_put_failure;
+
+		nla_nest_end(skb, mst_nest);
+	}
+
 done:
+
 	if (af)
 		nla_nest_end(skb, af);
 	nlmsg_end(skb, nlh);
@@ -803,6 +825,14 @@ static int br_afspec(struct net_bridge *br,
 			if (err)
 				return err;
 			break;
+		case IFLA_BRIDGE_MST:
+			if (cmd != RTM_SETLINK || !p)
+				return -EINVAL;
+
+			err = br_mst_parse(p, attr, extack);
+			if (err)
+				return err;
+			break;
 		}
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b907d389b63a..08d82578bd97 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1783,6 +1783,9 @@ int br_mst_vlan_set_msti(struct net_bridge_vlan *v, u16 msti);
 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);
+int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg);
+int br_mst_parse(struct net_bridge_port *p, struct nlattr *mst_attr,
+		 struct netlink_ext_ack *extack);
 #else
 static inline bool br_mst_is_enabled(struct net_bridge *br)
 {
@@ -1791,6 +1794,18 @@ static inline bool br_mst_is_enabled(struct net_bridge *br)
 
 static inline void br_mst_set_state(struct net_bridge_port *p,
 				    u16 msti, u8 state) {}
+static inline int br_mst_fill_info(struct sk_buff *skb,
+				   struct net_bridge_vlan_group *vg)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int br_mst_parse(struct net_bridge_port *p,
+			       struct nlattr *mst_attr,
+			       struct netlink_ext_ack *extack)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 struct nf_br_ops {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 04/14] net: bridge: mst: Notify switchdev drivers of MST mode changes
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (2 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 05/14] net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations Tobias Waldekranz
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Trigger a switchdev event whenever the bridge's MST mode is
enabled/disabled. This allows constituent ports to either perform any
required hardware config, or refuse the change if it not supported.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/net/switchdev.h |  2 ++
 net/bridge/br_mst.c     | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 3e424d40fae3..85dd004dc9ad 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -27,6 +27,7 @@ enum switchdev_attr_id {
 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL,
 	SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
 	SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
+	SWITCHDEV_ATTR_ID_BRIDGE_MST,
 	SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
 };
 
@@ -48,6 +49,7 @@ struct switchdev_attr {
 		clock_t ageing_time;			/* BRIDGE_AGEING_TIME */
 		bool vlan_filtering;			/* BRIDGE_VLAN_FILTERING */
 		u16 vlan_protocol;			/* BRIDGE_VLAN_PROTOCOL */
+		bool mst;				/* BRIDGE_MST */
 		bool mc_disabled;			/* MC_DISABLED */
 		u8 mrp_port_role;			/* MRP_PORT_ROLE */
 	} u;
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index df65aa7701c1..39057585c302 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -99,8 +99,14 @@ 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)
 {
+	struct switchdev_attr attr = {
+		.id = SWITCHDEV_ATTR_ID_BRIDGE_MST,
+		.orig_dev = br->dev,
+		.u.mst = on,
+	};
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_port *p;
+	int err;
 
 	list_for_each_entry(p, &br->port_list, list) {
 		vg = nbp_vlan_group(p);
@@ -116,6 +122,10 @@ int br_mst_set_enabled(struct net_bridge *br, bool on,
 	if (br_opt_get(br, BROPT_MST_ENABLED) == on)
 		return 0;
 
+	err = switchdev_port_attr_set(br->dev, &attr, extack);
+	if (err && err != -EOPNOTSUPP)
+		return err;
+
 	if (on)
 		static_branch_enable(&br_mst_used);
 	else
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 05/14] net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (3 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 04/14] net: bridge: mst: Notify switchdev drivers of MST mode changes Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 06/14] net: bridge: mst: Notify switchdev drivers of MST state changes Tobias Waldekranz
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Whenever a VLAN moves to a new MSTI, send a switchdev notification so
that switchdevs can track a bridge's VID to MSTI mappings.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/net/switchdev.h   |  7 ++++++
 net/bridge/br_mst.c       | 14 ++++++++++++
 net/bridge/br_switchdev.c | 46 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 85dd004dc9ad..53dfa0f7cf5b 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -29,6 +29,7 @@ enum switchdev_attr_id {
 	SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
 	SWITCHDEV_ATTR_ID_BRIDGE_MST,
 	SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
+	SWITCHDEV_ATTR_ID_VLAN_MSTI,
 };
 
 struct switchdev_brport_flags {
@@ -36,6 +37,11 @@ struct switchdev_brport_flags {
 	unsigned long mask;
 };
 
+struct switchdev_vlan_msti {
+	u16 vid;
+	u16 msti;
+};
+
 struct switchdev_attr {
 	struct net_device *orig_dev;
 	enum switchdev_attr_id id;
@@ -52,6 +58,7 @@ struct switchdev_attr {
 		bool mst;				/* BRIDGE_MST */
 		bool mc_disabled;			/* MC_DISABLED */
 		u8 mrp_port_role;			/* MRP_PORT_ROLE */
+		struct switchdev_vlan_msti vlan_msti;	/* VLAN_MSTI */
 	} u;
 };
 
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index 39057585c302..2f761d27d69e 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/kernel.h>
+#include <net/switchdev.h>
 
 #include "br_private.h"
 
@@ -65,13 +66,26 @@ static void br_mst_vlan_sync_state(struct net_bridge_vlan *pv, u16 msti)
 
 int br_mst_vlan_set_msti(struct net_bridge_vlan *mv, u16 msti)
 {
+	struct switchdev_attr attr = {
+		.id = SWITCHDEV_ATTR_ID_VLAN_MSTI,
+		.orig_dev = mv->br->dev,
+		.u.vlan_msti = {
+			.vid = mv->vid,
+			.msti = msti,
+		},
+	};
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_vlan *pv;
 	struct net_bridge_port *p;
+	int err;
 
 	if (mv->msti == msti)
 		return 0;
 
+	err = switchdev_port_attr_set(mv->br->dev, &attr, NULL);
+	if (err && err != -EOPNOTSUPP)
+		return err;
+
 	mv->msti = msti;
 
 	list_for_each_entry(p, &mv->br->port_list, list) {
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 6f6a70121a5e..8cc44c367231 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -331,6 +331,46 @@ br_switchdev_fdb_replay(const struct net_device *br_dev, const void *ctx,
 	return err;
 }
 
+static int br_switchdev_vlan_attr_replay(struct net_device *br_dev,
+					 const void *ctx,
+					 struct notifier_block *nb,
+					 struct netlink_ext_ack *extack)
+{
+	struct switchdev_notifier_port_attr_info attr_info = {
+		.info = {
+			.dev = br_dev,
+			.extack = extack,
+			.ctx = ctx,
+		},
+	};
+	struct net_bridge *br = netdev_priv(br_dev);
+	struct net_bridge_vlan_group *vg;
+	struct switchdev_attr attr;
+	struct net_bridge_vlan *v;
+	int err;
+
+	attr_info.attr = &attr;
+	attr.orig_dev = br_dev;
+
+	vg = br_vlan_group(br);
+
+	list_for_each_entry(v, &vg->vlan_list, vlist) {
+		if (v->msti) {
+			attr.id = SWITCHDEV_ATTR_ID_VLAN_MSTI;
+			attr.u.vlan_msti.vid = v->vid;
+			attr.u.vlan_msti.msti = v->msti;
+
+			err = nb->notifier_call(nb, SWITCHDEV_PORT_ATTR_SET,
+						&attr_info);
+			err = notifier_to_errno(err);
+			if (err)
+				return err;
+		}
+	}
+
+	return 0;
+}
+
 static int
 br_switchdev_vlan_replay_one(struct notifier_block *nb,
 			     struct net_device *dev,
@@ -425,6 +465,12 @@ static int br_switchdev_vlan_replay(struct net_device *br_dev,
 			return err;
 	}
 
+	if (adding) {
+		err = br_switchdev_vlan_attr_replay(br_dev, ctx, nb, extack);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 06/14] net: bridge: mst: Notify switchdev drivers of MST state changes
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (4 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 05/14] net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 07/14] net: bridge: mst: Add helper to map an MSTI to a VID set Tobias Waldekranz
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Generate a switchdev notification whenever an MST state changes. This
notification is keyed by the VLANs MSTI rather than the VID, since
multiple VLANs may share the same MST instance.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/net/switchdev.h |  7 +++++++
 net/bridge/br_mst.c     | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 53dfa0f7cf5b..aa0171d5786d 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -19,6 +19,7 @@
 enum switchdev_attr_id {
 	SWITCHDEV_ATTR_ID_UNDEFINED,
 	SWITCHDEV_ATTR_ID_PORT_STP_STATE,
+	SWITCHDEV_ATTR_ID_PORT_MST_STATE,
 	SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
 	SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
 	SWITCHDEV_ATTR_ID_PORT_MROUTER,
@@ -32,6 +33,11 @@ enum switchdev_attr_id {
 	SWITCHDEV_ATTR_ID_VLAN_MSTI,
 };
 
+struct switchdev_mst_state {
+	u16 msti;
+	u8 state;
+};
+
 struct switchdev_brport_flags {
 	unsigned long val;
 	unsigned long mask;
@@ -50,6 +56,7 @@ struct switchdev_attr {
 	void (*complete)(struct net_device *dev, int err, void *priv);
 	union {
 		u8 stp_state;				/* PORT_STP_STATE */
+		struct switchdev_mst_state mst_state;	/* PORT_MST_STATE */
 		struct switchdev_brport_flags brport_flags; /* PORT_BRIDGE_FLAGS */
 		bool mrouter;				/* PORT_MROUTER */
 		clock_t ageing_time;			/* BRIDGE_AGEING_TIME */
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index 2f761d27d69e..7d16926a3a31 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -29,8 +29,17 @@ static void br_mst_vlan_set_state(struct net_bridge_port *p, struct net_bridge_v
 
 void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state)
 {
+	struct switchdev_attr attr = {
+		.id = SWITCHDEV_ATTR_ID_PORT_MST_STATE,
+		.orig_dev = p->dev,
+		.u.mst_state = {
+			.msti = msti,
+			.state = state,
+		},
+	};
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_vlan *v;
+	int err;
 
 	vg = nbp_vlan_group(p);
 	if (!vg)
@@ -42,6 +51,17 @@ void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state)
 
 		br_mst_vlan_set_state(p, v, state);
 	}
+
+	if (!msti)
+		/* MSTI 0 (CST) state changes are notified via the
+		 * regular SWITCHDEV_ATTR_ID_PORT_STP_STATE.
+		 */
+		return;
+
+	err = switchdev_port_attr_set(p->dev, &attr, NULL);
+	if (err && err != -EOPNOTSUPP)
+		br_warn(p->br, "unable to offload MST state on %s in MSTI %u",
+			netdev_name(p->dev), msti);
 }
 
 static void br_mst_vlan_sync_state(struct net_bridge_vlan *pv, u16 msti)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 07/14] net: bridge: mst: Add helper to map an MSTI to a VID set
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (5 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 06/14] net: bridge: mst: Notify switchdev drivers of MST state changes Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 10:42   ` Nikolay Aleksandrov
  2022-03-14  9:52 ` [PATCH v3 net-next 08/14] net: bridge: mst: Add helper to check if MST is enabled Tobias Waldekranz
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

br_mst_get_info answers the question: "On this bridge, which VIDs are
mapped to the given MSTI?"

This is useful in switchdev drivers, which might have to fan-out
operations, relating to an MSTI, per VLAN.

An example: When a port's MST state changes from forwarding to
blocking, a driver may choose to flush the dynamic FDB entries on that
port to get faster reconvergence of the network, but this should only
be done in the VLANs that are managed by the MSTI in question.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/linux/if_bridge.h |  6 ++++++
 net/bridge/br_mst.c       | 26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 3aae023a9353..46e6327fef06 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -119,6 +119,7 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
 		     struct bridge_vlan_info *p_vinfo);
 int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
 			 struct bridge_vlan_info *p_vinfo);
+int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids);
 #else
 static inline bool br_vlan_enabled(const struct net_device *dev)
 {
@@ -151,6 +152,11 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
 {
 	return -EINVAL;
 }
+static inline int br_mst_get_info(struct net_device *dev, u16 msti,
+				  unsigned long *vids)
+{
+	return -EINVAL;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_BRIDGE)
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index 7d16926a3a31..eb18dbd5838f 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -13,6 +13,32 @@
 
 DEFINE_STATIC_KEY_FALSE(br_mst_used);
 
+int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids)
+{
+	struct net_bridge_vlan_group *vg;
+	struct net_bridge_vlan *v;
+	struct net_bridge *br;
+
+	ASSERT_RTNL();
+
+	if (!netif_is_bridge_master(dev))
+		return -EINVAL;
+
+	br = netdev_priv(dev);
+	if (!br_opt_get(br, BROPT_MST_ENABLED))
+		return -EINVAL;
+
+	vg = br_vlan_group(br);
+
+	list_for_each_entry(v, &vg->vlan_list, vlist) {
+		if (v->msti == msti)
+			set_bit(v->vid, vids);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(br_mst_get_info);
+
 static void br_mst_vlan_set_state(struct net_bridge_port *p, struct net_bridge_vlan *v,
 				  u8 state)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 08/14] net: bridge: mst: Add helper to check if MST is enabled
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (6 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 07/14] net: bridge: mst: Add helper to map an MSTI to a VID set Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 10:43   ` Nikolay Aleksandrov
  2022-03-14  9:52 ` [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST Tobias Waldekranz
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

This is useful for switchdev drivers that might want to refuse to join
a bridge where MST is enabled, if the hardware can't support it.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/linux/if_bridge.h | 5 +++++
 net/bridge/br_mst.c       | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 46e6327fef06..5dbab0a280a6 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -119,6 +119,7 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
 		     struct bridge_vlan_info *p_vinfo);
 int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
 			 struct bridge_vlan_info *p_vinfo);
+bool br_mst_enabled(struct net_device *dev);
 int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids);
 #else
 static inline bool br_vlan_enabled(const struct net_device *dev)
@@ -152,6 +153,10 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
 {
 	return -EINVAL;
 }
+static inline bool br_mst_enabled(struct net_device *dev)
+{
+	return false;
+}
 static inline int br_mst_get_info(struct net_device *dev, u16 msti,
 				  unsigned long *vids)
 {
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index eb18dbd5838f..e5ab2ce451c2 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -13,6 +13,15 @@
 
 DEFINE_STATIC_KEY_FALSE(br_mst_used);
 
+bool br_mst_enabled(struct net_device *dev)
+{
+	if (!netif_is_bridge_master(dev))
+		return false;
+
+	return br_opt_get(netdev_priv(dev), BROPT_MST_ENABLED);
+}
+EXPORT_SYMBOL(br_mst_enabled);
+
 int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids)
 {
 	struct net_bridge_vlan_group *vg;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (7 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 08/14] net: bridge: mst: Add helper to check if MST is enabled Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 16:56   ` Vladimir Oltean
  2022-03-14 17:51   ` Vladimir Oltean
  2022-03-14  9:52 ` [PATCH v3 net-next 10/14] net: dsa: Pass VLAN MSTI migration notifications to driver Tobias Waldekranz
                   ` (4 subsequent siblings)
  13 siblings, 2 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

When joining a bridge where MST is enabled, we validate that the
proper offloading support is in place, otherwise we fallback to
software bridging.

When then mode is changed on a bridge in which we are members, we
refuse the change if offloading is not supported.

At the moment we only check for configurable learning, but this will
be further restricted as we support more MST related switchdev events.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 20 ++++++++++++++++++++
 net/dsa/slave.c    |  6 ++++++
 3 files changed, 28 insertions(+)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f20bdd8ea0a8..2aba420696ef 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -234,6 +234,8 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 			    struct netlink_ext_ack *extack);
 bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
 int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
+int dsa_port_mst_enable(struct dsa_port *dp, bool on,
+			struct netlink_ext_ack *extack);
 int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
 			bool targeted_match);
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 58291df14cdb..1a17a0efa2fa 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
+	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
+	if (err && err != -EOPNOTSUPP)
+		return err;
+
 	return 0;
 }
 
@@ -735,6 +739,22 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
 	return 0;
 }
 
+int dsa_port_mst_enable(struct dsa_port *dp, bool on,
+			struct netlink_ext_ack *extack)
+{
+	struct dsa_switch *ds = dp->ds;
+
+	if (!on)
+		return 0;
+
+	if (!dsa_port_can_configure_learning(dp)) {
+		NL_SET_ERR_MSG_MOD(extack, "Hardware does not support MST");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
 			      struct switchdev_brport_flags flags,
 			      struct netlink_ext_ack *extack)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index a61a7c54af20..333f5702ea4f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -463,6 +463,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
 
 		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
 		break;
+	case SWITCHDEV_ATTR_ID_BRIDGE_MST:
+		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
+		ret = dsa_port_mst_enable(dp, attr->u.mst, extack);
+		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
 		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
 			return -EOPNOTSUPP;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 10/14] net: dsa: Pass VLAN MSTI migration notifications to driver
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (8 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 17:07   ` Vladimir Oltean
  2022-03-14  9:52 ` [PATCH v3 net-next 11/14] net: dsa: Handle MST state changes Tobias Waldekranz
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Add the usual trampoline functionality from the generic DSA layer down
to the drivers for VLAN MSTI migrations.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/net/dsa.h  |  3 +++
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 14 +++++++++++++-
 net/dsa/slave.c    |  6 ++++++
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 9d16505fc0e2..1ddaa2cc5842 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -964,6 +964,9 @@ struct dsa_switch_ops {
 				 struct netlink_ext_ack *extack);
 	int	(*port_vlan_del)(struct dsa_switch *ds, int port,
 				 const struct switchdev_obj_port_vlan *vlan);
+	int	(*vlan_msti_set)(struct dsa_switch *ds, struct dsa_bridge bridge,
+				 const struct switchdev_vlan_msti *msti);
+
 	/*
 	 * Forwarding database
 	 */
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2aba420696ef..d90b4cf0c9d2 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -236,6 +236,8 @@ bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
 int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
 int dsa_port_mst_enable(struct dsa_port *dp, bool on,
 			struct netlink_ext_ack *extack);
+int dsa_port_vlan_msti(struct dsa_port *dp,
+		       const struct switchdev_vlan_msti *msti);
 int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
 			bool targeted_match);
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1a17a0efa2fa..f6a822d854cc 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -747,7 +747,8 @@ int dsa_port_mst_enable(struct dsa_port *dp, bool on,
 	if (!on)
 		return 0;
 
-	if (!dsa_port_can_configure_learning(dp)) {
+	if (!(ds->ops->vlan_msti_set &&
+	      dsa_port_can_configure_learning(dp))) {
 		NL_SET_ERR_MSG_MOD(extack, "Hardware does not support MST");
 		return -EINVAL;
 	}
@@ -798,6 +799,17 @@ int dsa_port_bridge_flags(struct dsa_port *dp,
 	return 0;
 }
 
+int dsa_port_vlan_msti(struct dsa_port *dp,
+		       const struct switchdev_vlan_msti *msti)
+{
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->vlan_msti_set)
+		return -EOPNOTSUPP;
+
+	return ds->ops->vlan_msti_set(ds, *dp->bridge, msti);
+}
+
 int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
 			bool targeted_match)
 {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 333f5702ea4f..cd2c57de9592 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -482,6 +482,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
 
 		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, extack);
 		break;
+	case SWITCHDEV_ATTR_ID_VLAN_MSTI:
+		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
+		ret = dsa_port_vlan_msti(dp, &attr->u.vlan_msti);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 11/14] net: dsa: Handle MST state changes
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (9 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 10/14] net: dsa: Pass VLAN MSTI migration notifications to driver Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 17:14   ` Vladimir Oltean
  2022-03-14  9:52 ` [PATCH v3 net-next 12/14] net: dsa: mv88e6xxx: Disentangle STU from VTU Tobias Waldekranz
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Add the usual trampoline functionality from the generic DSA layer down
to the drivers for MST state changes.

When a state changes to disabled/blocking/listening, make sure to fast
age any dynamic entries in the affected VLANs (those controlled by the
MSTI in question).

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 include/net/dsa.h  |  3 +++
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 67 +++++++++++++++++++++++++++++++++++++++++++---
 net/dsa/slave.c    |  6 +++++
 4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 1ddaa2cc5842..a171e7cdb3fe 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -945,7 +945,10 @@ struct dsa_switch_ops {
 				     struct dsa_bridge bridge);
 	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
 				      u8 state);
+	int	(*port_mst_state_set)(struct dsa_switch *ds, int port,
+				      const struct switchdev_mst_state *state);
 	void	(*port_fast_age)(struct dsa_switch *ds, int port);
+	void	(*port_vlan_fast_age)(struct dsa_switch *ds, int port, u16 vid);
 	int	(*port_pre_bridge_flags)(struct dsa_switch *ds, int port,
 					 struct switchdev_brport_flags flags,
 					 struct netlink_ext_ack *extack);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d90b4cf0c9d2..2ae8996cf7c8 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -215,6 +215,8 @@ static inline struct net_device *dsa_master_find_slave(struct net_device *dev,
 void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
 			       const struct dsa_device_ops *tag_ops);
 int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age);
+int dsa_port_set_mst_state(struct dsa_port *dp,
+			   const struct switchdev_mst_state *state);
 int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy);
 int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
 void dsa_port_disable_rt(struct dsa_port *dp);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index f6a822d854cc..223681e03321 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -30,12 +30,11 @@ static int dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
 	return dsa_tree_notify(dp->ds->dst, e, v);
 }
 
-static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp)
+static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp, u16 vid)
 {
 	struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
 	struct switchdev_notifier_fdb_info info = {
-		/* flush all VLANs */
-		.vid = 0,
+		.vid = vid,
 	};
 
 	/* When the port becomes standalone it has already left the bridge.
@@ -57,7 +56,39 @@ static void dsa_port_fast_age(const struct dsa_port *dp)
 
 	ds->ops->port_fast_age(ds, dp->index);
 
-	dsa_port_notify_bridge_fdb_flush(dp);
+	dsa_port_notify_bridge_fdb_flush(dp, 0);
+}
+
+static void dsa_port_vlan_fast_age(const struct dsa_port *dp, u16 vid)
+{
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->port_vlan_fast_age)
+		return;
+
+	ds->ops->port_vlan_fast_age(ds, dp->index, vid);
+
+	dsa_port_notify_bridge_fdb_flush(dp, vid);
+}
+
+static void dsa_port_msti_fast_age(const struct dsa_port *dp, u16 msti)
+{
+	unsigned long *vids;
+	int vid;
+
+	vids = bitmap_zalloc(VLAN_N_VID, 0);
+	if (!vids)
+		return;
+
+	if (br_mst_get_info(dsa_port_bridge_dev_get(dp), msti, vids))
+		goto out_free;
+
+	for_each_set_bit(vid, vids, VLAN_N_VID) {
+		dsa_port_vlan_fast_age(dp, vid);
+	}
+
+out_free:
+	kfree(vids);
 }
 
 static bool dsa_port_can_configure_learning(struct dsa_port *dp)
@@ -118,6 +149,32 @@ static void dsa_port_set_state_now(struct dsa_port *dp, u8 state,
 		pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
 }
 
+int dsa_port_set_mst_state(struct dsa_port *dp,
+			   const struct switchdev_mst_state *state)
+{
+	struct dsa_switch *ds = dp->ds;
+	int err;
+
+	if (!ds->ops->port_mst_state_set)
+		return -EOPNOTSUPP;
+
+	err = ds->ops->port_mst_state_set(ds, dp->index, state);
+	if (err)
+		return err;
+
+	if (dp->learning) {
+		switch (state->state) {
+		case BR_STATE_DISABLED:
+		case BR_STATE_BLOCKING:
+		case BR_STATE_LISTENING:
+			dsa_port_msti_fast_age(dp, state->msti);
+			break;
+		}
+	}
+
+	return 0;
+}
+
 int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy)
 {
 	struct dsa_switch *ds = dp->ds;
@@ -748,6 +805,8 @@ int dsa_port_mst_enable(struct dsa_port *dp, bool on,
 		return 0;
 
 	if (!(ds->ops->vlan_msti_set &&
+	      ds->ops->port_mst_state_set &&
+	      ds->ops->port_vlan_fast_age &&
 	      dsa_port_can_configure_learning(dp))) {
 		NL_SET_ERR_MSG_MOD(extack, "Hardware does not support MST");
 		return -EINVAL;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index cd2c57de9592..106b177ad1eb 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -450,6 +450,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
 
 		ret = dsa_port_set_state(dp, attr->u.stp_state, true);
 		break;
+	case SWITCHDEV_ATTR_ID_PORT_MST_STATE:
+		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
+		ret = dsa_port_set_mst_state(dp, &attr->u.mst_state);
+		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
 		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
 			return -EOPNOTSUPP;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 12/14] net: dsa: mv88e6xxx: Disentangle STU from VTU
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (10 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 11/14] net: dsa: Handle MST state changes Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 13/14] net: dsa: mv88e6xxx: Export STU as devlink region Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading Tobias Waldekranz
  13 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

In early LinkStreet silicon (e.g. 6095/6185), the per-VLAN STP states
were kept in the VTU - there was no concept of a SID. Later, the
information was split into two tables, where the VTU only tracked
memberships and deferred the STP state tracking to the STU via a
pointer (SID). This meant that a group of VLANs could share the same
STU entry. Most likely, this was done to align with MSTP (802.1Q-2018,
Clause 13), which is built on this principle.

While the VTU is still 4k lines on most devices, the STU is capped at
64 entries. This means that the current stategy, updating STU info
whenever a VTU entry is updated, can not easily support MSTP because:

- The maximum number of VIDs would also be capped at 64, as we would
  have to allocate one SID for every VTU entry - even if many VLANs
  would effectively share the same MST.

- MSTP updates would be unnecessarily slow as you would have to
  iterate over all VLANs that share the same MST.

In order to support MSTP offloading in the future, manage the STU as a
separate entity from the VTU.

Only add support for newer hardware with separate VTU and
STU. VTU-only devices can also be supported, but essentially this
requires a software implementation of an STU (fanning out state
changed to all VLANs tied to the same MST).

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c        |  54 ++++
 drivers/net/dsa/mv88e6xxx/chip.h        |  24 ++
 drivers/net/dsa/mv88e6xxx/global1.h     |  10 +
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 311 ++++++++++++++----------
 4 files changed, 264 insertions(+), 135 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 84b90fc36c58..c14a62aa6a6c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1791,6 +1791,33 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
 	return mv88e6xxx_g1_atu_flush(chip, *fid, true);
 }
 
+static int mv88e6xxx_stu_loadpurge(struct mv88e6xxx_chip *chip,
+				   struct mv88e6xxx_stu_entry *entry)
+{
+	if (!chip->info->ops->stu_loadpurge)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->stu_loadpurge(chip, entry);
+}
+
+static int mv88e6xxx_stu_setup(struct mv88e6xxx_chip *chip)
+{
+	struct mv88e6xxx_stu_entry stu = {
+		.valid = true,
+		.sid = 0
+	};
+
+	if (!mv88e6xxx_has_stu(chip))
+		return 0;
+
+	/* Make sure that SID 0 is always valid. This is used by VTU
+	 * entries that do not make use of the STU, e.g. when creating
+	 * a VLAN upper on a port that is also part of a VLAN
+	 * filtering bridge.
+	 */
+	return mv88e6xxx_stu_loadpurge(chip, &stu);
+}
+
 static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 					u16 vid)
 {
@@ -3427,6 +3454,13 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	if (err)
 		goto unlock;
 
+	/* Must be called after mv88e6xxx_vtu_setup (which flushes the
+	 * VTU, thereby also flushing the STU).
+	 */
+	err = mv88e6xxx_stu_setup(chip);
+	if (err)
+		goto unlock;
+
 	/* Setup Switch Port Registers */
 	for (i = 0; i < mv88e6xxx_num_ports(chip); i++) {
 		if (dsa_is_unused_port(ds, i))
@@ -3882,6 +3916,8 @@ static const struct mv88e6xxx_ops mv88e6097_ops = {
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.phylink_get_caps = mv88e6095_phylink_get_caps,
+	.stu_getnext = mv88e6352_g1_stu_getnext,
+	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
 	.set_max_frame_size = mv88e6185_g1_set_max_frame_size,
 };
 
@@ -4968,6 +5004,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
+	.stu_getnext = mv88e6352_g1_stu_getnext,
+	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
 	.serdes_get_lane = mv88e6352_serdes_get_lane,
 	.serdes_pcs_get_state = mv88e6352_serdes_pcs_get_state,
 	.serdes_pcs_config = mv88e6352_serdes_pcs_config,
@@ -5033,6 +5071,8 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
+	.stu_getnext = mv88e6390_g1_stu_getnext,
+	.stu_loadpurge = mv88e6390_g1_stu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
 	.serdes_get_lane = mv88e6390_serdes_get_lane,
 	/* Check status register pause & lpa register */
@@ -5098,6 +5138,8 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
+	.stu_getnext = mv88e6390_g1_stu_getnext,
+	.stu_loadpurge = mv88e6390_g1_stu_loadpurge,
 	.serdes_power = mv88e6390_serdes_power,
 	.serdes_get_lane = mv88e6390x_serdes_get_lane,
 	.serdes_pcs_get_state = mv88e6390_serdes_pcs_get_state,
@@ -5166,6 +5208,8 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6390_g1_vtu_loadpurge,
+	.stu_getnext = mv88e6390_g1_stu_getnext,
+	.stu_loadpurge = mv88e6390_g1_stu_loadpurge,
 	.serdes_power = mv88e6393x_serdes_power,
 	.serdes_get_lane = mv88e6393x_serdes_get_lane,
 	.serdes_pcs_get_state = mv88e6393x_serdes_pcs_get_state,
@@ -5234,6 +5278,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,
 		.num_internal_phys = 8,
 		.max_vid = 4095,
+		.max_sid = 63,
 		.port_base_addr = 0x10,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5487,6 +5532,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 9,
 		.num_gpio = 16,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5510,6 +5556,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 9,
 		.num_gpio = 16,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5532,6 +5579,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5554,6 +5602,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5576,6 +5625,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5815,6 +5865,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 5,
 		.num_gpio = 15,
 		.max_vid = 4095,
+		.max_sid = 63,
 		.port_base_addr = 0x10,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5839,6 +5890,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 9,
 		.num_gpio = 16,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5863,6 +5915,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 9,
 		.num_gpio = 16,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
@@ -5886,6 +5939,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.max_vid = 8191,
+		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 30b92a265613..be654be69982 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -20,6 +20,7 @@
 
 #define EDSA_HLEN		8
 #define MV88E6XXX_N_FID		4096
+#define MV88E6XXX_N_SID		64
 
 #define MV88E6XXX_FID_STANDALONE	0
 #define MV88E6XXX_FID_BRIDGED		1
@@ -130,6 +131,7 @@ struct mv88e6xxx_info {
 	unsigned int num_internal_phys;
 	unsigned int num_gpio;
 	unsigned int max_vid;
+	unsigned int max_sid;
 	unsigned int port_base_addr;
 	unsigned int phy_base_addr;
 	unsigned int global1_addr;
@@ -181,6 +183,12 @@ struct mv88e6xxx_vtu_entry {
 	bool	valid;
 	bool	policy;
 	u8	member[DSA_MAX_PORTS];
+	u8	state[DSA_MAX_PORTS];	/* Older silicon has no STU */
+};
+
+struct mv88e6xxx_stu_entry {
+	u8	sid;
+	bool	valid;
 	u8	state[DSA_MAX_PORTS];
 };
 
@@ -602,6 +610,12 @@ struct mv88e6xxx_ops {
 	int (*vtu_loadpurge)(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry);
 
+	/* Spanning Tree Unit operations */
+	int (*stu_getnext)(struct mv88e6xxx_chip *chip,
+			   struct mv88e6xxx_stu_entry *entry);
+	int (*stu_loadpurge)(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry);
+
 	/* GPIO operations */
 	const struct mv88e6xxx_gpio_ops *gpio_ops;
 
@@ -700,6 +714,11 @@ struct mv88e6xxx_hw_stat {
 	int type;
 };
 
+static inline bool mv88e6xxx_has_stu(struct mv88e6xxx_chip *chip)
+{
+	return chip->info->max_sid > 0;
+}
+
 static inline bool mv88e6xxx_has_pvt(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->pvt;
@@ -730,6 +749,11 @@ static inline unsigned int mv88e6xxx_max_vid(struct mv88e6xxx_chip *chip)
 	return chip->info->max_vid;
 }
 
+static inline unsigned int mv88e6xxx_max_sid(struct mv88e6xxx_chip *chip)
+{
+	return chip->info->max_sid;
+}
+
 static inline u16 mv88e6xxx_port_mask(struct mv88e6xxx_chip *chip)
 {
 	return GENMASK((s32)mv88e6xxx_num_ports(chip) - 1, 0);
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 2c1607c858a1..65958b2a0d3a 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -348,6 +348,16 @@ int mv88e6390_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 int mv88e6390_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_vtu_entry *entry);
 int mv88e6xxx_g1_vtu_flush(struct mv88e6xxx_chip *chip);
+int mv88e6xxx_g1_stu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry);
+int mv88e6352_g1_stu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry);
+int mv88e6352_g1_stu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_stu_entry *entry);
+int mv88e6390_g1_stu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry);
+int mv88e6390_g1_stu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_stu_entry *entry);
 int mv88e6xxx_g1_vtu_prob_irq_setup(struct mv88e6xxx_chip *chip);
 void mv88e6xxx_g1_vtu_prob_irq_free(struct mv88e6xxx_chip *chip);
 int mv88e6xxx_g1_atu_get_next(struct mv88e6xxx_chip *chip, u16 fid);
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index b1bd9274a562..38e18f5811bf 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -44,8 +44,7 @@ static int mv88e6xxx_g1_vtu_fid_write(struct mv88e6xxx_chip *chip,
 
 /* Offset 0x03: VTU SID Register */
 
-static int mv88e6xxx_g1_vtu_sid_read(struct mv88e6xxx_chip *chip,
-				     struct mv88e6xxx_vtu_entry *entry)
+static int mv88e6xxx_g1_vtu_sid_read(struct mv88e6xxx_chip *chip, u8 *sid)
 {
 	u16 val;
 	int err;
@@ -54,15 +53,14 @@ static int mv88e6xxx_g1_vtu_sid_read(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
-	entry->sid = val & MV88E6352_G1_VTU_SID_MASK;
+	*sid = val & MV88E6352_G1_VTU_SID_MASK;
 
 	return 0;
 }
 
-static int mv88e6xxx_g1_vtu_sid_write(struct mv88e6xxx_chip *chip,
-				      struct mv88e6xxx_vtu_entry *entry)
+static int mv88e6xxx_g1_vtu_sid_write(struct mv88e6xxx_chip *chip, u8 sid)
 {
-	u16 val = entry->sid & MV88E6352_G1_VTU_SID_MASK;
+	u16 val = sid & MV88E6352_G1_VTU_SID_MASK;
 
 	return mv88e6xxx_g1_write(chip, MV88E6352_G1_VTU_SID, val);
 }
@@ -91,7 +89,7 @@ static int mv88e6xxx_g1_vtu_op(struct mv88e6xxx_chip *chip, u16 op)
 /* Offset 0x06: VTU VID Register */
 
 static int mv88e6xxx_g1_vtu_vid_read(struct mv88e6xxx_chip *chip,
-				     struct mv88e6xxx_vtu_entry *entry)
+				     bool *valid, u16 *vid)
 {
 	u16 val;
 	int err;
@@ -100,25 +98,28 @@ static int mv88e6xxx_g1_vtu_vid_read(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
-	entry->vid = val & 0xfff;
+	if (vid) {
+		*vid = val & 0xfff;
 
-	if (val & MV88E6390_G1_VTU_VID_PAGE)
-		entry->vid |= 0x1000;
+		if (val & MV88E6390_G1_VTU_VID_PAGE)
+			*vid |= 0x1000;
+	}
 
-	entry->valid = !!(val & MV88E6XXX_G1_VTU_VID_VALID);
+	if (valid)
+		*valid = !!(val & MV88E6XXX_G1_VTU_VID_VALID);
 
 	return 0;
 }
 
 static int mv88e6xxx_g1_vtu_vid_write(struct mv88e6xxx_chip *chip,
-				      struct mv88e6xxx_vtu_entry *entry)
+				      bool valid, u16 vid)
 {
-	u16 val = entry->vid & 0xfff;
+	u16 val = vid & 0xfff;
 
-	if (entry->vid & 0x1000)
+	if (vid & 0x1000)
 		val |= MV88E6390_G1_VTU_VID_PAGE;
 
-	if (entry->valid)
+	if (valid)
 		val |= MV88E6XXX_G1_VTU_VID_VALID;
 
 	return mv88e6xxx_g1_write(chip, MV88E6XXX_G1_VTU_VID, val);
@@ -147,7 +148,7 @@ static int mv88e6185_g1_vtu_stu_data_read(struct mv88e6xxx_chip *chip,
 }
 
 static int mv88e6185_g1_vtu_data_read(struct mv88e6xxx_chip *chip,
-				      struct mv88e6xxx_vtu_entry *entry)
+				      u8 *member, u8 *state)
 {
 	u16 regs[3];
 	int err;
@@ -160,36 +161,20 @@ static int mv88e6185_g1_vtu_data_read(struct mv88e6xxx_chip *chip,
 	/* Extract MemberTag data */
 	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
 		unsigned int member_offset = (i % 4) * 4;
+		unsigned int state_offset = member_offset + 2;
 
-		entry->member[i] = (regs[i / 4] >> member_offset) & 0x3;
-	}
-
-	return 0;
-}
-
-static int mv88e6185_g1_stu_data_read(struct mv88e6xxx_chip *chip,
-				      struct mv88e6xxx_vtu_entry *entry)
-{
-	u16 regs[3];
-	int err;
-	int i;
-
-	err = mv88e6185_g1_vtu_stu_data_read(chip, regs);
-	if (err)
-		return err;
+		if (member)
+			member[i] = (regs[i / 4] >> member_offset) & 0x3;
 
-	/* Extract PortState data */
-	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
-		unsigned int state_offset = (i % 4) * 4 + 2;
-
-		entry->state[i] = (regs[i / 4] >> state_offset) & 0x3;
+		if (state)
+			state[i] = (regs[i / 4] >> state_offset) & 0x3;
 	}
 
 	return 0;
 }
 
 static int mv88e6185_g1_vtu_data_write(struct mv88e6xxx_chip *chip,
-				       struct mv88e6xxx_vtu_entry *entry)
+				       u8 *member, u8 *state)
 {
 	u16 regs[3] = { 0 };
 	int i;
@@ -199,8 +184,11 @@ static int mv88e6185_g1_vtu_data_write(struct mv88e6xxx_chip *chip,
 		unsigned int member_offset = (i % 4) * 4;
 		unsigned int state_offset = member_offset + 2;
 
-		regs[i / 4] |= (entry->member[i] & 0x3) << member_offset;
-		regs[i / 4] |= (entry->state[i] & 0x3) << state_offset;
+		if (member)
+			regs[i / 4] |= (member[i] & 0x3) << member_offset;
+
+		if (state)
+			regs[i / 4] |= (state[i] & 0x3) << state_offset;
 	}
 
 	/* Write all 3 VTU/STU Data registers */
@@ -268,48 +256,6 @@ static int mv88e6390_g1_vtu_data_write(struct mv88e6xxx_chip *chip, u8 *data)
 
 /* VLAN Translation Unit Operations */
 
-static int mv88e6xxx_g1_vtu_stu_getnext(struct mv88e6xxx_chip *chip,
-					struct mv88e6xxx_vtu_entry *entry)
-{
-	int err;
-
-	err = mv88e6xxx_g1_vtu_sid_write(chip, entry);
-	if (err)
-		return err;
-
-	err = mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_STU_GET_NEXT);
-	if (err)
-		return err;
-
-	err = mv88e6xxx_g1_vtu_sid_read(chip, entry);
-	if (err)
-		return err;
-
-	return mv88e6xxx_g1_vtu_vid_read(chip, entry);
-}
-
-static int mv88e6xxx_g1_vtu_stu_get(struct mv88e6xxx_chip *chip,
-				    struct mv88e6xxx_vtu_entry *vtu)
-{
-	struct mv88e6xxx_vtu_entry stu;
-	int err;
-
-	err = mv88e6xxx_g1_vtu_sid_read(chip, vtu);
-	if (err)
-		return err;
-
-	stu.sid = vtu->sid - 1;
-
-	err = mv88e6xxx_g1_vtu_stu_getnext(chip, &stu);
-	if (err)
-		return err;
-
-	if (stu.sid != vtu->sid || !stu.valid)
-		return -EINVAL;
-
-	return 0;
-}
-
 int mv88e6xxx_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 			     struct mv88e6xxx_vtu_entry *entry)
 {
@@ -327,7 +273,7 @@ int mv88e6xxx_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 	 * write the VID only once, when the entry is given as invalid.
 	 */
 	if (!entry->valid) {
-		err = mv88e6xxx_g1_vtu_vid_write(chip, entry);
+		err = mv88e6xxx_g1_vtu_vid_write(chip, false, entry->vid);
 		if (err)
 			return err;
 	}
@@ -336,7 +282,7 @@ int mv88e6xxx_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
-	return mv88e6xxx_g1_vtu_vid_read(chip, entry);
+	return mv88e6xxx_g1_vtu_vid_read(chip, &entry->valid, &entry->vid);
 }
 
 int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
@@ -350,11 +296,7 @@ int mv88e6185_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 		return err;
 
 	if (entry->valid) {
-		err = mv88e6185_g1_vtu_data_read(chip, entry);
-		if (err)
-			return err;
-
-		err = mv88e6185_g1_stu_data_read(chip, entry);
+		err = mv88e6185_g1_vtu_data_read(chip, entry->member, entry->state);
 		if (err)
 			return err;
 
@@ -384,7 +326,7 @@ int mv88e6352_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 		return err;
 
 	if (entry->valid) {
-		err = mv88e6185_g1_vtu_data_read(chip, entry);
+		err = mv88e6185_g1_vtu_data_read(chip, entry->member, NULL);
 		if (err)
 			return err;
 
@@ -392,12 +334,7 @@ int mv88e6352_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 		if (err)
 			return err;
 
-		/* Fetch VLAN PortState data from the STU */
-		err = mv88e6xxx_g1_vtu_stu_get(chip, entry);
-		if (err)
-			return err;
-
-		err = mv88e6185_g1_stu_data_read(chip, entry);
+		err = mv88e6xxx_g1_vtu_sid_read(chip, &entry->sid);
 		if (err)
 			return err;
 	}
@@ -420,16 +357,11 @@ int mv88e6390_g1_vtu_getnext(struct mv88e6xxx_chip *chip,
 		if (err)
 			return err;
 
-		/* Fetch VLAN PortState data from the STU */
-		err = mv88e6xxx_g1_vtu_stu_get(chip, entry);
-		if (err)
-			return err;
-
-		err = mv88e6390_g1_vtu_data_read(chip, entry->state);
+		err = mv88e6xxx_g1_vtu_fid_read(chip, entry);
 		if (err)
 			return err;
 
-		err = mv88e6xxx_g1_vtu_fid_read(chip, entry);
+		err = mv88e6xxx_g1_vtu_sid_read(chip, &entry->sid);
 		if (err)
 			return err;
 	}
@@ -447,12 +379,12 @@ int mv88e6185_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
-	err = mv88e6xxx_g1_vtu_vid_write(chip, entry);
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry->valid, entry->vid);
 	if (err)
 		return err;
 
 	if (entry->valid) {
-		err = mv88e6185_g1_vtu_data_write(chip, entry);
+		err = mv88e6185_g1_vtu_data_write(chip, entry->member, entry->state);
 		if (err)
 			return err;
 
@@ -479,27 +411,21 @@ int mv88e6352_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
-	err = mv88e6xxx_g1_vtu_vid_write(chip, entry);
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry->valid, entry->vid);
 	if (err)
 		return err;
 
 	if (entry->valid) {
-		/* Write MemberTag and PortState data */
-		err = mv88e6185_g1_vtu_data_write(chip, entry);
-		if (err)
-			return err;
-
-		err = mv88e6xxx_g1_vtu_sid_write(chip, entry);
+		/* Write MemberTag data */
+		err = mv88e6185_g1_vtu_data_write(chip, entry->member, NULL);
 		if (err)
 			return err;
 
-		/* Load STU entry */
-		err = mv88e6xxx_g1_vtu_op(chip,
-					  MV88E6XXX_G1_VTU_OP_STU_LOAD_PURGE);
+		err = mv88e6xxx_g1_vtu_fid_write(chip, entry);
 		if (err)
 			return err;
 
-		err = mv88e6xxx_g1_vtu_fid_write(chip, entry);
+		err = mv88e6xxx_g1_vtu_sid_write(chip, entry->sid);
 		if (err)
 			return err;
 	}
@@ -517,41 +443,113 @@ int mv88e6390_g1_vtu_loadpurge(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
-	err = mv88e6xxx_g1_vtu_vid_write(chip, entry);
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry->valid, entry->vid);
 	if (err)
 		return err;
 
 	if (entry->valid) {
-		/* Write PortState data */
-		err = mv88e6390_g1_vtu_data_write(chip, entry->state);
+		/* Write MemberTag data */
+		err = mv88e6390_g1_vtu_data_write(chip, entry->member);
 		if (err)
 			return err;
 
-		err = mv88e6xxx_g1_vtu_sid_write(chip, entry);
+		err = mv88e6xxx_g1_vtu_fid_write(chip, entry);
 		if (err)
 			return err;
 
-		/* Load STU entry */
-		err = mv88e6xxx_g1_vtu_op(chip,
-					  MV88E6XXX_G1_VTU_OP_STU_LOAD_PURGE);
+		err = mv88e6xxx_g1_vtu_sid_write(chip, entry->sid);
 		if (err)
 			return err;
+	}
 
-		/* Write MemberTag data */
-		err = mv88e6390_g1_vtu_data_write(chip, entry->member);
+	/* Load/Purge VTU entry */
+	return mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_VTU_LOAD_PURGE);
+}
+
+int mv88e6xxx_g1_vtu_flush(struct mv88e6xxx_chip *chip)
+{
+	int err;
+
+	err = mv88e6xxx_g1_vtu_op_wait(chip);
+	if (err)
+		return err;
+
+	return mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_FLUSH_ALL);
+}
+
+/* Spanning Tree Unit Operations */
+
+int mv88e6xxx_g1_stu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry)
+{
+	int err;
+
+	err = mv88e6xxx_g1_vtu_op_wait(chip);
+	if (err)
+		return err;
+
+	/* To get the next higher active SID, the STU GetNext operation can be
+	 * started again without setting the SID registers since it already
+	 * contains the last SID.
+	 *
+	 * To save a few hardware accesses and abstract this to the caller,
+	 * write the SID only once, when the entry is given as invalid.
+	 */
+	if (!entry->valid) {
+		err = mv88e6xxx_g1_vtu_sid_write(chip, entry->sid);
 		if (err)
 			return err;
+	}
 
-		err = mv88e6xxx_g1_vtu_fid_write(chip, entry);
+	err = mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_STU_GET_NEXT);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g1_vtu_vid_read(chip, &entry->valid, NULL);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6xxx_g1_vtu_sid_read(chip, &entry->sid);
 		if (err)
 			return err;
 	}
 
-	/* Load/Purge VTU entry */
-	return mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_VTU_LOAD_PURGE);
+	return 0;
 }
 
-int mv88e6xxx_g1_vtu_flush(struct mv88e6xxx_chip *chip)
+int mv88e6352_g1_stu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry)
+{
+	int err;
+
+	err = mv88e6xxx_g1_stu_getnext(chip, entry);
+	if (err)
+		return err;
+
+	if (!entry->valid)
+		return 0;
+
+	return mv88e6185_g1_vtu_data_read(chip, NULL, entry->state);
+}
+
+int mv88e6390_g1_stu_getnext(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_stu_entry *entry)
+{
+	int err;
+
+	err = mv88e6xxx_g1_stu_getnext(chip, entry);
+	if (err)
+		return err;
+
+	if (!entry->valid)
+		return 0;
+
+	return mv88e6390_g1_vtu_data_read(chip, entry->state);
+}
+
+int mv88e6352_g1_stu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_stu_entry *entry)
 {
 	int err;
 
@@ -559,16 +557,59 @@ int mv88e6xxx_g1_vtu_flush(struct mv88e6xxx_chip *chip)
 	if (err)
 		return err;
 
-	return mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_FLUSH_ALL);
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry->valid, 0);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g1_vtu_sid_write(chip, entry->sid);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6185_g1_vtu_data_write(chip, NULL, entry->state);
+		if (err)
+			return err;
+	}
+
+	/* Load/Purge STU entry */
+	return mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_STU_LOAD_PURGE);
+}
+
+int mv88e6390_g1_stu_loadpurge(struct mv88e6xxx_chip *chip,
+			       struct mv88e6xxx_stu_entry *entry)
+{
+	int err;
+
+	err = mv88e6xxx_g1_vtu_op_wait(chip);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g1_vtu_vid_write(chip, entry->valid, 0);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_g1_vtu_sid_write(chip, entry->sid);
+	if (err)
+		return err;
+
+	if (entry->valid) {
+		err = mv88e6390_g1_vtu_data_write(chip, entry->state);
+		if (err)
+			return err;
+	}
+
+	/* Load/Purge STU entry */
+	return mv88e6xxx_g1_vtu_op(chip, MV88E6XXX_G1_VTU_OP_STU_LOAD_PURGE);
 }
 
+/* VTU Violation Management */
+
 static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
 {
 	struct mv88e6xxx_chip *chip = dev_id;
-	struct mv88e6xxx_vtu_entry entry;
+	u16 val, vid;
 	int spid;
 	int err;
-	u16 val;
 
 	mv88e6xxx_reg_lock(chip);
 
@@ -580,7 +621,7 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
 	if (err)
 		goto out;
 
-	err = mv88e6xxx_g1_vtu_vid_read(chip, &entry);
+	err = mv88e6xxx_g1_vtu_vid_read(chip, NULL, &vid);
 	if (err)
 		goto out;
 
@@ -588,13 +629,13 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
 
 	if (val & MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION) {
 		dev_err_ratelimited(chip->dev, "VTU member violation for vid %d, source port %d\n",
-				    entry.vid, spid);
+				    vid, spid);
 		chip->ports[spid].vtu_member_violation++;
 	}
 
 	if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
 		dev_dbg_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
-				    entry.vid, spid);
+				    vid, spid);
 		chip->ports[spid].vtu_miss_violation++;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 13/14] net: dsa: mv88e6xxx: Export STU as devlink region
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (11 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 12/14] net: dsa: mv88e6xxx: Disentangle STU from VTU Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14  9:52 ` [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading Tobias Waldekranz
  13 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Export the raw STU data in a devlink region so that it can be
inspected from userspace and compared to the current bridge
configuration.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.h    |  1 +
 drivers/net/dsa/mv88e6xxx/devlink.c | 94 +++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index be654be69982..6d4daa24d3e5 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -287,6 +287,7 @@ enum mv88e6xxx_region_id {
 	MV88E6XXX_REGION_GLOBAL2,
 	MV88E6XXX_REGION_ATU,
 	MV88E6XXX_REGION_VTU,
+	MV88E6XXX_REGION_STU,
 	MV88E6XXX_REGION_PVT,
 
 	_MV88E6XXX_REGION_MAX,
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index 381068395c63..1266eabee086 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -503,6 +503,85 @@ static int mv88e6xxx_region_vtu_snapshot(struct devlink *dl,
 	return 0;
 }
 
+/**
+ * struct mv88e6xxx_devlink_stu_entry - Devlink STU entry
+ * @sid:   Global1/3:   SID, unknown filters and learning.
+ * @vid:   Global1/6:   Valid bit.
+ * @data:  Global1/7-9: Membership data and priority override.
+ * @resvd: Reserved. In case we forgot something.
+ *
+ * The STU entry format varies between chipset generations. Peridot
+ * and Amethyst packs the STU data into Global1/7-8. Older silicon
+ * spreads the information across all three VTU data registers -
+ * inheriting the layout of even older hardware that had no STU at
+ * all. Since this is a low-level debug interface, copy all data
+ * verbatim and defer parsing to the consumer.
+ */
+struct mv88e6xxx_devlink_stu_entry {
+	u16 sid;
+	u16 vid;
+	u16 data[3];
+	u16 resvd;
+};
+
+static int mv88e6xxx_region_stu_snapshot(struct devlink *dl,
+					 const struct devlink_region_ops *ops,
+					 struct netlink_ext_ack *extack,
+					 u8 **data)
+{
+	struct mv88e6xxx_devlink_stu_entry *table, *entry;
+	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct mv88e6xxx_stu_entry stu;
+	int err;
+
+	table = kcalloc(mv88e6xxx_max_sid(chip) + 1,
+			sizeof(struct mv88e6xxx_devlink_stu_entry),
+			GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	entry = table;
+	stu.sid = mv88e6xxx_max_sid(chip);
+	stu.valid = false;
+
+	mv88e6xxx_reg_lock(chip);
+
+	do {
+		err = mv88e6xxx_g1_stu_getnext(chip, &stu);
+		if (err)
+			break;
+
+		if (!stu.valid)
+			break;
+
+		err = err ? : mv88e6xxx_g1_read(chip, MV88E6352_G1_VTU_SID,
+						&entry->sid);
+		err = err ? : mv88e6xxx_g1_read(chip, MV88E6XXX_G1_VTU_VID,
+						&entry->vid);
+		err = err ? : mv88e6xxx_g1_read(chip, MV88E6XXX_G1_VTU_DATA1,
+						&entry->data[0]);
+		err = err ? : mv88e6xxx_g1_read(chip, MV88E6XXX_G1_VTU_DATA2,
+						&entry->data[1]);
+		err = err ? : mv88e6xxx_g1_read(chip, MV88E6XXX_G1_VTU_DATA3,
+						&entry->data[2]);
+		if (err)
+			break;
+
+		entry++;
+	} while (stu.sid < mv88e6xxx_max_sid(chip));
+
+	mv88e6xxx_reg_unlock(chip);
+
+	if (err) {
+		kfree(table);
+		return err;
+	}
+
+	*data = (u8 *)table;
+	return 0;
+}
+
 static int mv88e6xxx_region_pvt_snapshot(struct devlink *dl,
 					 const struct devlink_region_ops *ops,
 					 struct netlink_ext_ack *extack,
@@ -605,6 +684,12 @@ static struct devlink_region_ops mv88e6xxx_region_vtu_ops = {
 	.destructor = kfree,
 };
 
+static struct devlink_region_ops mv88e6xxx_region_stu_ops = {
+	.name = "stu",
+	.snapshot = mv88e6xxx_region_stu_snapshot,
+	.destructor = kfree,
+};
+
 static struct devlink_region_ops mv88e6xxx_region_pvt_ops = {
 	.name = "pvt",
 	.snapshot = mv88e6xxx_region_pvt_snapshot,
@@ -640,6 +725,11 @@ static struct mv88e6xxx_region mv88e6xxx_regions[] = {
 		.ops = &mv88e6xxx_region_vtu_ops
 	  /* calculated at runtime */
 	},
+	[MV88E6XXX_REGION_STU] = {
+		.ops = &mv88e6xxx_region_stu_ops,
+		.cond = mv88e6xxx_has_stu,
+	  /* calculated at runtime */
+	},
 	[MV88E6XXX_REGION_PVT] = {
 		.ops = &mv88e6xxx_region_pvt_ops,
 		.size = MV88E6XXX_MAX_PVT_ENTRIES * sizeof(u16),
@@ -706,6 +796,10 @@ int mv88e6xxx_setup_devlink_regions_global(struct dsa_switch *ds)
 			size = (mv88e6xxx_max_vid(chip) + 1) *
 				sizeof(struct mv88e6xxx_devlink_vtu_entry);
 			break;
+		case MV88E6XXX_REGION_STU:
+			size = (mv88e6xxx_max_sid(chip) + 1) *
+				sizeof(struct mv88e6xxx_devlink_stu_entry);
+			break;
 		}
 
 		region = dsa_devlink_region_create(ds, ops, 1, size);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading
  2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
                   ` (12 preceding siblings ...)
  2022-03-14  9:52 ` [PATCH v3 net-next 13/14] net: dsa: mv88e6xxx: Export STU as devlink region Tobias Waldekranz
@ 2022-03-14  9:52 ` Tobias Waldekranz
  2022-03-14 16:27   ` Vladimir Oltean
  13 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14  9:52 UTC (permalink / raw)
  To: davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

Allocate a SID in the STU for each MSTID in use by a bridge and handle
the mapping of MSTIDs to VLANs using the SID field of each VTU entry.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 251 ++++++++++++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/chip.h |  13 ++
 2 files changed, 257 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c14a62aa6a6c..c23dbf37aeec 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1667,24 +1667,32 @@ static int mv88e6xxx_pvt_setup(struct mv88e6xxx_chip *chip)
 	return 0;
 }
 
-static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
+static void mv88e6xxx_port_fast_age_fid(struct mv88e6xxx_chip *chip, int port,
+					u16 fid)
 {
-	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
-	if (dsa_to_port(ds, port)->lag)
+	if (dsa_to_port(chip->ds, port)->lag)
 		/* Hardware is incapable of fast-aging a LAG through a
 		 * regular ATU move operation. Until we have something
 		 * more fancy in place this is a no-op.
 		 */
 		return;
 
-	mv88e6xxx_reg_lock(chip);
-	err = mv88e6xxx_g1_atu_remove(chip, 0, port, false);
-	mv88e6xxx_reg_unlock(chip);
+	err = mv88e6xxx_g1_atu_remove(chip, fid, port, false);
 
 	if (err)
-		dev_err(ds->dev, "p%d: failed to flush ATU\n", port);
+		dev_err(chip->ds->dev, "p%d: failed to flush ATU (FID %u)\n",
+			port, fid);
+}
+
+static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+
+	mv88e6xxx_reg_lock(chip);
+	mv88e6xxx_port_fast_age_fid(chip, port, 0);
+	mv88e6xxx_reg_unlock(chip);
 }
 
 static int mv88e6xxx_vtu_setup(struct mv88e6xxx_chip *chip)
@@ -1818,6 +1826,159 @@ static int mv88e6xxx_stu_setup(struct mv88e6xxx_chip *chip)
 	return mv88e6xxx_stu_loadpurge(chip, &stu);
 }
 
+static int mv88e6xxx_sid_get(struct mv88e6xxx_chip *chip, u8 *sid)
+{
+	DECLARE_BITMAP(busy, MV88E6XXX_N_SID) = { 0 };
+	struct mv88e6xxx_mst *mst;
+
+	set_bit(0, busy);
+
+	list_for_each_entry(mst, &chip->msts, node) {
+		set_bit(mst->stu.sid, busy);
+	}
+
+	*sid = find_first_zero_bit(busy, MV88E6XXX_N_SID);
+
+	return (*sid >= mv88e6xxx_max_sid(chip)) ? -ENOSPC : 0;
+}
+
+static int mv88e6xxx_mst_put(struct mv88e6xxx_chip *chip, u8 sid)
+{
+	struct mv88e6xxx_mst *mst, *tmp;
+	int err;
+
+	if (!sid)
+		return 0;
+
+	list_for_each_entry_safe(mst, tmp, &chip->msts, node) {
+		if (mst->stu.sid != sid)
+			continue;
+
+		if (!refcount_dec_and_test(&mst->refcnt))
+			return 0;
+
+		mst->stu.valid = false;
+		err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
+		if (err)
+			return err;
+
+		list_del(&mst->node);
+		kfree(mst);
+		return 0;
+	}
+
+	return -ENOENT;
+}
+
+static int mv88e6xxx_mst_get(struct mv88e6xxx_chip *chip, struct net_device *br,
+			     u16 msti, u8 *sid)
+{
+	struct mv88e6xxx_mst *mst;
+	int err, i;
+
+	if (!mv88e6xxx_has_stu(chip)) {
+		err = -EOPNOTSUPP;
+		goto err;
+	}
+
+	if (!msti) {
+		*sid = 0;
+		return 0;
+	}
+
+	list_for_each_entry(mst, &chip->msts, node) {
+		if (mst->br == br && mst->msti == msti) {
+			refcount_inc(&mst->refcnt);
+			*sid = mst->stu.sid;
+			return 0;
+		}
+	}
+
+	err = mv88e6xxx_sid_get(chip, sid);
+	if (err)
+		goto err;
+
+	mst = kzalloc(sizeof(*mst), GFP_KERNEL);
+	if (!mst) {
+		err = -ENOMEM;
+		goto err;
+	}
+
+	INIT_LIST_HEAD(&mst->node);
+	refcount_set(&mst->refcnt, 1);
+	mst->br = br;
+	mst->msti = msti;
+	mst->stu.valid = true;
+	mst->stu.sid = *sid;
+
+	/* The bridge starts out all ports in the disabled state. But
+	 * a STU state of disabled means to go by the port-global
+	 * state. So we set all user port's initial state to blocking,
+	 * to match the bridge's behavior.
+	 */
+	for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
+		mst->stu.state[i] = dsa_is_user_port(chip->ds, i) ?
+			MV88E6XXX_PORT_CTL0_STATE_BLOCKING :
+			MV88E6XXX_PORT_CTL0_STATE_DISABLED;
+
+	err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
+	if (err)
+		goto err_free;
+
+	list_add_tail(&mst->node, &chip->msts);
+	return 0;
+
+err_free:
+	kfree(mst);
+err:
+	return err;
+}
+
+static int mv88e6xxx_port_mst_state_set(struct dsa_switch *ds, int port,
+					const struct switchdev_mst_state *st)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct mv88e6xxx_mst *mst;
+	u8 state;
+	int err;
+
+	if (!mv88e6xxx_has_stu(chip))
+		return -EOPNOTSUPP;
+
+	switch (st->state) {
+	case BR_STATE_DISABLED:
+	case BR_STATE_BLOCKING:
+	case BR_STATE_LISTENING:
+		state = MV88E6XXX_PORT_CTL0_STATE_BLOCKING;
+		break;
+	case BR_STATE_LEARNING:
+		state = MV88E6XXX_PORT_CTL0_STATE_LEARNING;
+		break;
+	case BR_STATE_FORWARDING:
+		state = MV88E6XXX_PORT_CTL0_STATE_FORWARDING;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	list_for_each_entry(mst, &chip->msts, node) {
+		if (mst->br == dsa_port_bridge_dev_get(dp) &&
+		    mst->msti == st->msti) {
+			if (mst->stu.state[port] == state)
+				return 0;
+
+			mst->stu.state[port] = state;
+			mv88e6xxx_reg_lock(chip);
+			err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
+			mv88e6xxx_reg_unlock(chip);
+			return err;
+		}
+	}
+
+	return -ENOENT;
+}
+
 static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 					u16 vid)
 {
@@ -2437,6 +2598,12 @@ static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
 	if (err)
 		return err;
 
+	if (!vlan.valid && vlan.sid) {
+		err = mv88e6xxx_mst_put(chip, vlan.sid);
+		if (err)
+			return err;
+	}
+
 	return mv88e6xxx_g1_atu_remove(chip, vlan.fid, port, false);
 }
 
@@ -2482,6 +2649,72 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 	return err;
 }
 
+static void mv88e6xxx_port_vlan_fast_age(struct dsa_switch *ds, int port, u16 vid)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct mv88e6xxx_vtu_entry vlan;
+	int err;
+
+	mv88e6xxx_reg_lock(chip);
+
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
+	if (err)
+		goto unlock;
+
+	mv88e6xxx_port_fast_age_fid(chip, port, vlan.fid);
+
+unlock:
+	mv88e6xxx_reg_unlock(chip);
+
+	if (err)
+		dev_err(ds->dev, "p%d: failed to flush ATU in VID %u\n",
+			port, vid);
+}
+
+static int mv88e6xxx_vlan_msti_set(struct dsa_switch *ds,
+				   struct dsa_bridge bridge,
+				   const struct switchdev_vlan_msti *msti)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct mv88e6xxx_vtu_entry vlan;
+	u8 old_sid, new_sid;
+	int err;
+
+	mv88e6xxx_reg_lock(chip);
+
+	err = mv88e6xxx_vtu_get(chip, msti->vid, &vlan);
+	if (err)
+		goto unlock;
+
+	if (!vlan.valid) {
+		err = -EINVAL;
+		goto unlock;
+	}
+
+	old_sid = vlan.sid;
+
+	err = mv88e6xxx_mst_get(chip, bridge.dev, msti->msti, &new_sid);
+	if (err)
+		goto unlock;
+
+	if (new_sid != old_sid) {
+		vlan.sid = new_sid;
+
+		err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
+		if (err) {
+			mv88e6xxx_mst_put(chip, new_sid);
+			goto unlock;
+		}
+	}
+
+	if (old_sid)
+		err = mv88e6xxx_mst_put(chip, old_sid);
+
+unlock:
+	mv88e6xxx_reg_unlock(chip);
+	return err;
+}
+
 static int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 				  const unsigned char *addr, u16 vid,
 				  struct dsa_db db)
@@ -6008,6 +6241,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
 	mutex_init(&chip->reg_lock);
 	INIT_LIST_HEAD(&chip->mdios);
 	idr_init(&chip->policies);
+	INIT_LIST_HEAD(&chip->msts);
 
 	return chip;
 }
@@ -6540,10 +6774,13 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.port_pre_bridge_flags	= mv88e6xxx_port_pre_bridge_flags,
 	.port_bridge_flags	= mv88e6xxx_port_bridge_flags,
 	.port_stp_state_set	= mv88e6xxx_port_stp_state_set,
+	.port_mst_state_set	= mv88e6xxx_port_mst_state_set,
 	.port_fast_age		= mv88e6xxx_port_fast_age,
+	.port_vlan_fast_age	= mv88e6xxx_port_vlan_fast_age,
 	.port_vlan_filtering	= mv88e6xxx_port_vlan_filtering,
 	.port_vlan_add		= mv88e6xxx_port_vlan_add,
 	.port_vlan_del		= mv88e6xxx_port_vlan_del,
+	.vlan_msti_set		= mv88e6xxx_vlan_msti_set,
 	.port_fdb_add           = mv88e6xxx_port_fdb_add,
 	.port_fdb_del           = mv88e6xxx_port_fdb_del,
 	.port_fdb_dump          = mv88e6xxx_port_fdb_dump,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 6d4daa24d3e5..6a0b66354e1d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -297,6 +297,16 @@ struct mv88e6xxx_region_priv {
 	enum mv88e6xxx_region_id id;
 };
 
+struct mv88e6xxx_mst {
+	struct list_head node;
+
+	refcount_t refcnt;
+	struct net_device *br;
+	u16 msti;
+
+	struct mv88e6xxx_stu_entry stu;
+};
+
 struct mv88e6xxx_chip {
 	const struct mv88e6xxx_info *info;
 
@@ -397,6 +407,9 @@ struct mv88e6xxx_chip {
 
 	/* devlink regions */
 	struct devlink_region *regions[_MV88E6XXX_REGION_MAX];
+
+	/* Bridge MST to SID mappings */
+	struct list_head msts;
 };
 
 struct mv88e6xxx_bus_ops {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode
  2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
@ 2022-03-14 10:37   ` Nikolay Aleksandrov
  2022-03-14 11:09     ` Nikolay Aleksandrov
  2022-03-14 12:31   ` kernel test robot
  2022-03-14 16:43   ` kernel test robot
  2 siblings, 1 reply; 36+ messages in thread
From: Nikolay Aleksandrov @ 2022-03-14 10:37 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

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 <tobias@waldekranz.com>
> ---
[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;
>  


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states
  2022-03-14  9:52 ` [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states Tobias Waldekranz
@ 2022-03-14 10:37   ` Nikolay Aleksandrov
  2022-03-14 12:38     ` Tobias Waldekranz
  2022-03-14 14:58   ` Vladimir Oltean
  1 sibling, 1 reply; 36+ messages in thread
From: Nikolay Aleksandrov @ 2022-03-14 10:37 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

On 14/03/2022 11:52, Tobias Waldekranz wrote:
> Make it possible to change the port state in a given MSTI by extending
> the bridge port netlink interface (RTM_SETLINK on PF_BRIDGE).The
> proposed iproute2 interface would be:
> 
>     bridge mst set dev <PORT> msti <MSTI> state <STATE>
> 
> Current states in all applicable MSTIs can also be dumped via a
> corresponding RTM_GETLINK. The proposed iproute interface looks like
> this:
> 
> $ bridge mst
> port              msti
> vb1               0
> 		    state forwarding
> 		  100
> 		    state disabled
> vb2               0
> 		    state forwarding
> 		  100
> 		    state forwarding
> 
> The preexisting per-VLAN states are still valid in the MST
> mode (although they are read-only), and can be queried as usual if one
> is interested in knowing a particular VLAN's state without having to
> care about the VID to MSTI mapping (in this example VLAN 20 and 30 are
> bound to MSTI 100):
> 
> $ bridge -d vlan
> port              vlan-id
> vb1               10
> 		    state forwarding mcast_router 1
> 		  20
> 		    state disabled mcast_router 1
> 		  30
> 		    state disabled mcast_router 1
> 		  40
> 		    state forwarding mcast_router 1
> vb2               10
> 		    state forwarding mcast_router 1
> 		  20
> 		    state forwarding mcast_router 1
> 		  30
> 		    state forwarding mcast_router 1
> 		  40
> 		    state forwarding mcast_router 1
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

Hi Tobias,
A few comments below..

>  include/uapi/linux/if_bridge.h |  17 ++++++
>  include/uapi/linux/rtnetlink.h |   1 +
>  net/bridge/br_mst.c            | 105 +++++++++++++++++++++++++++++++++
>  net/bridge/br_netlink.c        |  32 +++++++++-
>  net/bridge/br_private.h        |  15 +++++
>  5 files changed, 169 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index f60244b747ae..879dfaef8da0 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -122,6 +122,7 @@ enum {
>  	IFLA_BRIDGE_VLAN_TUNNEL_INFO,
>  	IFLA_BRIDGE_MRP,
>  	IFLA_BRIDGE_CFM,
> +	IFLA_BRIDGE_MST,
>  	__IFLA_BRIDGE_MAX,
>  };
>  #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
> @@ -453,6 +454,21 @@ enum {
>  
>  #define IFLA_BRIDGE_CFM_CC_PEER_STATUS_MAX (__IFLA_BRIDGE_CFM_CC_PEER_STATUS_MAX - 1)
>  
> +enum {
> +	IFLA_BRIDGE_MST_UNSPEC,
> +	IFLA_BRIDGE_MST_ENTRY,
> +	__IFLA_BRIDGE_MST_MAX,
> +};
> +#define IFLA_BRIDGE_MST_MAX (__IFLA_BRIDGE_MST_MAX - 1)
> +
> +enum {
> +	IFLA_BRIDGE_MST_ENTRY_UNSPEC,
> +	IFLA_BRIDGE_MST_ENTRY_MSTI,
> +	IFLA_BRIDGE_MST_ENTRY_STATE,
> +	__IFLA_BRIDGE_MST_ENTRY_MAX,
> +};
> +#define IFLA_BRIDGE_MST_ENTRY_MAX (__IFLA_BRIDGE_MST_ENTRY_MAX - 1)
> +
>  struct bridge_stp_xstats {
>  	__u64 transition_blk;
>  	__u64 transition_fwd;
> @@ -786,4 +802,5 @@ enum {
>  	__BRIDGE_QUERIER_MAX
>  };
>  #define BRIDGE_QUERIER_MAX (__BRIDGE_QUERIER_MAX - 1)
> +

stray new line

>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 51530aade46e..83849a37db5b 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -817,6 +817,7 @@ enum {
>  #define RTEXT_FILTER_MRP	(1 << 4)
>  #define RTEXT_FILTER_CFM_CONFIG	(1 << 5)
>  #define RTEXT_FILTER_CFM_STATUS	(1 << 6)
> +#define RTEXT_FILTER_MST	(1 << 7)
>  
>  /* End of information exported to user level */
>  
> diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
> index 78ef5fea4d2b..df65aa7701c1 100644
> --- a/net/bridge/br_mst.c
> +++ b/net/bridge/br_mst.c
> @@ -124,3 +124,108 @@ int br_mst_set_enabled(struct net_bridge *br, bool on,
>  	br_opt_toggle(br, BROPT_MST_ENABLED, on);
>  	return 0;
>  }
> +
> +int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg)

const vg

> +{
> +	struct net_bridge_vlan *v;

const v

> +	struct nlattr *nest;
> +	unsigned long *seen;
> +	int err = 0;
> +
> +	seen = bitmap_zalloc(VLAN_N_VID, 0);
> +	if (!seen)
> +		return -ENOMEM;
> +
> +	list_for_each_entry(v, &vg->vlan_list, vlist) {
> +		if (test_bit(v->brvlan->msti, seen))
> +			continue;
> +
> +		nest = nla_nest_start_noflag(skb, IFLA_BRIDGE_MST_ENTRY);
> +		if (!nest ||
> +		    nla_put_u16(skb, IFLA_BRIDGE_MST_ENTRY_MSTI, v->brvlan->msti) ||
> +		    nla_put_u8(skb, IFLA_BRIDGE_MST_ENTRY_STATE, v->state)) {
> +			err = -EMSGSIZE;
> +			break;
> +		}
> +		nla_nest_end(skb, nest);
> +
> +		set_bit(v->brvlan->msti, seen);

__set_bit()

> +	}
> +
> +	kfree(seen);
> +	return err;
> +}
> +
> +static const struct nla_policy br_mst_nl_policy[IFLA_BRIDGE_MST_ENTRY_MAX + 1] = {
> +	[IFLA_BRIDGE_MST_ENTRY_MSTI] = NLA_POLICY_RANGE(NLA_U16,
> +						   1, /* 0 reserved for CST */
> +						   VLAN_N_VID - 1),
> +	[IFLA_BRIDGE_MST_ENTRY_STATE] = NLA_POLICY_RANGE(NLA_U8,
> +						    BR_STATE_DISABLED,
> +						    BR_STATE_BLOCKING),
> +};
> +
> +static int br_mst_parse_one(struct net_bridge_port *p,
> +			    const struct nlattr *attr,
> +			    struct netlink_ext_ack *extack)
> +{

I'd either set the state after parsing, so this function just does what it
says (parse) or I'd rename it.

> +	struct nlattr *tb[IFLA_BRIDGE_MST_ENTRY_MAX + 1];
> +	u16 msti;
> +	u8 state;
> +	int err;
> +
> +	err = nla_parse_nested(tb, IFLA_BRIDGE_MST_ENTRY_MAX, attr,
> +			       br_mst_nl_policy, extack);
> +	if (err)
> +		return err;
> +
> +	if (!tb[IFLA_BRIDGE_MST_ENTRY_MSTI]) {
> +		NL_SET_ERR_MSG_MOD(extack, "MSTI not specified");
> +		return -EINVAL;
> +	}
> +
> +	if (!tb[IFLA_BRIDGE_MST_ENTRY_STATE]) {
> +		NL_SET_ERR_MSG_MOD(extack, "State not specified");
> +		return -EINVAL;
> +	}
> +
> +	msti = nla_get_u16(tb[IFLA_BRIDGE_MST_ENTRY_MSTI]);
> +	state = nla_get_u8(tb[IFLA_BRIDGE_MST_ENTRY_STATE]);
> +
> +	br_mst_set_state(p, msti, state);
> +	return 0;
> +}
> +
> +int br_mst_parse(struct net_bridge_port *p, struct nlattr *mst_attr,
> +		 struct netlink_ext_ack *extack)

This doesn't just parse though, it also sets the state. Please rename it to
something more appropriate.

const mst_attr

> +{
> +	struct nlattr *attr;
> +	int err, msts = 0;
> +	int rem;
> +
> +	if (!br_opt_get(p->br, BROPT_MST_ENABLED)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Can't modify MST state when MST is disabled");
> +		return -EBUSY;
> +	}
> +
> +	nla_for_each_nested(attr, mst_attr, rem) {
> +		switch (nla_type(attr)) {
> +		case IFLA_BRIDGE_MST_ENTRY:
> +			err = br_mst_parse_one(p, attr, extack);
> +			break;
> +		default:
> +			continue;
> +		}
> +
> +		msts++;
> +		if (err)
> +			break;
> +	}
> +
> +	if (!msts) {
> +		NL_SET_ERR_MSG_MOD(extack, "Found no MST entries to process");
> +		err = -EINVAL;
> +	}
> +
> +	return err;
> +}
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 7d4432ca9a20..d2b4550f30d6 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -485,7 +485,8 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>  			   RTEXT_FILTER_BRVLAN_COMPRESSED |
>  			   RTEXT_FILTER_MRP |
>  			   RTEXT_FILTER_CFM_CONFIG |
> -			   RTEXT_FILTER_CFM_STATUS)) {
> +			   RTEXT_FILTER_CFM_STATUS |
> +			   RTEXT_FILTER_MST)) {
>  		af = nla_nest_start_noflag(skb, IFLA_AF_SPEC);
>  		if (!af)
>  			goto nla_put_failure;
> @@ -564,7 +565,28 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>  		nla_nest_end(skb, cfm_nest);
>  	}
>  
> +	if ((filter_mask & RTEXT_FILTER_MST) &&
> +	    br_opt_get(br, BROPT_MST_ENABLED) && port) {
> +		struct net_bridge_vlan_group *vg = nbp_vlan_group(port);

const vg

> +		struct nlattr *mst_nest;
> +		int err;
> +
> +		if (!vg || !vg->num_vlans)
> +			goto done;
> +
> +		mst_nest = nla_nest_start(skb, IFLA_BRIDGE_MST);
> +		if (!mst_nest)
> +			goto nla_put_failure;
> +
> +		err = br_mst_fill_info(skb, vg);
> +		if (err)
> +			goto nla_put_failure;
> +
> +		nla_nest_end(skb, mst_nest);
> +	}
> +

I think you should also update br_get_link_af_size_filtered() to account for the
new dump attributes based on the filter. I'd adjust vinfo_sz based on the filter
flag.

>  done:
> +
>  	if (af)
>  		nla_nest_end(skb, af);
>  	nlmsg_end(skb, nlh);
> @@ -803,6 +825,14 @@ static int br_afspec(struct net_bridge *br,
>  			if (err)
>  				return err;
>  			break;
> +		case IFLA_BRIDGE_MST:
> +			if (cmd != RTM_SETLINK || !p)
> +				return -EINVAL;

These are two different errors, please set extack appropriately
for each error.

> +
> +			err = br_mst_parse(p, attr, extack);
> +			if (err)
> +				return err;
> +			break;
>  		}
>  	}
>  
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index b907d389b63a..08d82578bd97 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1783,6 +1783,9 @@ int br_mst_vlan_set_msti(struct net_bridge_vlan *v, u16 msti);
>  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);
> +int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg);
> +int br_mst_parse(struct net_bridge_port *p, struct nlattr *mst_attr,
> +		 struct netlink_ext_ack *extack);
>  #else
>  static inline bool br_mst_is_enabled(struct net_bridge *br)
>  {
> @@ -1791,6 +1794,18 @@ static inline bool br_mst_is_enabled(struct net_bridge *br)
>  
>  static inline void br_mst_set_state(struct net_bridge_port *p,
>  				    u16 msti, u8 state) {}
> +static inline int br_mst_fill_info(struct sk_buff *skb,
> +				   struct net_bridge_vlan_group *vg)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int br_mst_parse(struct net_bridge_port *p,
> +			       struct nlattr *mst_attr,
> +			       struct netlink_ext_ack *extack)
> +{
> +	return -EOPNOTSUPP;
> +}
>  #endif
>  
>  struct nf_br_ops {


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 07/14] net: bridge: mst: Add helper to map an MSTI to a VID set
  2022-03-14  9:52 ` [PATCH v3 net-next 07/14] net: bridge: mst: Add helper to map an MSTI to a VID set Tobias Waldekranz
@ 2022-03-14 10:42   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 36+ messages in thread
From: Nikolay Aleksandrov @ 2022-03-14 10:42 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

On 14/03/2022 11:52, Tobias Waldekranz wrote:
> br_mst_get_info answers the question: "On this bridge, which VIDs are
> mapped to the given MSTI?"
> 
> This is useful in switchdev drivers, which might have to fan-out
> operations, relating to an MSTI, per VLAN.
> 
> An example: When a port's MST state changes from forwarding to
> blocking, a driver may choose to flush the dynamic FDB entries on that
> port to get faster reconvergence of the network, but this should only
> be done in the VLANs that are managed by the MSTI in question.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  include/linux/if_bridge.h |  6 ++++++
>  net/bridge/br_mst.c       | 26 ++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 3aae023a9353..46e6327fef06 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -119,6 +119,7 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
>  		     struct bridge_vlan_info *p_vinfo);
>  int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>  			 struct bridge_vlan_info *p_vinfo);
> +int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids);
>  #else
>  static inline bool br_vlan_enabled(const struct net_device *dev)
>  {
> @@ -151,6 +152,11 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>  {
>  	return -EINVAL;
>  }
> +static inline int br_mst_get_info(struct net_device *dev, u16 msti,
> +				  unsigned long *vids)
> +{
> +	return -EINVAL;
> +}
>  #endif
>  
>  #if IS_ENABLED(CONFIG_BRIDGE)
> diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
> index 7d16926a3a31..eb18dbd5838f 100644
> --- a/net/bridge/br_mst.c
> +++ b/net/bridge/br_mst.c
> @@ -13,6 +13,32 @@
>  
>  DEFINE_STATIC_KEY_FALSE(br_mst_used);
>  
> +int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids)
> +{
> +	struct net_bridge_vlan_group *vg;
> +	struct net_bridge_vlan *v;
> +	struct net_bridge *br;

const dev, vg, v, br

> +
> +	ASSERT_RTNL();
> +
> +	if (!netif_is_bridge_master(dev))
> +		return -EINVAL;
> +
> +	br = netdev_priv(dev);
> +	if (!br_opt_get(br, BROPT_MST_ENABLED))
> +		return -EINVAL;
> +
> +	vg = br_vlan_group(br);
> +
> +	list_for_each_entry(v, &vg->vlan_list, vlist) {
> +		if (v->msti == msti)
> +			set_bit(v->vid, vids);

__set_bit()

> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(br_mst_get_info);

EXPORT_SYMBOL_GPL

> +
>  static void br_mst_vlan_set_state(struct net_bridge_port *p, struct net_bridge_vlan *v,
>  				  u8 state)
>  {


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 08/14] net: bridge: mst: Add helper to check if MST is enabled
  2022-03-14  9:52 ` [PATCH v3 net-next 08/14] net: bridge: mst: Add helper to check if MST is enabled Tobias Waldekranz
@ 2022-03-14 10:43   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 36+ messages in thread
From: Nikolay Aleksandrov @ 2022-03-14 10:43 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

On 14/03/2022 11:52, Tobias Waldekranz wrote:
> This is useful for switchdev drivers that might want to refuse to join
> a bridge where MST is enabled, if the hardware can't support it.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  include/linux/if_bridge.h | 5 +++++
>  net/bridge/br_mst.c       | 9 +++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 46e6327fef06..5dbab0a280a6 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -119,6 +119,7 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
>  		     struct bridge_vlan_info *p_vinfo);
>  int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>  			 struct bridge_vlan_info *p_vinfo);
> +bool br_mst_enabled(struct net_device *dev);
>  int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids);
>  #else
>  static inline bool br_vlan_enabled(const struct net_device *dev)
> @@ -152,6 +153,10 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>  {
>  	return -EINVAL;
>  }
> +static inline bool br_mst_enabled(struct net_device *dev)
> +{
> +	return false;
> +}
>  static inline int br_mst_get_info(struct net_device *dev, u16 msti,
>  				  unsigned long *vids)
>  {
> diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
> index eb18dbd5838f..e5ab2ce451c2 100644
> --- a/net/bridge/br_mst.c
> +++ b/net/bridge/br_mst.c
> @@ -13,6 +13,15 @@
>  
>  DEFINE_STATIC_KEY_FALSE(br_mst_used);
>  
> +bool br_mst_enabled(struct net_device *dev)

const dev

> +{
> +	if (!netif_is_bridge_master(dev))
> +		return false;
> +
> +	return br_opt_get(netdev_priv(dev), BROPT_MST_ENABLED);
> +}
> +EXPORT_SYMBOL(br_mst_enabled);

EXPORT_SYMBOL_GPL

> +
>  int br_mst_get_info(struct net_device *dev, u16 msti, unsigned long *vids)
>  {
>  	struct net_bridge_vlan_group *vg;


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 02/14] net: bridge: mst: Allow changing a VLAN's MSTI
  2022-03-14  9:52 ` [PATCH v3 net-next 02/14] net: bridge: mst: Allow changing a VLAN's MSTI Tobias Waldekranz
@ 2022-03-14 10:45   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 36+ messages in thread
From: Nikolay Aleksandrov @ 2022-03-14 10:45 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

On 14/03/2022 11:52, Tobias Waldekranz wrote:
> Allow a VLAN to move out of the CST (MSTI 0), to an independent tree.
> 
> The user manages the VID to MSTI mappings via a global VLAN
> setting. The proposed iproute2 interface would be:
> 
>     bridge vlan global set dev br0 vid <VID> msti <MSTI>
> 
> Changing the state in non-zero MSTIs is still not supported, but will
> be addressed in upcoming changes.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  include/uapi/linux/if_bridge.h |  1 +
>  net/bridge/br_mst.c            | 42 ++++++++++++++++++++++++++++++++++
>  net/bridge/br_private.h        |  1 +
>  net/bridge/br_vlan_options.c   | 15 ++++++++++++
>  4 files changed, 59 insertions(+)
> 

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode
  2022-03-14 10:37   ` Nikolay Aleksandrov
@ 2022-03-14 11:09     ` Nikolay Aleksandrov
  0 siblings, 0 replies; 36+ messages in thread
From: Nikolay Aleksandrov @ 2022-03-14 11:09 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

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 <tobias@waldekranz.com>
>> ---
> [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 <razor@blackwall.org>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode
  2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
  2022-03-14 10:37   ` Nikolay Aleksandrov
@ 2022-03-14 12:31   ` kernel test robot
  2022-03-14 16:43   ` kernel test robot
  2 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2022-03-14 12:31 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: kbuild-all, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, Jiri Pirko, Ivan Vecera, Roopa Prabhu,
	Nikolay Aleksandrov, Russell King, Ido Schimmel, Petr Machata,
	Cooper Lees, Matt Johnston, linux-kernel, netdev, bridge

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/202203142009.7OAfQ0fR-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states
  2022-03-14 10:37   ` Nikolay Aleksandrov
@ 2022-03-14 12:38     ` Tobias Waldekranz
  0 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14 12:38 UTC (permalink / raw)
  To: Nikolay Aleksandrov, davem, kuba
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Russell King,
	Ido Schimmel, Petr Machata, Cooper Lees, Matt Johnston,
	linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 12:37, Nikolay Aleksandrov <razor@blackwall.org> wrote:
> On 14/03/2022 11:52, Tobias Waldekranz wrote:
>> Make it possible to change the port state in a given MSTI by extending
>> the bridge port netlink interface (RTM_SETLINK on PF_BRIDGE).The
>> proposed iproute2 interface would be:
>> 
>>     bridge mst set dev <PORT> msti <MSTI> state <STATE>
>> 
>> Current states in all applicable MSTIs can also be dumped via a
>> corresponding RTM_GETLINK. The proposed iproute interface looks like
>> this:
>> 
>> $ bridge mst
>> port              msti
>> vb1               0
>> 		    state forwarding
>> 		  100
>> 		    state disabled
>> vb2               0
>> 		    state forwarding
>> 		  100
>> 		    state forwarding
>> 
>> The preexisting per-VLAN states are still valid in the MST
>> mode (although they are read-only), and can be queried as usual if one
>> is interested in knowing a particular VLAN's state without having to
>> care about the VID to MSTI mapping (in this example VLAN 20 and 30 are
>> bound to MSTI 100):
>> 
>> $ bridge -d vlan
>> port              vlan-id
>> vb1               10
>> 		    state forwarding mcast_router 1
>> 		  20
>> 		    state disabled mcast_router 1
>> 		  30
>> 		    state disabled mcast_router 1
>> 		  40
>> 		    state forwarding mcast_router 1
>> vb2               10
>> 		    state forwarding mcast_router 1
>> 		  20
>> 		    state forwarding mcast_router 1
>> 		  30
>> 		    state forwarding mcast_router 1
>> 		  40
>> 		    state forwarding mcast_router 1
>> 
>> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>> ---
>
> Hi Tobias,
> A few comments below..
>
>>  include/uapi/linux/if_bridge.h |  17 ++++++
>>  include/uapi/linux/rtnetlink.h |   1 +
>>  net/bridge/br_mst.c            | 105 +++++++++++++++++++++++++++++++++
>>  net/bridge/br_netlink.c        |  32 +++++++++-
>>  net/bridge/br_private.h        |  15 +++++
>>  5 files changed, 169 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index f60244b747ae..879dfaef8da0 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -122,6 +122,7 @@ enum {
>>  	IFLA_BRIDGE_VLAN_TUNNEL_INFO,
>>  	IFLA_BRIDGE_MRP,
>>  	IFLA_BRIDGE_CFM,
>> +	IFLA_BRIDGE_MST,
>>  	__IFLA_BRIDGE_MAX,
>>  };
>>  #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>> @@ -453,6 +454,21 @@ enum {
>>  
>>  #define IFLA_BRIDGE_CFM_CC_PEER_STATUS_MAX (__IFLA_BRIDGE_CFM_CC_PEER_STATUS_MAX - 1)
>>  
>> +enum {
>> +	IFLA_BRIDGE_MST_UNSPEC,
>> +	IFLA_BRIDGE_MST_ENTRY,
>> +	__IFLA_BRIDGE_MST_MAX,
>> +};
>> +#define IFLA_BRIDGE_MST_MAX (__IFLA_BRIDGE_MST_MAX - 1)
>> +
>> +enum {
>> +	IFLA_BRIDGE_MST_ENTRY_UNSPEC,
>> +	IFLA_BRIDGE_MST_ENTRY_MSTI,
>> +	IFLA_BRIDGE_MST_ENTRY_STATE,
>> +	__IFLA_BRIDGE_MST_ENTRY_MAX,
>> +};
>> +#define IFLA_BRIDGE_MST_ENTRY_MAX (__IFLA_BRIDGE_MST_ENTRY_MAX - 1)
>> +
>>  struct bridge_stp_xstats {
>>  	__u64 transition_blk;
>>  	__u64 transition_fwd;
>> @@ -786,4 +802,5 @@ enum {
>>  	__BRIDGE_QUERIER_MAX
>>  };
>>  #define BRIDGE_QUERIER_MAX (__BRIDGE_QUERIER_MAX - 1)
>> +
>
> stray new line

Well that's embarrassing :)

>>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */
>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> index 51530aade46e..83849a37db5b 100644
>> --- a/include/uapi/linux/rtnetlink.h
>> +++ b/include/uapi/linux/rtnetlink.h
>> @@ -817,6 +817,7 @@ enum {
>>  #define RTEXT_FILTER_MRP	(1 << 4)
>>  #define RTEXT_FILTER_CFM_CONFIG	(1 << 5)
>>  #define RTEXT_FILTER_CFM_STATUS	(1 << 6)
>> +#define RTEXT_FILTER_MST	(1 << 7)
>>  
>>  /* End of information exported to user level */
>>  
>> diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
>> index 78ef5fea4d2b..df65aa7701c1 100644
>> --- a/net/bridge/br_mst.c
>> +++ b/net/bridge/br_mst.c
>> @@ -124,3 +124,108 @@ int br_mst_set_enabled(struct net_bridge *br, bool on,
>>  	br_opt_toggle(br, BROPT_MST_ENABLED, on);
>>  	return 0;
>>  }
>> +
>> +int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg)
>
> const vg
>
>> +{
>> +	struct net_bridge_vlan *v;
>
> const v
>
>> +	struct nlattr *nest;
>> +	unsigned long *seen;
>> +	int err = 0;
>> +
>> +	seen = bitmap_zalloc(VLAN_N_VID, 0);
>> +	if (!seen)
>> +		return -ENOMEM;
>> +
>> +	list_for_each_entry(v, &vg->vlan_list, vlist) {
>> +		if (test_bit(v->brvlan->msti, seen))
>> +			continue;
>> +
>> +		nest = nla_nest_start_noflag(skb, IFLA_BRIDGE_MST_ENTRY);
>> +		if (!nest ||
>> +		    nla_put_u16(skb, IFLA_BRIDGE_MST_ENTRY_MSTI, v->brvlan->msti) ||
>> +		    nla_put_u8(skb, IFLA_BRIDGE_MST_ENTRY_STATE, v->state)) {
>> +			err = -EMSGSIZE;
>> +			break;
>> +		}
>> +		nla_nest_end(skb, nest);
>> +
>> +		set_bit(v->brvlan->msti, seen);
>
> __set_bit()
>
>> +	}
>> +
>> +	kfree(seen);
>> +	return err;
>> +}
>> +
>> +static const struct nla_policy br_mst_nl_policy[IFLA_BRIDGE_MST_ENTRY_MAX + 1] = {
>> +	[IFLA_BRIDGE_MST_ENTRY_MSTI] = NLA_POLICY_RANGE(NLA_U16,
>> +						   1, /* 0 reserved for CST */
>> +						   VLAN_N_VID - 1),
>> +	[IFLA_BRIDGE_MST_ENTRY_STATE] = NLA_POLICY_RANGE(NLA_U8,
>> +						    BR_STATE_DISABLED,
>> +						    BR_STATE_BLOCKING),
>> +};
>> +
>> +static int br_mst_parse_one(struct net_bridge_port *p,
>> +			    const struct nlattr *attr,
>> +			    struct netlink_ext_ack *extack)
>> +{
>
> I'd either set the state after parsing, so this function just does what it
> says (parse) or I'd rename it.
>
>> +	struct nlattr *tb[IFLA_BRIDGE_MST_ENTRY_MAX + 1];
>> +	u16 msti;
>> +	u8 state;
>> +	int err;
>> +
>> +	err = nla_parse_nested(tb, IFLA_BRIDGE_MST_ENTRY_MAX, attr,
>> +			       br_mst_nl_policy, extack);
>> +	if (err)
>> +		return err;
>> +
>> +	if (!tb[IFLA_BRIDGE_MST_ENTRY_MSTI]) {
>> +		NL_SET_ERR_MSG_MOD(extack, "MSTI not specified");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!tb[IFLA_BRIDGE_MST_ENTRY_STATE]) {
>> +		NL_SET_ERR_MSG_MOD(extack, "State not specified");
>> +		return -EINVAL;
>> +	}
>> +
>> +	msti = nla_get_u16(tb[IFLA_BRIDGE_MST_ENTRY_MSTI]);
>> +	state = nla_get_u8(tb[IFLA_BRIDGE_MST_ENTRY_STATE]);
>> +
>> +	br_mst_set_state(p, msti, state);
>> +	return 0;
>> +}
>> +
>> +int br_mst_parse(struct net_bridge_port *p, struct nlattr *mst_attr,
>> +		 struct netlink_ext_ack *extack)
>
> This doesn't just parse though, it also sets the state. Please rename it to
> something more appropriate.
>
> const mst_attr
>
>> +{
>> +	struct nlattr *attr;
>> +	int err, msts = 0;
>> +	int rem;
>> +
>> +	if (!br_opt_get(p->br, BROPT_MST_ENABLED)) {
>> +		NL_SET_ERR_MSG_MOD(extack, "Can't modify MST state when MST is disabled");
>> +		return -EBUSY;
>> +	}
>> +
>> +	nla_for_each_nested(attr, mst_attr, rem) {
>> +		switch (nla_type(attr)) {
>> +		case IFLA_BRIDGE_MST_ENTRY:
>> +			err = br_mst_parse_one(p, attr, extack);
>> +			break;
>> +		default:
>> +			continue;
>> +		}
>> +
>> +		msts++;
>> +		if (err)
>> +			break;
>> +	}
>> +
>> +	if (!msts) {
>> +		NL_SET_ERR_MSG_MOD(extack, "Found no MST entries to process");
>> +		err = -EINVAL;
>> +	}
>> +
>> +	return err;
>> +}
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 7d4432ca9a20..d2b4550f30d6 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -485,7 +485,8 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>>  			   RTEXT_FILTER_BRVLAN_COMPRESSED |
>>  			   RTEXT_FILTER_MRP |
>>  			   RTEXT_FILTER_CFM_CONFIG |
>> -			   RTEXT_FILTER_CFM_STATUS)) {
>> +			   RTEXT_FILTER_CFM_STATUS |
>> +			   RTEXT_FILTER_MST)) {
>>  		af = nla_nest_start_noflag(skb, IFLA_AF_SPEC);
>>  		if (!af)
>>  			goto nla_put_failure;
>> @@ -564,7 +565,28 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>>  		nla_nest_end(skb, cfm_nest);
>>  	}
>>  
>> +	if ((filter_mask & RTEXT_FILTER_MST) &&
>> +	    br_opt_get(br, BROPT_MST_ENABLED) && port) {
>> +		struct net_bridge_vlan_group *vg = nbp_vlan_group(port);
>
> const vg
>
>> +		struct nlattr *mst_nest;
>> +		int err;
>> +
>> +		if (!vg || !vg->num_vlans)
>> +			goto done;
>> +
>> +		mst_nest = nla_nest_start(skb, IFLA_BRIDGE_MST);
>> +		if (!mst_nest)
>> +			goto nla_put_failure;
>> +
>> +		err = br_mst_fill_info(skb, vg);
>> +		if (err)
>> +			goto nla_put_failure;
>> +
>> +		nla_nest_end(skb, mst_nest);
>> +	}
>> +
>
> I think you should also update br_get_link_af_size_filtered() to account for the
> new dump attributes based on the filter. I'd adjust vinfo_sz based on the filter
> flag.
>
>>  done:
>> +
>>  	if (af)
>>  		nla_nest_end(skb, af);
>>  	nlmsg_end(skb, nlh);
>> @@ -803,6 +825,14 @@ static int br_afspec(struct net_bridge *br,
>>  			if (err)
>>  				return err;
>>  			break;
>> +		case IFLA_BRIDGE_MST:
>> +			if (cmd != RTM_SETLINK || !p)
>> +				return -EINVAL;
>
> These are two different errors, please set extack appropriately
> for each error.

Thanks for the review, all of the above will be fixed in v4.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states
  2022-03-14  9:52 ` [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states Tobias Waldekranz
  2022-03-14 10:37   ` Nikolay Aleksandrov
@ 2022-03-14 14:58   ` Vladimir Oltean
  2022-03-14 15:42     ` Tobias Waldekranz
  1 sibling, 1 reply; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 14:58 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 10:52:20AM +0100, Tobias Waldekranz wrote:
> +int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg)
> +{
> +	struct net_bridge_vlan *v;
> +	struct nlattr *nest;
> +	unsigned long *seen;
> +	int err = 0;
> +
> +	seen = bitmap_zalloc(VLAN_N_VID, 0);

I see there is precedent in the bridge driver for using dynamic
allocation as opposed to on-stack declaration using DECLARE_BITMAP().
I imagine this isn't just to be "heapsters", but why?

I don't have a very good sense of how much on-stack memory is too much
(a lot probably depends on the expected depth of the call stack too, and here it
doesn't appear to be too deep), but I see that mlxsw_sp_bridge_vxlan_vlan_is_valid()
has a DECLARE_BITMAP(vlans, VLAN_N_VID) too.

The comment applies for callers of br_mst_get_info() too.

> +	if (!seen)
> +		return -ENOMEM;
> +
> +	list_for_each_entry(v, &vg->vlan_list, vlist) {
> +		if (test_bit(v->brvlan->msti, seen))
> +			continue;
> +
> +		nest = nla_nest_start_noflag(skb, IFLA_BRIDGE_MST_ENTRY);
> +		if (!nest ||
> +		    nla_put_u16(skb, IFLA_BRIDGE_MST_ENTRY_MSTI, v->brvlan->msti) ||
> +		    nla_put_u8(skb, IFLA_BRIDGE_MST_ENTRY_STATE, v->state)) {
> +			err = -EMSGSIZE;
> +			break;
> +		}
> +		nla_nest_end(skb, nest);
> +
> +		set_bit(v->brvlan->msti, seen);
> +	}
> +
> +	kfree(seen);
> +	return err;
> +}

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states
  2022-03-14 14:58   ` Vladimir Oltean
@ 2022-03-14 15:42     ` Tobias Waldekranz
  0 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14 15:42 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 16:58, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Mar 14, 2022 at 10:52:20AM +0100, Tobias Waldekranz wrote:
>> +int br_mst_fill_info(struct sk_buff *skb, struct net_bridge_vlan_group *vg)
>> +{
>> +	struct net_bridge_vlan *v;
>> +	struct nlattr *nest;
>> +	unsigned long *seen;
>> +	int err = 0;
>> +
>> +	seen = bitmap_zalloc(VLAN_N_VID, 0);
>
> I see there is precedent in the bridge driver for using dynamic
> allocation as opposed to on-stack declaration using DECLARE_BITMAP().
> I imagine this isn't just to be "heapsters", but why?
>
> I don't have a very good sense of how much on-stack memory is too much
> (a lot probably depends on the expected depth of the call stack too, and here it
> doesn't appear to be too deep), but I see that mlxsw_sp_bridge_vxlan_vlan_is_valid()
> has a DECLARE_BITMAP(vlans, VLAN_N_VID) too.
>
> The comment applies for callers of br_mst_get_info() too.

In v4, things become even worse, as I need to allocate the bitmap in a
context where I can't return an error. So if it's ok to keep it on the
stack, then that would be great.

Here's the code in question:

size_t br_mst_info_size(const struct net_bridge_vlan_group *vg)
{
	const struct net_bridge_vlan *v;
	unsigned long *seen;
	size_t sz;

	seen = bitmap_zalloc(VLAN_N_VID, 0);
	if (WARN_ON(!seen))
		return 0;

	/* IFLA_BRIDGE_MST */
	sz = nla_total_size(0);

	list_for_each_entry(v, &vg->vlan_list, vlist) {
		if (test_bit(v->brvlan->msti, seen))
			continue;

		/* IFLA_BRIDGE_MST_ENTRY */
		sz += nla_total_size(0) +
			/* IFLA_BRIDGE_MST_ENTRY_MSTI */
			nla_total_size(sizeof(u16)) +
			/* IFLA_BRIDGE_MST_ENTRY_STATE */
			nla_total_size(sizeof(u8));

		__set_bit(v->brvlan->msti, seen);
	}

	kfree(seen);
	return sz;
}

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading
  2022-03-14  9:52 ` [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading Tobias Waldekranz
@ 2022-03-14 16:27   ` Vladimir Oltean
  2022-03-14 21:57     ` Tobias Waldekranz
  0 siblings, 1 reply; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 16:27 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 10:52:31AM +0100, Tobias Waldekranz wrote:
> Allocate a SID in the STU for each MSTID in use by a bridge and handle
> the mapping of MSTIDs to VLANs using the SID field of each VTU entry.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 251 ++++++++++++++++++++++++++++++-
>  drivers/net/dsa/mv88e6xxx/chip.h |  13 ++
>  2 files changed, 257 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index c14a62aa6a6c..c23dbf37aeec 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1667,24 +1667,32 @@ static int mv88e6xxx_pvt_setup(struct mv88e6xxx_chip *chip)
>  	return 0;
>  }
>  
> -static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
> +static void mv88e6xxx_port_fast_age_fid(struct mv88e6xxx_chip *chip, int port,
> +					u16 fid)
>  {
> -	struct mv88e6xxx_chip *chip = ds->priv;
>  	int err;
>  
> -	if (dsa_to_port(ds, port)->lag)
> +	if (dsa_to_port(chip->ds, port)->lag)
>  		/* Hardware is incapable of fast-aging a LAG through a
>  		 * regular ATU move operation. Until we have something
>  		 * more fancy in place this is a no-op.
>  		 */
>  		return;
>  
> -	mv88e6xxx_reg_lock(chip);
> -	err = mv88e6xxx_g1_atu_remove(chip, 0, port, false);
> -	mv88e6xxx_reg_unlock(chip);
> +	err = mv88e6xxx_g1_atu_remove(chip, fid, port, false);
>  
>  	if (err)
> -		dev_err(ds->dev, "p%d: failed to flush ATU\n", port);
> +		dev_err(chip->ds->dev, "p%d: failed to flush ATU (FID %u)\n",
> +			port, fid);
> +}
> +
> +static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
> +{
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +
> +	mv88e6xxx_reg_lock(chip);
> +	mv88e6xxx_port_fast_age_fid(chip, port, 0);
> +	mv88e6xxx_reg_unlock(chip);
>  }
>  
>  static int mv88e6xxx_vtu_setup(struct mv88e6xxx_chip *chip)
> @@ -1818,6 +1826,159 @@ static int mv88e6xxx_stu_setup(struct mv88e6xxx_chip *chip)
>  	return mv88e6xxx_stu_loadpurge(chip, &stu);
>  }
>  
> +static int mv88e6xxx_sid_get(struct mv88e6xxx_chip *chip, u8 *sid)
> +{
> +	DECLARE_BITMAP(busy, MV88E6XXX_N_SID) = { 0 };
> +	struct mv88e6xxx_mst *mst;
> +
> +	set_bit(0, busy);

__set_bit

> +
> +	list_for_each_entry(mst, &chip->msts, node) {
> +		set_bit(mst->stu.sid, busy);
> +	}

Up to you, but parentheses are generally not used for single-line blocks.

> +
> +	*sid = find_first_zero_bit(busy, MV88E6XXX_N_SID);
> +
> +	return (*sid >= mv88e6xxx_max_sid(chip)) ? -ENOSPC : 0;
> +}
> +
> +static int mv88e6xxx_mst_put(struct mv88e6xxx_chip *chip, u8 sid)
> +{
> +	struct mv88e6xxx_mst *mst, *tmp;
> +	int err;
> +
> +	if (!sid)
> +		return 0;

Very minor nitpick: since mv88e6xxx_mst_put already checks this, could
you drop the "!sid" check from callers?

> +
> +	list_for_each_entry_safe(mst, tmp, &chip->msts, node) {
> +		if (mst->stu.sid != sid)
> +			continue;
> +
> +		if (!refcount_dec_and_test(&mst->refcnt))
> +			return 0;
> +
> +		mst->stu.valid = false;
> +		err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
> +		if (err)

Should we bother with a refcount_set(&mst->refcount, 1) on error?

> +			return err;
> +
> +		list_del(&mst->node);
> +		kfree(mst);
> +		return 0;
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +static int mv88e6xxx_mst_get(struct mv88e6xxx_chip *chip, struct net_device *br,
> +			     u16 msti, u8 *sid)
> +{
> +	struct mv88e6xxx_mst *mst;
> +	int err, i;
> +
> +	if (!mv88e6xxx_has_stu(chip)) {
> +		err = -EOPNOTSUPP;
> +		goto err;
> +	}
> +
> +	if (!msti) {
> +		*sid = 0;
> +		return 0;
> +	}
> +
> +	list_for_each_entry(mst, &chip->msts, node) {
> +		if (mst->br == br && mst->msti == msti) {
> +			refcount_inc(&mst->refcnt);
> +			*sid = mst->stu.sid;
> +			return 0;
> +		}
> +	}
> +
> +	err = mv88e6xxx_sid_get(chip, sid);
> +	if (err)
> +		goto err;
> +
> +	mst = kzalloc(sizeof(*mst), GFP_KERNEL);
> +	if (!mst) {
> +		err = -ENOMEM;
> +		goto err;
> +	}
> +
> +	INIT_LIST_HEAD(&mst->node);
> +	refcount_set(&mst->refcnt, 1);
> +	mst->br = br;
> +	mst->msti = msti;
> +	mst->stu.valid = true;
> +	mst->stu.sid = *sid;
> +
> +	/* The bridge starts out all ports in the disabled state. But
> +	 * a STU state of disabled means to go by the port-global
> +	 * state. So we set all user port's initial state to blocking,
> +	 * to match the bridge's behavior.
> +	 */
> +	for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
> +		mst->stu.state[i] = dsa_is_user_port(chip->ds, i) ?
> +			MV88E6XXX_PORT_CTL0_STATE_BLOCKING :
> +			MV88E6XXX_PORT_CTL0_STATE_DISABLED;
> +
> +	err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
> +	if (err)
> +		goto err_free;
> +
> +	list_add_tail(&mst->node, &chip->msts);
> +	return 0;
> +
> +err_free:
> +	kfree(mst);
> +err:
> +	return err;
> +}
> +
> +static int mv88e6xxx_port_mst_state_set(struct dsa_switch *ds, int port,
> +					const struct switchdev_mst_state *st)
> +{
> +	struct dsa_port *dp = dsa_to_port(ds, port);
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	struct mv88e6xxx_mst *mst;
> +	u8 state;
> +	int err;
> +
> +	if (!mv88e6xxx_has_stu(chip))
> +		return -EOPNOTSUPP;
> +
> +	switch (st->state) {
> +	case BR_STATE_DISABLED:
> +	case BR_STATE_BLOCKING:
> +	case BR_STATE_LISTENING:
> +		state = MV88E6XXX_PORT_CTL0_STATE_BLOCKING;
> +		break;
> +	case BR_STATE_LEARNING:
> +		state = MV88E6XXX_PORT_CTL0_STATE_LEARNING;
> +		break;
> +	case BR_STATE_FORWARDING:
> +		state = MV88E6XXX_PORT_CTL0_STATE_FORWARDING;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry(mst, &chip->msts, node) {
> +		if (mst->br == dsa_port_bridge_dev_get(dp) &&
> +		    mst->msti == st->msti) {
> +			if (mst->stu.state[port] == state)
> +				return 0;
> +
> +			mst->stu.state[port] = state;
> +			mv88e6xxx_reg_lock(chip);
> +			err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
> +			mv88e6xxx_reg_unlock(chip);
> +			return err;
> +		}
> +	}
> +
> +	return -ENOENT;
> +}
> +
>  static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
>  					u16 vid)
>  {
> @@ -2437,6 +2598,12 @@ static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
>  	if (err)
>  		return err;
>  
> +	if (!vlan.valid && vlan.sid) {
> +		err = mv88e6xxx_mst_put(chip, vlan.sid);
> +		if (err)
> +			return err;
> +	}
> +
>  	return mv88e6xxx_g1_atu_remove(chip, vlan.fid, port, false);
>  }
>  
> @@ -2482,6 +2649,72 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
>  	return err;
>  }
>  
> +static void mv88e6xxx_port_vlan_fast_age(struct dsa_switch *ds, int port, u16 vid)
> +{
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	struct mv88e6xxx_vtu_entry vlan;
> +	int err;
> +
> +	mv88e6xxx_reg_lock(chip);
> +
> +	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
> +	if (err)
> +		goto unlock;
> +
> +	mv88e6xxx_port_fast_age_fid(chip, port, vlan.fid);
> +
> +unlock:
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	if (err)
> +		dev_err(ds->dev, "p%d: failed to flush ATU in VID %u\n",
> +			port, vid);

This error message actually corresponds to an mv88e6xxx_vtu_get() error,
so the message is kind of incorrect. mv88e6xxx_port_fast_age_fid(),
whose error code isn't propagated here, has its own "failed to flush ATU"
error message.

> +}

Otherwise this looks pretty good.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode
  2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
  2022-03-14 10:37   ` Nikolay Aleksandrov
  2022-03-14 12:31   ` kernel test robot
@ 2022-03-14 16:43   ` kernel test robot
  2 siblings, 0 replies; 36+ messages in thread
From: kernel test robot @ 2022-03-14 16:43 UTC (permalink / raw)
  To: Tobias Waldekranz, davem, kuba
  Cc: llvm, kbuild-all, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, Jiri Pirko, Ivan Vecera, Roopa Prabhu,
	Nikolay Aleksandrov, Russell King, Ido Schimmel, Petr Machata,
	Cooper Lees, Matt Johnston, linux-kernel, netdev, bridge

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/202203150034.m0Fvevlq-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14  9:52 ` [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST Tobias Waldekranz
@ 2022-03-14 16:56   ` Vladimir Oltean
  2022-03-14 17:55     ` Vladimir Oltean
  2022-03-14 17:51   ` Vladimir Oltean
  1 sibling, 1 reply; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 16:56 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 10:52:26AM +0100, Tobias Waldekranz wrote:
> When joining a bridge where MST is enabled, we validate that the
> proper offloading support is in place, otherwise we fallback to
> software bridging.
> 
> When then mode is changed on a bridge in which we are members, we
> refuse the change if offloading is not supported.
> 
> At the moment we only check for configurable learning, but this will
> be further restricted as we support more MST related switchdev events.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  net/dsa/dsa_priv.h |  2 ++
>  net/dsa/port.c     | 20 ++++++++++++++++++++
>  net/dsa/slave.c    |  6 ++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index f20bdd8ea0a8..2aba420696ef 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -234,6 +234,8 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
>  			    struct netlink_ext_ack *extack);
>  bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
>  int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
> +int dsa_port_mst_enable(struct dsa_port *dp, bool on,
> +			struct netlink_ext_ack *extack);
>  int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
>  			bool targeted_match);
>  int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 58291df14cdb..1a17a0efa2fa 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
>  	if (err && err != -EOPNOTSUPP)
>  		return err;
>  
> +	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
> +	if (err && err != -EOPNOTSUPP)
> +		return err;

Sadly this will break down because we don't have unwinding on error in
place (sorry). We'd end up with an unoffloaded bridge port with
partially synced bridge port attributes. Could you please add a patch
previous to this one that handles this, and unoffloads those on error?

> +
>  	return 0;
>  }
>  
> @@ -735,6 +739,22 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
>  	return 0;
>  }
>  
> +int dsa_port_mst_enable(struct dsa_port *dp, bool on,
> +			struct netlink_ext_ack *extack)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +
> +	if (!on)
> +		return 0;
> +
> +	if (!dsa_port_can_configure_learning(dp)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Hardware does not support MST");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
>  			      struct switchdev_brport_flags flags,
>  			      struct netlink_ext_ack *extack)
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index a61a7c54af20..333f5702ea4f 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -463,6 +463,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
>  
>  		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
>  		break;
> +	case SWITCHDEV_ATTR_ID_BRIDGE_MST:
> +		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
> +			return -EOPNOTSUPP;
> +
> +		ret = dsa_port_mst_enable(dp, attr->u.mst, extack);
> +		break;
>  	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
>  		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
>  			return -EOPNOTSUPP;
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 10/14] net: dsa: Pass VLAN MSTI migration notifications to driver
  2022-03-14  9:52 ` [PATCH v3 net-next 10/14] net: dsa: Pass VLAN MSTI migration notifications to driver Tobias Waldekranz
@ 2022-03-14 17:07   ` Vladimir Oltean
  0 siblings, 0 replies; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 17:07 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 10:52:27AM +0100, Tobias Waldekranz wrote:
> Add the usual trampoline functionality from the generic DSA layer down
> to the drivers for VLAN MSTI migrations.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 11/14] net: dsa: Handle MST state changes
  2022-03-14  9:52 ` [PATCH v3 net-next 11/14] net: dsa: Handle MST state changes Tobias Waldekranz
@ 2022-03-14 17:14   ` Vladimir Oltean
  0 siblings, 0 replies; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 17:14 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 10:52:28AM +0100, Tobias Waldekranz wrote:
> Add the usual trampoline functionality from the generic DSA layer down
> to the drivers for MST state changes.
> 
> When a state changes to disabled/blocking/listening, make sure to fast
> age any dynamic entries in the affected VLANs (those controlled by the
> MSTI in question).
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  include/net/dsa.h  |  3 +++
>  net/dsa/dsa_priv.h |  2 ++
>  net/dsa/port.c     | 67 +++++++++++++++++++++++++++++++++++++++++++---
>  net/dsa/slave.c    |  6 +++++
>  4 files changed, 74 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 1ddaa2cc5842..a171e7cdb3fe 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -945,7 +945,10 @@ struct dsa_switch_ops {
>  				     struct dsa_bridge bridge);
>  	void	(*port_stp_state_set)(struct dsa_switch *ds, int port,
>  				      u8 state);
> +	int	(*port_mst_state_set)(struct dsa_switch *ds, int port,
> +				      const struct switchdev_mst_state *state);
>  	void	(*port_fast_age)(struct dsa_switch *ds, int port);
> +	void	(*port_vlan_fast_age)(struct dsa_switch *ds, int port, u16 vid);
>  	int	(*port_pre_bridge_flags)(struct dsa_switch *ds, int port,
>  					 struct switchdev_brport_flags flags,
>  					 struct netlink_ext_ack *extack);
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index d90b4cf0c9d2..2ae8996cf7c8 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -215,6 +215,8 @@ static inline struct net_device *dsa_master_find_slave(struct net_device *dev,
>  void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
>  			       const struct dsa_device_ops *tag_ops);
>  int dsa_port_set_state(struct dsa_port *dp, u8 state, bool do_fast_age);
> +int dsa_port_set_mst_state(struct dsa_port *dp,
> +			   const struct switchdev_mst_state *state);
>  int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy);
>  int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
>  void dsa_port_disable_rt(struct dsa_port *dp);
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index f6a822d854cc..223681e03321 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -30,12 +30,11 @@ static int dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
>  	return dsa_tree_notify(dp->ds->dst, e, v);
>  }
>  
> -static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp)
> +static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp, u16 vid)
>  {
>  	struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
>  	struct switchdev_notifier_fdb_info info = {
> -		/* flush all VLANs */
> -		.vid = 0,
> +		.vid = vid,
>  	};
>  
>  	/* When the port becomes standalone it has already left the bridge.
> @@ -57,7 +56,39 @@ static void dsa_port_fast_age(const struct dsa_port *dp)
>  
>  	ds->ops->port_fast_age(ds, dp->index);
>  
> -	dsa_port_notify_bridge_fdb_flush(dp);

Could you please migrate the "flush all VLANs" comment here?

> +	dsa_port_notify_bridge_fdb_flush(dp, 0);
> +}
> +
> +static void dsa_port_vlan_fast_age(const struct dsa_port *dp, u16 vid)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +
> +	if (!ds->ops->port_vlan_fast_age)
> +		return;
> +
> +	ds->ops->port_vlan_fast_age(ds, dp->index, vid);
> +
> +	dsa_port_notify_bridge_fdb_flush(dp, vid);
> +}
> +
> +static void dsa_port_msti_fast_age(const struct dsa_port *dp, u16 msti)
> +{
> +	unsigned long *vids;
> +	int vid;
> +
> +	vids = bitmap_zalloc(VLAN_N_VID, 0);
> +	if (!vids)
> +		return;

As discussed, I think you can/should use DECLARE_BITMAP().

> +
> +	if (br_mst_get_info(dsa_port_bridge_dev_get(dp), msti, vids))
> +		goto out_free;
> +
> +	for_each_set_bit(vid, vids, VLAN_N_VID) {
> +		dsa_port_vlan_fast_age(dp, vid);

This has an error propagation path to user space, since just user space
sets it via IFLA_BRIDGE_MST_ENTRY_STATE, does it not?
So could we actually propagate an error all the way from
ds->ops->port_vlan_fast_age to the mstpd daemon?

> +	}
> +
> +out_free:
> +	kfree(vids);
>  }
>  
>  static bool dsa_port_can_configure_learning(struct dsa_port *dp)
> @@ -118,6 +149,32 @@ static void dsa_port_set_state_now(struct dsa_port *dp, u8 state,
>  		pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
>  }
>  
> +int dsa_port_set_mst_state(struct dsa_port *dp,
> +			   const struct switchdev_mst_state *state)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +	int err;
> +
> +	if (!ds->ops->port_mst_state_set)
> +		return -EOPNOTSUPP;
> +
> +	err = ds->ops->port_mst_state_set(ds, dp->index, state);
> +	if (err)
> +		return err;
> +
> +	if (dp->learning) {
> +		switch (state->state) {
> +		case BR_STATE_DISABLED:
> +		case BR_STATE_BLOCKING:
> +		case BR_STATE_LISTENING:
> +			dsa_port_msti_fast_age(dp, state->msti);
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy)
>  {
>  	struct dsa_switch *ds = dp->ds;
> @@ -748,6 +805,8 @@ int dsa_port_mst_enable(struct dsa_port *dp, bool on,
>  		return 0;
>  
>  	if (!(ds->ops->vlan_msti_set &&
> +	      ds->ops->port_mst_state_set &&
> +	      ds->ops->port_vlan_fast_age &&
>  	      dsa_port_can_configure_learning(dp))) {
>  		NL_SET_ERR_MSG_MOD(extack, "Hardware does not support MST");
>  		return -EINVAL;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index cd2c57de9592..106b177ad1eb 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -450,6 +450,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
>  
>  		ret = dsa_port_set_state(dp, attr->u.stp_state, true);
>  		break;
> +	case SWITCHDEV_ATTR_ID_PORT_MST_STATE:
> +		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
> +			return -EOPNOTSUPP;
> +
> +		ret = dsa_port_set_mst_state(dp, &attr->u.mst_state);
> +		break;
>  	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
>  		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
>  			return -EOPNOTSUPP;
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14  9:52 ` [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST Tobias Waldekranz
  2022-03-14 16:56   ` Vladimir Oltean
@ 2022-03-14 17:51   ` Vladimir Oltean
  1 sibling, 0 replies; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 17:51 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 10:52:26AM +0100, Tobias Waldekranz wrote:
> When joining a bridge where MST is enabled, we validate that the
> proper offloading support is in place, otherwise we fallback to
> software bridging.
> 
> When then mode is changed on a bridge in which we are members, we
> refuse the change if offloading is not supported.
> 
> At the moment we only check for configurable learning, but this will
> be further restricted as we support more MST related switchdev events.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  net/dsa/dsa_priv.h |  2 ++
>  net/dsa/port.c     | 20 ++++++++++++++++++++
>  net/dsa/slave.c    |  6 ++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index f20bdd8ea0a8..2aba420696ef 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -234,6 +234,8 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
>  			    struct netlink_ext_ack *extack);
>  bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
>  int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
> +int dsa_port_mst_enable(struct dsa_port *dp, bool on,
> +			struct netlink_ext_ack *extack);
>  int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
>  			bool targeted_match);
>  int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 58291df14cdb..1a17a0efa2fa 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
>  	if (err && err != -EOPNOTSUPP)
>  		return err;
>  
> +	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
> +	if (err && err != -EOPNOTSUPP)
> +		return err;

The "err && err != -EOPNOTSUPP" scheme is in place for compatibility
with drivers that don't all support the bridge port attributes, but used
to work prior to dsa_port_switchdev_sync_attrs()'s existence.

I don't think this scheme is necessary for dsa_port_mst_enable(), you
can just return -EOPNOTSUPP and remove the special handling for this code.

> +
>  	return 0;
>  }
>  
> @@ -735,6 +739,22 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
>  	return 0;
>  }
>  
> +int dsa_port_mst_enable(struct dsa_port *dp, bool on,
> +			struct netlink_ext_ack *extack)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +
> +	if (!on)
> +		return 0;
> +
> +	if (!dsa_port_can_configure_learning(dp)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Hardware does not support MST");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
>  			      struct switchdev_brport_flags flags,
>  			      struct netlink_ext_ack *extack)
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index a61a7c54af20..333f5702ea4f 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -463,6 +463,12 @@ static int dsa_slave_port_attr_set(struct net_device *dev, const void *ctx,
>  
>  		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
>  		break;
> +	case SWITCHDEV_ATTR_ID_BRIDGE_MST:
> +		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
> +			return -EOPNOTSUPP;
> +
> +		ret = dsa_port_mst_enable(dp, attr->u.mst, extack);
> +		break;
>  	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
>  		if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
>  			return -EOPNOTSUPP;
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14 16:56   ` Vladimir Oltean
@ 2022-03-14 17:55     ` Vladimir Oltean
  2022-03-14 20:01       ` Tobias Waldekranz
  0 siblings, 1 reply; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 17:55 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 06:56:49PM +0200, Vladimir Oltean wrote:
> > diff --git a/net/dsa/port.c b/net/dsa/port.c
> > index 58291df14cdb..1a17a0efa2fa 100644
> > --- a/net/dsa/port.c
> > +++ b/net/dsa/port.c
> > @@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
> >  	if (err && err != -EOPNOTSUPP)
> >  		return err;
> >  
> > +	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
> > +	if (err && err != -EOPNOTSUPP)
> > +		return err;
> 
> Sadly this will break down because we don't have unwinding on error in
> place (sorry). We'd end up with an unoffloaded bridge port with
> partially synced bridge port attributes. Could you please add a patch
> previous to this one that handles this, and unoffloads those on error?

Actually I would rather rename the entire dsa_port_mst_enable() function
to dsa_port_mst_validate() and move it to the beginning of dsa_port_bridge_join().
This simplifies the unwinding that needs to take place quite a bit.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14 17:55     ` Vladimir Oltean
@ 2022-03-14 20:01       ` Tobias Waldekranz
  2022-03-14 20:20         ` Vladimir Oltean
  0 siblings, 1 reply; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14 20:01 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 19:55, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Mar 14, 2022 at 06:56:49PM +0200, Vladimir Oltean wrote:
>> > diff --git a/net/dsa/port.c b/net/dsa/port.c
>> > index 58291df14cdb..1a17a0efa2fa 100644
>> > --- a/net/dsa/port.c
>> > +++ b/net/dsa/port.c
>> > @@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
>> >  	if (err && err != -EOPNOTSUPP)
>> >  		return err;
>> >  
>> > +	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
>> > +	if (err && err != -EOPNOTSUPP)
>> > +		return err;
>> 
>> Sadly this will break down because we don't have unwinding on error in
>> place (sorry). We'd end up with an unoffloaded bridge port with
>> partially synced bridge port attributes. Could you please add a patch
>> previous to this one that handles this, and unoffloads those on error?
>
> Actually I would rather rename the entire dsa_port_mst_enable() function
> to dsa_port_mst_validate() and move it to the beginning of dsa_port_bridge_join().
> This simplifies the unwinding that needs to take place quite a bit.

Well you still need to unwind vlan filtering if setting the ageing time
fails, which is the most complicated one, right? Still, I agree that
_validate is a better name, and then _bridge_join seems like a more
reasonable placement. Should the unwinding patch still be part of this
series then?

While we're here, I actually made this a hard error in both scenarios
(but forgot to update the log - will do that in v4, depending on what we
decide here). There's a dilemma:

- When reacting to the attribute event, i.e. changing the mode on a
  member we're apart of, we _can't_ return -EOPNOTSUPP as it will be
  ignored, which is why dsa_port_mst_validate (nee _enable) returns
  -EINVAL.

- When joining a bridge, we _must_ return -EOPNOTSUPP to trigger the
  software fallback.

Having something like this in dsa_port_bridge_join...

err = dsa_port_mst_validate(dp);
if (err == -EINVAL)
	return -EOPNOTSUPP;
else if (err)
	return err;

...works I suppose, but feels somewhat awkwark. Any better ideas?


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14 20:01       ` Tobias Waldekranz
@ 2022-03-14 20:20         ` Vladimir Oltean
  2022-03-14 22:13           ` Tobias Waldekranz
  0 siblings, 1 reply; 36+ messages in thread
From: Vladimir Oltean @ 2022-03-14 20:20 UTC (permalink / raw)
  To: Tobias Waldekranz
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 09:01:12PM +0100, Tobias Waldekranz wrote:
> On Mon, Mar 14, 2022 at 19:55, Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Mon, Mar 14, 2022 at 06:56:49PM +0200, Vladimir Oltean wrote:
> >> > diff --git a/net/dsa/port.c b/net/dsa/port.c
> >> > index 58291df14cdb..1a17a0efa2fa 100644
> >> > --- a/net/dsa/port.c
> >> > +++ b/net/dsa/port.c
> >> > @@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
> >> >  	if (err && err != -EOPNOTSUPP)
> >> >  		return err;
> >> >  
> >> > +	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
> >> > +	if (err && err != -EOPNOTSUPP)
> >> > +		return err;
> >> 
> >> Sadly this will break down because we don't have unwinding on error in
> >> place (sorry). We'd end up with an unoffloaded bridge port with
> >> partially synced bridge port attributes. Could you please add a patch
> >> previous to this one that handles this, and unoffloads those on error?
> >
> > Actually I would rather rename the entire dsa_port_mst_enable() function
> > to dsa_port_mst_validate() and move it to the beginning of dsa_port_bridge_join().
> > This simplifies the unwinding that needs to take place quite a bit.
> 
> Well you still need to unwind vlan filtering if setting the ageing time
> fails, which is the most complicated one, right?

Yes, but we can leave that for another day :)

...ergo

> Should the unwinding patch still be part of this series then?

no.

> Still, I agree that _validate is a better name, and then _bridge_join
> seems like a more reasonable placement.
> 
> While we're here, I actually made this a hard error in both scenarios
> (but forgot to update the log - will do that in v4, depending on what we
> decide here). There's a dilemma:
> 
> - When reacting to the attribute event, i.e. changing the mode on a
>   member we're apart of, we _can't_ return -EOPNOTSUPP as it will be
>   ignored, which is why dsa_port_mst_validate (nee _enable) returns
>   -EINVAL.
> 
> - When joining a bridge, we _must_ return -EOPNOTSUPP to trigger the
>   software fallback.
> 
> Having something like this in dsa_port_bridge_join...
> 
> err = dsa_port_mst_validate(dp);
> if (err == -EINVAL)
> 	return -EOPNOTSUPP;
> else if (err)
> 	return err;
> 
> ...works I suppose, but feels somewhat awkwark. Any better ideas?

What you can do is follow the model of dsa_switch_supports_uc_filtering(),
and create a dsa_switch_supports_mst() which is called inside an
"if br_mst_enabled(br)" check, and returns bool. When false, you could
return -EINVAL or -EOPNOTSUPP, as appropriate.

This is mostly fine, except for the pesky dsa_port_can_configure_learning(dp)
check :) So while you could name it dsa_port_supports_mst() and pass it
a dsa_port, the problem is that you can't put the implementation of this
new dsa_port_supports_mst() next to dsa_switch_supports_uc_filtering()
where it would be nice to sit for symmetry, because the latter is static
inline and we're missing the definition of dsa_port_can_configure_learning().
So.. the second best thing is to keep dsa_port_supports_mst() in the
same place where dsa_port_mst_enable() currently is.

What do you think?

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading
  2022-03-14 16:27   ` Vladimir Oltean
@ 2022-03-14 21:57     ` Tobias Waldekranz
  0 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14 21:57 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 18:27, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Mar 14, 2022 at 10:52:31AM +0100, Tobias Waldekranz wrote:
>> Allocate a SID in the STU for each MSTID in use by a bridge and handle
>> the mapping of MSTIDs to VLANs using the SID field of each VTU entry.
>> 
>> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>> ---
>>  drivers/net/dsa/mv88e6xxx/chip.c | 251 ++++++++++++++++++++++++++++++-
>>  drivers/net/dsa/mv88e6xxx/chip.h |  13 ++
>>  2 files changed, 257 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index c14a62aa6a6c..c23dbf37aeec 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -1667,24 +1667,32 @@ static int mv88e6xxx_pvt_setup(struct mv88e6xxx_chip *chip)
>>  	return 0;
>>  }
>>  
>> -static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
>> +static void mv88e6xxx_port_fast_age_fid(struct mv88e6xxx_chip *chip, int port,
>> +					u16 fid)
>>  {
>> -	struct mv88e6xxx_chip *chip = ds->priv;
>>  	int err;
>>  
>> -	if (dsa_to_port(ds, port)->lag)
>> +	if (dsa_to_port(chip->ds, port)->lag)
>>  		/* Hardware is incapable of fast-aging a LAG through a
>>  		 * regular ATU move operation. Until we have something
>>  		 * more fancy in place this is a no-op.
>>  		 */
>>  		return;
>>  
>> -	mv88e6xxx_reg_lock(chip);
>> -	err = mv88e6xxx_g1_atu_remove(chip, 0, port, false);
>> -	mv88e6xxx_reg_unlock(chip);
>> +	err = mv88e6xxx_g1_atu_remove(chip, fid, port, false);
>>  
>>  	if (err)
>> -		dev_err(ds->dev, "p%d: failed to flush ATU\n", port);
>> +		dev_err(chip->ds->dev, "p%d: failed to flush ATU (FID %u)\n",
>> +			port, fid);
>> +}
>> +
>> +static void mv88e6xxx_port_fast_age(struct dsa_switch *ds, int port)
>> +{
>> +	struct mv88e6xxx_chip *chip = ds->priv;
>> +
>> +	mv88e6xxx_reg_lock(chip);
>> +	mv88e6xxx_port_fast_age_fid(chip, port, 0);
>> +	mv88e6xxx_reg_unlock(chip);
>>  }
>>  
>>  static int mv88e6xxx_vtu_setup(struct mv88e6xxx_chip *chip)
>> @@ -1818,6 +1826,159 @@ static int mv88e6xxx_stu_setup(struct mv88e6xxx_chip *chip)
>>  	return mv88e6xxx_stu_loadpurge(chip, &stu);
>>  }
>>  
>> +static int mv88e6xxx_sid_get(struct mv88e6xxx_chip *chip, u8 *sid)
>> +{
>> +	DECLARE_BITMAP(busy, MV88E6XXX_N_SID) = { 0 };
>> +	struct mv88e6xxx_mst *mst;
>> +
>> +	set_bit(0, busy);
>
> __set_bit
>

Ack

>> +
>> +	list_for_each_entry(mst, &chip->msts, node) {
>> +		set_bit(mst->stu.sid, busy);
>> +	}
>
> Up to you, but parentheses are generally not used for single-line blocks.
>

Ack

>> +
>> +	*sid = find_first_zero_bit(busy, MV88E6XXX_N_SID);
>> +
>> +	return (*sid >= mv88e6xxx_max_sid(chip)) ? -ENOSPC : 0;
>> +}
>> +
>> +static int mv88e6xxx_mst_put(struct mv88e6xxx_chip *chip, u8 sid)
>> +{
>> +	struct mv88e6xxx_mst *mst, *tmp;
>> +	int err;
>> +
>> +	if (!sid)
>> +		return 0;
>
> Very minor nitpick: since mv88e6xxx_mst_put already checks this, could
> you drop the "!sid" check from callers?

Dropping

>> +
>> +	list_for_each_entry_safe(mst, tmp, &chip->msts, node) {
>> +		if (mst->stu.sid != sid)
>> +			continue;
>> +
>> +		if (!refcount_dec_and_test(&mst->refcnt))
>> +			return 0;
>> +
>> +		mst->stu.valid = false;
>> +		err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
>> +		if (err)
>
> Should we bother with a refcount_set(&mst->refcount, 1) on error?

We might as well. Thanks.

>> +			return err;
>> +
>> +		list_del(&mst->node);
>> +		kfree(mst);
>> +		return 0;
>> +	}
>> +
>> +	return -ENOENT;
>> +}
>> +
>> +static int mv88e6xxx_mst_get(struct mv88e6xxx_chip *chip, struct net_device *br,
>> +			     u16 msti, u8 *sid)
>> +{
>> +	struct mv88e6xxx_mst *mst;
>> +	int err, i;
>> +
>> +	if (!mv88e6xxx_has_stu(chip)) {
>> +		err = -EOPNOTSUPP;
>> +		goto err;
>> +	}
>> +
>> +	if (!msti) {
>> +		*sid = 0;
>> +		return 0;
>> +	}
>> +
>> +	list_for_each_entry(mst, &chip->msts, node) {
>> +		if (mst->br == br && mst->msti == msti) {
>> +			refcount_inc(&mst->refcnt);
>> +			*sid = mst->stu.sid;
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	err = mv88e6xxx_sid_get(chip, sid);
>> +	if (err)
>> +		goto err;
>> +
>> +	mst = kzalloc(sizeof(*mst), GFP_KERNEL);
>> +	if (!mst) {
>> +		err = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	INIT_LIST_HEAD(&mst->node);
>> +	refcount_set(&mst->refcnt, 1);
>> +	mst->br = br;
>> +	mst->msti = msti;
>> +	mst->stu.valid = true;
>> +	mst->stu.sid = *sid;
>> +
>> +	/* The bridge starts out all ports in the disabled state. But
>> +	 * a STU state of disabled means to go by the port-global
>> +	 * state. So we set all user port's initial state to blocking,
>> +	 * to match the bridge's behavior.
>> +	 */
>> +	for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
>> +		mst->stu.state[i] = dsa_is_user_port(chip->ds, i) ?
>> +			MV88E6XXX_PORT_CTL0_STATE_BLOCKING :
>> +			MV88E6XXX_PORT_CTL0_STATE_DISABLED;
>> +
>> +	err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
>> +	if (err)
>> +		goto err_free;
>> +
>> +	list_add_tail(&mst->node, &chip->msts);
>> +	return 0;
>> +
>> +err_free:
>> +	kfree(mst);
>> +err:
>> +	return err;
>> +}
>> +
>> +static int mv88e6xxx_port_mst_state_set(struct dsa_switch *ds, int port,
>> +					const struct switchdev_mst_state *st)
>> +{
>> +	struct dsa_port *dp = dsa_to_port(ds, port);
>> +	struct mv88e6xxx_chip *chip = ds->priv;
>> +	struct mv88e6xxx_mst *mst;
>> +	u8 state;
>> +	int err;
>> +
>> +	if (!mv88e6xxx_has_stu(chip))
>> +		return -EOPNOTSUPP;
>> +
>> +	switch (st->state) {
>> +	case BR_STATE_DISABLED:
>> +	case BR_STATE_BLOCKING:
>> +	case BR_STATE_LISTENING:
>> +		state = MV88E6XXX_PORT_CTL0_STATE_BLOCKING;
>> +		break;
>> +	case BR_STATE_LEARNING:
>> +		state = MV88E6XXX_PORT_CTL0_STATE_LEARNING;
>> +		break;
>> +	case BR_STATE_FORWARDING:
>> +		state = MV88E6XXX_PORT_CTL0_STATE_FORWARDING;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	list_for_each_entry(mst, &chip->msts, node) {
>> +		if (mst->br == dsa_port_bridge_dev_get(dp) &&
>> +		    mst->msti == st->msti) {
>> +			if (mst->stu.state[port] == state)
>> +				return 0;
>> +
>> +			mst->stu.state[port] = state;
>> +			mv88e6xxx_reg_lock(chip);
>> +			err = mv88e6xxx_stu_loadpurge(chip, &mst->stu);
>> +			mv88e6xxx_reg_unlock(chip);
>> +			return err;
>> +		}
>> +	}
>> +
>> +	return -ENOENT;
>> +}
>> +
>>  static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
>>  					u16 vid)
>>  {
>> @@ -2437,6 +2598,12 @@ static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
>>  	if (err)
>>  		return err;
>>  
>> +	if (!vlan.valid && vlan.sid) {
>> +		err = mv88e6xxx_mst_put(chip, vlan.sid);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>>  	return mv88e6xxx_g1_atu_remove(chip, vlan.fid, port, false);
>>  }
>>  
>> @@ -2482,6 +2649,72 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
>>  	return err;
>>  }
>>  
>> +static void mv88e6xxx_port_vlan_fast_age(struct dsa_switch *ds, int port, u16 vid)
>> +{
>> +	struct mv88e6xxx_chip *chip = ds->priv;
>> +	struct mv88e6xxx_vtu_entry vlan;
>> +	int err;
>> +
>> +	mv88e6xxx_reg_lock(chip);
>> +
>> +	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
>> +	if (err)
>> +		goto unlock;
>> +
>> +	mv88e6xxx_port_fast_age_fid(chip, port, vlan.fid);
>> +
>> +unlock:
>> +	mv88e6xxx_reg_unlock(chip);
>> +
>> +	if (err)
>> +		dev_err(ds->dev, "p%d: failed to flush ATU in VID %u\n",
>> +			port, vid);
>
> This error message actually corresponds to an mv88e6xxx_vtu_get() error,
> so the message is kind of incorrect. mv88e6xxx_port_fast_age_fid(),
> whose error code isn't propagated here, has its own "failed to flush ATU"
> error message.

Not sure I agree. If this fails, the symptom will be lingering dynamic
entries in the affected VLAN. In that case I think the current message,
or something similar, will make it as easy as possible to establish a
correlation.

Yes, it failed because the VTU get op failed, but that is more of an
internal affair in the driver.

Anyway, it's a moot point, because I think we should just allow the
error to bubble up to userspace instead - as you suggested in 11/14.

>> +}
>
> Otherwise this looks pretty good.

Careful now, don't spoil me :)

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST
  2022-03-14 20:20         ` Vladimir Oltean
@ 2022-03-14 22:13           ` Tobias Waldekranz
  0 siblings, 0 replies; 36+ messages in thread
From: Tobias Waldekranz @ 2022-03-14 22:13 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jiri Pirko, Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov,
	Russell King, Ido Schimmel, Petr Machata, Cooper Lees,
	Matt Johnston, linux-kernel, netdev, bridge

On Mon, Mar 14, 2022 at 22:20, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Mar 14, 2022 at 09:01:12PM +0100, Tobias Waldekranz wrote:
>> On Mon, Mar 14, 2022 at 19:55, Vladimir Oltean <olteanv@gmail.com> wrote:
>> > On Mon, Mar 14, 2022 at 06:56:49PM +0200, Vladimir Oltean wrote:
>> >> > diff --git a/net/dsa/port.c b/net/dsa/port.c
>> >> > index 58291df14cdb..1a17a0efa2fa 100644
>> >> > --- a/net/dsa/port.c
>> >> > +++ b/net/dsa/port.c
>> >> > @@ -240,6 +240,10 @@ static int dsa_port_switchdev_sync_attrs(struct dsa_port *dp,
>> >> >  	if (err && err != -EOPNOTSUPP)
>> >> >  		return err;
>> >> >  
>> >> > +	err = dsa_port_mst_enable(dp, br_mst_enabled(br), extack);
>> >> > +	if (err && err != -EOPNOTSUPP)
>> >> > +		return err;
>> >> 
>> >> Sadly this will break down because we don't have unwinding on error in
>> >> place (sorry). We'd end up with an unoffloaded bridge port with
>> >> partially synced bridge port attributes. Could you please add a patch
>> >> previous to this one that handles this, and unoffloads those on error?
>> >
>> > Actually I would rather rename the entire dsa_port_mst_enable() function
>> > to dsa_port_mst_validate() and move it to the beginning of dsa_port_bridge_join().
>> > This simplifies the unwinding that needs to take place quite a bit.
>> 
>> Well you still need to unwind vlan filtering if setting the ageing time
>> fails, which is the most complicated one, right?
>
> Yes, but we can leave that for another day :)
>
> ...ergo
>
>> Should the unwinding patch still be part of this series then?
>
> no.

Agreed

>> Still, I agree that _validate is a better name, and then _bridge_join
>> seems like a more reasonable placement.
>> 
>> While we're here, I actually made this a hard error in both scenarios
>> (but forgot to update the log - will do that in v4, depending on what we
>> decide here). There's a dilemma:
>> 
>> - When reacting to the attribute event, i.e. changing the mode on a
>>   member we're apart of, we _can't_ return -EOPNOTSUPP as it will be
>>   ignored, which is why dsa_port_mst_validate (nee _enable) returns
>>   -EINVAL.
>> 
>> - When joining a bridge, we _must_ return -EOPNOTSUPP to trigger the
>>   software fallback.
>> 
>> Having something like this in dsa_port_bridge_join...
>> 
>> err = dsa_port_mst_validate(dp);
>> if (err == -EINVAL)
>> 	return -EOPNOTSUPP;
>> else if (err)
>> 	return err;
>> 
>> ...works I suppose, but feels somewhat awkwark. Any better ideas?
>
> What you can do is follow the model of dsa_switch_supports_uc_filtering(),
> and create a dsa_switch_supports_mst() which is called inside an
> "if br_mst_enabled(br)" check, and returns bool. When false, you could
> return -EINVAL or -EOPNOTSUPP, as appropriate.
>
> This is mostly fine, except for the pesky dsa_port_can_configure_learning(dp)
> check :) So while you could name it dsa_port_supports_mst() and pass it
> a dsa_port, the problem is that you can't put the implementation of this
> new dsa_port_supports_mst() next to dsa_switch_supports_uc_filtering()
> where it would be nice to sit for symmetry, because the latter is static
> inline and we're missing the definition of dsa_port_can_configure_learning().
> So.. the second best thing is to keep dsa_port_supports_mst() in the
> same place where dsa_port_mst_enable() currently is.
>
> What do you think?

I think that would mostly work. It would have to be positioned higher up
in the file though, so that it can be called from _bridge_join. Unless
we add a forward for it of course, but that seems to break with existing
conventions.

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2022-03-14 22:13 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14  9:52 [PATCH v3 net-next 00/14] net: bridge: Multiple Spanning Trees Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 01/14] net: bridge: mst: Multiple Spanning Tree (MST) mode Tobias Waldekranz
2022-03-14 10:37   ` Nikolay Aleksandrov
2022-03-14 11:09     ` Nikolay Aleksandrov
2022-03-14 12:31   ` kernel test robot
2022-03-14 16:43   ` kernel test robot
2022-03-14  9:52 ` [PATCH v3 net-next 02/14] net: bridge: mst: Allow changing a VLAN's MSTI Tobias Waldekranz
2022-03-14 10:45   ` Nikolay Aleksandrov
2022-03-14  9:52 ` [PATCH v3 net-next 03/14] net: bridge: mst: Support setting and reporting MST port states Tobias Waldekranz
2022-03-14 10:37   ` Nikolay Aleksandrov
2022-03-14 12:38     ` Tobias Waldekranz
2022-03-14 14:58   ` Vladimir Oltean
2022-03-14 15:42     ` Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 04/14] net: bridge: mst: Notify switchdev drivers of MST mode changes Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 05/14] net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 06/14] net: bridge: mst: Notify switchdev drivers of MST state changes Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 07/14] net: bridge: mst: Add helper to map an MSTI to a VID set Tobias Waldekranz
2022-03-14 10:42   ` Nikolay Aleksandrov
2022-03-14  9:52 ` [PATCH v3 net-next 08/14] net: bridge: mst: Add helper to check if MST is enabled Tobias Waldekranz
2022-03-14 10:43   ` Nikolay Aleksandrov
2022-03-14  9:52 ` [PATCH v3 net-next 09/14] net: dsa: Validate hardware support for MST Tobias Waldekranz
2022-03-14 16:56   ` Vladimir Oltean
2022-03-14 17:55     ` Vladimir Oltean
2022-03-14 20:01       ` Tobias Waldekranz
2022-03-14 20:20         ` Vladimir Oltean
2022-03-14 22:13           ` Tobias Waldekranz
2022-03-14 17:51   ` Vladimir Oltean
2022-03-14  9:52 ` [PATCH v3 net-next 10/14] net: dsa: Pass VLAN MSTI migration notifications to driver Tobias Waldekranz
2022-03-14 17:07   ` Vladimir Oltean
2022-03-14  9:52 ` [PATCH v3 net-next 11/14] net: dsa: Handle MST state changes Tobias Waldekranz
2022-03-14 17:14   ` Vladimir Oltean
2022-03-14  9:52 ` [PATCH v3 net-next 12/14] net: dsa: mv88e6xxx: Disentangle STU from VTU Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 13/14] net: dsa: mv88e6xxx: Export STU as devlink region Tobias Waldekranz
2022-03-14  9:52 ` [PATCH v3 net-next 14/14] net: dsa: mv88e6xxx: MST Offloading Tobias Waldekranz
2022-03-14 16:27   ` Vladimir Oltean
2022-03-14 21:57     ` Tobias Waldekranz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).