Multiple Spanning Tree Protocol (MSTP) HLD #1298

hamnarauf · 2023-03-21T14:46:06Z

This document describes the design details for IEEE 802.1s - Multiple Spanning Tree Protocol (MSTP) support in SONiC.

ridahanif96 · 2023-06-05T09:11:54Z

@zhangyanzhao can you please assign this HLD to @hamnarauf Thanks!

zhangyanzhao · 2023-07-11T15:06:17Z

SONiC community review recording https://zoom.us/rec/share/3XxqJ20eag07iK0x7T_4YqBq6jhbyWXGjpk72AvHL35peWSOXZ04w_ZQjGlwXJbM.0DPyXu03R_bS0Gry

rck-innovium · 2023-07-11T15:54:40Z

doc/stp/MSTP_HLD.md

+||SAI_SWITCH_ATTR_MAX_STP_INSTANCE|
+
+## New SAI Attributes
+MSTP design requires one new attribute `SAI_HOSTIF_TRAP_TYPE_MSTP` for control trap packets which will be defined in saihostif.h.


Please explain why SAI_HOSTIF_TRAP_TYPE_STP is not sufficient. The DMAC and LLC fields are identical for STP and MSTP. Creating separate trap types for STP and MSTP would required the hardware to parse/match the BPDU protocol identifier.

We can discuss this in coming SAI meeting. For reference, please see this PR Add hostif trap for MSTP

ridahanif96 · 2023-07-11T16:01:57Z

@lguohan , Please find following are unmerged PRs of PVST, that needs to merged for MSTP:
SWSS: sonic-net/sonic-swss#1058
Utilities: sonic-net/sonic-utilities#648
Buildimage: sonic-net/sonic-buildimage#3463

ridahanif96 · 2023-08-11T08:14:39Z

@zhangyanzhao pls add @wajahatrazi as assignee for this HLD.

zhangyanzhao · 2023-10-07T06:01:54Z

@hamnarauf can you please help to add the code PR list in this HLD? Thanks.

ridahanif96 · 2023-10-07T06:24:23Z

@hamnarauf can you please help to add the code PR list in this HLD? Thanks.

Hi @zhangyanzhao , Hamna is no longer assignee on this feature. Please consider @Wajahat as assignee on this from now onward. We are working on MSTP code and waiting for @adyeung help in unmereged code PR of PVST. Thanks!

zhangyanzhao · 2023-10-12T18:55:51Z

@ridahanif96 can you please help to add the code PRs to this HLD by referring to #806 ? Thanks!

ridahanif96 · 2024-03-19T10:46:31Z

@divyachandralekha Hi Divya,can you please review this HLD!

divyachandralekha · 2024-03-18T08:31:37Z

doc/stp/MSTP_HLD.md

+The table holds the state of a port i.e forwarding, learning, blocking with respect to each instance.
+#### STP_VLAN_INSTANCE_TABLE 
+The table holds the VLAN to instance mapping.
+#### STP_FASTAGEING_FLUSH_TABLE 


A per-VLAN flush may trigger a lot of churn, especially considering the possibility of 4K VLANs mapped to one MST instance. Here, only one flush is necessary. You may need to consider optimisation in this scenario.

Yes, it might be cumber-sum to flush for each VLAN. We will consider one FDB Flush for every MSTI for optimization.

divyachandralekha · 2024-03-18T08:40:48Z

doc/stp/MSTP_HLD.md

+#  Sequence Diagrams
+## MSTP global enable
+
+Only the VLANs that are currently present will be mapped to IST instance. A VLAN cannot be mapped to an instance if it has not been created yet.


When we have this limitation , managing the IST configuration becomes more complex. Especially in environments where VLANs are frequently added or removed. Administrators must continuously update the IST configuration to reflect changes in VLAN membership.
Creating a VLAN and then adding it to the Internal Spanning Tree (IST) configuration can potentially create convergence churn and disturb the existing network.
You may need to consider pre-configuring VLAN-to-instance mapping before actually creating the VLAN.

Agreed, we will consider pre-configuring VLAN-to instance mapping to avoid convergence churn.

divyachandralekha · 2024-03-18T09:48:13Z

doc/stp/MSTP_HLD.md

+Below commands allow configuring on region basis:
+
+- **config spanning_tree region name \<region-name\>**
+  - Edit the name of region


Do you support empty region name ?

Name is an empty string it could have 0 value by default for region name

divyachandralekha · 2024-03-18T09:51:47Z

doc/stp/MSTP_HLD.md

+- **config spanning_tree instance (add|del) \<instance-id\>**
+  - Creation or deletion of an instance.
+  - instance-id: Default: 0, range: 1-63
+  - Instance can not be deleted if VLAN(s) are associated with it. 


What is the reason behind this restriction?

The restriction here on an MSTP instance when VLANs are associated with it serves to prevent accidental deletions that could lead to network inconsistencies and loops in VLAN topologies. This approach aligns with industry standards followed by Cisco, Huawei, and SONiC, ensuring consistency and clarity for users familiar with these platforms. Additionally, in SONiC, an interface associated with a VLAN also prevents the deletion of that VLAN, maintaining the integrity of the network configuration.

ridahanif96 · 2024-04-03T13:23:22Z

Hi @divyachandralekha , do you have more suggestions on the design? if no can you please help approve this PR. Also we can discuss offline for PVST code PRs and our workflows for bringing STP/PVST/MSTP to SONiC

adyeung · 2024-04-03T19:08:27Z

@ridahanif96 The earlier PVST code PRs will be updated and reposted in the next community release

ridahanif96 · 2024-04-04T01:21:52Z

@ridahanif96 The earlier PVST code PRs will be updated and reposted in the next community release

@adyeung thanks Adam!, till then it would be great if we also finalize this design and merge this.

ridahanif96 · 2024-04-04T01:25:01Z

@rck-innovium Hi Ravi, can you please help review this design. As per suggestion earlier in SAI community we had updated SAI Modification for MSTP control packets.

Update CONFIG/APP DB and yang model

linux-foundation-easycla · 2024-05-06T06:49:47Z

The committers listed above are authorized under a signed CLA.

✅ login: hamnarauf / name: Hamna Rauf (064ae6e, a19b757, e9c22c3, 6370e96, 865134a, 07c9f56, 6bf70a3, 3f584ba, b789c19)
✅ login: ridahanif96 / name: Rida Hanif (c799392, 8ebdc14, 6c3db77, cb56cb6, 22d5811)
✅ login: divyachandralekha / name: Divya Kumaran Chandralekha (387f5b2)

praveenraja1 · 2024-05-13T16:29:27Z

doc/stp/MSTP_HLD.md

+2. Assigning VLAN to instance via SAI STP API and SAI VLAN API.
+3. Creation of STP Port and assigning port state with respect to each instance via SAI STP API.
+4. Flushing FDB entries via SAI FDB API.
+


BPDU guard functionality requires link-state control, maybe we need to add SAI api that control link state here as well.

PortOrch already has link state control SAI attributes. STP notifies OrchAgent of any link state changes, which are then managed accordingly.

praveenraja1 · 2024-05-13T16:38:39Z

doc/stp/MSTP_HLD.md

+  - Configure an interface as a bpdu_guard. 
+
+Interface level CLI configurations of root guard, BPDU guard will also be supported for spanning-tree mode `mstp`.
+


I see STP can be enabled on "portchannel" as well, so im assuming we may need to add CLI restrictions to ensure that when STP is enabled on a portchannel, the portchannel group cannot be deleted/modified.

If thats true, please ensure its captured here or a different section of HLD

Design will handle Portchannel modification/deletion automatically , so we don't plan to add any restrictions to prevent these operation on a STP enabled PortChannel.

praveenraja1 · 2024-05-13T17:06:23Z

doc/stp/MSTP_HLD.md

+Responsible for all MST protocol related calculations. BPDUs are sent and received in STPd and states are updated accordingly.
+
+### STPSync
+A process running as a part of STPD. Responsible for updating all the MSTP states in APP DB.


Couple of questions here,

is "stpSync" process forked within "STPd".

Is "STPd" taken from a open-source implementation for instance https://github.com/sonic-net/sonic-stp ?

Assuming the answer to the above is 'yes', would be it cleaner to have "STPsync" implemented outside of "STPd" and then have IPCs to communicate port-states, fdb flush & link state events.

Doing this will ensure "STPd" repo has minimal integration changes and syncing it later to open-source repo would be easier.

stpsync is not a process. STPSync is a component integrated within STPd responsible for managing and updating all operational STP data to APP DB.

We will update the same in HLD.

sutharsansr · 2024-05-26T17:00:37Z

doc/stp/MSTP_HLD.md

+### STPSync
+A process running as a part of STPD. Responsible for updating all the MSTP states in APP DB.
+
+The BPDU rx/tx, BPDU processing, handling of timers, handling of changes related to port or LAG using netlink events and STP port state sync to Linux Kernel will function the same as PVST.


If the port state programming fails, how to handle the use-case. Can you please provide your thought process on the same

if port state programming we will keep trying to set it until succeeds.

sutharsansr · 2024-05-26T17:02:27Z

doc/stp/MSTP_HLD.md

+            }
+            default 128;
+            description
+                "The manageable component of the Port Identifier,


Can you please add the information that priority values should be increments of 16 in the description?

We will update the HLD

sutharsansr · 2024-06-08T08:49:14Z

doc/stp/MSTP_HLD.md

+  - Specify configuring the port level priority for root bridge in seconds.
+  Default: 128,  range 0-240 
+- **config spanning_tree mst cost \<cost-value\>**
+  - Specify configuring the port level priority for root bridge in seconds.


Does this description needs an update to match the command? And path cost configuration should be supported only per interface or global?

Path cost works for both cases per interface and global. These are two separate set of commands for each case.

sutharsansr · 2024-06-09T04:49:16Z

doc/stp/MSTP_HLD.md

+- **config spanning_tree interface \<ifname\> bpdu_guard {enable|disable}**
+  - Configure an interface as a bpdu_guard. 
+
+- **config spanning_tree interface \<ifname\> guard {root|bpdu}**


Is this CLI command a condensed version of the above two seperate bpdu and root guard configuration commands?

We will update HLD with reference to this point

TafkaMax · 2024-06-09T10:32:16Z

Is the plan to merge the code for both PVST and MSTP together for the community sonic or would one be merged before?

praveenraja1

Looks good

ridahanif96 · 2024-06-11T11:31:10Z

Is the plan to merge the code for both PVST and MSTP together for the community sonic or would one be merged before?

No, As discussed in PENS WG , we will only provide MSTP for the community in this release ,we will not be looking into PVST code merged.

ridahanif96 · 2024-07-24T07:56:43Z

@madhupalu , @praveenraja1 , @sutharsansr @adyeung @zhangyanzhao ,
As per our Discussion in PENS WG, New Updated Design is ready for review. Please consider Multiple Spanning Tree Protocol (MSTP) Updated HLD from onwards. Please help review the new design.

This PR will be closed to avoid confusion.

adyeung · 2024-08-02T00:14:57Z

Closing this PR, revised HLD in the following submission

#1761

hamnarauf force-pushed the mstp_hld branch 3 times, most recently from 5321c1e to 7d5eb3e Compare March 22, 2023 07:25

Add MSTP HLD

064ae6e

hamnarauf force-pushed the mstp_hld branch from 7d5eb3e to 064ae6e Compare March 22, 2023 07:28

hamnarauf added 2 commits April 18, 2023 11:25

Add YANG model

a19b757

Update revision history for YANG

6370e96

zhangyanzhao assigned ridahanif96 May 30, 2023

adyeung assigned hamnarauf and unassigned ridahanif96 Jun 21, 2023

hamnarauf added 4 commits June 23, 2023 10:46

Merge branch 'sonic-net:master' into mstp_hld

6bf70a3

Update HLD

e9c22c3

Add copyright

865134a

Update HLD

07c9f56

rck-innovium reviewed Jul 11, 2023

View reviewed changes

Yuval-Mellanox mentioned this pull request Jul 12, 2023

Multiple Spanning Tree Protocol #1422

Open

hamnarauf added 2 commits July 17, 2023 15:18

Address review comments

3f584ba

Update table of contents

b789c19

hamnarauf removed their assignment Aug 15, 2023

ridahanif96 mentioned this pull request Aug 18, 2023

MSTP Traps for Control Packet opencomputeproject/SAI#1853

Closed

zhangyanzhao assigned hamnarauf Oct 7, 2023

zhangyanzhao unassigned hamnarauf Oct 12, 2023

divyachandralekha reviewed Mar 19, 2024

View reviewed changes

Update MSTP_HLD.md

cb56cb6

adyeung closed this Apr 3, 2024

adyeung reopened this Apr 3, 2024

adyeung self-assigned this Apr 17, 2024

ridahanif96 and others added 2 commits April 25, 2024 16:47

Removed COPP Changes

22d5811

Update MSTP_HLD.md

387f5b2

Update CONFIG/APP DB and yang model

Updated CLI Command

8ebdc14

praveenraja1 reviewed May 13, 2024

View reviewed changes

Updated Show Commands

6c3db77

sutharsansr reviewed May 26, 2024

View reviewed changes

sutharsansr reviewed Jun 8, 2024

View reviewed changes

sutharsansr reviewed Jun 9, 2024

View reviewed changes

praveenraja1 approved these changes Jun 10, 2024

View reviewed changes

sutharsansr approved these changes Jun 12, 2024

View reviewed changes

ridahanif96 mentioned this pull request Jul 24, 2024

Multiple Spanning Tree Protocol (MSTP) Updated HLD #1761

Merged

adyeung closed this Aug 2, 2024

		- Configure an interface as a bpdu_guard.

		Interface level CLI configurations of root guard, BPDU guard will also be supported for spanning-tree mode `mstp`.

Multiple Spanning Tree Protocol (MSTP) HLD #1298

Multiple Spanning Tree Protocol (MSTP) HLD #1298

Conversation

hamnarauf commented Mar 21, 2023 • edited Loading

ridahanif96 commented Jun 5, 2023

zhangyanzhao commented Jul 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ridahanif96 commented Jul 11, 2023 • edited Loading

ridahanif96 commented Aug 11, 2023

zhangyanzhao commented Oct 7, 2023

ridahanif96 commented Oct 7, 2023 • edited Loading

zhangyanzhao commented Oct 12, 2023

ridahanif96 commented Mar 19, 2024

Choose a reason for hiding this comment

ridahanif96 Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ridahanif96 commented Apr 3, 2024

adyeung commented Apr 3, 2024

ridahanif96 commented Apr 4, 2024

ridahanif96 commented Apr 4, 2024

linux-foundation-easycla bot commented May 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ridahanif96 Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TafkaMax commented Jun 9, 2024

praveenraja1 left a comment

Choose a reason for hiding this comment

ridahanif96 commented Jun 11, 2024

ridahanif96 commented Jul 24, 2024

adyeung commented Aug 2, 2024

hamnarauf commented Mar 21, 2023 •

edited

Loading

ridahanif96 commented Jul 11, 2023 •

edited

Loading

ridahanif96 commented Oct 7, 2023 •

edited

Loading

ridahanif96 Mar 21, 2024 •

edited

Loading

linux-foundation-easycla bot commented May 6, 2024 •

edited

Loading

ridahanif96 Jun 11, 2024 •

edited

Loading