Skip to content
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

HLD Port FEC BER #1829

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

vincentpcng
Copy link

What I did

Add design proposal for Port FEC BER feature

Summary of Changes:

Enhance the collect and compute the FEC BER on each port.
Modify the port_rates.lua , portstat.py and netstat.py

@vincentpcng vincentpcng marked this pull request as ready for review October 15, 2024 22:42
@qinchuanares
Copy link
Contributor

When the port is down or flapping, or even postFEC error > 0, preFEC BER calculation does not make sense.

After link flap or link down, when link is stably up, the preFEC BER calculation uses counters reset from 0 at link up.

@vincentpcng

This comment was marked as duplicate.

@vincentpcng vincentpcng reopened this Oct 30, 2024
@vincentpcng
Copy link
Author

vincentpcng commented Oct 30, 2024

hit the wrong button and accidental closed the PR, reopen it.

@vincentpcng
Copy link
Author

When the port is down or flapping, or even postFEC error > 0, preFEC BER calculation does not make sense.

After link flap or link down, when link is stably up, the preFEC BER calculation uses counters reset from 0 at link up.

Agree, the measurement could be compromised if the link is not stable, like flapping occur, etc

@Junchao-Mellanox
Copy link
Contributor

The feature only capture the realtime port FEC BER. Is it possible to capture any history port FEC BER issue so that we will be able to get some debug information when an issue only happens in a short period?

doc/port_fec-ber/port_fec_ber.md Outdated Show resolved Hide resolved
- monotonically increasing
- return not support if its not working for an interface
- SAI_PORT_STAT_IF_IN_FEC_NOT_CORRECTABLE_FRAMES
- monotonically increasing

Choose a reason for hiding this comment

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

after link status change the counter isn't reliable. After offline discussion with Microsoft it was decided to remove this requirement

@vincentpcng
Copy link
Author

The following code review PR is for the implementation of this HLD
(1) sonic-net/sonic-swss#3363
(2) sonic-net/sonic-utilities#3607
(3) sonic-net/sonic-mgmt#15481

@vincentpcng
Copy link
Author

vincentpcng commented Nov 12, 2024

Review feedback , the following key points were mentions

(1) can we add min , max, average BER
(2) what about future serdes , can we use user data rate when compute the BER.
(3) what about other devices support
(4) the statistical average 1E-8

response:
(1) we can add the min, max, and average to the show command... this can be done as a future enhancement
(2) unless IEEE defined the enoding, otherwise we dont know (i) what the future serdes speed, (ii) what type of encoding were use in it, hence can;t be done.
One suggestion is to use the user data rate for computing. This is a possible trade off. However, the resulted BER will become not a true reflection. The community future suggest can we add more columns and use the "user data rate".
This will be left as future enhancement discussion
(3) This feature is as stated, require the SAI support. If SAI layer can provide the counter, than we should be able to support the devices.
(4) when calculate the uncorrectable BER (post FEC), the frame were dropped, we have no idea how many error is in the frame. So we can take the best, worst or statistical average for the calculation. During the document review the 1e-8 statistically average were recommended for it.

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@vincentpcng can you please link all the code PRs to the HLD?

@vincentpcng
Copy link
Author

I added the code PR(s) links before the meeting minute comments...

Let me include it again here

The following code review PR is for the implementation of this HLD
(1) sonic-net/sonic-swss#3363
(2) sonic-net/sonic-utilities#3607
(3) sonic-net/sonic-mgmt#15481


Step 4 : assume statistical average error rate as 1e-8 per frame

rs_average_frame_ber = 1e-8
Copy link
Contributor

Choose a reason for hiding this comment

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

@AndyRodan, @vincentpcng can you please provide the reference for this?

Copy link

@AndyRodan AndyRodan Nov 18, 2024

Choose a reason for hiding this comment

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

for 802.3 ck PMD's the minimal requirement in Clause 162 is as follows for "frame loss ratio" (FEC frame error rate):

"For the 100GBASE-CR1 PHY, in order to support the required frame loss ratio (see 1.4.275) of less than
6.2 × 10–10 for 64-octet frames with minimum interpacket gap, the PMD and the adjacent PMA are expected
to detect bits from a compliant input signal at a BER lower than 2.4 × 10−4 assuming errors are sufficiently
uncorrelated. "

For the 200GBASE-CR2 and 400GBASE-CR4 PHYs, in order to support the required frame loss ratio (see
1.4.275) of less than 6.2 × 10–11 for 64-octet frames with minimum interpacket gap, the PMD and the adjacent PMA are expected to detect bits from a compliant input signal at a BER lower than 2.4 × 10−4
assuming errors are sufficiently uncorrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants