The bridge has had per-VLAN STP support for a while now, since:
https://lore.kernel.org/netdev/[email protected]/
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/15)
- Ingress STP filtering is deferred until the frame's VLAN has been
resolved (1/15)
- The preexisting per-VLAN states can no longer be controlled directly
(1/15). They are instead placed under the MST module's control,
which is managed using a new netlink interface (described in 3/15)
- VLANs can br mapped to MSTIs in an arbitrary M:N fashion, using a
new global VLAN option (2/15)
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
v3 -> v4:
Bridge:
- Constify arguments where possible (Nik)
- Use non-atomic bitmap operators (Nik)
- Rename br_mst_parse -> br_mst_process (Nik)
- Account for the dynamic size of generated MST netlink data (Nik)
- Provide proper error reporting on invalid input (Nik)
- Export bridge helpers under GPL (Nik)
- Fix build when bridge VLAN filtering is compiled out (Intel bot)
- Allocate VLAN bitmaps on the stack (Vladimir)
DSA:
- Propagate MST state change errors back to the bridge layer
(Vladimir)
- Fix issue with software fallback (Vladimir)
- Ignore FDB events on software bridged ports
mv88e6xxx:
- Use non-atomic bitmap operators (Vladimir)
- Restore refcount in error path (Vladimir)
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 (15):
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: Never offload FDB entries on standalone ports
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 | 304 +++++++++++++++++++++-
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 | 13 +
include/net/dsa.h | 6 +
include/net/switchdev.h | 16 ++
include/uapi/linux/if_bridge.h | 18 ++
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 | 332 ++++++++++++++++++++++++
net/bridge/br_netlink.c | 44 +++-
net/bridge/br_private.h | 56 ++++
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 | 106 +++++++-
net/dsa/slave.c | 21 ++
23 files changed, 1337 insertions(+), 152 deletions(-)
create mode 100644 net/bridge/br_mst.c
--
2.25.1
Allow the user to switch from the current per-VLAN STP mode to an MST
mode.
Up to this point, per-VLAN STP states where always isolated from each
other. This is in contrast to the MSTP standard (802.1Q-2018, Clause
13.5), where VLANs are grouped into MST instances (MSTIs), and the
state is managed on a per-MSTI level, rather that at the per-VLAN
level.
Perhaps due to the prevalence of the standard, many switching ASICs
are built after the same model. Therefore, add a corresponding MST
mode to the bridge, which we can later add offloading support for in a
straight-forward way.
For now, all VLANs are fixed to MSTI 0, also called the Common
Spanning Tree (CST). That is, all VLANs will follow the port-global
state.
Upcoming changes will make this actually useful by allowing VLANs to
be mapped to arbitrary MSTIs and allow individual MSTI states to be
changed.
Signed-off-by: Tobias Waldekranz <[email protected]>
Acked-by: Nikolay Aleksandrov <[email protected]>
---
include/uapi/linux/if_bridge.h | 1 +
net/bridge/Makefile | 2 +-
net/bridge/br.c | 5 ++
net/bridge/br_input.c | 17 ++++++-
net/bridge/br_mst.c | 84 ++++++++++++++++++++++++++++++++++
net/bridge/br_private.h | 32 +++++++++++++
net/bridge/br_stp.c | 3 ++
net/bridge/br_vlan.c | 20 +++++++-
net/bridge/br_vlan_options.c | 5 ++
9 files changed, 165 insertions(+), 4 deletions(-)
create mode 100644 net/bridge/br_mst.c
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 2711c3522010..30a242195ced 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -759,6 +759,7 @@ struct br_mcast_stats {
enum br_boolopt_id {
BR_BOOLOPT_NO_LL_LEARN,
BR_BOOLOPT_MCAST_VLAN_SNOOPING,
+ BR_BOOLOPT_MST_ENABLE,
BR_BOOLOPT_MAX
};
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index 7fb9a021873b..24bd1c0a9a5a 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -20,7 +20,7 @@ obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o br_multicast_eht.o
-bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o br_vlan_tunnel.o br_vlan_options.o
+bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o br_vlan_tunnel.o br_vlan_options.o br_mst.o
bridge-$(CONFIG_NET_SWITCHDEV) += br_switchdev.o
diff --git a/net/bridge/br.c b/net/bridge/br.c
index b1dea3febeea..96e91d69a9a8 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -265,6 +265,9 @@ int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on,
case BR_BOOLOPT_MCAST_VLAN_SNOOPING:
err = br_multicast_toggle_vlan_snooping(br, on, extack);
break;
+ case BR_BOOLOPT_MST_ENABLE:
+ err = br_mst_set_enabled(br, on, extack);
+ break;
default:
/* shouldn't be called with unsupported options */
WARN_ON(1);
@@ -281,6 +284,8 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt)
return br_opt_get(br, BROPT_NO_LL_LEARN);
case BR_BOOLOPT_MCAST_VLAN_SNOOPING:
return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED);
+ case BR_BOOLOPT_MST_ENABLE:
+ return br_opt_get(br, BROPT_MST_ENABLED);
default:
/* shouldn't be called with unsupported options */
WARN_ON(1);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index e0c13fcc50ed..196417859c4a 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -78,13 +78,22 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
u16 vid = 0;
u8 state;
- if (!p || p->state == BR_STATE_DISABLED)
+ if (!p)
goto drop;
br = p->br;
+
+ if (br_mst_is_enabled(br)) {
+ state = BR_STATE_FORWARDING;
+ } else {
+ if (p->state == BR_STATE_DISABLED)
+ goto drop;
+
+ state = p->state;
+ }
+
brmctx = &p->br->multicast_ctx;
pmctx = &p->multicast_ctx;
- state = p->state;
if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, &vid,
&state, &vlan))
goto out;
@@ -370,9 +379,13 @@ static rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
return RX_HANDLER_PASS;
forward:
+ if (br_mst_is_enabled(p->br))
+ goto defer_stp_filtering;
+
switch (p->state) {
case BR_STATE_FORWARDING:
case BR_STATE_LEARNING:
+defer_stp_filtering:
if (ether_addr_equal(p->br->dev->dev_addr, dest))
skb->pkt_type = PACKET_HOST;
diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
new file mode 100644
index 000000000000..e1ec9d39c660
--- /dev/null
+++ b/net/bridge/br_mst.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Bridge Multiple Spanning Tree Support
+ *
+ * Authors:
+ * Tobias Waldekranz <[email protected]>
+ */
+
+#include <linux/kernel.h>
+
+#include "br_private.h"
+
+DEFINE_STATIC_KEY_FALSE(br_mst_used);
+
+static void br_mst_vlan_set_state(struct net_bridge_port *p, struct net_bridge_vlan *v,
+ u8 state)
+{
+ struct net_bridge_vlan_group *vg = nbp_vlan_group(p);
+
+ if (v->state == state)
+ return;
+
+ br_vlan_set_state(v, state);
+
+ if (v->vid == vg->pvid)
+ br_vlan_set_pvid_state(vg, state);
+}
+
+void br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state)
+{
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_vlan *v;
+
+ vg = nbp_vlan_group(p);
+ if (!vg)
+ return;
+
+ list_for_each_entry(v, &vg->vlan_list, vlist) {
+ if (v->brvlan->msti != msti)
+ continue;
+
+ br_mst_vlan_set_state(p, v, state);
+ }
+}
+
+void br_mst_vlan_init_state(struct net_bridge_vlan *v)
+{
+ /* VLANs always start out in MSTI 0 (CST) */
+ v->msti = 0;
+
+ if (br_vlan_is_master(v))
+ v->state = BR_STATE_FORWARDING;
+ else
+ v->state = v->port->state;
+}
+
+int br_mst_set_enabled(struct net_bridge *br, bool on,
+ struct netlink_ext_ack *extack)
+{
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_port *p;
+
+ list_for_each_entry(p, &br->port_list, list) {
+ vg = nbp_vlan_group(p);
+
+ if (!vg->num_vlans)
+ continue;
+
+ NL_SET_ERR_MSG(extack,
+ "MST mode can't be changed while VLANs exist");
+ return -EBUSY;
+ }
+
+ if (br_opt_get(br, BROPT_MST_ENABLED) == on)
+ return 0;
+
+ if (on)
+ static_branch_enable(&br_mst_used);
+ else
+ static_branch_disable(&br_mst_used);
+
+ br_opt_toggle(br, BROPT_MST_ENABLED, on);
+ return 0;
+}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 48bc61ebc211..934c3dc4a927 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,34 @@ 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) {}
+static inline int br_mst_set_enabled(struct net_bridge *br, bool on,
+ struct netlink_ext_ack *extack)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
struct nf_br_ops {
int (*br_dev_xmit_hook)(struct sk_buff *skb);
};
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 1d80f34a139c..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
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 <[email protected]>
---
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 355ad102d6b1..051b9358946b 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
On Mon, Mar 14, 2022 at 22:32, Jakub Kicinski <[email protected]> wrote:
> On Tue, 15 Mar 2022 01:25:32 +0100 Tobias Waldekranz wrote:
>> 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 <[email protected]>
>
> ../net/bridge/br_mst.c: In function ‘br_mst_set_enabled’:
> ../net/bridge/br_mst.c:102:16: error: variable ‘attr’ has initializer but incomplete type
> 102 | struct switchdev_attr attr = {
> | ^~~~~~~~~~~~~~
> ../net/bridge/br_mst.c:103:18: error: ‘struct switchdev_attr’ has no member named ‘id’
> 103 | .id = SWITCHDEV_ATTR_ID_BRIDGE_MST,
> | ^~
> ../net/bridge/br_mst.c:103:23: error: ‘SWITCHDEV_ATTR_ID_BRIDGE_MST’ undeclared (first use in this function)
> 103 | .id = SWITCHDEV_ATTR_ID_BRIDGE_MST,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../net/bridge/br_mst.c:103:23: note: each undeclared identifier is reported only once for each function it appears in
> ../net/bridge/br_mst.c:103:23: warning: excess elements in struct initializer
> ../net/bridge/br_mst.c:103:23: note: (near initialization for ‘attr’)
> ../net/bridge/br_mst.c:104:18: error: ‘struct switchdev_attr’ has no member named ‘orig_dev’
> 104 | .orig_dev = br->dev,
> | ^~~~~~~~
> ../net/bridge/br_mst.c:104:29: warning: excess elements in struct initializer
> 104 | .orig_dev = br->dev,
> | ^~
> ../net/bridge/br_mst.c:104:29: note: (near initialization for ‘attr’)
> ../net/bridge/br_mst.c:105:18: error: ‘struct switchdev_attr’ has no member named ‘u’
> 105 | .u.mst = on,
> | ^
> ../net/bridge/br_mst.c:105:26: warning: excess elements in struct initializer
> 105 | .u.mst = on,
> | ^~
> ../net/bridge/br_mst.c:105:26: note: (near initialization for ‘attr’)
> ../net/bridge/br_mst.c:102:31: error: storage size of ‘attr’ isn’t known
> 102 | struct switchdev_attr attr = {
> | ^~~~
> ../net/bridge/br_mst.c:125:15: error: implicit declaration of function ‘switchdev_port_attr_set’; did you mean ‘br_switchdev_port_vlan_del’? [-Werror=implicit-function-declaration]
> 125 | err = switchdev_port_attr_set(br->dev, &attr, extack);
> | ^~~~~~~~~~~~~~~~~~~~~~~
> | br_switchdev_port_vlan_del
> ../net/bridge/br_mst.c:102:31: warning: unused variable ‘attr’ [-Wunused-variable]
> 102 | struct switchdev_attr attr = {
> | ^~~~
Sorry about that. Forgot to run the incremental build after the
rebase. Will be fixed in v5.
On Tue, 15 Mar 2022 01:25:32 +0100 Tobias Waldekranz wrote:
> 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 <[email protected]>
../net/bridge/br_mst.c: In function ‘br_mst_set_enabled’:
../net/bridge/br_mst.c:102:16: error: variable ‘attr’ has initializer but incomplete type
102 | struct switchdev_attr attr = {
| ^~~~~~~~~~~~~~
../net/bridge/br_mst.c:103:18: error: ‘struct switchdev_attr’ has no member named ‘id’
103 | .id = SWITCHDEV_ATTR_ID_BRIDGE_MST,
| ^~
../net/bridge/br_mst.c:103:23: error: ‘SWITCHDEV_ATTR_ID_BRIDGE_MST’ undeclared (first use in this function)
103 | .id = SWITCHDEV_ATTR_ID_BRIDGE_MST,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../net/bridge/br_mst.c:103:23: note: each undeclared identifier is reported only once for each function it appears in
../net/bridge/br_mst.c:103:23: warning: excess elements in struct initializer
../net/bridge/br_mst.c:103:23: note: (near initialization for ‘attr’)
../net/bridge/br_mst.c:104:18: error: ‘struct switchdev_attr’ has no member named ‘orig_dev’
104 | .orig_dev = br->dev,
| ^~~~~~~~
../net/bridge/br_mst.c:104:29: warning: excess elements in struct initializer
104 | .orig_dev = br->dev,
| ^~
../net/bridge/br_mst.c:104:29: note: (near initialization for ‘attr’)
../net/bridge/br_mst.c:105:18: error: ‘struct switchdev_attr’ has no member named ‘u’
105 | .u.mst = on,
| ^
../net/bridge/br_mst.c:105:26: warning: excess elements in struct initializer
105 | .u.mst = on,
| ^~
../net/bridge/br_mst.c:105:26: note: (near initialization for ‘attr’)
../net/bridge/br_mst.c:102:31: error: storage size of ‘attr’ isn’t known
102 | struct switchdev_attr attr = {
| ^~~~
../net/bridge/br_mst.c:125:15: error: implicit declaration of function ‘switchdev_port_attr_set’; did you mean ‘br_switchdev_port_vlan_del’? [-Werror=implicit-function-declaration]
125 | err = switchdev_port_attr_set(br->dev, &attr, extack);
| ^~~~~~~~~~~~~~~~~~~~~~~
| br_switchdev_port_vlan_del
../net/bridge/br_mst.c:102:31: warning: unused variable ‘attr’ [-Wunused-variable]
102 | struct switchdev_attr attr = {
| ^~~~
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 <[email protected]>
---
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 051b9358946b..5d3e034b9030 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
If a port joins a bridge that it can't offload, it will fallback to
standalone mode and software bridging. In this case, we never want to
offload any FDB entries to hardware either.
Signed-off-by: Tobias Waldekranz <[email protected]>
---
net/dsa/slave.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index a61a7c54af20..647adee97f7f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2624,6 +2624,9 @@ static int dsa_slave_fdb_event(struct net_device *dev,
if (ctx && ctx != dp)
return 0;
+ if (!dp->bridge)
+ return 0;
+
if (switchdev_fdb_is_dynamically_learned(fdb_info)) {
if (dsa_port_offloads_bridge_port(dp, orig_dev))
return 0;
--
2.25.1
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 <[email protected]>
---
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