Adding initial MSTP changes over PVST code #5
Conversation
…lities (sonic-net#3056) [cisco|express-boot]: Add support for cisco express boot in sonic-utilities
…ch are not applicable to Supervisor (sonic-net#3646) hat I did On 202205 branch, execute "show techsupport " on Supervisor. It hung in the scripts which call "show queue counters". This PR filters out a list of command which are not applicable to Supervisor. Fixes Nokia-ION/ndk#60 How I did it Modify generate_dump script by adding code to check if IS_SUPERVISOR, not to executes the "show queue counters". And also return at the beginning of the following functions which are not applicable on Supervisor: save_bfd_info() save_bgp_info() save_evpn_info() save_frr_info() Also fix some of the failure which are shown during the show techsupport. a) check if file /etc//cron.d/logrote exists before access it in function disable_logrotate() and enable_logrotate() b) check if directory /var/log/sai_failure_dump/ exists before using this directory. Add "show reboot-cause history" to the generate_dump Check if output of "show vrf" is empty, don't access the empty list with "awk" command How to verify it "show techsupport" shoudl work fine on Supervisor as well as on LC Signed-off-by: mlok <marty.lok@nokia.com>
config/stp.py
Outdated
| ctx.fail("MST is already configured") | ||
|
|
||
| if mode == "pvst": | ||
| disable_global_mst(db) |
There was a problem hiding this comment.
Let's throw a warning or error for any STP mode change, as this action should not be allowed. This approach ensures that users first disable the existing mode before configuring a new one, will better control and avoiding conflicts
There was a problem hiding this comment.
warning should be like "MSTP is already configured; please disable PVST before enabling MST" and viceversa
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| enable_stp_for_vlans(db) # Enable STP for VLAN by default | ||
|
|
||
| elif mode == "mst": | ||
| disable_global_pvst(db) |
There was a problem hiding this comment.
Lets remove this disable api and add error message
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| } | ||
| db.mod_entry('STP', "GLOBAL", fvs) | ||
|
|
||
| bridge_mac = get_bridge_mac_address(db) |
There was a problem hiding this comment.
We don't need to set the default values for STP_MST table
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| } | ||
| db.set_entry('STP_MST', "GLOBAL", mst_fvs) | ||
|
|
||
| do_vlan_to_instance0(db) # VLANs to Instance 0 mapping as part of global configuration |
There was a problem hiding this comment.
do_vlan_to_instance0 - this call is also not required. When MST mode is enabled , STPd will add all VLANs to CIST
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| print("PVST is disabled") | ||
| elif mode == "mst" and current_mode == "mst": | ||
| disable_global_mst(db) | ||
| print("MST is disabled") |
There was a problem hiding this comment.
Please remove the print
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
|
|
||
| if mode == "pvst" and current_mode == "pvst": | ||
| disable_global_pvst(db) | ||
| print("PVST is disabled") |
There was a problem hiding this comment.
Please remove this print
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
|
|
||
|
|
||
| # cmd: STP global root guard timeout | ||
| # MST CONFIGURATION IN THE STP_PORT TABLE |
There was a problem hiding this comment.
Root guard timeout is not valid for MST mode
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| if param_type not in allowed_params: | ||
| ctx.fail("Invalid parameter") | ||
|
|
||
| mst_inst_table = db.get_table('STP_MST_INST') |
There was a problem hiding this comment.
This value should be added in STP_MST table (Global).
MSTP has only one timer which is global
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| db.mod_entry('STP', "GLOBAL", {'forward_delay': forward_delay}) | ||
| elif current_mode == "mst": | ||
| db.mod_entry('STP_MST', "GLOBAL", {'forward_delay': forward_delay}) | ||
| update_mst_instance_parameters(ctx, db, 'forward_delay', forward_delay) |
There was a problem hiding this comment.
We dnt need this call update_mst_instance_parameters
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| db.mod_entry('STP', "GLOBAL", {'hello_time': hello_interval}) | ||
| elif current_mode == "mst": | ||
| db.mod_entry('STP_MST', "GLOBAL", {'hello_time': hello_interval}) | ||
| update_mst_instance_parameters(ctx, db, 'hello_time', hello_interval) |
There was a problem hiding this comment.
We dnt need this instance update call
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| db.mod_entry('STP', "GLOBAL", {'max_age': max_age}) | ||
| elif current_mode == "mst": | ||
| db.mod_entry('STP_MST', "GLOBAL", {'max_age': max_age}) | ||
| update_mst_instance_parameters(ctx, db, 'max_age', max_age) |
There was a problem hiding this comment.
We dnt need this instance update call
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| db.mod_entry('STP', "GLOBAL", {'max_hops': max_hops}) | ||
| if current_mode == "mst": | ||
| db.mod_entry('STP_MST', "GLOBAL", {'max_hops': max_hops}) | ||
| update_mst_instance_parameters(ctx, db, 'max_hops', max_hops) |
There was a problem hiding this comment.
We dnt need this instance update call
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| # cmd: STP global bridge priority | ||
| # MST CONFIGURATIONS IN THE STP_MST_INST TABLE |
There was a problem hiding this comment.
Global priority set is not applicable for MST
There was a problem hiding this comment.
I have addressed this comment, please review
config/stp.py
Outdated
| ctx = click.get_current_context() | ||
| db = _db.cfgdb | ||
| check_if_global_stp_enabled(db, ctx) | ||
| if revision not in range(MST_MIN_REVISION, MST_MAX_REVISION + 1): |
There was a problem hiding this comment.
I have addressed this comment, please review
| @@ -465,6 +778,7 @@ def check_if_stp_enabled_for_vlan(ctx, db, vlan_name): | |||
| ctx.fail("STP is not enabled for VLAN") | |||
|
|
|||
|
|
|||
| #config spanning_tree vlan enable <vlan-id> | |||
There was a problem hiding this comment.
This command is valid only for PVST. Please add check.
There was a problem hiding this comment.
I have addressed this comment, please review
| @@ -500,6 +814,7 @@ def stp_vlan_enable(_db, vid): | |||
| db.mod_entry('STP_VLAN_PORT', vlan_intf_key, vlan_intf_entry) | |||
|
|
|||
|
|
|||
| # config spanning_tree vlan disable <vlan-id> | |||
There was a problem hiding this comment.
This command is valid only for PVST. Please add check.
There was a problem hiding this comment.
I have addressed this comment, please review
| @@ -512,6 +827,7 @@ def stp_vlan_disable(_db, vid): | |||
| db.mod_entry('STP_VLAN', vlan_name, {'enabled': 'false'}) | |||
|
|
|||
|
|
|||
| # config spanning_tree vlan forward_delay <vlan-id> <4-30 seconds> | |||
There was a problem hiding this comment.
This command is valid only for PVST. Please add check.
There was a problem hiding this comment.
I have addressed this comment, please review
| @@ -527,7 +843,7 @@ def stp_vlan_forward_delay(_db, vid, forward_delay): | |||
| is_valid_stp_vlan_parameters(ctx, db, vlan_name, "forward_delay", forward_delay) | |||
| db.mod_entry('STP_VLAN', vlan_name, {'forward_delay': forward_delay}) | |||
|
|
|||
|
|
|||
| # config spanning_tree vlan hello <vlan-id> <1-10 seconds> | |||
There was a problem hiding this comment.
This command is valid only for PVST. Please add check.
There was a problem hiding this comment.
I have addressed this comment, please review
| @@ -543,7 +859,7 @@ def stp_vlan_hello_interval(_db, vid, hello_interval): | |||
| is_valid_stp_vlan_parameters(ctx, db, vlan_name, "hello_time", hello_interval) | |||
| db.mod_entry('STP_VLAN', vlan_name, {'hello_time': hello_interval}) | |||
|
|
|||
|
|
|||
| # config spanning_tree vlan max_age <vlan-id> <6-40 seconds> | |||
There was a problem hiding this comment.
This command is valid only for PVST. Please add check.
There was a problem hiding this comment.
I have addressed this comment, please review
| @@ -560,6 +876,7 @@ def stp_vlan_max_age(_db, vid, max_age): | |||
| db.mod_entry('STP_VLAN', vlan_name, {'max_age': max_age}) | |||
|
|
|||
|
|
|||
| # config spanning_tree vlan priority <vlan-id> <0-61440> | |||
There was a problem hiding this comment.
This command is valid only for PVST. Please add check.
There was a problem hiding this comment.
I have addressed this comment, please review
* Update PR checker pipeline to point to bookworm
What I did The sonic vm image has yang validation issue with default config setup. Limit YANG check to golden config only before fix default factory config. How I did it Restrict YANG validation to golden config only. How to verify it Unit test and integration test.
* Revert "Speed up route_check script (sonic-net#3544)" This reverts commit 7cbcfda. * Revert Speed up route_check script
* Fix save_file command in generate_dump * Fix save_file command in generate_dump
Update DB migrator script to next branch 202505
What I did config_sonic_basedon_testbed.yml creates an empty golden_config_db.json file under /etc/sonic for non mx platforms. When this empty file exist db_migration fails for all asics for non mx platforms. How I did it Update db_migrator to support empty golden config file. How to verify it Run unit test and end to end test
Currently adding CLI changes in the PVST code for MSTP