[FEC] Design for auto-fec #1416

dgsudharsan · 2023-07-06T01:46:47Z

This HLD introduces design for auto fec. This document tries to bring in a deterministic approach when FEC and autoneg are configured together which is currently left to vendor implementation.

Repo	PR title	State
sonic-swss	[FEC]Auto FEC initial changes(sonic-net/sonic-swss#2874)
sonic-sairedis	[FEC]Auto FEC initial changes(sonic-net/sonic-sairedis#1271)
sonic-swss	[FEC]Auto FEC initial changes(sonic-net/sonic-swss#2893)
sonic-utilities	[portconfig]Auto FEC initial changes(sonic-net/sonic-utilities#2965 )
sonic-buildimage	[yang] Update sonic-port yang model to support auto FEC (sonic-net/sonic-buildimage#16389)

dgsudharsan · 2023-07-18T18:21:01Z

@prgeor @ridahanif96 Please review this HLD

doc/port_auto_neg/auto-fec.md

dgsudharsan · 2023-08-29T19:44:00Z

@venkatmahalingam @ridahanif96 @madhupalu @prgeor Can you please review and sign off?

dgsudharsan · 2023-09-08T19:15:53Z

@venkatmahalingam @prgeor @ridahanif96 @madhupalu Gentle reminder to review and signoff the HLD

prgeor · 2023-09-11T06:07:42Z

@dgsudharsan can you attach the code PRs in the description. @yutongzhang-microsoft FYI

prgeor · 2023-09-11T04:49:26Z

doc/port_auto_neg/auto-fec.md

+### Requirements
+
+Primary requirements for configuring FEC are
+- Honor the backward compatibility.


@dgsudharsan can you provide more context on the backward compatibility

Backward compatibility means if an image upgrade is done from an image without the feature to image with the feature, there should be no issues. E.g a port with 'rs' fec and AN enabled should work the same before and after the upgrade to image with the feature.

@dgsudharsan thanks. can you capture this in the doc

doc/port_auto_neg/auto-fec.md

prgeor · 2023-09-11T06:01:38Z

doc/port_auto_neg/auto-fec.md

+    /**
+     * @brief Operational FEC mode
+     *
+     * If port is down, the returned value should be zero.


@dgsudharsan return value zero or none?

Updated from latest SAI headers

doc/port_auto_neg/auto-fec.md

prgeor · 2023-09-11T06:20:39Z

doc/port_auto_neg/auto-fec.md

+ |  2  |       False        | False      | auto       | CLI will throw error. No FEC attributes will be set                             |
+ |  3  |       False        | False      | N/A        | No FEC attributes will be set                                                   |
+ |  4  |       False        | True       | none/rs/fc | SAI_PORT_ATTR_FEC_MODE=none/rs/fc. Behavior is undeterministic when AN is enabled and user has configured FEC |
+ |  5  |       False        | True       | auto       | No FEC attributes will be set                                                   |


@dgsudharsan why not throw the error if the platform does not support auto?

doc/port_auto_neg/auto-fec.md

prgeor · 2023-09-11T06:28:15Z

doc/port_auto_neg/auto-fec.md

+ |  6  |       False        | True       | N/A        | No FEC attributes will be set                                                   |
+ |  7  |       True         | True       | none/rs/fc | SAI_PORT_ATTR_FEC_MODE=none/rs/fc SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE=True |
+ |  8  |       True         | True       | auto       | SAI_PORT_ATTR_FEC_MODE=none, SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE=False     |
+ |  9  |       True         | True       | N/A        | SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE will not be set                        |


@dgsudharsan why not set override = False. What is the issue?

This is the case when there is no fec config. N/A means not available. FEC override will be set only when FEC is configured.

@dgsudharsan FEC override = False or FEC override not set aren't the same?

FEC override attribute will be set only in the flow of setting FEC. When FEC itself is not set there will be no override attribute that needs to be programmed.

In addition by default the attribute is false, so if not set the default value expected from SAI is false. However the table is to capture what is programmed from orchagent perspective on specific combinations

doc/port_auto_neg/auto-fec.md

Co-authored-by: Prince George <[email protected]>

prgeor · 2023-09-24T06:14:50Z

doc/port_auto_neg/auto-fec.md

+ |  6  |       False        | True       | N/A        | No FEC attributes will be set                                                   |
+ |  7  |       True         | True       | none/rs/fc | SAI_PORT_ATTR_FEC_MODE=none/rs/fc SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE=True |
+ |  8  |       True         | True       | auto       | SAI_PORT_ATTR_FEC_MODE=none, SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE=False     |
+ |  9  |       True         | True       | N/A        | SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE will not be set                        |


@dgsudharsan FEC override = False or FEC override not set aren't the same?

prgeor · 2023-09-24T06:16:23Z

doc/port_auto_neg/auto-fec.md

+ |  8  |       True         | True       | auto       | SAI_PORT_ATTR_FEC_MODE=none, SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE=False     |
+ |  9  |       True         | True       | N/A        | SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE will not be set                        |
+ | 10  |       True         | False      | None/rs/fc | SAI_PORT_ATTR_FEC_MODE=none/rs/fc                                               |
+ | 11  |       True         | False      | auto       | SAI_PORT_ATTR_FEC_MODE=none, SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE=False. However the auto will take effect only when AN is enabled and until then the mode will be none. |


@dgsudharsan ok. please add notice log

doc/port_auto_neg/auto-fec.md

prgeor · 2023-09-24T06:45:03Z

doc/port_auto_neg/auto-fec.md

+
+### Restrictions/Limitations
+
+Since previously the behavior of FEC with autoneg is undefined, if a vendor always gives precedence for auto negotiated FEC over user configured FEC, implementing the new attribute may lead to backward incompatibility issues when SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE is implemented. Since orchagent will always set SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE to true when FEC is explicitly configured and the attribute is supported by SAI, there will be difference in behavior before and after upgrade.


SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE support by the platform itself means that the platform is going to OVERRIDE its existing behavior. isn't it? If so, the difference in behavior is expected...i mean it will be a breaking change though...If the platform choose to keep the original behavior, then it means the platform is not supporting the override SAI attribute in which case why support override attribute?

If there is difference in behavior, it should be also made backward compatible through db_migrator. It is upto the vendors to decide whether they want to support new attribute and introduce db_migrator if they need it for solving the backward incompatibility or avoid introducing the attribute altogether.

Co-authored-by: Prince George <[email protected]>

* [FEC]Auto FEC initial changes Initial Changes to support Auto FEC feature. Added support for mode "auto" in FEC Why I did it To implement auto FEC feature HLD: sonic-net/SONiC#1416

skg-net · 2024-02-06T00:30:51Z

@dgsudharsan Can you please update the Quality Metric (Alpha/Beta/GA) for the feature either in this PR comments or in HLD itself based on https://github.com/sonic-net/SONiC/blob/master/doc/SONiC%20feature%20quality%20definition.md
Thanks

Initial changes for auto FEC

ee4edcc

dgsudharsan force-pushed the auto_fec branch from 67712bd to ee4edcc Compare July 6, 2023 01:50

Update SAI attributes section

d8fae6b

dgsudharsan requested review from prgeor and ridahanif96 July 18, 2023 18:19

Update auto-fec.md based on community review comments

b603241

venkatmahalingam reviewed Aug 16, 2023

View reviewed changes

doc/port_auto_neg/auto-fec.md Outdated Show resolved Hide resolved

venkatmahalingam reviewed Aug 16, 2023

View reviewed changes

doc/port_auto_neg/auto-fec.md Outdated Show resolved Hide resolved

venkatmahalingam reviewed Aug 16, 2023

View reviewed changes

doc/port_auto_neg/auto-fec.md Outdated Show resolved Hide resolved

ridahanif96 reviewed Aug 16, 2023

View reviewed changes

doc/port_auto_neg/auto-fec.md Show resolved Hide resolved

madhupalu reviewed Aug 16, 2023

View reviewed changes

doc/port_auto_neg/auto-fec.md Outdated Show resolved Hide resolved

Update auto-fec.md

40cf5cb

dgsudharsan requested review from venkatmahalingam, ridahanif96 and madhupalu August 26, 2023 15:25

Update auto-fec.md

9dbf98a

venkatmahalingam approved these changes Sep 8, 2023

View reviewed changes

ridahanif96 approved these changes Sep 8, 2023

View reviewed changes

prgeor reviewed Sep 11, 2023

View reviewed changes

dgsudharsan and others added 4 commits September 11, 2023 09:13

Update doc/port_auto_neg/auto-fec.md

a1f4379

Co-authored-by: Prince George <[email protected]>

Update SAI_PORT_ATTR_OPER_PORT_FEC_MODE definition

6ea8106

Addressing review comments

da07329

Update auto-fec.md

e87422d

dgsudharsan requested a review from prgeor September 12, 2023 03:13

Update auto-fec.md

ff4eb00

prgeor reviewed Sep 24, 2023

View reviewed changes

dgsudharsan and others added 2 commits September 25, 2023 10:39

Update doc/port_auto_neg/auto-fec.md

4230d59

Co-authored-by: Prince George <[email protected]>

Update auto-fec.md

c485763

dgsudharsan requested a review from prgeor September 25, 2023 17:53

Update auto-fec.md

a0bafa3

This was referenced Sep 26, 2023

[FEC]Auto FEC initial changes sonic-net/sonic-swss#2893

Merged

[portconfig]Auto FEC initial changes sonic-net/sonic-utilities#2965

Merged

prgeor approved these changes Sep 26, 2023

View reviewed changes

liat-grozovik merged commit 5cfdfbb into sonic-net:master Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEC] Design for auto-fec #1416

[FEC] Design for auto-fec #1416

dgsudharsan commented Jul 6, 2023 •

edited

Loading

dgsudharsan commented Jul 18, 2023

dgsudharsan commented Aug 29, 2023

dgsudharsan commented Sep 8, 2023

prgeor commented Sep 11, 2023

prgeor Sep 11, 2023

dgsudharsan Sep 12, 2023

prgeor Sep 19, 2023

dgsudharsan Sep 20, 2023

prgeor Sep 11, 2023

dgsudharsan Sep 12, 2023

prgeor Sep 11, 2023

dgsudharsan Sep 12, 2023

prgeor Sep 11, 2023

dgsudharsan Sep 12, 2023

prgeor Sep 24, 2023

dgsudharsan Sep 25, 2023

dgsudharsan Sep 25, 2023

prgeor Sep 24, 2023

prgeor Sep 24, 2023

prgeor Sep 24, 2023

dgsudharsan Sep 25, 2023

skg-net commented Feb 6, 2024


		### Restrictions/Limitations

		Since previously the behavior of FEC with autoneg is undefined, if a vendor always gives precedence for auto negotiated FEC over user configured FEC, implementing the new attribute may lead to backward incompatibility issues when SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE is implemented. Since orchagent will always set SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE to true when FEC is explicitly configured and the attribute is supported by SAI, there will be difference in behavior before and after upgrade.

[FEC] Design for auto-fec #1416

[FEC] Design for auto-fec #1416

Conversation

dgsudharsan commented Jul 6, 2023 • edited Loading

dgsudharsan commented Jul 18, 2023

dgsudharsan commented Aug 29, 2023

dgsudharsan commented Sep 8, 2023

prgeor commented Sep 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skg-net commented Feb 6, 2024

dgsudharsan commented Jul 6, 2023 •

edited

Loading