Re: [PATCH v2 net-next 01/10] net: bridge: mst: Multiple Spanning Tree (MST) mode

From: Tobias Waldekranz <tobias@waldekranz.com>
To: Nikolay Aleksandrov <razor@blackwall.org>,
	davem@davemloft.net, kuba@kernel.org
Cc: Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>, Ivan Vecera <ivecera@redhat.com>,
	Roopa Prabhu <roopa@nvidia.com>,
	Russell King <linux@armlinux.org.uk>,
	Petr Machata <petrm@nvidia.com>, Cooper Lees <me@cooperlees.com>,
	Ido Schimmel <idosch@nvidia.com>,
	Matt Johnston <matt@codeconstruct.com.au>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bridge@lists.linux-foundation.org
Subject: Re: [PATCH v2 net-next 01/10] net: bridge: mst: Multiple Spanning Tree (MST) mode
Date: Mon, 07 Mar 2022 15:53:30 +0100	[thread overview]
Message-ID: <87a6e1olud.fsf@waldekranz.com> (raw)
In-Reply-To: <F908AE50-EDF4-4B83-98BD-ECB872CAD776@blackwall.org>

On Wed, Mar 02, 2022 at 00:01, Nikolay Aleksandrov <razor@blackwall.org> wrote:
> On 1 March 2022 11:03:12 CET, Tobias Waldekranz <tobias@waldekranz.com> 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>
>>---
>> include/uapi/linux/if_link.h |  1 +
>> net/bridge/Makefile          |  2 +-
>> net/bridge/br_input.c        | 17 +++++++-
>> net/bridge/br_mst.c          | 83 ++++++++++++++++++++++++++++++++++++
>> net/bridge/br_netlink.c      | 14 +++++-
>> net/bridge/br_private.h      | 26 +++++++++++
>> net/bridge/br_stp.c          |  3 ++
>> net/bridge/br_vlan.c         | 20 ++++++++-
>> net/bridge/br_vlan_options.c |  5 +++
>> 9 files changed, 166 insertions(+), 5 deletions(-)
>> create mode 100644 net/bridge/br_mst.c
>>
>
> Hi,
> As I mentioned in another review, I'm currently traveling and will have pc access
> end of this week (Sun), I'll try to review the set as much as I can through my phone in the
> meantime. Thanks for reworking it, generally looks good.
> A few comments below,
>
>
>>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>index e315e53125f4..7e0a653aafa3 100644
>>--- a/include/uapi/linux/if_link.h
>>+++ b/include/uapi/linux/if_link.h
>>@@ -482,6 +482,7 @@ enum {
>> 	IFLA_BR_VLAN_STATS_PER_PORT,
>> 	IFLA_BR_MULTI_BOOLOPT,
>> 	IFLA_BR_MCAST_QUERIER_STATE,
>>+	IFLA_BR_MST_ENABLED,
>
> Please use the boolopt api for new bridge boolean options like this one.

Ahh, I was not aware of that. Will change it in v3. Thanks.