Dynamic transceiver tuning support #821

dgsudharsan · 2019-03-28T07:30:06Z

- What I did
Dynamic transceiver tuning support
- How I did it
Dynamically program pre-emphasis and other settings based on media detected in xcvrd. The various supported media are defined in media_settings.json

- How to verify it
Perform OIR of different media types. Dump the pre-emphasis values from hardware and verify that they got reflected

- Description for the changelog

Support for dynamically programming pre-emphasis

DO NOT MERGE THIS PULL REQUEST UNTIL BCM SDK SUPPORT FOR SAI PREEMPHASIS ATTRIBUTES HAVE BEEN MERGED

- A picture of a cute animal (not mandatory but encouraged)

wschwartz · 2019-03-28T18:43:50Z

orchagent/portsorch.cpp

+            {
+                attr_id = SAI_PORT_ATTR_SERDES_IPREDRIVER;
+            } 
+            else if(fvField(i) == "port") 


Hi Sudharsan,

I am reviewing the SAI piece of this as well at Broadcom and one thing I am wondering is how you handle interface mode. Along with pre-emphasis settings, for DACs we will want to set either CR/XFI where for copper we would want to program SR/SFI. The transceiver compliance field is all we would need but that would need to get passed down to SAI to set the interface mode properly.

Hi @wschwartz Currently I don't think SONiC has a mechanism to program the interface mode. This pull request is just to handle pre-emphasis related settings alone. The interface mode can later be extended in this logic or can be incorporated in some other different logic. For now without setting interface mode, the pre-emphasis settings alone work good in SONiC and if there is a future requirement we can create a new PR for the related settings.

yes, it can. check this attribute. SAI_PORT_ATTR_MEDIA_TYPE

@wschwatz, do we think we can fix this media type in another PR, or it has to be this PR?

Hi @lguohan @dgsudharsan I think this could be extended in the future to support interface mode in a future PR. With TH3 coming we have a new set of interface modes to support so its probably good to take time and investigate those. I think @ben-gale is working to find another reviewer as well.

lguohan · 2019-04-24T08:05:28Z

orchagent/portsorch.cpp

@@ -3166,6 +3171,46 @@ void PortsOrch::doTask(NotificationConsumer &consumer)
        }

        sai_deserialize_free_port_oper_status_ntf(count, portoperstatus);
+    } else if (op == "media_change") {


media settings should go to standard port app db, like the admin_status in the port. I do not understand why choose the notification consumer here.

Please make sure your code style is consistent, you need to keep a space for after if.

Hi Guohan,
I feel that media event is analogous to oper state notification generated by ASIC. These are from South bound APIs. If we see the oper status implementation, a notification consumer is used which is from syncd to orchagent. I used the same logic to implement this as similar to oper status. On other hand I feel admin status is application triggerd change from user.
Please let me know your views. If i still want to go ahead and implement it similar to admin status I will go ahead and update the PR.

lguohan

use appdb instead of notification consumer.

lguohan · 2019-04-24T16:22:38Z

also since you add new function, you also need to add vs test on this.

dgsudharsan · 2019-05-01T02:09:12Z

Hi @lguohan. I have added vs tests for the attributes I introduced. Below is the output of vs test
sudo pytest -s -v --dvsname=vs test_port.py
========================================================= test session starts =========================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.3.0, py-1.8.0, pluggy-0.6.0 -- /usr/bin/python
cachedir: .cache
rootdir: /home/admin/code/sonic-swss/tests, inifile:
collected 6 items

test_port.py::TestPort::test_PortMtu PASSED [ 16%]
test_port.py::test_PortNotification PASSED [ 33%]
test_port.py::test_PortFec PASSED [ 50%]
test_port.py::test_PortPreemp PASSED [ 66%]
test_port.py::test_PortIdriver PASSED [ 83%]
test_port.py::test_PortIpredriver PASSED [100%]

====================================================== 6 passed in 43.30 seconds ======================================================

stevenlu99

Look OK for me

stevenlu99

We are calling SAI API to configure serdes tuning values thru orchagent, why not thru syncd?
How we determine thru which entity to access asic? Do we have a general guidance?

dgsudharsan · 2019-05-02T22:48:08Z

We are calling SAI API to configure serdes tuning values thru orchagent, why not thru syncd?
How we determine thru which entity to access asic? Do we have a general guidance?

Hi @stevenlu99 , The API called in orchagent actually calls the libsairedis API. This will write into ASIC_DB and which in turn messages the syncd. Syncd then deserializes the parameters and invokes the vendor specific SAI API that programs the NPU.
This is the general design followed in all the SAI calls and i followed the same here too.

stevenlu99

Looks good

lguohan · 2019-05-10T05:25:25Z

orchagent/portsorch.h

@@ -189,6 +189,12 @@ class PortsOrch : public Orch, public Subject

    bool getPortOperStatus(const Port& port, sai_port_oper_status_t& status) const;
    void updatePortOperStatus(Port &port, sai_port_oper_status_t status);
+
+    void sai_get_port_serdes_values (const std::string& s,


( [](start = 35, length = 2)

no space here.

lguohan · 2019-05-10T05:28:46Z

orchagent/portsorch.cpp

+                /* Set port serdes Pre-emphasis */
+                if (fvField(i) == "preemphasis")
+                {
+                    sai_get_port_serdes_values(fvValue(i), pre_emphasis);


sai_get_port_serdes_values [](start = 20, length = 26)

app db value do not use json format. Please propose a text format and use that in the app db.

for example:

value,value

please add the proposed scheme in here.

https://github.com/Azure/sonic-swss/blob/master/doc/swss-schema.md

In reply to: 282750165 [](ancestors = 282750165)

lguohan · 2019-05-10T05:29:15Z

orchagent/portsorch.cpp

@@ -22,6 +22,9 @@
 #include "crmorch.h"
 #include "countercheckorch.h"
 #include "notifier.h"
+#include "swss/json.hpp"
+
+using json = nlohmann::json;


do not use json for app db attribute value.

lguohan · 2019-05-10T05:31:01Z

orchagent/portsorch.cpp

@@ -1855,6 +1879,52 @@ void PortsOrch::doPortTask(Consumer &consumer)
                    }
                }

+                if(pre_emphasis.size() != 0)


if(p [](start = 16, length = 4)

leave space here.

orchagent/portsorch.cpp

dgsudharsan · 2019-05-14T19:31:31Z

Ran the test after the changes.
sudo pytest -s -v --dvsname=vs test_port.py
===================================================== test session starts =====================================================
platform linux2 -- Python 2.7.15rc1, pytest-3.3.0, py-1.8.0, pluggy-0.6.0 -- /usr/bin/python
cachedir: .cache
rootdir: /home/ubuntu/preemp/sonic-swss/tests, inifile:
collected 6 items

test_port.py::TestPort::test_PortMtu PASSED [ 16%]
test_port.py::test_PortNotification PASSED [ 33%]
test_port.py::test_PortFec PASSED [ 50%]
test_port.py::test_PortPreemp PASSED [ 66%]
test_port.py::test_PortIdriver PASSED [ 83%]
test_port.py::test_PortIpredriver PASSED [100%]

================================================== 6 passed in 40.25 seconds ==================================================

fixing compilation Removing debug logs Reverting unwanted change Addressing review comments Clean up Adding vs Test cases Addressing code review comments Addressing review comment Modifying schema

lguohan · 2019-06-01T06:30:33Z

retest this please

lguohan · 2019-06-03T23:41:37Z

retest this please

lguohan · 2019-06-04T12:28:18Z

retest this please

…c-net#821) Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com>

wschwartz reviewed Mar 28, 2019

View reviewed changes

lguohan reviewed Apr 24, 2019

View reviewed changes

lguohan requested changes Apr 24, 2019

View reviewed changes

dgsudharsan force-pushed the dgpreemp branch 2 times, most recently from 4c5e465 to 1fc2011 Compare April 27, 2019 03:50

stevenlu99 approved these changes May 2, 2019

View reviewed changes

stevenlu99 reviewed May 2, 2019

View reviewed changes

stevenlu99 approved these changes May 3, 2019

View reviewed changes

dgsudharsan force-pushed the dgpreemp branch from 652a50b to 95a42d4 Compare May 7, 2019 18:48

lguohan reviewed May 10, 2019

View reviewed changes

orchagent/portsorch.cpp Outdated Show resolved Hide resolved

dgsudharsan force-pushed the dgpreemp branch from 95a42d4 to 0f33b65 Compare May 14, 2019 19:09

preemp orch changes

35659d5

fixing compilation Removing debug logs Reverting unwanted change Addressing review comments Clean up Adding vs Test cases Addressing code review comments Addressing review comment Modifying schema

dgsudharsan force-pushed the dgpreemp branch from 0f33b65 to 35659d5 Compare May 15, 2019 00:12

lguohan approved these changes May 29, 2019

View reviewed changes

lguohan merged commit d616764 into sonic-net:master Jun 4, 2019

stcheng added the Enhancement ➕ label Jun 4, 2019

dgsudharsan deleted the dgpreemp branch October 8, 2019 05:38

EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022

[fwutil]: Set default socket timeout for FW download to 30 sec. (soni…

c5c5ffc

…c-net#821) Signed-off-by: Nazarii Hnydyn <nazariig@mellanox.com>

oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023

[syncd] Bring back TimerWatchdog (sonic-net#821)

bb341e9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic transceiver tuning support #821

Dynamic transceiver tuning support #821

dgsudharsan commented Mar 28, 2019

wschwartz Mar 28, 2019

dgsudharsan Mar 29, 2019

lguohan Apr 11, 2019

wschwartz Apr 14, 2019

lguohan Apr 24, 2019

dgsudharsan Apr 24, 2019

lguohan left a comment

lguohan commented Apr 24, 2019

dgsudharsan commented May 1, 2019

stevenlu99 left a comment

stevenlu99 left a comment

dgsudharsan commented May 2, 2019

stevenlu99 left a comment

lguohan May 10, 2019

lguohan May 10, 2019

lguohan May 10, 2019

lguohan May 10, 2019

lguohan May 10, 2019

dgsudharsan commented May 14, 2019

lguohan commented Jun 1, 2019

lguohan commented Jun 3, 2019

lguohan commented Jun 4, 2019

Dynamic transceiver tuning support #821

Dynamic transceiver tuning support #821

Conversation

dgsudharsan commented Mar 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

lguohan left a comment

Choose a reason for hiding this comment

lguohan commented Apr 24, 2019

dgsudharsan commented May 1, 2019

stevenlu99 left a comment

Choose a reason for hiding this comment

stevenlu99 left a comment

Choose a reason for hiding this comment

dgsudharsan commented May 2, 2019

stevenlu99 left a 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

dgsudharsan commented May 14, 2019

lguohan commented Jun 1, 2019

lguohan commented Jun 3, 2019

lguohan commented Jun 4, 2019