From 8f6c2f2b2beb8bd7e9bff82e26edaca536153c69 Mon Sep 17 00:00:00 2001 From: Maksym Belei Date: Tue, 2 Feb 2021 10:40:49 +0200 Subject: [PATCH 1/4] [orchagent] Init field of supported speeds while port initialization * Reading of supported speed list from the ASIC and saving it inside PORT_TABLE:EthernetXX table in APPL_DB to be able to check supported speeds while changing a current speed through CLI commands. Signed-off-by: Maksym Belei --- orchagent/portsorch.cpp | 110 ++++++++++++++++++++++++++++++++++++---- orchagent/portsorch.h | 1 + 2 files changed, 101 insertions(+), 10 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 44919bfc43..0e24aa973f 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1783,21 +1783,73 @@ bool PortsOrch::setHostIntfsStripTag(Port &port, sai_hostif_vlan_tag_t strip) return true; } -bool PortsOrch::isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed) +const PortSupportedSpeeds& PortsOrch::getSupportedSpeed(const std::string& alias, sai_object_id_t port_id) { - // 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) - initPortSupportedSpeeds(alias, port_id); + // This method will return vector of supported speeds for the port + // The method will return empty vector if there was something wrong during method execution. - const auto &supp_speeds = m_portSupportedSpeeds[port_id]; - if (supp_speeds.empty()) + 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)) { - // we don't have the list for this port, so return true to change speed anyway - return true; + const auto size_guess = 25; // Guess the size which could be enough + + std::vector 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(speeds.size()); + attr.value.u32list.list = speeds.data(); + + status = sai_port_api->get_port_attribute(port_id, 1, &attr); + if (status != SAI_STATUS_BUFFER_OVERFLOW) + { + break; + } + + speeds.resize(attr.value.u32list.count); // if our guess was wrong + // retry with the correct value + } + + if (status == SAI_STATUS_SUCCESS) + { + speeds.resize(attr.value.u32list.count); + m_portSupportedSpeeds[port_id] = speeds; + } + else + { + 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=%" PRIx64 ". 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=%" PRIx64 ". 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=%" PRIx64 ". 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 std::find(supp_speeds.begin(), supp_speeds.end(), speed) != supp_speeds.end(); + return m_portSupportedSpeeds[port_id]; } void PortsOrch::getPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id, PortSupportedSpeeds &supported_speeds) @@ -1873,6 +1925,22 @@ void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_ m_portStateTable.set(alias, v); } +bool PortsOrch::isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed) +{ + // This method will return false if we get a list of supported speeds and the requested speed + // is not supported + // Otherwise the method will return true (even if did not receive list of supported speed) + + const PortSupportedSpeeds &supp_speeds = getSupportedSpeed(alias, 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(); +} + /* * If Gearbox is enabled and this is a Gearbox port then set the attributes accordingly. */ @@ -4167,6 +4235,28 @@ bool PortsOrch::initializePort(Port &port) return false; } + /* + * initialize field with supported speeds of the port. + */ + auto supp_speed_temp = getSupportedSpeed(port.m_alias, port.m_port_id); + /* Copy the vector to be able to modify it */ + auto supp_speed(supp_speed_temp); + if (supp_speed.size() > 0) + { + sort(supp_speed.begin(), supp_speed.end()); + + string tmp_supp_speed_str = ""; + /* Add (count - 1) elements to the string with separator*/ + for (auto iter = supp_speed.begin(); iter < prev(supp_speed.end()); iter++) + { + tmp_supp_speed_str += to_string(*iter) + ","; + } + /* Add the last element w/o separator*/ + tmp_supp_speed_str += to_string(supp_speed.back()); + + m_portTable->hset(port.m_alias, "supp_speed", tmp_supp_speed_str); + } + return true; } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 851200b47c..b802cb805a 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -289,6 +289,7 @@ class PortsOrch : public Orch, public Subject bool setBridgePortAdminStatus(sai_object_id_t id, bool up); + const PortSupportedSpeeds& getSupportedSpeed(const std::string& alias, sai_object_id_t port_id); bool isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed); void getPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id, PortSupportedSpeeds &supported_speeds); void initPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id); From ad7de0ae88273a2f24da740c724286ee6702fa9a Mon Sep 17 00:00:00 2001 From: KonstiantynHalushka Date: Tue, 16 Nov 2021 17:41:01 +0200 Subject: [PATCH 2/4] [orchagent/portsorch] Refactoring, remove duplicate code --- orchagent/portsorch.cpp | 87 +++++++++-------------------------------- orchagent/portsorch.h | 4 +- 2 files changed, 20 insertions(+), 71 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 0e24aa973f..42cb9f4a0b 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1783,75 +1783,6 @@ bool PortsOrch::setHostIntfsStripTag(Port &port, sai_hostif_vlan_tag_t strip) return true; } -const PortSupportedSpeeds& PortsOrch::getSupportedSpeed(const std::string& alias, sai_object_id_t port_id) -{ - // This method will return vector of supported speeds for the port - // The method will return empty vector if there was something wrong during method execution. - - 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)) - { - const auto size_guess = 25; // Guess the size which could be enough - - std::vector 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(speeds.size()); - attr.value.u32list.list = speeds.data(); - - status = sai_port_api->get_port_attribute(port_id, 1, &attr); - if (status != SAI_STATUS_BUFFER_OVERFLOW) - { - break; - } - - speeds.resize(attr.value.u32list.count); // if our guess was wrong - // retry with the correct value - } - - if (status == SAI_STATUS_SUCCESS) - { - speeds.resize(attr.value.u32list.count); - m_portSupportedSpeeds[port_id] = speeds; - } - else - { - 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=%" PRIx64 ". 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=%" PRIx64 ". 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=%" PRIx64 ". 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 m_portSupportedSpeeds[port_id]; -} - void PortsOrch::getPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id, PortSupportedSpeeds &supported_speeds) { sai_attribute_t attr; @@ -1909,6 +1840,24 @@ void PortsOrch::getPortSupportedSpeeds(const std::string& alias, sai_object_id_t } } +const PortSupportedSpeeds& PortsOrch::getSupportedSpeed(const std::string& alias, sai_object_id_t port_id) +{ + // This method will return vector of supported speeds for the port + // The method will return empty vector if there was something wrong during method execution. + + // "Lazy" query of supported speeds for given port + // Once received the list will be stored in m_portSupportedSpeeds + + if (!m_portSupportedSpeeds.count(port_id)) + { + PortSupportedSpeeds supported_speeds; + getPortSupportedSpeeds(alias, port_id, &supported_speeds); + m_portSupportedSpeeds[port_id] = supported_speeds; + } + + return m_portSupportedSpeeds[port_id]; +} + void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id) { // If port supported speeds map already contains the information, save the SAI call diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index b802cb805a..dd009459e5 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -289,10 +289,10 @@ class PortsOrch : public Orch, public Subject bool setBridgePortAdminStatus(sai_object_id_t id, bool up); - const PortSupportedSpeeds& getSupportedSpeed(const std::string& alias, sai_object_id_t port_id); - bool isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed); void getPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id, PortSupportedSpeeds &supported_speeds); + const PortSupportedSpeeds& getSupportedSpeed(const std::string& alias, sai_object_id_t port_id); void initPortSupportedSpeeds(const std::string& alias, sai_object_id_t port_id); + bool isSpeedSupported(const std::string& alias, sai_object_id_t port_id, sai_uint32_t speed); task_process_status setPortSpeed(Port &port, sai_uint32_t speed); bool getPortSpeed(sai_object_id_t id, sai_uint32_t &speed); bool setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value); From addfe2ca89bcaf2ad7a35325ab9373ae4da5febd Mon Sep 17 00:00:00 2001 From: KonstiantynHalushka Date: Wed, 17 Nov 2021 10:32:51 +0200 Subject: [PATCH 3/4] [orchagent/portsorch] Fixed transfer by reference --- orchagent/portsorch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 42cb9f4a0b..90eb611e21 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1851,7 +1851,7 @@ const PortSupportedSpeeds& PortsOrch::getSupportedSpeed(const std::string& alias if (!m_portSupportedSpeeds.count(port_id)) { PortSupportedSpeeds supported_speeds; - getPortSupportedSpeeds(alias, port_id, &supported_speeds); + getPortSupportedSpeeds(alias, port_id, supported_speeds); m_portSupportedSpeeds[port_id] = supported_speeds; } From 144b6c1b873b363784513119ac8058dac9d50438 Mon Sep 17 00:00:00 2001 From: KonstiantynHalushka Date: Wed, 17 Nov 2021 18:39:19 +0200 Subject: [PATCH 4/4] [orchagent/portsorch] Return of a call to the initPortSupportedSpeeds function, since when calling isSpeedSupported, you need to initialize m_portStateTable if it has not already been done Signed-off-by: KonstiantynHalushka --- orchagent/portsorch.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 90eb611e21..acba85d16e 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1879,6 +1879,7 @@ bool PortsOrch::isSpeedSupported(const std::string& alias, sai_object_id_t port_ // This method will return false if we get a list of supported speeds and the requested speed // is not supported // Otherwise the method will return true (even if did not receive list of supported speed) + initPortSupportedSpeeds(alias, port_id); const PortSupportedSpeeds &supp_speeds = getSupportedSpeed(alias, port_id); if (supp_speeds.size() == 0)