[mstp] Fix for incorrectly passing mst internal index to stpsync#77
[mstp] Fix for incorrectly passing mst internal index to stpsync#77vganesan-nokia wants to merge 1 commit intosonic-net:masterfrom
Conversation
Changes to fix wrongly passing mst internal index to stpsync api-s, which expect instance id. For MSTP, since the internal index and MST instance ids are not one-to-one mapped (for example, for CIST, the MST instance id 0 but the internal index 64). With these changes, the index are converted to MST instance id and passed to stpsync api-s Signed-off-by: vedganes <veda.ganesan@nokia.com>
|
@divyachandralekha, would you please review this? |
| /* Delete CIST port from APP DB*/ | ||
| stpsync_del_mst_port_info(ifname, MSTP_MSTID_CIST); | ||
| stpsync_del_port_state(ifname, MSTP_INDEX_CIST); | ||
| stpsync_del_port_state(ifname, MSTP_MSTID_CIST); |
There was a problem hiding this comment.
@vganesan-nokia
This call is intended for orchagent, and orchagent operates based on the MSTP index, not the MST ID.The MST ID is local to the STPd context and is used only for configuration and display purposes.
Therefore, for this call — as well as the other modified calls below — mstp_index is the correct parameter to use.
There was a problem hiding this comment.
@divyachandralekha, thanks for the review comment.
In orchangent there is mapping between the given number (either MST ID or MSTP index or any number) and the OID of the created STP instance m_stpInstOId (Refer src/sonic-swss/orchagent/stporch.cpp addStpInstance(), for example). So, functinoality-wise orchagent is indifferent to MST ID (which is user given) or any internal number (like MSTP index corrsponding to the user given MST ID). As long we choose one (either MST ID or internal MSTP index corresponding to the user given MST ID) and use it consistently the MSTP works.
However, based on the orchagent code, it seems the STP_PORT_STATE_TABLE records in APPL_DB use the key STP_PORT_STATE_TABLE:<interface name>:<instance id>. If we use internal MSTP index, we'll see the port state records of CIST with key STP_PORT_STATE_TABLE:<interface name>:64 and the port records for non CIST MST ID (say MST 1), we'll see those records with key STP_PORT_STATE_TABLE:<interface name>:0. This is bit confusing and also the apis related to stp instance in src/sonic-stp/stpsync/stp_sync.cpp and the orchagent functions for STP_PORT_STATE_TABLE indicate using instance.
If passing mstp_index is the preferred way, I'll close this PR. But, I would recommend to document the schema for APPL_DB STP_PORT_STATE_TABLE to indicate that the key has internal MSTP index but not the user given MST ID and add a fv pair in this records to give MST ID of the index in the key in addition to the state value.
I'll be happy to update the HLD: STP_PORT_STATE_TABLE. to provide the schema details.
2 participants
What was added
Changes to fix wrongly passing mst internal index to stpsync api-s, which expect instance id. For MSTP, since the internal index and MST instance ids are not one-to-one mapped (for example, for CIST, the MST instance id 0 but the internal index 64). With these changes, the index are converted to MST instance id and passed to stpsync api-s
Verified