[hash]: Add ECMP/LAG hash algorithm to OA (#2953)

* [hash]: Add ECMP/LAG hash algorithm to OA.
* Implement as defined in sonic-net/SONiC#1501

orchagent/switch/switch_capabilities.h

@@ -1,9 +1,10 @@
 #pragma once
 
 extern "C" {
-#include <saiobject.h>
 #include <saitypes.h>
+#include <saiobject.h>
 #include <saihash.h>
+#include <saiswitch.h>
 }
 
 #include <vector>
@@ -21,21 +22,39 @@ class SwitchCapabilities final
     bool isSwitchEcmpHashSupported() const;
     bool isSwitchLagHashSupported() const;
 
+    bool isSwitchEcmpHashAlgorithmSupported() const;
+    bool isSwitchLagHashAlgorithmSupported() const;
+
     bool validateSwitchHashFieldCap(const std::set<sai_native_hash_field_t> &hfSet) const;
 
+    bool validateSwitchEcmpHashAlgorithmCap(sai_hash_algorithm_t haValue) const;
+    bool validateSwitchLagHashAlgorithmCap(sai_hash_algorithm_t haValue) const;
+
 private:
+    template<typename T>
+    bool validateSwitchHashAlgorithmCap(const T &obj, sai_hash_algorithm_t haValue) const;
+
     swss::FieldValueTuple makeHashFieldCapDbEntry() const;
+
     swss::FieldValueTuple makeEcmpHashCapDbEntry() const;
     swss::FieldValueTuple makeLagHashCapDbEntry() const;
 
+    std::vector<swss::FieldValueTuple> makeEcmpHashAlgorithmCapDbEntry() const;
+    std::vector<swss::FieldValueTuple> makeLagHashAlgorithmCapDbEntry() const;
+
     sai_status_t queryEnumCapabilitiesSai(std::vector<sai_int32_t> &capList, sai_object_type_t objType, sai_attr_id_t attrId) const;
     sai_status_t queryAttrCapabilitiesSai(sai_attr_capability_t &attrCap, sai_object_type_t objType, sai_attr_id_t attrId) const;
 
     void queryHashNativeHashFieldListEnumCapabilities();
     void queryHashNativeHashFieldListAttrCapabilities();
 
-    void querySwitchEcmpHashCapabilities();
-    void querySwitchLagHashCapabilities();
+    void querySwitchEcmpHashAttrCapabilities();
+    void querySwitchLagHashAttrCapabilities();
+
+    void querySwitchEcmpHashAlgorithmEnumCapabilities();
+    void querySwitchEcmpHashAlgorithmAttrCapabilities();
+    void querySwitchLagHashAlgorithmEnumCapabilities();
+    void querySwitchLagHashAlgorithmAttrCapabilities();
 
     void queryHashCapabilities();
     void querySwitchCapabilities();
@@ -61,6 +80,18 @@ class SwitchCapabilities final
         struct {
             bool isAttrSupported = false;
         } lagHash;
+
+        struct {
+            std::set<sai_hash_algorithm_t> haSet;
+            bool isEnumSupported = false;
+            bool isAttrSupported = false;
+        } ecmpHashAlgorithm;
+
+        struct {
+            std::set<sai_hash_algorithm_t> haSet;
+            bool isEnumSupported = false;
+            bool isAttrSupported = false;
+        } lagHashAlgorithm;
     } switchCapabilities;
 
     static swss::DBConnector stateDb;

orchagent/switch/switch_container.h

@@ -1,6 +1,7 @@
 #pragma once
 
 extern "C" {
+#include <saiswitch.h>
 #include <saihash.h>
 }
 
@@ -24,5 +25,15 @@ class SwitchHash final
         bool is_set = false;
     } lag_hash;
 
+    struct {
+        sai_hash_algorithm_t value;
+        bool is_set = false;
+    } ecmp_hash_algorithm;
+
+    struct {
+        sai_hash_algorithm_t value;
+        bool is_set = false;
+    } lag_hash_algorithm;
+
     std::unordered_map<std::string, std::string> fieldValueMap;
 };

orchagent/switch/switch_helper.cpp

@@ -1,5 +1,10 @@
 // includes -----------------------------------------------------------------------------------------------------------
 
+extern "C" {
+#include <saihash.h>
+#include <saiswitch.h>
+}
+
 #include <unordered_map>
 #include <unordered_set>
 #include <string>
@@ -36,6 +41,17 @@ static const std::unordered_map<std::string, sai_native_hash_field_t> swHashHash
     { SWITCH_HASH_FIELD_INNER_L4_SRC_PORT, SAI_NATIVE_HASH_FIELD_INNER_L4_SRC_PORT }
 };
 
+static const std::unordered_map<std::string, sai_hash_algorithm_t> swHashAlgorithmMap =
+{
+    { SWITCH_HASH_ALGORITHM_CRC,       SAI_HASH_ALGORITHM_CRC       },
+    { SWITCH_HASH_ALGORITHM_XOR,       SAI_HASH_ALGORITHM_XOR       },
+    { SWITCH_HASH_ALGORITHM_RANDOM,    SAI_HASH_ALGORITHM_RANDOM    },
+    { SWITCH_HASH_ALGORITHM_CRC_32LO,  SAI_HASH_ALGORITHM_CRC_32LO  },
+    { SWITCH_HASH_ALGORITHM_CRC_32HI,  SAI_HASH_ALGORITHM_CRC_32HI  },
+    { SWITCH_HASH_ALGORITHM_CRC_CCITT, SAI_HASH_ALGORITHM_CRC_CCITT },
+    { SWITCH_HASH_ALGORITHM_CRC_XOR,   SAI_HASH_ALGORITHM_CRC_XOR   }
+};
+
 // switch helper ------------------------------------------------------------------------------------------------------
 
 const SwitchHash& SwitchHelper::getSwHash() const
@@ -86,6 +102,30 @@ bool SwitchHelper::parseSwHashFieldList(T &obj, const std::string &field, const
     return true;
 }
 
+template<typename T>
+bool SwitchHelper::parseSwHashAlgorithm(T &obj, const std::string &field, const std::string &value) const
+{
+    SWSS_LOG_ENTER();
+
+    if (value.empty())
+    {
+        SWSS_LOG_ERROR("Failed to parse field(%s): empty value is prohibited", field.c_str());
+        return false;
+    }
+
+    const auto &cit = swHashAlgorithmMap.find(value);
+    if (cit == swHashAlgorithmMap.cend())
+    {
+        SWSS_LOG_ERROR("Failed to parse field(%s): invalid value(%s)", field.c_str(), value.c_str());
+        return false;
+    }
+
+    obj.value = cit->second;
+    obj.is_set = true;
+
+    return true;
+}
+
 bool SwitchHelper::parseSwHashEcmpHash(SwitchHash &hash, const std::string &field, const std::string &value) const
 {
     return parseSwHashFieldList(hash.ecmp_hash, field, value);
@@ -96,6 +136,16 @@ bool SwitchHelper::parseSwHashLagHash(SwitchHash &hash, const std::string &field
     return parseSwHashFieldList(hash.lag_hash, field, value);
 }
 
+bool SwitchHelper::parseSwHashEcmpHashAlgorithm(SwitchHash &hash, const std::string &field, const std::string &value) const
+{
+    return parseSwHashAlgorithm(hash.ecmp_hash_algorithm, field, value);
+}
+
+bool SwitchHelper::parseSwHashLagHashAlgorithm(SwitchHash &hash, const std::string &field, const std::string &value) const
+{
+    return parseSwHashAlgorithm(hash.lag_hash_algorithm, field, value);
+}
+
 bool SwitchHelper::parseSwHash(SwitchHash &hash) const
 {
     SWSS_LOG_ENTER();
@@ -119,6 +169,20 @@ bool SwitchHelper::parseSwHash(SwitchHash &hash) const
                 return false;
             }
         }
+        else if (field == SWITCH_HASH_ECMP_HASH_ALGORITHM)
+        {
+            if (!parseSwHashEcmpHashAlgorithm(hash, field, value))
+            {
+                return false;
+            }
+        }
+        else if (field == SWITCH_HASH_LAG_HASH_ALGORITHM)
+        {
+            if (!parseSwHashLagHashAlgorithm(hash, field, value))
+            {
+                return false;
+            }
+        }
         else
         {
             SWSS_LOG_WARN("Unknown field(%s): skipping ...", field.c_str());
@@ -132,7 +196,10 @@ bool SwitchHelper::validateSwHash(SwitchHash &hash) const
 {
     SWSS_LOG_ENTER();
 
-    if (!hash.ecmp_hash.is_set && !hash.lag_hash.is_set)
+    auto cond = hash.ecmp_hash.is_set || hash.lag_hash.is_set;
+    cond = cond || hash.ecmp_hash_algorithm.is_set || hash.lag_hash_algorithm.is_set;
+
+    if (!cond)
     {
         SWSS_LOG_ERROR("Validation error: missing valid fields");
         return false;

orchagent/switch/switch_helper.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <string>
+
 #include "switch_container.h"
 
 class SwitchHelper final
@@ -16,9 +18,13 @@ class SwitchHelper final
 private:
     template<typename T>
     bool parseSwHashFieldList(T &obj, const std::string &field, const std::string &value) const;
+    template<typename T>
+    bool parseSwHashAlgorithm(T &obj, const std::string &field, const std::string &value) const;
 
     bool parseSwHashEcmpHash(SwitchHash &hash, const std::string &field, const std::string &value) const;
     bool parseSwHashLagHash(SwitchHash &hash, const std::string &field, const std::string &value) const;
+    bool parseSwHashEcmpHashAlgorithm(SwitchHash &hash, const std::string &field, const std::string &value) const;
+    bool parseSwHashLagHashAlgorithm(SwitchHash &hash, const std::string &field, const std::string &value) const;
 
     bool validateSwHash(SwitchHash &hash) const;
 

orchagent/switch/switch_schema.h

@@ -23,3 +23,14 @@
 
 #define SWITCH_HASH_ECMP_HASH "ecmp_hash"
 #define SWITCH_HASH_LAG_HASH  "lag_hash"
+
+#define SWITCH_HASH_ALGORITHM_CRC       "CRC"
+#define SWITCH_HASH_ALGORITHM_XOR       "XOR"
+#define SWITCH_HASH_ALGORITHM_RANDOM    "RANDOM"
+#define SWITCH_HASH_ALGORITHM_CRC_32LO  "CRC_32LO"
+#define SWITCH_HASH_ALGORITHM_CRC_32HI  "CRC_32HI"
+#define SWITCH_HASH_ALGORITHM_CRC_CCITT "CRC_CCITT"
+#define SWITCH_HASH_ALGORITHM_CRC_XOR   "CRC_XOR"
+
+#define SWITCH_HASH_ECMP_HASH_ALGORITHM "ecmp_hash_algorithm"
+#define SWITCH_HASH_LAG_HASH_ALGORITHM  "lag_hash_algorithm"

orchagent/switchorch.cpp

@@ -497,6 +497,17 @@ bool SwitchOrch::setSwitchHashFieldListSai(const SwitchHash &hash, bool isEcmpHa
     return status == SAI_STATUS_SUCCESS;
 }
 
+bool SwitchOrch::setSwitchHashAlgorithmSai(const SwitchHash &hash, bool isEcmpHash) const
+{
+    sai_attribute_t attr;
+
+    attr.id = isEcmpHash ? SAI_SWITCH_ATTR_ECMP_DEFAULT_HASH_ALGORITHM : SAI_SWITCH_ATTR_LAG_DEFAULT_HASH_ALGORITHM;
+    attr.value.s32 = static_cast<sai_int32_t>(isEcmpHash ? hash.ecmp_hash_algorithm.value : hash.lag_hash_algorithm.value);
+
+    auto status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
+    return status == SAI_STATUS_SUCCESS;
+}
+
 bool SwitchOrch::setSwitchHash(const SwitchHash &hash)
 {
     SWSS_LOG_ENTER();
@@ -574,6 +585,76 @@ bool SwitchOrch::setSwitchHash(const SwitchHash &hash)
         }
     }
 
+    if (hash.ecmp_hash_algorithm.is_set)
+    {
+        if (!hObj.ecmp_hash_algorithm.is_set || (hObj.ecmp_hash_algorithm.value != hash.ecmp_hash_algorithm.value))
+        {
+            if (swCap.isSwitchEcmpHashAlgorithmSupported())
+            {
+                if (!swCap.validateSwitchEcmpHashAlgorithmCap(hash.ecmp_hash_algorithm.value))
+                {
+                    SWSS_LOG_ERROR("Failed to validate switch ECMP hash algorithm: capability is not supported");
+                    return false;
+                }
+
+                if (!setSwitchHashAlgorithmSai(hash, true))
+                {
+                    SWSS_LOG_ERROR("Failed to set switch ECMP hash algorithm in SAI");
+                    return false;
+                }
+
+                cfgUpd = true;
+            }
+            else
+            {
+                SWSS_LOG_WARN("Switch ECMP hash algorithm configuration is not supported: skipping ...");
+            }
+        }
+    }
+    else
+    {
+        if (hObj.ecmp_hash_algorithm.is_set)
+        {
+            SWSS_LOG_ERROR("Failed to remove switch ECMP hash algorithm configuration: operation is not supported");
+            return false;
+        }
+    }
+
+    if (hash.lag_hash_algorithm.is_set)
+    {
+        if (!hObj.lag_hash_algorithm.is_set || (hObj.lag_hash_algorithm.value != hash.lag_hash_algorithm.value))
+        {
+            if (swCap.isSwitchLagHashAlgorithmSupported())
+            {
+                if (!swCap.validateSwitchLagHashAlgorithmCap(hash.lag_hash_algorithm.value))
+                {
+                    SWSS_LOG_ERROR("Failed to validate switch LAG hash algorithm: capability is not supported");
+                    return false;
+                }
+
+                if (!setSwitchHashAlgorithmSai(hash, false))
+                {
+                    SWSS_LOG_ERROR("Failed to set switch LAG hash algorithm in SAI");
+                    return false;
+                }
+
+                cfgUpd = true;
+            }
+            else
+            {
+                SWSS_LOG_WARN("Switch LAG hash algorithm configuration is not supported: skipping ...");
+            }
+        }
+    }
+    else
+    {
+        if (hObj.lag_hash_algorithm.is_set)
+        {
+            SWSS_LOG_ERROR("Failed to remove switch LAG hash algorithm configuration: operation is not supported");
+            return false;
+        }
+    }
+
     // Don't update internal cache when config remains unchanged
     if (!cfgUpd)
     {

orchagent/switchorch.h

@@ -58,6 +58,7 @@ class SwitchOrch : public Orch
 
     // Switch hash
     bool setSwitchHashFieldListSai(const SwitchHash &hash, bool isEcmpHash) const;
+    bool setSwitchHashAlgorithmSai(const SwitchHash &hash, bool isEcmpHash) const;
     bool setSwitchHash(const SwitchHash &hash);
 
     bool getSwitchHashOidSai(sai_object_id_t &oid, bool isEcmpHash) const;

tests/conftest.py

@@ -27,6 +27,7 @@
 from dvslib import dvs_mirror
 from dvslib import dvs_policer
 from dvslib import dvs_hash
+from dvslib import dvs_switch
 
 from buffer_model import enable_dynamic_buffer
 
@@ -160,6 +161,8 @@ def _populate_default_asic_db_values(self) -> None:
 
         self.default_hash_keys = self.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_HASH")
 
+        self.default_switch_keys = self.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_SWITCH")
+
         self.default_copp_policers = self.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_POLICER")
 
 
@@ -1348,6 +1351,7 @@ def get_asic_db(self) -> AsicDbValidator:
             db.default_acl_tables = self.asicdb.default_acl_tables
             db.default_acl_entries = self.asicdb.default_acl_entries
             db.default_hash_keys = self.asicdb.default_hash_keys
+            db.default_switch_keys = self.asicdb.default_switch_keys
             db.default_copp_policers = self.asicdb.default_copp_policers
             db.port_name_map = self.asicdb.portnamemap
             db.default_vlan_id = self.asicdb.default_vlan_id
@@ -1937,6 +1941,10 @@ def dvs_hash_manager(request, dvs):
     request.cls.dvs_hash = dvs_hash.DVSHash(dvs.get_asic_db(),
                                             dvs.get_config_db())
 
+@pytest.fixture(scope="class")
+def dvs_switch_manager(request, dvs):
+    request.cls.dvs_switch = dvs_switch.DVSSwitch(dvs.get_asic_db())
+
 ##################### DPB fixtures ###########################################
 def create_dpb_config_file(dvs):
     cmd = "sonic-cfggen -j /etc/sonic/init_cfg.json -j /tmp/ports.json --print-data > /tmp/dpb_config_db.json"