Fixing MSTP global command enable issue #4127

divyachandralekha · 2025-11-19T14:30:46Z

What I did
Added support to display MSTP output for "show spanning-tree" command
Fixed the issue on MSTP global enable command issue
Added testcases to cover mstp display command changes

mssonicbld · 2025-11-19T14:30:55Z

/azp run

azure-pipelines · 2025-11-19T14:31:06Z

Azure Pipelines successfully started running 1 pipeline(s).

divyachandralekha · 2025-11-19T14:38:15Z

@skulambi-cisco
Please review

vganesan-nokia · 2025-11-21T21:32:36Z

@divyachandralekha, currently all the commands starting with "config spanning-tree mst instance" expects an entry for given instance already exists in STP_MST_INST table in config db. If an entry for the given instance does not already exist in config db, there is failure.
We would like to suggest to have a command to newly create an entry in "STP_MST_INST" table. We can use the command "config spanning-tree mst instance priority " to create an entry in STP_MST_INST table if one does not exist already before setting the priority.

divyachandralekha · 2025-11-25T15:03:14Z

@divyachandralekha, currently all the commands starting with "config spanning-tree mst instance" expects an entry for given instance already exists in STP_MST_INST table in config db. If an entry for the given instance does not already exist in config db, there is failure. We would like to suggest to have a command to newly create an entry in "STP_MST_INST" table. We can use the command "config spanning-tree mst instance priority " to create an entry in STP_MST_INST table if one does not exist already before setting the priority.

@vganesan-nokia , This can be taken up as an enhancement. The current behaviour is intentional, as it prevents creating or consuming an MST instance without any VLAN mapping. This ensures that MST instances are only created when they have a valid purpose in the configuration.

mssonicbld · 2025-11-25T17:08:30Z

/azp run

azure-pipelines · 2025-11-25T17:08:40Z

Azure Pipelines successfully started running 1 pipeline(s).

divyachandralekha · 2025-11-25T17:17:59Z

@vganesan-nokia , Please approve the changes

vganesan-nokia · 2025-11-25T19:34:14Z

show/stp.py

+    vlan_str = mst_entry.get('vlan@', '')
+    vlan_list = format_vlan_list(vlan_str)
+    click.echo("")
+    click.echo("#######  MST{:<8} Vlans mapped : {}".format(mst_id, vlan_list))


If the instance is 0 we can add the word (CIST) as propsed in the HLD

I will take care in next PR.

vganesan-nokia · 2025-11-25T19:52:25Z

show/stp.py

-            ctx.invoke(show_stp_vlan, vlanid=vlanid)
+        # For MSTP mode, use new format
+        if g_stp_mode == 'MSTP':
+            ctx.invoke(show_mstp_summary)


Suggestion for alternate approach:
If we can do this similar to how it is done for PVST we can show spanning tree for all MST instances and also for a specific instance (for "show spanning-tree mst instance command) using the same function show_mst_instnace_detail().

I will take care in next PR.

vganesan-nokia · 2025-11-25T19:59:34Z

show/stp.py

+    show_mst_region_info()
+
+
+def show_mst_instance_detail(mst_id):


When I compare the implemented show output to the show information in HLD, I see that the information shown here is for the summary. i..e, the interface information is shown in single row for each interface. Should we rename this function to "show_mst_instance_summary()" to make the name approriately mean what is done in the function?

I will take care in next PR.

vganesan-nokia · 2025-11-25T20:02:23Z

show/stp.py

+            show_mst_port_info(mst_id, ifname)
+
+
+def show_mst_port_info(mst_id, ifname):


We may need another function like "show_mst_port_info_detail()" which can be used for showing MST instance detail where the each port info is displayed in deatil (as per "show spanning-tree mst detail" info in the HLD)

I will take care in next PR.

vganesan-nokia · 2025-11-25T20:10:34Z

show/stp.py

+    # CIST Root timers (typically same as bridge timers)
+    click.echo("CIST Root Timers                : MaxAge {}s Hello {}s FwdDly {}s MaxHops {}".format(
+        mst_global['max_age'], mst_global['hello_time'], 
+        mst_global['forward_delay'], mst_global['max_hops']))


Recommendation: It is better to implement the "show spanning-tree statistics [mst ]" as part of this PR. We already have some statistics (such as bpd rx, bpdu tx and num fwd transitions) in the STP_MST_PORT_TABLE entries. We can also implement "show spanning-tree mst interface " as part of the PR to meet all the show commands specified in HLD

it seems the MSTP show commands are missed during implementation.
Currently I have enhanced the existing show command to verify the MSTP. Rest all show MSTP related changes will add in next PR.

vganesan-nokia · 2025-11-25T20:12:57Z

tests/test_config_mstp.py

            assert result.exit_code != 0
            assert "Invalid value" in result.output
-            assert "is not one of 'enable', 'disable'" in result.output
+            assert "choose from enable, disable" in result.output


A generic commnent not for this line.
We need unit test for the show commands impelemented in show/stp.py.

vganesan-nokia · 2025-11-25T20:15:06Z

@divyachandralekha, currently all the commands starting with "config spanning-tree mst instance" expects an entry for given instance already exists in STP_MST_INST table in config db. If an entry for the given instance does not already exist in config db, there is failure. We would like to suggest to have a command to newly create an entry in "STP_MST_INST" table. We can use the command "config spanning-tree mst instance priority " to create an entry in STP_MST_INST table if one does not exist already before setting the priority.

@vganesan-nokia , This can be taken up as an enhancement. The current behaviour is intentional, as it prevents creating or consuming an MST instance without any VLAN mapping. This ensures that MST instances are only created when they have a valid purpose in the configuration.

@divyachandralekha, thanks for confirming that this is was intentional. Yes, we can do this as enhancement.

vganesan-nokia · 2025-11-25T20:17:21Z

@vganesan-nokia , Please approve the changes

@divyachandralekha, I have some reivew comments for the show commands implementation. I think the show command code was added after I post my previous review comments.

mssonicbld · 2025-11-26T08:53:38Z

/azp run

azure-pipelines · 2025-11-26T08:53:49Z

Azure Pipelines successfully started running 1 pipeline(s).

divyachandralekha · 2025-11-26T08:58:58Z

@vganesan-nokia , All your review comments are noted. I will take care of those in next PR.

mssonicbld · 2025-11-26T11:04:38Z

/azp run

azure-pipelines · 2025-11-26T11:04:49Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-26T11:17:29Z

/azp run

azure-pipelines · 2025-11-26T11:17:39Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-26T15:49:12Z

/azp run

azure-pipelines · 2025-11-26T15:49:23Z

Azure Pipelines successfully started running 1 pipeline(s).

vganesan-nokia

Thanks for respoinding to the review comments. Looking forward to the next PR.

mssonicbld · 2025-11-26T18:03:42Z

/azp run

azure-pipelines · 2025-11-26T18:03:53Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-27T15:19:52Z

/azp run

azure-pipelines · 2025-11-27T15:20:03Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-27T16:04:18Z

/azp run

azure-pipelines · 2025-11-27T16:04:29Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-27T17:35:19Z

/azp run

azure-pipelines · 2025-11-27T17:35:30Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T04:56:57Z

/azp run

azure-pipelines · 2025-11-28T04:57:08Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T08:28:02Z

/azp run

azure-pipelines · 2025-11-28T08:28:13Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T09:17:12Z

/azp run

azure-pipelines · 2025-11-28T09:17:22Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T12:21:33Z

/azp run

azure-pipelines · 2025-11-28T12:21:43Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T13:50:42Z

/azp run

azure-pipelines · 2025-11-28T13:50:53Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T17:06:23Z

/azp run

azure-pipelines · 2025-11-28T17:06:34Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-28T17:41:56Z

/azp run

azure-pipelines · 2025-11-28T17:42:11Z

Azure Pipelines successfully started running 1 pipeline(s).

divyachandralekha · 2025-11-29T08:56:57Z

@qiluo-msft , Please merge the changes

Fixing MSTP global command enable issue

522aa7f

Fixing MSTP global command enable issue

af11fa0

vganesan-nokia reviewed Nov 25, 2025

View reviewed changes

Fixing MSTP global command enable issue

da56043

Fixing MSTP global command enable issue

32216c1

Fixing MSTP global command enable issue

1b34555

Fixing MSTP global command enable issue

066f77b

vganesan-nokia approved these changes Nov 26, 2025

View reviewed changes

Fixing MSTP global command enable issue

6338ae2

Fixing MSTP global command enable issue

5348f0e

Fixing MSTP global command enable issue

da7c4b8

Fixing MSTP global command enable issue

5c2745a

Fixing MSTP global command enable issue

92608c8

Fixing MSTP global command enable issue

449b11f

Fixing MSTP global command enable issue

235ebff

Fixing MSTP global command enable issue

44d54b7

Fixing MSTP global command enable issue

c26b693

Fixing MSTP global command enable issue

3e5f50b

		show_mst_port_info(mst_id, ifname)


		def show_mst_port_info(mst_id, ifname):

Fixing MSTP global command enable issue #4127

Are you sure you want to change the base?

Fixing MSTP global command enable issue #4127

Uh oh!

Conversation

divyachandralekha commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Nov 19, 2025

Uh oh!

azure-pipelines bot commented Nov 19, 2025

Uh oh!

divyachandralekha commented Nov 19, 2025

Uh oh!

vganesan-nokia commented Nov 21, 2025

Uh oh!

divyachandralekha commented Nov 25, 2025

Uh oh!

mssonicbld commented Nov 25, 2025

Uh oh!

azure-pipelines bot commented Nov 25, 2025

Uh oh!

divyachandralekha commented Nov 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vganesan-nokia Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vganesan-nokia commented Nov 25, 2025

Uh oh!

vganesan-nokia commented Nov 25, 2025

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

divyachandralekha commented Nov 26, 2025

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

vganesan-nokia left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

mssonicbld commented Nov 27, 2025

Uh oh!

azure-pipelines bot commented Nov 27, 2025

divyachandralekha commented Nov 19, 2025 •

edited

Loading

vganesan-nokia Nov 25, 2025 •

edited

Loading