-
Notifications
You must be signed in to change notification settings - Fork 753
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.
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
Fix the conditional check for FEC errors #16700
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
If we do this change, does that mean passed test now will fail, and failed test now will pass? |
Discussed offline, better to check if FEC counter is 0. The comparison can be skipped if FEC counter is 0. |
* Fix the conditional check for FEC errors * Fix the comment
Cherry-pick PR to 202405: #16715 |
@bingwang-ms No, test cases which are failing are the ones with non-zero's. So this will not be true. |
@bingwang-ms I have opened #16722 PR to address this additional check. Thank you! |
* Fix the conditional check for FEC errors * Fix the comment
Cherry-pick PR to 202411: #16724 |
* Fix the conditional check for FEC errors * Fix the comment
* Fix the conditional check for FEC errors * Fix the comment
Description of PR
FEC correctable codeword errors should not exceed FEC symbol errors, as a codeword is composed of multiple symbols. If this occurs, it likely indicates a reporting bug, misconfiguration, or data misinterpretation, requiring validation of counters, FEC settings, and system behavior. Fix the conditional check in the test case accordingly.
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Issue in the conditional check
How did you do it?
Fix the conditional check for FEC errors
How did you verify/test it?
Verified on SN4700 platform.
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation