[portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it (#2333)

stephenxs · web-flow · commit dc8bc1c40f64 · 2022-07-24T10:20:05.000+03:00
- What I did
Expose supported FEC modes to STATE_DB.PORT_TABLE|&lt;port&gt;.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.
diff --git 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,27 +1195,39 @@ 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)
         {
             return parseHandleSaiStatusFailure(handle_status);
         }
     }
 
-    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
@@ -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
