Skip to content

[Port-breakout] Fix Unable to find key NPU_SI_SETTINGS_SYNC_STATUS ERR #622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Port-breakout] Fix Unable to find key NPU_SI_SETTINGS_SYNC_STATUS ERR #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions sonic-xcvrd/tests/test_xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3597,6 +3597,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_del_port_sfp_dom
mock_table_helper.get_int_tbl = MagicMock(return_value=mock_table)
mock_table_helper.get_dom_tbl = MagicMock(return_value=mock_table)
mock_table_helper.get_dom_threshold_tbl = MagicMock(return_value=mock_table)
mock_table_helper.get_state_port_tbl = MagicMock(return_value=mock_table)
stop_event = threading.Event()
sfp_error_event = threading.Event()
port_mapping = PortMapping()
Expand All @@ -3606,6 +3607,7 @@ def test_SfpStateUpdateTask_handle_port_change_event(self, mock_del_port_sfp_dom
task.xcvr_table_helper.get_status_tbl = mock_table_helper.get_status_tbl
task.xcvr_table_helper.get_intf_tbl = mock_table_helper.get_intf_tbl
task.xcvr_table_helper.get_dom_tbl = mock_table_helper.get_dom_tbl
task.xcvr_table_helper.get_state_port_tbl = mock_table_helper.get_state_port_tbl
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
wait_time = 5
while wait_time > 0:
Expand Down Expand Up @@ -3836,9 +3838,13 @@ class MockTable:
dom_threshold_tbl = MockTable()
dom_threshold_tbl.get = MagicMock(return_value=(True, (('key4', 'value4'),)))
dom_threshold_tbl.set = MagicMock()
state_port_tbl = MockTable()
state_port_tbl.get = MagicMock(return_value=(True, (('key5', 'value5'),)))
state_port_tbl.set = MagicMock()
mock_table_helper.get_status_sw_tbl = MagicMock(return_value=status_sw_tbl)
mock_table_helper.get_intf_tbl = MagicMock(return_value=int_tbl)
mock_table_helper.get_dom_threshold_tbl = MagicMock(return_value=dom_threshold_tbl)
mock_table_helper.get_state_port_tbl = MagicMock(return_value=state_port_tbl)

port_mapping = PortMapping()
mock_sfp_obj_dict = MagicMock()
Expand All @@ -3849,6 +3855,7 @@ class MockTable:
task.xcvr_table_helper.get_status_sw_tbl = mock_table_helper.get_status_sw_tbl
task.xcvr_table_helper.get_intf_tbl = mock_table_helper.get_intf_tbl
task.xcvr_table_helper.get_dom_threshold_tbl = mock_table_helper.get_dom_threshold_tbl
task.xcvr_table_helper.get_state_port_tbl = mock_table_helper.get_state_port_tbl
task.dom_db_utils.post_port_dom_thresholds_to_db = MagicMock()
task.vdm_db_utils.post_port_vdm_thresholds_to_db = MagicMock()
port_change_event = PortChangeEvent('Ethernet0', 1, 0, PortChangeEvent.PORT_ADD)
Expand Down
10 changes: 10 additions & 0 deletions sonic-xcvrd/xcvrd/xcvrd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,16 @@ def on_add_logical_port(self, port_change_event):
# 3. SFP is not present. Only update TRANSCEIVER_STATUS_INFO table.
status_sw_tbl = self.xcvr_table_helper.get_status_sw_tbl(port_change_event.asic_id)
int_tbl = self.xcvr_table_helper.get_intf_tbl(port_change_event.asic_id)
# Initialize the NPU_SI_SETTINGS_SYNC_STATUS to default value
state_port_table = self.xcvr_table_helper.get_state_port_tbl(port_change_event.asic_id)
found, state_port_table_fvs = state_port_table.get(port_change_event.port_name)
if not found:
helper_logger.log_notice("Add logical port: Creating STATE_DB PORT_TABLE as unable to find for lport {}".format(port_change_event.port_name))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keshavg-marvell can we make it debug log?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @prgeor for the review. Same log level is there in normal port add during xcvrd init at below place. Do we change there as well to be consistent for both the cases?

sonic-platform-daemons/sonic-xcvrd/xcvrd/xcvrd.py

Line 2079 in 700ccd4

self.log_notice("Port init control: Creating STATE_DB PORT_TABLE as unable to find for lport {}".format(lport))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keshavg-marvell @mihirpat1 can we call initialize_port_init_control_fields_in_port_table() in on_add_logical_port() instead of calling it in daemon init() function? This way we dont have to handle breakout scenario differently?

state_port_table_fvs = []
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider verifying the conversion of state_port_table_fvs to a dictionary when it may be an empty list. Adding a clarifying comment on the expected structure of state_port_table_fvs could improve maintainability.

Suggested change
state_port_table_fvs = []
state_port_table_fvs = []
# Verify the structure of state_port_table_fvs and clarify its expected format.
# Expected: A list of key-value pairs (e.g., [(key1, value1), (key2, value2)]) or an empty list.
if not isinstance(state_port_table_fvs, list) or not all(isinstance(item, tuple) and len(item) == 2 for item in state_port_table_fvs):
helper_logger.log_error("Invalid structure for state_port_table_fvs: {}".format(state_port_table_fvs))
state_port_table_fvs = []

Copilot uses AI. Check for mistakes.

if NPU_SI_SETTINGS_SYNC_STATUS_KEY not in dict(state_port_table_fvs):
state_port_table.set(port_change_event.port_name, [(NPU_SI_SETTINGS_SYNC_STATUS_KEY,
NPU_SI_SETTINGS_DEFAULT_VALUE)])
helper_logger.log_notice("Add logical port: Initialized NPU_SI_SETTINGS_SYNC_STATUS for lport {}".format(port_change_event.port_name))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keshavg-marvell debug instaed of notice log?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @prgeor for the review. Same log level is there in normal port add during xcvrd init at below place. Do we change there as well to be consistent for both the cases?

sonic-platform-daemons/sonic-xcvrd/xcvrd/xcvrd.py

Line 2085 in 700ccd4

self.log_notice("Port init control: Initialized NPU_SI_SETTINGS_SYNC_STATUS for lport {}".format(lport))


error_description = 'N/A'
status = None
Expand Down
Loading