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

Keshavg-marvell · 2025-06-03T10:44:58Z

Description

What is the Issue:

For newly created breakout ports, the NPU_SI_SETTINGS_SYNC_STATUS value is not getting set in the STATE_DB's PORT_TABLE. This is normally done during xcvrd init for all the port present in system using initialize_port_init_control_fields_in_port_table(). Due to this during SI setting programming for DPB ports, the code fails to locate the NPU_SI_SETTINGS_SYNC_STATUS key in the STATE_DB, resulting in an below syslog error.

sonic ERR pmon#chassis: Get value by key from STATE_DB: Unable to
 find key NPU_SI_SETTINGS_SYNC_STATUS state_port_table_fvs_dict
 {'host_tx_ready': 'false', 'state': 'ok', 'netdev_oper_status':
 'down', 'admin_status': 'down', 'mtu': '9100', 'supported_speeds':
 '50000,100000', 'supported_fecs': 'rs'} for lport Ethernet448

Fix:
Set the NPU_SI_SETTINGS_SYNC_STATUS to NPU_SI_SETTINGS_DEFAULT_VALUE value in State DB on the event of "port_event_helper.PortChangeEvent.PORT_ADD" in on_add_logical_port() which get trigged for newly added DPB ports.

This PR fixes below issue:
#621

Motivation and Context

To fix error seen in syslog during port breakout

How Has This Been Tested?

Verified that syslog ERR is not seen with breakout CLI
sudo config interface breakout Ethernet448 "8x100G[50G,25G]" -y -f

Back port request

Additional Information (Optional)

sonic-xcvrd/xcvrd/xcvrd.py

…slog error What is the Issue: For newly created breakout ports, the NPU_SI_SETTINGS_SYNC_STATUS value is not getting set in the STATE_DB's PORT_TABLE. This is normally done during xcvrd init for all the port present in system using initialize_port_init_control_fields_in_port_table(). Due to this during SI setting programming for DPB ports, the code fails to locate the NPU_SI_SETTINGS_SYNC_STATUS key in the STATE_DB, resulting in an below syslog error. sonic ERR pmon#chassis: Get value by key from STATE_DB: Unable to find key NPU_SI_SETTINGS_SYNC_STATUS state_port_table_fvs_dict {'host_tx_ready': 'false', 'state': 'ok', 'netdev_oper_status': 'down', 'admin_status': 'down', 'mtu': '9100', 'supported_speeds': '50000,100000', 'supported_fecs': 'rs'} for lport Ethernet448 Fix: Set the NPU_SI_SETTINGS_SYNC_STATUS to NPU_SI_SETTINGS_DEFAULT_VALUE value in State DB on the event of "port_event_helper.PortChangeEvent.PORT_ADD" in on_add_logical_port() which get trigged for newly added DPB ports. Signed-off-by: Keshav Gupta <[email protected]>

Copilot

Pull Request Overview

This PR addresses an issue with breakout ports where the NPU_SI_SETTINGS_SYNC_STATUS field was missing in the STATE_DB PORT_TABLE, causing errors during SI setting programming. Key changes include adding initialization of NPU_SI_SETTINGS_SYNC_STATUS in the xcvrd on_add_logical_port() function and updating tests to mock the state port table.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
sonic-xcvrd/xcvrd/xcvrd.py	Added logic to initialize NPU_SI_SETTINGS_SYNC_STATUS for new logical ports
sonic-xcvrd/tests/test_xcvrd.py	Updated tests to include the state port table mock setup

Comments suppressed due to low confidence (1)

sonic-xcvrd/tests/test_xcvrd.py:3600

Consider adding explicit assertions to verify that NPU_SI_SETTINGS_SYNC_STATUS is correctly set in the state DB during the logical port addition. This would help ensure the new behavior for DPB ports is properly tested.

mock_table_helper.get_state_port_tbl = MagicMock(return_value=mock_table)

Copilot · 2025-06-05T02:08:50Z

sonic-xcvrd/xcvrd/xcvrd.py

+        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))
+            state_port_table_fvs = []


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 = []

mihirpat1 · 2025-06-05T20:53:22Z

/azpw run

mssonicbld · 2025-06-05T20:53:25Z

Only PR owner can use /azpw run

mihirpat1 · 2025-06-05T20:53:40Z

/azp run

azure-pipelines · 2025-06-05T20:53:46Z

Commenter does not have sufficient privileges for PR 622 in repo sonic-net/sonic-platform-daemons

mihirpat1 · 2025-06-05T21:12:11Z

@Keshavg-marvell Where is the below log defined? Asking since I couldn't find it in xcvrd

sonic ERR pmon#chassis: Get value by key from STATE_DB: Unable to
 find key NPU_SI_SETTINGS_SYNC_STATUS state_port_table_fvs_dict
 {'host_tx_ready': 'false', 'state': 'ok', 'netdev_oper_status':
 'down', 'admin_status': 'down', 'mtu': '9100', 'supported_speeds':
 '50000,100000', 'supported_fecs': 'rs'} for lport Ethernet448

Keshavg-marvell · 2025-06-06T04:46:08Z

@Keshavg-marvell Where is the below log defined? Asking since I couldn't find it in xcvrd

sonic ERR pmon#chassis: Get value by key from STATE_DB: Unable to
 find key NPU_SI_SETTINGS_SYNC_STATUS state_port_table_fvs_dict
 {'host_tx_ready': 'false', 'state': 'ok', 'netdev_oper_status':
 'down', 'admin_status': 'down', 'mtu': '9100', 'supported_speeds':
 '50000,100000', 'supported_fecs': 'rs'} for lport Ethernet448

this logs get printed in below code.

https://github.com/sonic-net/sonic-platform-daemons/blob/master/sonic-xcvrd/xcvrd/xcvrd_utilities/xcvr_table_helper.py#L216

mihirpat1 · 2025-06-06T17:38:42Z

@Keshavg-marvell Where is the below log defined? Asking since I couldn't find it in xcvrd
sonic ERR pmon#chassis: Get value by key from STATE_DB: Unable to
 find key NPU_SI_SETTINGS_SYNC_STATUS state_port_table_fvs_dict
 {'host_tx_ready': 'false', 'state': 'ok', 'netdev_oper_status':
 'down', 'admin_status': 'down', 'mtu': '9100', 'supported_speeds':
 '50000,100000', 'supported_fecs': 'rs'} for lport Ethernet448
this logs get printed in below code.

https://github.com/sonic-net/sonic-platform-daemons/blob/master/sonic-xcvrd/xcvrd/xcvrd_utilities/xcvr_table_helper.py#L216

@Keshavg-marvell Wondering why are we seeing chassis instead of xcvrd in the process log.
Also, the changes look good to me.

Keshavg-marvell · 2025-06-16T14:14:14Z

@Keshavg-marvell Where is the below log defined? Asking since I couldn't find it in xcvrd
sonic ERR pmon#chassis: Get value by key from STATE_DB: Unable to
 find key NPU_SI_SETTINGS_SYNC_STATUS state_port_table_fvs_dict
 {'host_tx_ready': 'false', 'state': 'ok', 'netdev_oper_status':
 'down', 'admin_status': 'down', 'mtu': '9100', 'supported_speeds':
 '50000,100000', 'supported_fecs': 'rs'} for lport Ethernet448
this logs get printed in below code.
https://github.com/sonic-net/sonic-platform-daemons/blob/master/sonic-xcvrd/xcvrd/xcvrd_utilities/xcvr_table_helper.py#L216
@Keshavg-marvell Wondering why are we seeing chassis instead of xcvrd in the process log. Also, the changes look good to me.

@mihirpat1 I believe this chassis is appended from common syslog infra used by xcvrd. if changes looks good. Can you please review and merge this PR.

mihirpat1

LGTM. However, not sure why the PR checkers have not yet triggered for this PR.

Keshavg-marvell · 2025-06-17T03:37:36Z

/azpw run Azure.sonic-platform-daemons

mssonicbld · 2025-06-17T03:37:39Z

/AzurePipelines run Azure.sonic-platform-daemons

azure-pipelines · 2025-06-17T03:37:48Z

Azure Pipelines successfully started running 1 pipeline(s).

Keshavg-marvell · 2025-06-18T04:25:10Z

Thanks @mihirpat1 for the review. I have retrigged the pipelines, and now all checks are passing. Can you please merge this PR now?

Keshavg-marvell · 2025-06-26T11:37:17Z

@prgeor Can you please review and merge this PR?

prgeor · 2025-06-30T20:25:04Z

sonic-xcvrd/xcvrd/xcvrd.py

+        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))


@Keshavg-marvell can we make it debug log?

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))

@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?

prgeor · 2025-06-30T20:25:31Z

sonic-xcvrd/xcvrd/xcvrd.py

+        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))


@Keshavg-marvell debug instaed of notice log?

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))

mihirpat1 mentioned this pull request Jun 3, 2025

"Unable to find key NPU_SI_SETTINGS_SYNC_STATUS .." Error logged for newly created DPB ports. #621

Open

mihirpat1 reviewed Jun 3, 2025

View reviewed changes

sonic-xcvrd/xcvrd/xcvrd.py Outdated Show resolved Hide resolved

Keshavg-marvell force-pushed the port_breakout_error branch from 59578f8 to bdcf95a Compare June 4, 2025 04:06

Keshavg-marvell requested a review from mihirpat1 June 4, 2025 04:19

mihirpat1 requested a review from Copilot June 5, 2025 02:08

Copilot AI reviewed Jun 5, 2025

View reviewed changes

mihirpat1 approved these changes Jun 16, 2025

View reviewed changes

prgeor reviewed Jun 30, 2025

View reviewed changes

saiarcot895 requested a review from prgeor July 23, 2025 15:44

pavannaregundi mentioned this pull request Aug 1, 2025

Fix port SI setting per speed during speed change #649

Open

1 task

-            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 = []

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

Keshavg-marvell commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Back port request

Additional Information (Optional)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

mihirpat1 commented Jun 5, 2025

Uh oh!

mssonicbld commented Jun 5, 2025

Uh oh!

mihirpat1 commented Jun 5, 2025

Uh oh!

azure-pipelines bot commented Jun 5, 2025

Uh oh!

mihirpat1 commented Jun 5, 2025

Uh oh!

Keshavg-marvell commented Jun 6, 2025

Uh oh!

mihirpat1 commented Jun 6, 2025

Uh oh!

Keshavg-marvell commented Jun 16, 2025

Uh oh!

mihirpat1 left a comment

Choose a reason for hiding this comment

Uh oh!

Keshavg-marvell commented Jun 17, 2025

Uh oh!

mssonicbld commented Jun 17, 2025

Uh oh!

azure-pipelines bot commented Jun 17, 2025

Uh oh!

Keshavg-marvell commented Jun 18, 2025

Uh oh!

Keshavg-marvell commented Jun 26, 2025

Uh oh!

prgeor Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Keshavg-marvell Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

prgeor Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

prgeor Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Keshavg-marvell Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Keshavg-marvell commented Jun 3, 2025 •

edited

Loading