From 872f7bf9f3cf14112efc58005c9f5acfc520e894 Mon Sep 17 00:00:00 2001
From: Vaibhav Hemant Dixit <vaibhav.dixit@microsoft.com>
Date: Wed, 7 Dec 2022 14:09:14 -0800
Subject: [PATCH] [portinit] Do not call GET on SAI_PORT_ATTR_SPEED when
 AUTONEG is enabled (#2484)

What I did
During port init if AutoNeg is enabled do not do GET oper on SAI_PORT_ATTR_SPEED (do not call getPortSpeed)

Why I did it
This PR fixes an issue where in some platforms syncd crashes when AutoNeg is enabled and port is oper down.
The crash happens in the warmboot recovery path:

In the target image as part of portinit, GET operation on SAI_PORT_ATTR_SPEED returns a random value.
This random value does not match the speed in the base image.
Diff in speed causes syncd comparison logic to attempt to set newly detected speed.
Comparison logic crashes in APPLY_VIEW when doing a SET speed operation on port with the new speed.
Why SAI returns random value in GET oper on SAI_PORT_ATTR_SPEED:
SAI returns random value as when autoneg is enabled attribute SAI_PORT_ATTR_SPEED is not set in the first place.
When autoneg is enabled a list of speeds is set using attribute SAI_PORT_ATTR_ADVERTISED_SPEED (instead of SAI_PORT_ATTR_SPEED when autoneg is not enabled.

In autoneg enabled case, get oper on SAI_PORT_ATTR_SPEED is not allowed as the speed on the port is not certain, and instead depends on advertised/negotiated speed.
---
 orchagent/p4orch/tests/fake_portorch.cpp |  5 +++++
 orchagent/portsorch.cpp                  | 19 ++++++++++++++++++-
 orchagent/portsorch.h                    |  1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp
index f9098f663918..d2b7651a882c 100644
--- a/orchagent/p4orch/tests/fake_portorch.cpp
+++ b/orchagent/p4orch/tests/fake_portorch.cpp
@@ -606,6 +606,11 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
     return true;
 }
 
+bool PortsOrch::isAutoNegEnabled(sai_object_id_t id)
+{
+    return true;
+}
+
 task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
 {
     return task_success;
diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
index 7748ab4c8240..eaeba5ae82ad 100755
--- a/orchagent/portsorch.cpp
+++ b/orchagent/portsorch.cpp
@@ -2484,6 +2484,23 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
     return true;
 }
 
+bool PortsOrch::isAutoNegEnabled(sai_object_id_t id)
+{
+    SWSS_LOG_ENTER();
+
+    sai_attribute_t attr;
+    attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE;
+
+    sai_status_t status = sai_port_api->get_port_attribute(id, 1, &attr);
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_ERROR("Failed to get port AutoNeg status for port pid:%" PRIx64, id);
+        return false;
+    }
+
+    return attr.value.booldata;
+}
+
 task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
 {
     SWSS_LOG_ENTER();
@@ -4758,7 +4775,7 @@ bool PortsOrch::initializePort(Port &port)
     }
 
     /* initialize port admin speed */
-    if (!getPortSpeed(port.m_port_id, port.m_speed))
+    if (!isAutoNegEnabled(port.m_port_id) && !getPortSpeed(port.m_port_id, port.m_speed))
     {
         SWSS_LOG_ERROR("Failed to get initial port admin speed %d", port.m_speed);
         return false;
diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h
index 446d6bc8de45..cec49a7bda51 100644
--- a/orchagent/portsorch.h
+++ b/orchagent/portsorch.h
@@ -363,6 +363,7 @@ class PortsOrch : public Orch, public Subject
     bool m_isPortCounterMapGenerated = false;
     bool m_isPortBufferDropCounterMapGenerated = false;
 
+    bool isAutoNegEnabled(sai_object_id_t id);
     task_process_status setPortAutoNeg(sai_object_id_t id, int an);
     bool setPortFecMode(sai_object_id_t id, int fec);
     task_process_status setPortInterfaceType(sai_object_id_t id, sai_port_interface_type_t interface_type);