From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=waldekranz-com.20210112.gappssmtp.com; s=20210112; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version; bh=V0LbW5LueiWt1tPi33D71OMFfIFyzm670dcDqQa5c5Q=; b=CxfWZXX0kv32BD1WQ/OJcVIGTDr4mpYvX56gBYB4lFMI4e8M2MOFdiwYSYgpRCe+TV 4SPxRzXQEvwIgxda5Ymbh2hfRKRMFdzl/A5DCtNwFYo937O58rin+LBpyg/WiuGQrJoC tk0QW3gTx3wonWCM8CQqjk6TN1YATpkxxJmVlCzut4UvLY0leGV53BFpmuqk3ScMWnjg VlbSkLyQj5YP1VxKhdl7jkFWwd5lGbfeFGCsp2gFFzE835ikvILERttvSuw22ecUqLNJ Slx4HOJgfqyCSbPmg3ychRF8bNbMCYqitgUCV107TpctqoH4jV/QfncMRd3OgzX25RgA OrAg== From: Tobias Waldekranz In-Reply-To: <610eb6cc-4df4-f0fc-462a-b33145334a12@blackwall.org> References: <20220316150857.2442916-1-tobias@waldekranz.com> <610eb6cc-4df4-f0fc-462a-b33145334a12@blackwall.org> Date: Thu, 17 Mar 2022 10:50:21 +0100 Message-ID: <87tubwkiw2.fsf@waldekranz.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Bridge] [PATCH v5 net-next 00/15] net: bridge: Multiple Spanning Trees List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikolay Aleksandrov , davem@davemloft.net, kuba@kernel.org Cc: Ivan Vecera , Andrew Lunn , Florian Fainelli , Jiri Pirko , Petr Machata , Ido Schimmel , bridge@lists.linux-foundation.org, Russell King , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Cooper Lees , Roopa Prabhu , Matt Johnston , Vladimir Oltean , Vivien Didelot On Thu, Mar 17, 2022 at 11:00, Nikolay Aleksandrov 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.