GCU: New test case for port speed change by okaravasi · Pull Request #21235 · sonic-net/sonic-mgmt

okaravasi · 2025-11-07T15:31:14Z

Description of PR

Summary:
Fixes # (issue)
This PR implements the test case described in this test plan #21388.

This PR has dependency on #14887 as it imports and uses functions that are added there.

Type of change

Back port request

Approach

What is the motivation for this PR?

To add new test case for gcu port speed change.

How did you do it?

How did you verify/test it?

Any platform specific information?

generic

Supported testbed topology if it's a new test case?

t2

Documentation

mssonicbld · 2025-11-07T15:31:22Z

/azp run

azure-pipelines · 2025-11-07T15:31:38Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-17T16:00:31Z

/azp run

azure-pipelines · 2025-11-17T16:00:51Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-21T05:33:11Z

/azp run

azure-pipelines · 2025-11-21T05:33:24Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-11-26T15:51:20Z

/azp run

azure-pipelines · 2025-11-26T15:51:34Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

This PR adds a new test case for validating port speed changes using Generic Config Updater (GCU) on T2 topology. The test verifies that port speed can be changed dynamically through GCU patches, with proper updates propagated to all databases (CONFIG_DB, APPL_DB, ASIC), correct buffer profile creation, and successful traffic forwarding with ACL rule verification.

Key changes:

Implements comprehensive port speed change test including setup, verification, and traffic scenarios
Adds platform-specific mappings for supported speeds, lanes, and FEC values
Validates database consistency, buffer profile generation, and ACL rule enforcement after speed changes

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 21 comments.

File	Description
tests/generic_config_updater/add_cluster/test_port_speed_change.py	New test case implementing port speed change validation with fixtures for port selection, speed/FEC configuration, database verification, ACL setup, and traffic validation across multiple scenarios
tests/generic_config_updater/add_cluster/platform_constants.py	Platform-specific mappings defining supported speeds, lane counts per speed, and FEC modes for different platforms

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

arlakshm

as comments

arlakshm · 2025-12-08T01:29:56Z

+    For example:
+        400G speeds might support rs fec while 100G speeds might support fc fec.
+    """
+    current_fec = get_port_fec(duthost, cli_namespace_prefix, selected_random_port)


here we are setting the same fec value before and after speed change. Normally, the scenario is 400g with fec as None and 100g with fec=rs. How is the scenario handled?

Hi @arlakshm

Currently, in the scenario where a port is removed and its speed is changed, a random FEC is selected from the FECs supported for the target speed. This comes from the get_fec_for_speed function, which returns a value based on a hardcoded mapping for the specific platform and speed in the add_cluster/constants.py file.

At this step, we allow the port to be non-operational, as the speed might not be supported by the SFP, so we are more relaxed about the updated FEC value. However, during the actual test operation—when we change the speed to the required one—we use the FEC that the port had initially, to ensure that it is a supported FEC.

For future reference, the get_target_fec function is implemented to return a supported FEC value based on the target speed and the FECs supported for that speed by the chip. Currently, this code is not used because the Broadcom chip on Nokia platforms has limitations and returns supported FEC values only for the initially configured speed.

arlakshm · 2025-12-08T01:30:24Z

+    cmd = "sonic-db-cli {} CONFIG_DB HGET \"PORT|{}\" \"speed\"".format(cli_namespace_prefix, selected_random_port)
+    port_speed_config_db = duthost.shell(cmd, module_ignore_errors=True)['stdout']
+
+    cmd = "sonic-db-cli {} APPL_DB HGET \"PORT_TABLE:{}\" \"speed\"".format(cli_namespace_prefix, selected_random_port)


iso adding individual shell commands here, can we update sonic_asic.py to support single and multi asic? also it can be used in other tests if needed?

Could we handle this via a separate PR and update this code after that change? That would be more convenient for the progress of this PR. Thank you.

arlakshm · 2025-12-08T01:33:11Z

+    with allure.step("Re-enabling loganalyzer before removing cluster - changing speeds."):
+        if loganalyzer and loganalyzer[duthost.hostname]:
+            loganalyzer[duthost.hostname].add_end_ignore_mark()
+


Could we verify whether BGP and the port channel are operational after the speed change, and confirm that all critical processes are running? Also, please check if the neighbor and system_neighbor entries are correct.

@arlakshm , wondering if we might limit the scope of this test case to be a layer1 only test? For port level only perspective, does the test case cover everything? If we combine this test with bgp fact test etc, will it cover BGP/PO sanity? thanks.

anamehra · 2025-12-18T04:53:29Z

Hi @okaravasi , could you please add some details in description on what the test case expected to do? It will be helpful to review if you could add the expected base config, config after remove operations and the patch generated for config apply-patch to understand the expectation and validate of the test meet those expectation.

mssonicbld · 2025-12-29T14:54:56Z

/azp run

azure-pipelines · 2025-12-29T14:55:11Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-12-29T15:02:03Z

/azp run

azure-pipelines · 2025-12-29T15:02:18Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: karavasi <[email protected]>

This reverts commit fd04b44. Signed-off-by: karavasi <[email protected]>

…speed updated in SAI, verify updated pg lossless profile created Signed-off-by: karavasi <[email protected]>

Signed-off-by: karavasi <[email protected]>

… speed change case Signed-off-by: karavasi <[email protected]>

…tches only upon suggestion, 1st patch removes/adds port info + changes speed, 2nd patch applies QUEUE config Signed-off-by: karavasi <[email protected]>

Signed-off-by: karavasi <[email protected]>

mssonicbld · 2025-12-29T15:05:44Z

/azp run

azure-pipelines · 2025-12-29T15:05:58Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: karavasi <[email protected]>

mssonicbld · 2026-01-02T12:44:03Z

/azp run

azure-pipelines · 2026-01-02T12:44:17Z

Azure Pipelines successfully started running 1 pipeline(s).

okaravasi · 2026-01-02T13:06:21Z

Hi @okaravasi , could you please add some details in description on what the test case expected to do? It will be helpful to review if you could add the expected base config, config after remove operations and the patch generated for config apply-patch to understand the expectation and validate of the test meet those expectation.

Hi @anamehra , Please refer to Test Plan #21388 as I have updated with the json example files.

rlhui · 2026-01-08T02:36:03Z

Hi @okaravasi , could you please add some details in description on what the test case expected to do? It will be helpful to review if you could add the expected base config, config after remove operations and the patch generated for config apply-patch to understand the expectation and validate of the test meet those expectation.

@okaravasi is this answered?

rawal01 · 2026-01-08T03:08:47Z

Hi @okaravasi , could you please add some details in description on what the test case expected to do? It will be helpful to review if you could add the expected base config, config after remove operations and the patch generated for config apply-patch to understand the expectation and validate of the test meet those expectation.

@okaravasi is this answered?

@rlhui Olympia pointed to test plan where we have detailed the test case expectations and operation
@anamehra do you need more information ?

anamehra · 2026-01-08T04:55:15Z

@anamehra do you need more information ?

Please update the description as well with the correct link to test plan PR.

rawal01 · 2026-01-09T14:12:07Z

@anamehra do you need more information ?

Please update the description as well with the correct link to test plan PR.

@anamehra the description was updated thanks @arlakshm for help

rawal01 · 2026-01-09T15:21:00Z

@mssonicbld please run the checks again since 14887 is merged it should not run into this error

arlakshm · 2026-01-12T18:24:32Z

/Azp run Azure.sonic-mgmt

azure-pipelines · 2026-01-12T18:24:47Z

Azure Pipelines successfully started running 1 pipeline(s).

rlhui · 2026-01-14T17:55:01Z

this is a new PR replacing this one #21882

okaravasi marked this pull request as draft November 7, 2025 15:32

rawal01 mentioned this pull request Nov 7, 2025

Enhancement: New GCU test case needed for dynamic port speed change for 400G to 100G #21157

Closed

mlok-nokia added this to SONiC Chassis Nov 7, 2025

vmittal-msft self-requested a review November 11, 2025 01:58

abdosi added the P0 label Nov 12, 2025

abdosi requested review from anamehra and kenneth-arista November 12, 2025 18:30

rlhui requested review from prgeor and xincunli-sonic November 19, 2025 18:17

okaravasi marked this pull request as ready for review November 21, 2025 05:35

rlhui assigned okaravasi Dec 3, 2025

rlhui requested a review from arlakshm December 3, 2025 17:10

arlakshm requested a review from Copilot December 8, 2025 00:59

Copilot started reviewing on behalf of arlakshm December 8, 2025 00:59 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

arlakshm reviewed Dec 8, 2025

View reviewed changes

github-actions bot requested review from isabelmsft and yutongzhang-microsoft December 29, 2025 14:55

okaravasi added 8 commits December 29, 2025 16:05

Add Test Plan for port speed change via gcu

cbb2546

Signed-off-by: karavasi <[email protected]>

Adding test case port speed change

161fbd7

Signed-off-by: karavasi <[email protected]>

Revert "Add Test Plan for port speed change via gcu"

bdfb7d2

This reverts commit fd04b44. Signed-off-by: karavasi <[email protected]>

remove port dependencies before speed change, add acl config, verify …

d45ebd1

…speed updated in SAI, verify updated pg lossless profile created Signed-off-by: karavasi <[email protected]>

fixing hardcoded value, acl port indexing

651e9ad

Signed-off-by: karavasi <[email protected]>

Ignoring expecting ERR log based on patch sorting algorithm, for port…

1cd7737

… speed change case Signed-off-by: karavasi <[email protected]>

Modifying patch operations so that to apply port speed change in 2 pa…

7893fc5

…tches only upon suggestion, 1st patch removes/adds port info + changes speed, 2nd patch applies QUEUE config Signed-off-by: karavasi <[email protected]>

Added docstring to module platform_constants.py

ad8503e

Signed-off-by: karavasi <[email protected]>

okaravasi force-pushed the gcu-test-port-speed-change branch from 7d07e8a to ad8503e Compare December 29, 2025 15:05

Resolve pr comments

a9bf567

Signed-off-by: karavasi <[email protected]>

rlhui removed the P0 label Jan 14, 2026

Conversation

okaravasi commented Nov 7, 2025 • edited by arlakshm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Nov 7, 2025

Uh oh!

azure-pipelines bot commented Nov 7, 2025

Uh oh!

mssonicbld commented Nov 17, 2025

Uh oh!

azure-pipelines bot commented Nov 17, 2025

Uh oh!

mssonicbld commented Nov 21, 2025

Uh oh!

azure-pipelines bot commented Nov 21, 2025

Uh oh!

mssonicbld commented Nov 26, 2025

Uh oh!

azure-pipelines bot commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arlakshm left a comment

Choose a reason for hiding this comment

Uh oh!

arlakshm Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

okaravasi Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

arlakshm Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

okaravasi Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arlakshm Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

rlhui Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

anamehra commented Dec 18, 2025

Uh oh!

mssonicbld commented Dec 29, 2025

Uh oh!

azure-pipelines bot commented Dec 29, 2025

Uh oh!

mssonicbld commented Dec 29, 2025

Uh oh!

azure-pipelines bot commented Dec 29, 2025

Uh oh!

mssonicbld commented Dec 29, 2025

okaravasi commented Nov 7, 2025 •

edited by arlakshm

Loading

rawal01 commented Jan 9, 2026 •

edited

Loading