Multiple Spanning Tree Protocol (MSTP) Updated HLD #1761
Conversation
|
@praveenraja1 @sutharsansr @balajib-cisco @rck-innovium @jeff-yin please review |
|
|
||
|
|
||
| ## Uplink Fast | ||
| MSTP standard does not support uplink fast so uplink fast functionality will be disable for MSTP. |
There was a problem hiding this comment.
does this mean HLD is not supporting uplink fast?
There was a problem hiding this comment.
UplinkFast was developed by Cisco as an enhancement for classic STP. Since MSTP is an IEEE standard and incorporates RSTP's rapid convergence capabilities, there was no need to include or replicate the UplinkFast feature in MSTP. So this HLD wont support uplink fast.
doc/MSTP/MSTP.md
Outdated
| - Configure an interface for MSTP. | ||
|
|
||
|
|
||
| - **config spanning_tree interface \<ifname\> edgeport {enable|disable}** |
There was a problem hiding this comment.
Does this correspond to AdminEdge Functionality? Will there be a seperate command to support autoEdge?
There was a problem hiding this comment.
This correspond to AdminEdge. autoEdge will not be supported.
doc/MSTP/MSTP.md
Outdated
| - Specify configuring the port level priority for root bridge in seconds. | ||
| - Default: 128, range 0-240 | ||
|
|
||
| - **config spanning_tree interface \<ifname\> cost \<cost-value\>** |
There was a problem hiding this comment.
should we add a command to configure the link type? P2P/Shared Lan/Auto?
There was a problem hiding this comment.
We will add a command for link type.
doc/MSTP/MSTP.md
Outdated
|
|
||
| ### Functional Test Cases | ||
|
|
||
| 1. Verify CONFIG DB is populated with configured MSTP parameters. |
There was a problem hiding this comment.
The numbering seems to be stuck with 1 (in other places too) Is it a problem with the template?
There was a problem hiding this comment.
This is the template, numbering are done automatically. if you see preview format it will show the correct format.
doc/MSTP/MSTP.md
Outdated
|
|
||
| 1. Verify MSTP over LAG. | ||
|
|
||
| 1. Verify MSTP over static breakout ports. |
There was a problem hiding this comment.
Can we add test-cases for BPDU guard and other functionalities?
There was a problem hiding this comment.
Added more test-cases for BPDU guard and other functionalities
Hello All, I want to update here so that all the implementations are aligned on STP, PVST feature is implemented using existing open PRs on stp, validated and automated based on https://github.com/sonic-net/SONiC/blob/master/doc/stp/SONiC_PVST_HLD.md. The complete PR will be raised soon to integrate into 202411 from CISCO. |
|
@madhupalu please invite @bendrapubalareddy to the next PENS WG meeting to discuss the code PRs plan, we have reached a consensus in the WG earlier this year that the older PRs to be dropped to align with MSTP, to avoid further delay of this feature, we need to make sure the community is coherent with the approach |
|
|
@bendrapubalareddy please join us next PENS WG meeting here https://zoom-lfx.platform.linuxfoundation.org/meeting/93042804249?password=179075a6-61fe-448e-b4bc-638ea6b6a7ea |
|
@madhupalu don't think you meant to close this HLD, as it's still under review |
@adyeung sure, we never close this HLD until it is being reviewed in bigger community - tuesday! |
|
Thank you @adyeung <https://github.com/adyeung> @madhupalu
<https://github.com/madhupalu>. I will join .
Bala
…On Thu, Aug 22, 2024 at 12:18 AM Madhu Paluru ***@***.***> wrote:
@bendrapubalareddy <https://github.com/bendrapubalareddy> please join us
next PENS WG meeting here
https://zoom-lfx.platform.linuxfoundation.org/meeting/93042804249?password=179075a6-61fe-448e-b4bc-638ea6b6a7ea
—
Reply to this email directly, view it on GitHub
<#1761 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKZN6TABQXVM6HXZFX4QX3ZSTOHPAVCNFSM6AAAAABLLVWPXWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBSG42DANBWGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Community review recording https://zoom.us/rec/share/NeY9bEy76Zk5vtE8bjtVim3fs7rkuVXOUXkqrKBowwFBgLbRc6OyyYxB-Rryv2zk.iz7qR4m3uXRM_Q2d |
doc/MSTP/MSTP.md
Outdated
| #### STP_MST_INST_TABLE | ||
| ``` | ||
| ;Stores the STP per MSTI operational details | ||
| key = _STP_MST_INST_TABLE:"MST" id |
There was a problem hiding this comment.
As discussed in the meeting, remove "_" prefix from the tables.
| MSTP reduces convergence time compared to STP. When a network topology change occurs, only the affected MSTI needs to reconverge, minimizing the impact on the entire network. | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Mention kernel level changes expected to enable MSTP, if no changes expected and following PVST behavior, mention it accordingly.
doc/MSTP/MSTP.md
Outdated
|
|
||
| # Sequence Diagrams | ||
| ## MSTP global enable | ||
|
|
There was a problem hiding this comment.
Please update the flow diagrams with the details, where the APP_DB changes/updates are done.
There was a problem hiding this comment.
Please update the diagram with APP_DB details.
There was a problem hiding this comment.
We will update the HLD diagram.
doc/MSTP/MSTP.md
Outdated
| - debug spanning_tree mst instance \<instance-id\> | ||
| - debug spanning_tree mst bpdu [tx|rx] | ||
| - debug spanning_tree mst event | ||
| - debug spanning_tree mst verbose |
There was a problem hiding this comment.
can this be combined with "debug spanning_tee mst event" with an additional option as "detail"
There was a problem hiding this comment.
Can you please elaborate what specific "detail" needs to be done with additional option?
There was a problem hiding this comment.
Minor comment - debug spanning_tree mst event [detail] instead of having a verbose command separately
There was a problem hiding this comment.
The below debug commands are existing for PVST and we will reuse the same command for MSTP as well.
- debug spanning_tree bpdu [tx|rx]
- debug spanning_tree event
- debug spanning_tree verbose
The below will be new command added for MSTP
- debug spanning_tree mst instance <instance-id>
doc/MSTP/MSTP.md
Outdated
| path "../../../STP_PORT/STP_PORT_LIST/ifname"; | ||
| } | ||
| description | ||
| "Reference to Ethernet interface or PortChannel"; |
There was a problem hiding this comment.
rephrase it to "Reference to Ethernet interface or PortChannel in the STP database";
There was a problem hiding this comment.
Updated as per suggestion
doc/MSTP/MSTP.md
Outdated
| } | ||
| } | ||
|
|
||
| list _STP_MST_INST_TABLE_LIST { |
There was a problem hiding this comment.
As discussed in the call please remove "_" before the name of the list.
| mst_boundary_proto = BIT ; enabled or disabled | ||
| ``` | ||
|
|
||
| #### STP_INST_PORT_FLUSH_TABLE |
There was a problem hiding this comment.
Why is STP_INST_PORT_FLUSH_TABLE needed in APP_DB? >> it is an action, shouldn't it be put on the APP DB or not?
There was a problem hiding this comment.
In general, Redis-DB is used for communication between dockers in the SONiC architecture.
Are you looking of any alternative approaches to inter-docker communication in SONiC that you're familiar with?
doc/MSTP/MSTP.md
Outdated
|
|
||
| 1. Support protocol operation on static breakout ports | ||
|
|
||
| 1. Support protocol operation on Port-channel interfaces |
There was a problem hiding this comment.
Is the MSTP HLD supports VS image (no ASIC) ?
There was a problem hiding this comment.
Would Port Channel support aggregated path cost? If not, it should be explicitly mentioned here like in PVST HLD.
Updated in HLD
|
|
||
|  | ||
| ## STP Container | ||
| STP Container is responsible for actions taken for BPDU rx and BPDU tx. Following are the details for implementation: |
There was a problem hiding this comment.
Please point me to the stp-container in sonic community code. I couldnt find it.
I see the following PR which is in review state:
sonic-net/sonic-buildimage#3463
There was a problem hiding this comment.
SONIC has a repository as stp which holds all these details. Please see below link to repo:
https://github.com/sonic-net/sonic-stp
There was a problem hiding this comment.
Iam asking about the docker implementation in the PR sonic-net/sonic-buildimage#3463
https://github.com/sonic-net/sonic-stp -- provide stp implementation. I am looking for the stp docker, which is still in a PR sonic-net/sonic-buildimage#3463
There was a problem hiding this comment.
Yes Stp docker is still in above open PR.
There was a problem hiding this comment.
If the code is still in a PR, how are we basing our design, which is not present in the code base yet?
There was a problem hiding this comment.
As discussed in the PENS meeting , we will be pushing the STP docker PR as well.
|
|
||
| ``` | ||
| ;Defines instance and port for which FDB Flush needs to be performed | ||
| key = STP_INST_PORT_FLUSH_TABLE:instane:ifname ; FDB Flush instance id and port |
There was a problem hiding this comment.
why not just use "FLUSHFDBREQUEST PORT" notification, similar to how CLI fdb clear command handles it?
There was a problem hiding this comment.
Can you please share for reference code where it is being used.
There was a problem hiding this comment.
Sorry for the delayed response, please refer the below py script.
src/sonic-utilities/scripts/fdbclear:33: self.db.publish('APPL_DB','FLUSHFDBREQUEST', msg)
There was a problem hiding this comment.
"FLUSHFDBREQUEST PORT" is also a similar redis-DB operation which STPd does here.
| name | ||
| state = "true" | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Can you please add info on how "BPDU guard" functionality will be done, considering it requires port to go into "err-disabled" mode (link-down)
Will existing LINK/PORT tables in APPL_DB be leveraged or new ones be introduced?
There was a problem hiding this comment.
MSTP is going to follow the same approach as the PVST does.
|
Hi,
Can you pls share the meeting invite for today PENS WG meeting if there is
one scheduled? I could not find it on PENS WG page.
Thanks,
Bala
…On Wed, Aug 21, 2024 at 10:48 PM Adam Yeung ***@***.***> wrote:
@madhupalu <https://github.com/madhupalu> please invite @bendrapubalareddy
<https://github.com/bendrapubalareddy> to the next PENS WG meeting to
discuss the code PRs plan, we have reached a consensus in the WG earlier
this year that the older PRs to be dropped to align with MSTP, to avoid
further delay of this feature, we need to make sure the community is
coherent with the approach
—
Reply to this email directly, view it on GitHub
<#1761 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKZN6VWHWFUCS5TB43ULSDZSTDW7AVCNFSM6AAAAABLLVWPXWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBSGU4TCNZYGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi Bala, PENS lab meet is scheduled today. Please see below meeting joining details Regards |
| - Configure path cost of an interface for an instance. | ||
| - cost-value: Range: 1-200000000 | ||
|
|
||
| ## Interface Level |
There was a problem hiding this comment.
These interface level commands below are same as STP. Need to add additional 'mst' keyword, please correct this.
There was a problem hiding this comment.
We will be reusing the PVST commands to support MST configurations as well.
There was a problem hiding this comment.
Kindly add this as a note that the existing interface level PVST commands would be reused for MSTP as well since PVST and MSTP would be mutually exclusive on the same Vlan-Port combination.
doc/MSTP/MSTP.md
Outdated
| 1. Common and Internal Spanning Tree (CIST): The common and internal spanning tree (CIST) is a single spanning tree that connects all devices in a switched network. It consists of the ISTs in all MST regions and the CST. | ||
|
|
||
| 1. MST Instances (MSTIs): MSTP divides the network into multiple regions, each containing several MSTIs. Each MSTI operates independently, allowing for efficient use of network resources and optimized load balancing across different VLANs. | ||
| 1. MST Regions: An MST region is a group of interconnected bridges that share the same MST configuration, including the MST configuration name, revision number, and VLAN-to-instance mappings |
There was a problem hiding this comment.
MST region definition needs to be pre-cursor to IST, CST and CIST definitions that refers to this term. Also please take care of the uniform line spacing between the definitions.
doc/MSTP/MSTP.md
Outdated
| 1. MST Instances (MSTIs): MSTP divides the network into multiple regions, each containing several MSTIs. Each MSTI operates independently, allowing for efficient use of network resources and optimized load balancing across different VLANs. | ||
| 1. MST Regions: An MST region is a group of interconnected bridges that share the same MST configuration, including the MST configuration name, revision number, and VLAN-to-instance mappings | ||
| 1. VLAN-to-MSTI Mapping: MSTP maps VLANs to specific MSTIs using a VLAN mapping table. This mapping ensures that traffic within a VLAN follows the corresponding MSTI, optimizing the network path and improving performance. | ||
| 1. Optimized Bandwidth: By mapping VLANs to specific MSTIs, MSTP ensures that network paths are used efficiently, reducing link blocking and optimizing bandwidth usage |
There was a problem hiding this comment.
Please mention that VLAN-to-MSTI mapping can also help with traffic load sharing and link level redundancy. Its better to separate out this point from definitions as 'MSTP advantages'.
| MST BPDUs: MSTP uses Multiple Spanning Tree Bridge Protocol Data Units (MST BPDUs) to exchange information between switches. These BPDUs contain information about the MSTI and VLAN mappings, ensuring consistent spanning tree calculations across the network. | ||
|
|
||
| MSTP calculates spanning trees on the basis of Multiple Spanning Tree Bridge Protocol Data Units (MST BPDUs). | ||
|
|
There was a problem hiding this comment.
Add a point here that MSTP BPDUs (with IEEE Reserved Multicast MAC) are untagged unlike PVST BPDUs and they are backward compatible with legacy STP/RSTP & PVST+/RPVST+ BPDUs. Also add a note that a hash of MSTP-VLAN mapping knows as 'config digest' is carried in the BPDUs, coupled with region name and the revision number, determines the same/different MST region of the sending switch.
There was a problem hiding this comment.
Updated in HLD as per suggestion
doc/MSTP/MSTP.md
Outdated
|
|
||
| 1. The Destination Mac Address will be 01:80:C2:00:00:00 for MSTP BPDUs. | ||
|
|
||
| 1. Support compatibility with networks employing different spanning tree protocols, such as STP, RSTP. |
There was a problem hiding this comment.
Backward compatibility
There was a problem hiding this comment.
Updated in HLD as per suggestion
doc/MSTP/MSTP.md
Outdated
| The output of this command will be as follows for `mstp`: | ||
| ``` | ||
| Spanning-tree Mode: MSTP | ||
| ####### MST0 Vlans mapped : 1, 4-8, 202-4094 |
There was a problem hiding this comment.
Does it make more sense to display MST0 as CIST? Kindly re-check and change accordingly
There was a problem hiding this comment.
Updated in HLD as per suggestions
| ``` | ||
|
|
||
|
|
||
| - show spanning_tree bpdu_guard |
There was a problem hiding this comment.
Similar show output for Root guard is missing. Would it be available?
There was a problem hiding this comment.
Updated in HLD as per suggestion
doc/MSTP/MSTP.md
Outdated
|
|
||
| 1. Support protocol operation on static breakout ports | ||
|
|
||
| 1. Support protocol operation on Port-channel interfaces |
There was a problem hiding this comment.
Would Port Channel support aggregated path cost? If not, it should be explicitly mentioned here like in PVST HLD.
Updated in HLD
| - **config spanning_tree interface link-type {P2P|Shared-Lan|Auto} \<ifname\>** | ||
| - Specify configuring the interface at different link types. | ||
|
|
||
| ## Show Commands |
There was a problem hiding this comment.
Can any of these show commands display if the topology change is in progress for a particular MSTI when TCNs are being propagated for it, by displaying something such as TC flag set? This can help the user/scripts to understand that the convergence is in progress if the ports are in 'Designated Discarding' state etc. Once the topology is converged, TC flag can be reset and displayed accordingly.
There was a problem hiding this comment.
Currenlty there are no show commands to display this. Noted for future enhancement.
There was a problem hiding this comment.
My idea is to include one additional line like "TC in progress: Yes/No" in the existing show outputs that displays the MSTP port states/roles
| description "First Revision"; | ||
| } | ||
|
|
||
| grouping interfaceAttr { |
There was a problem hiding this comment.
Should this interface grouping also have leaf node for link-type setting? Kindly check.
There was a problem hiding this comment.
Updated in HLD as per suggestion
|
@praveenraja1 @sutharsansr @vinaykumar-kayyur Hi All, We have address all comments and updated HLD as per suggestion. Please help approve HLD |
| MSTP standard does not support uplink fast so uplink fast functionality will be disable for MSTP. | ||
|
|
||
|
|
||
| # Sequence Diagrams |
There was a problem hiding this comment.
Thank you @divyachandralekha! I still feel its important to show how different events arrive at the STP module and how they are handled until HW is programmed.
doc/MSTP/MSTP.md
Outdated
|
|
||
| - **config spanning_tree interface link-type {P2P|Shared-Lan|Auto} \<ifname\>** | ||
| - Specify configuring the interface at different link types. | ||
| - Deafault : Auto |
There was a problem hiding this comment.
Minor: Correct Spelling
| - **config spanning_tree interface link-type {P2P|Shared-Lan|Auto} \<ifname\>** | ||
| - Specify configuring the interface at different link types. | ||
|
|
||
| ## Show Commands |
There was a problem hiding this comment.
My idea is to include one additional line like "TC in progress: Yes/No" in the existing show outputs that displays the MSTP port states/roles
|
All the design aspect has been addressed. Minor enhancement requests can be addressed in future release by the submitters or community. The HLD has been reviewed twice in the SONiC community, if there is no further objection, the HLD will be merged for 202411 |
|
@ridahanif96 can you please help to add the code PRs to this HLD by referring to #806? Thanks. |
@zhangyanzhao , MSTP code PRs are dependent on PVST code which is in review. |
This document describes the design for Multiple Spanning Tree Protocol (MSTP) support in SONiC.
Previous MSTP Design will no longer be considered. The design is modified with a collaborative efforts of BRCM & xFlow Research to provide MSTP design for Community.