From 9507b57cb8c0e0a2540f1784c6a29ccdaa83a05d Mon Sep 17 00:00:00 2001
From: Stephen Sun <5379172+stephenxs@users.noreply.github.com>
Date: Sun, 24 Jul 2022 15:20:05 +0800
Subject: [PATCH] [portsorch] Expose supported FEC modes to STABE_DB and check
whether FEC mode is supported before setting it (#2333)
- What I did
Expose supported FEC modes to STATE_DB.PORT_TABLE|<port>.supported_fecs.
The orchagent calls get_port_attribute to get attribute SAI_PORT_ATTR_SUPPORTED_FEC_MODE during initialization and then records it into internal data.
1. By default, the supported FEC modes will be returned by SAI and exposed to STATE_DB. Eg. rs,none means only rs and none is supported on the port.
The orchagent will check whether the FEC mode is supported on the port before calling the SAI API to set it.
The CLI will check whether the FEC mode is in supported_fecs before setting FEC mode on a port to the CONFIG_DB
2. In case the SAI API does not support any FEC mode on the port, N/A will be exposed to STATE_DB
The orchagent will deny any FEC setting and prints log.
The CLI will deny FEC setting.
3. In case the SAI API get_port_attribute returns Not implemented
No check will be performed before setting a FEC mode to SAI on the port.
The CLI will check whether the FEC mode is defined before setting it to CONFIG_DB.
- Why I did it
It is not supported to set FEC mode on some platforms. To avoid error, we need to expose the supported FEC list.
- How I verified it
Manually test and mock test.
---
orchagent/portsorch.cpp | 119 ++++++++++++++--
orchagent/portsorch.h | 8 +-
tests/mock_tests/portsorch_ut.cpp | 227 ++++++++++++++++++++++++++++++
3 files changed, 342 insertions(+), 12 deletions(-)
diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
index e9a3afdc1e..fd96d6a3c2 100755
--- a/orchagent/portsorch.cpp
+++ b/orchagent/portsorch.cpp
@@ -77,6 +77,13 @@ static map<string, sai_port_fec_mode_t> fec_mode_map =
{ "fc", SAI_PORT_FEC_MODE_FC }
};
+static map<sai_port_fec_mode_t, string> fec_mode_reverse_map =
+{
+ { SAI_PORT_FEC_MODE_NONE, "none" },
+ { SAI_PORT_FEC_MODE_RS, "rs" },
+ { SAI_PORT_FEC_MODE_FC, "fc" }
+};
+
static map<string, sai_port_priority_flow_control_mode_t> pfc_asym_map =
{
{ "on", SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_SEPARATE },
@@ -1188,19 +1195,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)
{
@@ -1208,7 +1227,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);
@@ -1985,6 +2004,87 @@ void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_
m_portStateTable.set(alias, v);
}
+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<sai_int32_t> fecModes(fec_mode_reverse_map.size());
+
+ attr.id = SAI_PORT_ATTR_SUPPORTED_FEC_MODE;
+ attr.value.s32list.count = static_cast<uint32_t>(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<sai_port_fec_mode_t>(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<FieldValueTuple> 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.
*/
@@ -2978,6 +3078,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++;
}
@@ -3326,14 +3427,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;
}
@@ -3343,14 +3442,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 0fd3552e19..a3413790b1 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<sai_uint32_t> PortSupportedSpeeds;
+typedef std::set<std::string> PortSupportedFecModes;
static const map<sai_port_oper_status_t, string> oper_status_strings =
{
@@ -209,6 +210,8 @@ class PortsOrch : public Orch, public Subject
unique_ptr<Table> m_gbcounterTable;
std::map<sai_object_id_t, PortSupportedSpeeds> m_portSupportedSpeeds;
+ // Supported FEC modes on the system side.
+ std::map<sai_object_id_t, PortSupportedFecModes> m_portSupportedFecModes;
bool m_initDone = false;
Port m_cpuPort;
@@ -305,7 +308,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);
@@ -314,6 +317,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<sai_port_fec_mode_t> 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<swss::DBConnector> 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<KeyOpFieldsValuesTuple> 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<Orch *>(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<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
+ consumer->addToSync(entries);
+ static_cast<Orch *>(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<string> ts;
+
+ gPortsOrch->dumpPendingTasks(ts);
+ ASSERT_TRUE(ts.empty());
+
+ entries.push_back({"Ethernet0", "SET",
+ {
+ {"fec", "none"}
+ }});
+ consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
+ consumer->addToSync(entries);
+ static_cast<Orch *>(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<KeyOpFieldsValuesTuple> 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<Orch *>(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<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
+ consumer->addToSync(entries);
+ static_cast<Orch *>(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<string> 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<KeyOpFieldsValuesTuple> 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<Orch *>(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<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
+ consumer->addToSync(entries);
+ static_cast<Orch *>(gPortsOrch)->doTask();
+ entries.clear();
+
+ ASSERT_EQ(_sai_set_port_fec_count, current_sai_api_call_count);
+
+ vector<string> 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);