Media based port settings in SONIC #328

dgsudharsan · 2019-01-28T07:25:25Z

This pull request contains the high level document explaining the design of media based port settings in Sonic

doc/media-settings/Media-based-Port-settings.md

ben-gale · 2019-01-31T15:07:37Z

doc/media-settings/Media-based-Port-settings.md

+
+    "PORT_OPTICS_SETTINGS": {
+
+        "Ethernet20": {


"Ethernet20": As discussed on the call, I think it would be good to allow port lists (e.g. "Ethernet1, Ethernet3, Ethernet5 etc") and port ranges (e.g. "Ethernet1-Ethernet48") here. A list can also include range entries. I think this may make the file much shorter and more usable/maintainable.

Hi @ben-gale . Agreed. Will add this to the document

ben-gale · 2019-01-31T15:11:06Z

doc/media-settings/Media-based-Port-settings.md

+
+                    "preemphasis": [
+
+                         "0x1201",


Can you explain what these values represent please? They look like register contents, in which case, are these values similar/common in semantics and type across transceivers?

These are the best abstractions of relevant pre-emp, idriver and pre-driver registers that are used by vendors. These will be common in semantics across transceivers. ASIC vendors have APIs to program these. So, it's upto the libsaiASIC-VENDOR implementation to implement these to map to their SDKs

ben-gale · 2019-01-31T15:11:41Z

doc/media-settings/Media-based-Port-settings.md

+
+                        "0x1"
+
+                     ]


Did you think about adding a FEC setting here (for copper transceivers)?

FEC can already be provided in port_config.ini (e.g. wnc/x86_64-wnc_osw1800-r0/OSW1800-48x6q/port_config.ini). FEC settings should be coming from config DB through the configuration present in port_config.ini

correct me if I'm wrong but is not the FEC set in the INTERFACE section, at least that is how we do it for specific optics
"Ethernet36": {
...
"fec": "rs"
},

correct me if I'm wrong but is not the FEC set in the INTERFACE section, at least that is how we do it for specific optics
"Ethernet36": {
...
"fec": "rs"
},

Hi. What i meant to convey here is there are multiple other mechanisms used to configure FEC and hence this specific mechanism need not have provision to configure FEC settings

ben-gale · 2019-01-31T15:13:03Z

doc/media-settings/Media-based-Port-settings.md

+
+    "PORT_OPTICS_SETTINGS": {
+
+        "Ethernet20": {


As also discussed on the call, the file must also support Port Breakout scenarios - this may change the port references in the file. Please describe how will this be handled?

The document will be updated on breakout scenario.

Rename Media-based-Port-settings.md to doc/Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md Image files Rename doc/Media-based-Port-settings.md to doc/media-settings/Media-based-Port-settings.md Delete event_flow.png Delete key_selection_flow.png Add files via upload Update Media-based-Port-settings.md Update Media-based-Port-settings.md Addressing the global range option Adding a global range block where the common values for multiple ports can be specified as a range. Changing indentation Update Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md Update Media-based-Port-settings.md

lguohan · 2019-02-02T06:38:30Z

as part of the development, please also add a checker program to make sure the media setting file submit by vendor is correct. here is an example. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-device-data/tests/config_checker

dgsudharsan · 2019-02-05T03:11:17Z

as part of the development, please also add a checker program to make sure the media setting file submit by vendor is correct. here is an example. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-device-data/tests/config_checker

Sure Guohan. Will add that during development

Updating the document based on latest design

lguohan · 2019-03-28T07:29:51Z

doc/media-settings/Media-based-Port-settings.md

+Breakout Scenario:
+==================
+
+              For breakout the particular media, if it has a global value is listed in the media_settings.json with appropriate lane values. If the values are specific to a port, then the entry is listed under the port. If the logical port is going to be created after breakout (E.g Ethernet0 is modified to Ethernet0,Ethernet1,Ethernet2,Ethernet3) those corresponding ports are listed in the ports section in media_settings.json. In the current breakout scenario, when port_config.ini is modified and a config reload is done, the flow will be similar to bootup sequence and thus all the media settings will be pushed from xcvrd to portsorch on restart during config apply.


can you give detailed example for breakout scenario. have difficult to understand.

Hi @lguohan I have given an example of breakout. I have modified the design of media settings file slightly so that the users do not need to do anything additionally to handle breakout scenario

lguohan · 2019-03-28T07:31:20Z

can you format the doc? it is not well formatted.

lguohan · 2019-03-28T07:33:17Z

doc/media-settings/Media-based-Port-settings.md

+Media settings file:
+====================
+
+              Each vendor need to define the media settings file under device/<vendor-name>/ <ONIE_PLATFORM_STRING>/ <HARDWARE_SKU>/media_settings.json. This file contains the list of optics and DAC media-based settings for each port in the device. Each media setting for a media type comprises of key-value pairs per lane that is expected to be programmed when an optics or DAC is used. Examples of key value pairs include pre-emphasis, idriver and ipredriver.


since you are going to handle break out scenario, the media_settings should not be related to HARDWARE_SKU, it should only related to the true hardware platform.

Hi @lguohan I have modified the document to have the file under platform path rather than SKU path. I have also slightly changed design so as to handle breakout cases

Formatting the document

Editing file for proper indentation

Explaining the breakout scenario with examples

dgsudharsan · 2019-03-29T03:49:28Z

can you format the doc? it is not well formatted.

Hi @lguohan I had done some formatting now. Please let me know if this looks good

Rama-Nemani · 2019-04-05T12:08:02Z

The Pre-emphasis settings can be different, when the port operates in different speeds. How does the definition handle it?

dgsudharsan · 2019-04-08T00:51:40Z

The Pre-emphasis settings can be different, when the port operates in different speeds. How does the definition handle it?

Hi,
I assume pre-emphasis for different speed in-turn implies a different optic or DAC corresponding to the speed. Thus those are going to be different settings as such and would have different set of values listed. For example if a port can operate at 100G as well as 40G, there will be two sections like this. The QSFP28 implies a 100G DAC whereas QSFP implies a 40G DAC

            "QSFP28-40GBASE-CR4-1M":{
                "preemphasis": {
                    "lane0":"0x005678",
                    "lane1":"0x005678"
                },
                "idriver": {
                    "lane0":"0x1",
                    "lane1":"0x1"
                }
            }
            "QSFP-40GBASE-CR4-1M":{
                "preemphasis": {
                    "lane0":"0x001234",
                    "lane1":"0x001234"
                },
                "idriver": {
                    "lane0":"0x2",
                    "lane1":"0x2"
                }
            }

satishkumarks

For pre-emphasis, these abstractions be further be classified into specific TX_FIR setting values(like pre-tap, main, post_tap). Some vendor-types choose to have either 3-tap co-effs or more based on the different modulation type. The order of values in plain hex string would be misleading at times.

satishkumarks · 2019-04-09T10:23:03Z

For pre-emphasis, these abstractions be further be classified into specific TX_FIR setting values(like pre-tap, main, post_tap). Some vendor-types choose to have either 3-tap co-effs or more based on the different modulation type. The order of values in plain hex string would be misleading at times.

(This was supposed to be a review comment. Realised, it took as approval by mistake. Plz Ignore)

dgsudharsan · 2019-04-09T15:52:26Z

urther be classified into specific TX_FIR setting values(like pre-tap, main, post_tap). Some vendor-types choose to have either 3-t

Hi. Currently SAI doesn't have TX_FIR individual attributes. I think the Vendor SAI can handle decoding pre-emphasis values into TX_FIR setting values.

stevenlu99 · 2019-04-12T00:13:52Z

Actually there could cases that we can run different speed on same transceiver. e.g people can run 40G on a 100G optic. Seems we do not have such supported yet, but could we consider add in json schema?

dgsudharsan · 2019-04-12T02:08:04Z

Actually there could cases that we can run different speed on same transceiver. e.g people can run 40G on a 100G optic. Seems we do not have such supported yet, but could we consider add in json schema?

Hi,
Yes there will be cases where the media can be used at a lower speed but I am not sure if someone wants this use case.
One more question i have in mind is if someone uses a 100G media at 40G rate will the pre-emphasis change? From my understanding pre-emphasis or any transceiver tuning parameter does not vary with the speed at which port operates. There might be different pre-emphasis settings for a 100G media and 40G media on same port, but if the 100G media operates at 40G speed (due to port speed) I think it still should be programmed with 100G pre-emphasis settings. Please let me know your thoughts on this.

stevenlu99 · 2019-04-12T21:49:55Z

The tuning value will change if speed changed even with same transceiver module.

stevenlu99 · 2019-04-12T22:06:30Z

More comments:

How about warm boot case? in case of warm boot, everything remaining same, but we should not invoke SAI function calls to set tunning value to avoid data path impact.
There seems a bug in xcvrd function get_media_settings_key()
there are some transceiver modules could be not properly programmed and we can not assume they have keys like "manufacturenamer", "modelname", 'cable_type', etc. basically need check key exiting before referring.
The "GLOBAL" settings new support range, which is pretty good and convenient. How about make it better and support mixed ranges and ports. e.g. 1-5, 7, 9-32, so that we can handle cases that some ports might need special tuning values?

dgsudharsan · 2019-04-15T18:10:23Z

The tuning value will change if speed changed even with same transceiver module.

I assume most optics/DAC do not support dual speeds. Even if few does, I assume this might not be a normal use case where the media is used at a lower than possible speed. Please let me know your views.

stevenlu99 · 2019-04-15T18:13:54Z

dgsudharsan, that's right, we do not see many cases that same optics/DAC support dual speed. We do not necessary have it support in this change. Just want make sure we have this fact considered and we can extend our schema if this case happens later.

dgsudharsan · 2019-04-18T23:37:56Z

More comments:

How about warm boot case? in case of warm boot, everything remaining same, but we should not invoke SAI function calls to set tunning value to avoid data path impact.

There seems a bug in xcvrd function get_media_settings_key()
there are some transceiver modules could be not properly programmed and we can not assume they have keys like "manufacturenamer", "modelname", 'cable_type', etc. basically need check key exiting before referring.

The "GLOBAL" settings new support range, which is pretty good and convenient. How about make it better and support mixed ranges and ports. e.g. 1-5, 7, 9-32, so that we can handle cases that some ports might need special tuning values?

Hi @stevenlu99

Thanks for the catch. I handled warmboot case and updated xcvrd pull request
I am not directly reading from eeprom but from a transceiver_info_dict API which i assume will fill up keys with empty values if they are not read(else even without my changes xcvrd would crash) Thus i am not doing checks there.
Yes it does support mixed port ranges currently. You can give range like you specified.

Modified few places according to latest design.

Updated with latest design. Included default at global level, supporting multiple ranges at global level.

shihjeff · 2019-08-25T06:22:56Z

@dgsudharsan - Sorry to bother you. I want to add some functionality on your PR for media serdes attributes and wanted to make sure my idea is complete and don't break yours.

I want to add: SAI_PORT_ATTR_HW_PROFILE_ID to SONiC to support other vendors and not have to add new SAI attributes for the various ASIC types.

Since SONiC has quite a few repos just wanted to piggyback on yours and wanted to make sure I have all of them:

#328
https://github.com/Azure/sonic-swss/pull/821/files
sonic-net/sonic-buildimage#2713

Did these also already handle the port-breakout scenario (multiple logical ports per physical port)?

keboliu reviewed Jan 30, 2019

View reviewed changes

doc/media-settings/Media-based-Port-settings.md Outdated Show resolved Hide resolved

doc/media-settings/Media-based-Port-settings.md Outdated Show resolved Hide resolved

doc/media-settings/Media-based-Port-settings.md Outdated Show resolved Hide resolved

ben-gale reviewed Jan 31, 2019

View reviewed changes

dgsudharsan force-pushed the hld_media branch from 33cddec to 30a1500 Compare February 1, 2019 08:27

Added section on breakout scenario

71abb8e

Sudharsan D.G added 2 commits March 27, 2019 20:01

Update Media-based-Port-settings.md

64b036b

Updating the document based on latest design

Update Media-based-Port-settings.md

328f511

lguohan reviewed Mar 28, 2019

View reviewed changes

lguohan requested a review from jleveque March 28, 2019 07:30

lguohan reviewed Mar 28, 2019

View reviewed changes

Sudharsan D.G added 3 commits March 28, 2019 15:02

Update Media-based-Port-settings.md

6e63fe9

Formatting the document

Update Media-based-Port-settings.md

3397bb0

Editing file for proper indentation

Update Media-based-Port-settings.md

9c656a0

Explaining the breakout scenario with examples

satishkumarks approved these changes Apr 9, 2019

View reviewed changes

stevenlu99 approved these changes May 2, 2019

View reviewed changes

Sudharsan D.G added 2 commits May 14, 2019 18:55

Update Media-based-Port-settings.md

688514d

Modified few places according to latest design.

Update Media-based-Port-settings.md

0e2ad60

Updated with latest design. Included default at global level, supporting multiple ranges at global level.

lguohan merged commit 909a2fc into sonic-net:master Jun 7, 2019

rajendra-dendukuri pushed a commit to rajendra-dendukuri/SONiC that referenced this pull request Jul 29, 2019

Media based port settings in SONIC (sonic-net#328)

c26d833

dgsudharsan deleted the hld_media branch October 8, 2019 05:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media based port settings in SONIC #328

Media based port settings in SONIC #328

dgsudharsan commented Jan 28, 2019

ben-gale Jan 31, 2019

dgsudharsan Feb 1, 2019

ben-gale Jan 31, 2019

dgsudharsan Feb 1, 2019

ben-gale Jan 31, 2019

dgsudharsan Feb 1, 2019

MichelMoriniaux Mar 15, 2019

dgsudharsan Mar 19, 2019

ben-gale Jan 31, 2019

dgsudharsan Feb 1, 2019

lguohan commented Feb 2, 2019

dgsudharsan commented Feb 5, 2019

lguohan Mar 28, 2019 •

edited

Loading

dgsudharsan Mar 29, 2019

lguohan commented Mar 28, 2019

lguohan Mar 28, 2019

dgsudharsan Mar 29, 2019

dgsudharsan commented Mar 29, 2019

Rama-Nemani commented Apr 5, 2019

dgsudharsan commented Apr 8, 2019

satishkumarks left a comment •

edited

Loading

satishkumarks commented Apr 9, 2019

dgsudharsan commented Apr 9, 2019

stevenlu99 commented Apr 12, 2019

dgsudharsan commented Apr 12, 2019

stevenlu99 commented Apr 12, 2019

stevenlu99 commented Apr 12, 2019

dgsudharsan commented Apr 15, 2019

stevenlu99 commented Apr 15, 2019

dgsudharsan commented Apr 18, 2019

shihjeff commented Aug 25, 2019

Media based port settings in SONIC #328

Media based port settings in SONIC #328

Conversation

dgsudharsan commented Jan 28, 2019

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

lguohan commented Feb 2, 2019

dgsudharsan commented Feb 5, 2019

lguohan Mar 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Mar 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgsudharsan commented Mar 29, 2019

Rama-Nemani commented Apr 5, 2019

dgsudharsan commented Apr 8, 2019

satishkumarks left a comment • edited Loading

Choose a reason for hiding this comment

satishkumarks commented Apr 9, 2019

dgsudharsan commented Apr 9, 2019

stevenlu99 commented Apr 12, 2019

dgsudharsan commented Apr 12, 2019

stevenlu99 commented Apr 12, 2019

stevenlu99 commented Apr 12, 2019

dgsudharsan commented Apr 15, 2019

stevenlu99 commented Apr 15, 2019

dgsudharsan commented Apr 18, 2019

shihjeff commented Aug 25, 2019

lguohan Mar 28, 2019 •

edited

Loading

satishkumarks left a comment •

edited

Loading