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>
| 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
| 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
| } | ||
| 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
| } | ||
| 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
| 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
|
|
||
| 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
|
|
||
|
|
||
| # 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
| 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
| 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
| 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
| 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
| 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
|
|
||
| # Bridge priority cannot be set without Instance ID | ||
| # 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
| 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
| 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
| 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
| 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
| 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
| 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
| 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
divyachandralekha
left a comment
divyachandralekha
left a comment
There was a problem hiding this comment.
Added review comments. Please check
* 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
11 participants
Currently adding CLI changes in the PVST code for MSTP