| Message ID | 20220316150857.2442916-1-tobias@waldekranz.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: bridge: Multiple Spanning Trees | expand |
On 16/03/2022 17:08, Tobias Waldekranz wrote: > 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/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 > Hi Tobias, One major missing thing is the selftests for this new feature. Do you have a plan to upstream them? Cheers, Nik
On Thu, Mar 17, 2022 at 11:00, Nikolay Aleksandrov <razor@blackwall.org> wrote: > On 16/03/2022 17:08, Tobias Waldekranz wrote: >> 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/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 >> > > Hi Tobias, > One major missing thing is the selftests for this new feature. Do you > have a plan to upstream them? 100% agree. I have an internal test that I plan to adapt to run as a kselftest. There's a bootstrapping problem here though. I can't send the iproute2 series until the kernel support is merged - and until I know how the iproute2 support ends up looking I can't add a kselftest. Ideally, tools/iproute2 would be a thing in the kernel. Then you could send the entire implementation as one series. I'm sure that's probably been discussed many times already, but my Google-fu fails me.
On 17/03/2022 11:50, Tobias Waldekranz wrote: > On Thu, Mar 17, 2022 at 11:00, Nikolay Aleksandrov <razor@blackwall.org> wrote: >> On 16/03/2022 17:08, Tobias Waldekranz wrote: >>> 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/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 >>> >> >> Hi Tobias, >> One major missing thing is the selftests for this new feature. Do you >> have a plan to upstream them? > > 100% agree. I have an internal test that I plan to adapt to run as a > kselftest. There's a bootstrapping problem here though. I can't send the > iproute2 series until the kernel support is merged - and until I know > how the iproute2 support ends up looking I can't add a kselftest. > That's ok, some people choose to send the iproute2 with the set, others send the iproute2 patches separately and add selftests after those are accepted (that's my personal preference for the same reasons above). Personally I don't mind either way as long as the tests end up materializing. :) Just in case you've missed it - most of the bridge tests reside in tools/testing/selftests/net/forwarding. > Ideally, tools/iproute2 would be a thing in the kernel. Then you could > send the entire implementation as one series. I'm sure that's probably > been discussed many times already, but my Google-fu fails me. Cheers, Nik
Hello: This series was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Wed, 16 Mar 2022 16:08:42 +0100 you wrote: > 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. > > [...] Here is the summary with links: - [v5,net-next,01/15] net: bridge: mst: Multiple Spanning Tree (MST) mode https://git.kernel.org/netdev/net-next/c/ec7328b59176 - [v5,net-next,02/15] net: bridge: mst: Allow changing a VLAN's MSTI https://git.kernel.org/netdev/net-next/c/8c678d60562f - [v5,net-next,03/15] net: bridge: mst: Support setting and reporting MST port states https://git.kernel.org/netdev/net-next/c/122c29486e1f - [v5,net-next,04/15] net: bridge: mst: Notify switchdev drivers of MST mode changes https://git.kernel.org/netdev/net-next/c/87c167bb94ee - [v5,net-next,05/15] net: bridge: mst: Notify switchdev drivers of VLAN MSTI migrations https://git.kernel.org/netdev/net-next/c/6284c723d9b9 - [v5,net-next,06/15] net: bridge: mst: Notify switchdev drivers of MST state changes https://git.kernel.org/netdev/net-next/c/7ae9147f4312 - [v5,net-next,07/15] net: bridge: mst: Add helper to map an MSTI to a VID set https://git.kernel.org/netdev/net-next/c/cceac97afa09 - [v5,net-next,08/15] net: bridge: mst: Add helper to check if MST is enabled https://git.kernel.org/netdev/net-next/c/48d57b2e5f43 - [v5,net-next,09/15] net: bridge: mst: Add helper to query a port's MST state https://git.kernel.org/netdev/net-next/c/f54fd0e16306 - [v5,net-next,10/15] net: dsa: Validate hardware support for MST https://git.kernel.org/netdev/net-next/c/332afc4c8c0d - [v5,net-next,11/15] net: dsa: Pass VLAN MSTI migration notifications to driver https://git.kernel.org/netdev/net-next/c/8e6598a7b0fa - [v5,net-next,12/15] net: dsa: Handle MST state changes https://git.kernel.org/netdev/net-next/c/7414af30b7d8 - [v5,net-next,13/15] net: dsa: mv88e6xxx: Disentangle STU from VTU https://git.kernel.org/netdev/net-next/c/49c98c1dc7d9 - [v5,net-next,14/15] net: dsa: mv88e6xxx: Export STU as devlink region https://git.kernel.org/netdev/net-next/c/7dc96039b967 - [v5,net-next,15/15] net: dsa: mv88e6xxx: MST Offloading https://git.kernel.org/netdev/net-next/c/acaf4d2e36b3 You are awesome, thank you!
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/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 v4 -> v5: Bridge: - Fix build error in intermediate commit (Jakub) - Use rcu safe list iterator in br_mst_info_size (Nik) - Propagate any errors back to the caller when changing an MST state (Vladimir) DSA: - Boolean algebra workshop (Vladimir, feat. De Morgan) - Only flush FDBs on ports when transitioning from forwarding/learning to listening/blocking/disabled (Vladimir) 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: bridge: mst: Add helper to query a port's MST state 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 | 19 ++ 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 | 357 ++++++++++++++++++++++++ net/bridge/br_netlink.c | 44 ++- net/bridge/br_private.h | 61 ++++ net/bridge/br_stp.c | 6 + net/bridge/br_switchdev.c | 46 +++ net/bridge/br_vlan.c | 20 +- net/bridge/br_vlan_options.c | 20 ++ net/dsa/dsa_priv.h | 7 + net/dsa/port.c | 113 +++++++- net/dsa/slave.c | 18 ++ 23 files changed, 1381 insertions(+), 152 deletions(-) create mode 100644 net/bridge/br_mst.c