From 1e1438e46ea8e85cffce283935c97c45bfde8dc2 Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Tue, 9 Aug 2022 00:57:04 +0800 Subject: [PATCH] [portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it (#2400) * Revert "Revert "[portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it (#2333)" (#2396)" This reverts commit 6565b502f01e739b4961f216084ffedf78395640. * Adjust the prototype of setPortFec Signed-off-by: Stephen Sun --- orchagent/p4orch/tests/fake_portorch.cpp | 2 +- orchagent/portsorch.cpp | 120 ++++++++++-- orchagent/portsorch.h | 8 +- tests/mock_tests/portsorch_ut.cpp | 227 +++++++++++++++++++++++ 4 files changed, 344 insertions(+), 13 deletions(-) diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp index 1cd3e20ae3..0762dd0357 100644 --- a/orchagent/p4orch/tests/fake_portorch.cpp +++ b/orchagent/p4orch/tests/fake_portorch.cpp @@ -518,7 +518,7 @@ bool PortsOrch::getPortPvid(Port &port, sai_uint32_t &pvid) return true; } -bool PortsOrch::setPortFec(Port &port, sai_port_fec_mode_t mode) +bool PortsOrch::setPortFec(Port &port, std::string &mode) { return true; } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 3a0b4e75f0..26219d4d35 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -79,6 +79,13 @@ static map fec_mode_map = { "fc", SAI_PORT_FEC_MODE_FC } }; +static map fec_mode_reverse_map = +{ + { SAI_PORT_FEC_MODE_NONE, "none" }, + { SAI_PORT_FEC_MODE_RS, "rs" }, + { SAI_PORT_FEC_MODE_FC, "fc" } +}; + static map pfc_asym_map = { { "on", SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_SEPARATE }, @@ -1214,19 +1221,31 @@ bool PortsOrch::setPortTpid(sai_object_id_t id, sai_uint16_t tpid) return true; } - -bool PortsOrch::setPortFec(Port &port, sai_port_fec_mode_t mode) +bool PortsOrch::setPortFec(Port &port, string &mode) { SWSS_LOG_ENTER(); + auto searchRef = m_portSupportedFecModes.find(port.m_port_id); + if (searchRef != m_portSupportedFecModes.end()) + { + auto &supportedFecModes = searchRef->second; + if (!supportedFecModes.empty() && (supportedFecModes.find(mode) == supportedFecModes.end())) + { + SWSS_LOG_ERROR("Unsupported mode %s on port %s", mode.c_str(), port.m_alias.c_str()); + // We return true becase the caller will keep the item in m_toSync and retry it later if we return false + // As the FEC mode is not supported it doesn't make sense to retry. + return true; + } + } + sai_attribute_t attr; attr.id = SAI_PORT_ATTR_FEC_MODE; - attr.value.s32 = mode; + attr.value.s32 = port.m_fec_mode; sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr); if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to set fec mode %d to port pid:%" PRIx64, mode, port.m_port_id); + SWSS_LOG_ERROR("Failed to set FEC mode %s to port %s", mode.c_str(), port.m_alias.c_str()); task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status); if (handle_status != task_success) { @@ -1234,7 +1253,7 @@ bool PortsOrch::setPortFec(Port &port, sai_port_fec_mode_t mode) } } - SWSS_LOG_INFO("Set fec mode %d to port pid:%" PRIx64, mode, port.m_port_id); + SWSS_LOG_NOTICE("Set port %s FEC mode %s", port.m_alias.c_str(), mode.c_str()); setGearboxPortsAttr(port, SAI_PORT_ATTR_FEC_MODE, &mode); @@ -2011,6 +2030,7 @@ void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_ m_portStateTable.set(alias, v); } + void PortsOrch::initPortCapAutoNeg(Port &port) { sai_status_t status; @@ -2040,6 +2060,87 @@ void PortsOrch::initPortCapLinkTraining(Port &port) SWSS_LOG_WARN("Unable to get %s LT support capability", port.m_alias.c_str()); } +void PortsOrch::getPortSupportedFecModes(const std::string& alias, sai_object_id_t port_id, PortSupportedFecModes &supported_fecmodes) +{ + sai_attribute_t attr; + sai_status_t status; + vector fecModes(fec_mode_reverse_map.size()); + + attr.id = SAI_PORT_ATTR_SUPPORTED_FEC_MODE; + attr.value.s32list.count = static_cast(fecModes.size()); + attr.value.s32list.list = fecModes.data(); + + status = sai_port_api->get_port_attribute(port_id, 1, &attr); + fecModes.resize(attr.value.s32list.count); + if (status == SAI_STATUS_SUCCESS) + { + if (fecModes.empty()) + { + supported_fecmodes.insert("N/A"); + } + else + { + for(auto fecMode : fecModes) + { + supported_fecmodes.insert(fec_mode_reverse_map[static_cast(fecMode)]); + } + } + } + else + { + if (SAI_STATUS_IS_ATTR_NOT_SUPPORTED(status) || + SAI_STATUS_IS_ATTR_NOT_IMPLEMENTED(status) || + status == SAI_STATUS_NOT_IMPLEMENTED) + { + // unable to validate FEC mode if attribute is not supported on platform + SWSS_LOG_NOTICE("Unable to validate FEC mode for port %s id=%" PRIx64 " due to unsupported by platform", + alias.c_str(), port_id); + } + else + { + SWSS_LOG_ERROR("Failed to get a list of supported FEC modes for port %s id=%" PRIx64 ". Error=%d", + alias.c_str(), port_id, status); + } + + supported_fecmodes.clear(); // return empty + } +} + +void PortsOrch::initPortSupportedFecModes(const std::string& alias, sai_object_id_t port_id) +{ + // If port supported speeds map already contains the information, save the SAI call + if (m_portSupportedFecModes.count(port_id)) + { + return; + } + PortSupportedFecModes supported_fec_modes; + getPortSupportedFecModes(alias, port_id, supported_fec_modes); + m_portSupportedFecModes[port_id] = supported_fec_modes; + + if (supported_fec_modes.empty()) + { + // Do not expose "supported_fecs" in case fetching FEC modes is not supported by the vendor + SWSS_LOG_INFO("No supported_fecs exposed to STATE_DB for port %s since fetching supported FEC modes is not supported by the vendor", + alias.c_str()); + return; + } + + vector v; + std::string supported_fec_modes_str; + bool first = true; + for(auto fec : supported_fec_modes) + { + if (first) + first = false; + else + supported_fec_modes_str += ','; + supported_fec_modes_str += fec; + } + + v.emplace_back(std::make_pair("supported_fecs", supported_fec_modes_str)); + m_portStateTable.set(alias, v); +} + /* * If Gearbox is enabled and this is a Gearbox port then set the attributes accordingly. */ @@ -3105,6 +3206,7 @@ void PortsOrch::doPortTask(Consumer &consumer) } initPortSupportedSpeeds(get<0>(it->second), m_portListLaneMap[it->first]); + initPortSupportedFecModes(get<0>(it->second), m_portListLaneMap[it->first]); it++; } @@ -3520,14 +3622,12 @@ void PortsOrch::doPortTask(Consumer &consumer) p.m_fec_mode = fec_mode_map[fec_mode]; p.m_fec_cfg = true; - if (setPortFec(p, p.m_fec_mode)) + if (setPortFec(p, fec_mode)) { m_portList[alias] = p; - SWSS_LOG_NOTICE("Set port %s fec to %s", alias.c_str(), fec_mode.c_str()); } else { - SWSS_LOG_ERROR("Failed to set port %s fec to %s", alias.c_str(), fec_mode.c_str()); it++; continue; } @@ -3537,14 +3637,12 @@ void PortsOrch::doPortTask(Consumer &consumer) /* Port is already down, setting fec mode*/ p.m_fec_mode = fec_mode_map[fec_mode]; p.m_fec_cfg = true; - if (setPortFec(p, p.m_fec_mode)) + if (setPortFec(p, fec_mode)) { m_portList[alias] = p; - SWSS_LOG_NOTICE("Set port %s fec to %s", alias.c_str(), fec_mode.c_str()); } else { - SWSS_LOG_ERROR("Failed to set port %s fec to %s", alias.c_str(), fec_mode.c_str()); it++; continue; } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 42e17d7fb9..28e576e906 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -27,6 +27,7 @@ #define PG_DROP_STAT_COUNTER_FLEX_COUNTER_GROUP "PG_DROP_STAT_COUNTER" typedef std::vector PortSupportedSpeeds; +typedef std::set PortSupportedFecModes; static const map oper_status_strings = { @@ -209,6 +210,8 @@ class PortsOrch : public Orch, public Subject unique_ptr m_gbcounterTable; std::map m_portSupportedSpeeds; + // Supported FEC modes on the system side. + std::map m_portSupportedFecModes; bool m_initDone = false; Port m_cpuPort; @@ -311,7 +314,7 @@ class PortsOrch : public Orch, public Subject bool setPortTpid(sai_object_id_t id, sai_uint16_t tpid); bool setPortPvid (Port &port, sai_uint32_t pvid); bool getPortPvid(Port &port, sai_uint32_t &pvid); - bool setPortFec(Port &port, sai_port_fec_mode_t mode); + bool setPortFec(Port &port, std::string &mode); bool setPortPfcAsym(Port &port, string pfc_asym); bool getDestPortId(sai_object_id_t src_port_id, dest_port_type_t port_type, sai_object_id_t &des_port_id); @@ -320,6 +323,9 @@ class PortsOrch : public Orch, public Subject 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); + // Get supported FEC modes on system side + void getPortSupportedFecModes(const std::string& alias, sai_object_id_t port_id, PortSupportedFecModes &supported_fecmodes); + void initPortSupportedFecModes(const std::string& alias, sai_object_id_t port_id); 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); diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index 78c633d4a1..93e73d9041 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -20,6 +20,70 @@ namespace portsorch_test using namespace std; + sai_port_api_t ut_sai_port_api; + sai_port_api_t *pold_sai_port_api; + + bool not_support_fetching_fec; + vector mock_port_fec_modes = {SAI_PORT_FEC_MODE_RS, SAI_PORT_FEC_MODE_FC}; + + sai_status_t _ut_stub_sai_get_port_attribute( + _In_ sai_object_id_t port_id, + _In_ uint32_t attr_count, + _Inout_ sai_attribute_t *attr_list) + { + sai_status_t status; + if (attr_count == 1 && attr_list[0].id == SAI_PORT_ATTR_SUPPORTED_FEC_MODE) + { + if (not_support_fetching_fec) + { + status = SAI_STATUS_NOT_IMPLEMENTED; + } + else + { + uint32_t i; + for (i = 0; i < attr_list[0].value.s32list.count && i < mock_port_fec_modes.size(); i++) + { + attr_list[0].value.s32list.list[i] = mock_port_fec_modes[i]; + } + attr_list[0].value.s32list.count = i; + status = SAI_STATUS_SUCCESS; + } + } + else + { + status = pold_sai_port_api->get_port_attribute(port_id, attr_count, attr_list); + } + return status; + } + + uint32_t _sai_set_port_fec_count; + int32_t _sai_port_fec_mode; + sai_status_t _ut_stub_sai_set_port_attribute( + _In_ sai_object_id_t port_id, + _In_ const sai_attribute_t *attr) + { + if (attr[0].id == SAI_PORT_ATTR_FEC_MODE) + { + _sai_set_port_fec_count++; + _sai_port_fec_mode = attr[0].value.s32; + } + return pold_sai_port_api->set_port_attribute(port_id, attr); + } + + void _hook_sai_port_api() + { + ut_sai_port_api = *sai_port_api; + pold_sai_port_api = sai_port_api; + ut_sai_port_api.get_port_attribute = _ut_stub_sai_get_port_attribute; + ut_sai_port_api.set_port_attribute = _ut_stub_sai_set_port_attribute; + sai_port_api = &ut_sai_port_api; + } + + void _unhook_sai_port_api() + { + sai_port_api = pold_sai_port_api; + } + struct PortsOrchTest : public ::testing::Test { shared_ptr m_app_db; @@ -173,6 +237,169 @@ namespace portsorch_test }; + TEST_F(PortsOrchTest, PortSupportedFecModes) + { + _hook_sai_port_api(); + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + std::deque entries; + + not_support_fetching_fec = false; + // Get SAI default ports to populate DB + auto ports = ut_helper::getInitialSaiPorts(); + + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + + // refill consumer + gPortsOrch->addExistingData(&portTable); + + // Apply configuration : + // create ports + static_cast(gPortsOrch)->doTask(); + + uint32_t current_sai_api_call_count = _sai_set_port_fec_count; + + entries.push_back({"Ethernet0", "SET", + { + {"fec", "rs"} + }}); + auto consumer = dynamic_cast(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gPortsOrch)->doTask(); + entries.clear(); + + ASSERT_EQ(_sai_set_port_fec_count, ++current_sai_api_call_count); + ASSERT_EQ(_sai_port_fec_mode, SAI_PORT_FEC_MODE_RS); + + vector ts; + + gPortsOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + + entries.push_back({"Ethernet0", "SET", + { + {"fec", "none"} + }}); + consumer = dynamic_cast(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gPortsOrch)->doTask(); + + ASSERT_EQ(_sai_set_port_fec_count, current_sai_api_call_count); + ASSERT_EQ(_sai_port_fec_mode, SAI_PORT_FEC_MODE_RS); + + gPortsOrch->dumpPendingTasks(ts); + ASSERT_EQ(ts.size(), 0); + + _unhook_sai_port_api(); + } + + /* + * Test case: SAI_PORT_ATTR_SUPPORTED_FEC_MODE is not supported by vendor + **/ + TEST_F(PortsOrchTest, PortNotSupportedFecModes) + { + _hook_sai_port_api(); + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + std::deque entries; + + not_support_fetching_fec = true; + // Get SAI default ports to populate DB + auto ports = ut_helper::getInitialSaiPorts(); + + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + + // refill consumer + gPortsOrch->addExistingData(&portTable); + + // Apply configuration : + // create ports + static_cast(gPortsOrch)->doTask(); + + uint32_t current_sai_api_call_count = _sai_set_port_fec_count; + + entries.push_back({"Ethernet0", "SET", + { + {"fec", "rs"} + }}); + auto consumer = dynamic_cast(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gPortsOrch)->doTask(); + entries.clear(); + + ASSERT_EQ(_sai_set_port_fec_count, ++current_sai_api_call_count); + ASSERT_EQ(_sai_port_fec_mode, SAI_PORT_FEC_MODE_RS); + + vector ts; + + gPortsOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + + _unhook_sai_port_api(); + } + + /* + * Test case: Fetching SAI_PORT_ATTR_SUPPORTED_FEC_MODE is supported but no FEC mode is supported on the port + **/ + TEST_F(PortsOrchTest, PortSupportNoFecModes) + { + _hook_sai_port_api(); + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + std::deque entries; + + not_support_fetching_fec = false; + auto old_mock_port_fec_modes = mock_port_fec_modes; + mock_port_fec_modes.clear(); + // Get SAI default ports to populate DB + auto ports = ut_helper::getInitialSaiPorts(); + + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + + // refill consumer + gPortsOrch->addExistingData(&portTable); + + // Apply configuration : + // create ports + static_cast(gPortsOrch)->doTask(); + + uint32_t current_sai_api_call_count = _sai_set_port_fec_count; + + entries.push_back({"Ethernet0", "SET", + { + {"fec", "rs"} + }}); + auto consumer = dynamic_cast(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gPortsOrch)->doTask(); + entries.clear(); + + ASSERT_EQ(_sai_set_port_fec_count, current_sai_api_call_count); + + vector ts; + + gPortsOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + + mock_port_fec_modes = old_mock_port_fec_modes; + _unhook_sai_port_api(); + } + TEST_F(PortsOrchTest, PortReadinessColdBoot) { Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);