Add a test case to validate FEC histogram #14661

vvolam · 2024-09-19T17:18:53Z

Description of PR

Summary:
This PR introduces a new test case to verify the FEC histogram on SONiC devices. The test validates the FEC histogram output and checks for errors in critical bins, failing if any errors are detected. This assists in identifying potential transmission issues or signal degradation on the interface.

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

Approach

What is the motivation for this PR?

The motivation for this PR is to add automated testing for validating the Forward Error Correction (FEC) histogram on interfaces. Monitoring FEC statistics is crucial for ensuring link stability, as high error counts in specific bins can indicate link issues or degradation.

How did you do it?

Test case retrieves FEC histogram data using the show interfaces counters fec-histogram command, validates the output, and checks the error count across various bins. The test fails if the error counts in critical bins (bins 8-15) exceed a defined threshold.

How did you verify/test it?

The test was verified by running it on multiple interfaces with SFP modules present and operational.
Test was verified on "Mellanox-SN4700-O8C48" testbed and "Arista-7060X6-64PE-C256S2" testbed.

Any platform specific information?

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

Documentation

N/A

mssonicbld · 2024-09-19T17:19:33Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/test_intf_fec.py:153:9: E128 continuation line under-indented for visual indent flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

tests/platform_tests/test_intf_fec.py

prgeor · 2024-09-26T17:38:46Z

@vvolam please fix the build error

tests/platform_tests/test_intf_fec.py

mssonicbld · 2024-10-09T19:21:05Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/test_intf_fec.py:188:13: F841 local variable 'acceptable_error_threshold' is assigned to but never used tests/platform_tests/test_intf_fec.py:213:121: E501 line too long (128 > 120 characters) tests/platform_tests/test_intf_fec.py:217:21: F821 undefined name 'time' tests/platform_tests/test_intf_fec.py:227:121: E501 line too long (132 > 120 characters) flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2024-10-09T19:26:57Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/test_intf_fec.py:189:13: F841 local variable 'acceptable_error_threshold' is assigned to but never used flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

tests/platform_tests/test_intf_fec.py

mssonicbld · 2024-10-10T01:36:54Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/test_intf_fec.py:142:5: F841 local variable 'e' is assigned to but never used flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

tests/common/platform/interface_utils.py

vvolam · 2024-10-18T21:54:27Z

/azpw run

mssonicbld · 2024-10-18T21:54:29Z

/AzurePipelines run

azure-pipelines · 2024-10-18T21:54:40Z

Azure Pipelines successfully started running 1 pipeline(s).

vvolam · 2024-10-22T16:54:07Z

@dgsudharsan , @roy-sror could you please review? Thank you.

mssonicbld · 2025-01-08T18:19:33Z

/azp run

azure-pipelines · 2025-01-08T18:19:47Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-10T03:03:22Z

/azp run

azure-pipelines · 2025-01-10T03:03:50Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-10T19:42:49Z

/azp run

azure-pipelines · 2025-01-10T19:43:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-10T19:47:49Z

/azp run

azure-pipelines · 2025-01-10T19:48:02Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

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

tests/common/platform/interface_utils.py

tests/platform_tests/test_intf_fec.py

mssonicbld · 2025-01-10T23:20:56Z

/azp run

azure-pipelines · 2025-01-10T23:21:07Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2025-01-22T22:27:36Z

tests/platform_tests/test_intf_fec.py

-                fec = get_fec_oper_mode(duthost, intf['interface'])
-                if fec == "n/a":
-                    pytest.fail("FEC status is N/A for interface {}".format(intf['interface']))
+    for intf in interfaces:


@vvolam some platforms like Arista 7060-x6 does not support fec operational mode yet. can you test there?

@prgeor yes, that's the reason this test is already skipped on broadcom platform which is at line 51.

prgeor · 2025-01-22T22:28:56Z

tests/platform_tests/test_intf_fec.py

+    duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]
+
+    if "broadcom" in duthost.facts.get('platform_asic'):
+        pytest.skip("Skipping this test on platforms with Broadcom ASICs")


@vvolam we need this to be tested on 7060-x6. FEC histogram does work on TH5

@prgeor Tested on Arista 7060x6 platform and enabled the test on that platform. Thank you!

mssonicbld · 2025-01-22T23:28:46Z

/azp run

azure-pipelines · 2025-01-22T23:28:58Z

Azure Pipelines successfully started running 1 pipeline(s).

vvolam requested a review from prgeor as a code owner September 19, 2024 17:18

vvolam requested a review from dgsudharsan September 19, 2024 18:59

prgeor reviewed Sep 25, 2024

View reviewed changes

tests/platform_tests/test_intf_fec.py Outdated Show resolved Hide resolved

prgeor reviewed Sep 26, 2024

View reviewed changes

tests/platform_tests/test_intf_fec.py Outdated Show resolved Hide resolved

vvolam requested review from vdahiya12, prgeor and roy-sror September 26, 2024 19:49

prgeor reviewed Sep 30, 2024

View reviewed changes

tests/platform_tests/test_intf_fec.py Show resolved Hide resolved

prgeor reviewed Oct 9, 2024

View reviewed changes

tests/platform_tests/test_intf_fec.py Outdated Show resolved Hide resolved

prgeor reviewed Oct 17, 2024

View reviewed changes

tests/common/platform/interface_utils.py Outdated Show resolved Hide resolved

vvolam force-pushed the fec-histogram branch from aea7fff to 83ef695 Compare October 18, 2024 00:21

vvolam added 10 commits October 28, 2024 17:18

Add a test case to validate FEC histogram

fd3ec05

Fix pre-commit issue

4f9a801

Bin errors must be ZERO for bin>=7

70edba5

Add a speed check for supported speeds

0d71d91

Add a log when skipping

0d47916

Take 3 samples before deciding bin error counters are increasing

f3fda4e

fix pre-commit errors

5e18761

Minor change

093e6cb

Modularize the test definition for more clarity

4538a8f

Minor change

463b46f

Merge remote-tracking branch 'public/master' into fec-histogram

10ff09b

Address review comments

6dc6fe6

Check for fec_hist only once

c51ef53

vvolam requested a review from Copilot January 10, 2025 19:50

Copilot AI reviewed Jan 10, 2025

View reviewed changes

tests/common/platform/interface_utils.py Show resolved Hide resolved

tests/common/platform/interface_utils.py Show resolved Hide resolved

tests/platform_tests/test_intf_fec.py Show resolved Hide resolved

tests/platform_tests/test_intf_fec.py Outdated Show resolved Hide resolved

Address review comments

dc53a7a

roy-sror approved these changes Jan 19, 2025

View reviewed changes

prgeor reviewed Jan 22, 2025

View reviewed changes

vvolam added 2 commits January 22, 2025 23:25

Merge remote-tracking branch 'public/master' into fec-histogram

0147d7a

Enable test_verify_fec_histogram test on Arista 7060x6 platforms

d21bae9

prgeor approved these changes Jan 22, 2025

View reviewed changes

vvolam added the Request for 202405 branch label Jan 23, 2025

prgeor merged commit 7f8faf6 into sonic-net:master Jan 23, 2025
18 checks passed

vvolam added Request for 202411 branch Request for 202405 branch and removed Request for 202405 branch labels Jan 23, 2025

vvolam deleted the fec-histogram branch January 23, 2025 03:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a test case to validate FEC histogram #14661

Add a test case to validate FEC histogram #14661

vvolam commented Sep 19, 2024 •

edited

Loading

mssonicbld commented Sep 19, 2024

prgeor commented Sep 26, 2024

mssonicbld commented Oct 9, 2024

mssonicbld commented Oct 9, 2024

mssonicbld commented Oct 10, 2024

vvolam commented Oct 18, 2024

mssonicbld commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

vvolam commented Oct 22, 2024

mssonicbld commented Jan 8, 2025

azure-pipelines bot commented Jan 8, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

Copilot AI left a comment

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

prgeor Jan 22, 2025

vvolam Jan 22, 2025

prgeor Jan 22, 2025

vvolam Jan 22, 2025

mssonicbld commented Jan 22, 2025

azure-pipelines bot commented Jan 22, 2025

Add a test case to validate FEC histogram #14661

Add a test case to validate FEC histogram #14661

Conversation

vvolam commented Sep 19, 2024 • edited Loading

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

mssonicbld commented Sep 19, 2024

prgeor commented Sep 26, 2024

mssonicbld commented Oct 9, 2024

mssonicbld commented Oct 9, 2024

mssonicbld commented Oct 10, 2024

vvolam commented Oct 18, 2024

mssonicbld commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

vvolam commented Oct 22, 2024

mssonicbld commented Jan 8, 2025

azure-pipelines bot commented Jan 8, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

mssonicbld commented Jan 10, 2025

azure-pipelines bot commented Jan 10, 2025

prgeor Jan 22, 2025

Choose a reason for hiding this comment

vvolam Jan 22, 2025

Choose a reason for hiding this comment

prgeor Jan 22, 2025

Choose a reason for hiding this comment

vvolam Jan 22, 2025

Choose a reason for hiding this comment

mssonicbld commented Jan 22, 2025

azure-pipelines bot commented Jan 22, 2025

vvolam commented Sep 19, 2024 •

edited

Loading