[portsorch]: Rewrite validatePortSpeed() method. Use true lazy approa…

…ch (sonic-net#506)

* We have enough logging inside of the method. Remove the excessive one

* Rewrite portsorch::validatespeed. Use true lazy approach to minimize SAI get operations on each port

* Restore the missed attr.id

* Rename validatePortSpeed to isSpeedSupported. Change logic to return true for erross also. Even we receive an error, we could move further. It's not critical

orchagent/portsorch.cpp

@@ -757,54 +757,80 @@ bool PortsOrch::setHostIntfsStripTag(Port &port, sai_hostif_vlan_tag_t strip)
     return true;
 }
 
-bool PortsOrch::validatePortSpeed(sai_object_id_t port_id, sai_uint32_t speed)
+bool PortsOrch::isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed)
 {
+    // This method will return false iff we get a list of supported speeds and the requested speed
+    // is not supported
+    // Otherwise the method will return true (even if we received errors)
+
     sai_attribute_t attr;
     sai_status_t status;
 
     // "Lazy" query of supported speeds for given port
     // Once received the list will be stored in m_portSupportedSpeeds
     if (!m_portSupportedSpeeds.count(port_id))
     {
-        attr.id = SAI_PORT_ATTR_SUPPORTED_SPEED;
-        attr.value.u32list.count = 0;
-        attr.value.u32list.list = NULL;
+        const auto size_guess = 25; // Guess the size which could be enough
 
-        status = sai_port_api->get_port_attribute(port_id, 1, &attr);
-        if (status == SAI_STATUS_BUFFER_OVERFLOW)
-        {
-            std::vector<sai_uint32_t> speeds(attr.value.u32list.count);
+        std::vector<sai_uint32_t> speeds(size_guess);
+
+        for (int attempt = 0; attempt < 2; ++attempt) // two attempts to get our value
+        {                                             // first with the guess,
+                                                      // other with the returned value
+            attr.id = SAI_PORT_ATTR_SUPPORTED_SPEED;
+            attr.value.u32list.count = static_cast<uint32_t>(speeds.size());
             attr.value.u32list.list = speeds.data();
+
             status = sai_port_api->get_port_attribute(port_id, 1, &attr);
-            if (status == SAI_STATUS_SUCCESS)
+            if (status != SAI_STATUS_BUFFER_OVERFLOW)
             {
-                m_portSupportedSpeeds[port_id] = speeds;
-            }
-            else
-            {
-                SWSS_LOG_ERROR("Failed to get supported speed list for port %lx", port_id);
-                return false;
+                break;
             }
+
+            speeds.resize(attr.value.u32list.count); // if our guess was wrong
+                                                     // retry with the correct value
         }
-        // TODO: change to macro SAI_STATUS_IS_ATTR_NOT_SUPPORTED once it is fixed in SAI
-        // https://github.com/opencomputeproject/SAI/pull/710
-        else if ((((status) & (~0xFFFF)) == SAI_STATUS_ATTR_NOT_SUPPORTED_0) ||
-                 (((status) & (~0xFFFF)) == SAI_STATUS_ATTR_NOT_IMPLEMENTED_0) ||
-                 (status == SAI_STATUS_NOT_IMPLEMENTED))
+
+        if (status == SAI_STATUS_SUCCESS)
         {
-            // unable to validate speed if attribute is not supported on platform
-            // assuming input value is correct
-            SWSS_LOG_WARN("Unable to validate speed for port %lx. Not supported by platform", port_id);
-            return true;
+                speeds.resize(attr.value.u32list.count);
+                m_portSupportedSpeeds[port_id] = speeds;
         }
         else
         {
-            SWSS_LOG_ERROR("Failed to get number of supported speeds for port %lx", port_id);
-            return false;
+            if (status == SAI_STATUS_BUFFER_OVERFLOW)
+            {
+                // something went wrong in SAI implementation
+                SWSS_LOG_ERROR("Failed to get supported speed list for port %s id=%lx. Not enough container size",
+                               alias.c_str(), port_id);
+            }
+            else if (SAI_STATUS_IS_ATTR_NOT_SUPPORTED(status) ||
+                     SAI_STATUS_IS_ATTR_NOT_IMPLEMENTED(status) ||
+                     status == SAI_STATUS_NOT_IMPLEMENTED)
+            {
+                // unable to validate speed if attribute is not supported on platform
+                // assuming input value is correct
+                SWSS_LOG_WARN("Unable to validate speed for port %s id=%lx. Not supported by platform",
+                              alias.c_str(), port_id);
+            }
+            else
+            {
+                SWSS_LOG_ERROR("Failed to get a list of supported speeds for port %s id=%lx. Error=%d",
+                               alias.c_str(), port_id, status);
+            }
+            m_portSupportedSpeeds[port_id] = {}; // use an empty list,
+                                                 // we don't want to get the port speed for this port again
+            return true; // we can't check if the speed is valid, so return true to change the speed
         }
+
     }
 
-    PortSupportedSpeeds &supp_speeds = m_portSupportedSpeeds[port_id];
+    const PortSupportedSpeeds &supp_speeds = m_portSupportedSpeeds[port_id];
+    if (supp_speeds.size() == 0)
+    {
+        // we don't have the list for this port, so return true to change speed anyway
+        return true;
+    }
 
     return std::find(supp_speeds.begin(), supp_speeds.end(), speed) != supp_speeds.end();
 }
@@ -1236,9 +1262,8 @@ void PortsOrch::doPortTask(Consumer &consumer)
                 {
                     sai_uint32_t current_speed;
 
-                    if (!validatePortSpeed(p.m_port_id, speed))
+                    if (!isSpeedSupported(alias, p.m_port_id, speed))
                     {
-                        SWSS_LOG_ERROR("Failed to set speed %u for port %s. The value is not supported", speed, alias.c_str());
                         it++;
                         continue;
                     }

orchagent/portsorch.h

@@ -134,7 +134,7 @@ class PortsOrch : public Orch, public Subject
 
     bool setBridgePortAdminStatus(sai_object_id_t id, bool up);
 
-    bool validatePortSpeed(sai_object_id_t port_id, sai_uint32_t speed);
+    bool isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed);
     bool setPortSpeed(sai_object_id_t port_id, sai_uint32_t speed);
     bool getPortSpeed(sai_object_id_t port_id, sai_uint32_t &speed);