[hash]: Extend VS lib with ECMP/LAG hash (#1192)

* [hash]: Extend VS lib with ECMP/LAG hash. Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com> * [hash]: Handle review comments. Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com> --------- Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
sonic-net · Feb 13, 2023 · 4d86af3 · 4d86af3
1 parent 3a960be
commit 4d86af3
Show file tree

Hide file tree

Showing 7 changed files with 300 additions and 7 deletions.
diff --git a/lib/RedisRemoteSaiInterface.cpp b/lib/RedisRemoteSaiInterface.cpp
@@ -1089,11 +1089,22 @@ sai_status_t RedisRemoteSaiInterface::waitForQueryAattributeEnumValuesCapability
             position++;
         }
     }
-    else if (status ==  SAI_STATUS_BUFFER_OVERFLOW)
+    else if (status == SAI_STATUS_BUFFER_OVERFLOW)
     {
-        // TODO on sai status overflow we should populate correct count on the list
+        const std::vector<swss::FieldValueTuple> &values = kfvFieldsValues(kco);
+
+        if (values.size() != 1)
+        {
+            SWSS_LOG_ERROR("Invalid response from syncd: expected 1 value, received %zu", values.size());
 
-        SWSS_LOG_ERROR("TODO need to handle SAI_STATUS_BUFFER_OVERFLOW, FIXME");
+            return SAI_STATUS_FAILURE;
+        }
+
+        const uint32_t num_capabilities = std::stoi(fvValue(values[0]));
+
+        SWSS_LOG_DEBUG("Received payload: count = %u", num_capabilities);
+
+        enumValuesCapability->count = num_capabilities;
     }
 
     return status;

diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp
@@ -484,6 +484,15 @@ sai_status_t Syncd::processAttrEnumValuesCapabilityQuery(
 
         SWSS_LOG_DEBUG("Sending response: capabilities = '%s', count = %d", strCap.c_str(), enumCapList.count);
     }
+    else if (status == SAI_STATUS_BUFFER_OVERFLOW)
+    {
+        entry =
+        {
+            swss::FieldValueTuple("ENUM_COUNT", std::to_string(enumCapList.count))
+        };
+
+        SWSS_LOG_DEBUG("Sending response: count = %u", enumCapList.count);
+    }
 
     m_selectableChannel->set(sai_serialize_status(status), entry, REDIS_ASIC_STATE_COMMAND_ATTR_ENUM_VALUES_CAPABILITY_RESPONSE);
 

diff --git a/syncd/tests/TestSyncdMlnx.cpp b/syncd/tests/TestSyncdMlnx.cpp
@@ -4,7 +4,9 @@
 #include <memory>
 #include <vector>
 #include <array>
+#include <set>
 #include <thread>
+#include <algorithm>
 
 #include <gtest/gtest.h>
 
@@ -160,6 +162,53 @@ class SyncdMlnxTest : public ::testing::Test
     sai_object_id_t m_switchId = SAI_NULL_OBJECT_ID;
 };
 
+TEST_F(SyncdMlnxTest, queryAttrEnumValuesCapability)
+{
+    sai_s32_list_t data = { .count = 0, .list = nullptr };
+
+    auto status = m_sairedis->queryAattributeEnumValuesCapability(
+        m_switchId, SAI_OBJECT_TYPE_HASH, SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST, &data
+    );
+    ASSERT_EQ(status, SAI_STATUS_BUFFER_OVERFLOW);
+
+    std::vector<sai_int32_t> hfList(data.count);
+    data.list = hfList.data();
+
+    status = m_sairedis->queryAattributeEnumValuesCapability(
+        m_switchId, SAI_OBJECT_TYPE_HASH, SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST, &data
+    );
+    ASSERT_EQ(status, SAI_STATUS_SUCCESS);
+
+    const std::set<sai_native_hash_field_t> hfSet1 = {
+        SAI_NATIVE_HASH_FIELD_IN_PORT,
+        SAI_NATIVE_HASH_FIELD_DST_MAC,
+        SAI_NATIVE_HASH_FIELD_SRC_MAC,
+        SAI_NATIVE_HASH_FIELD_ETHERTYPE,
+        SAI_NATIVE_HASH_FIELD_VLAN_ID,
+        SAI_NATIVE_HASH_FIELD_IP_PROTOCOL,
+        SAI_NATIVE_HASH_FIELD_DST_IP,
+        SAI_NATIVE_HASH_FIELD_SRC_IP,
+        SAI_NATIVE_HASH_FIELD_L4_DST_PORT,
+        SAI_NATIVE_HASH_FIELD_L4_SRC_PORT,
+        SAI_NATIVE_HASH_FIELD_INNER_DST_MAC,
+        SAI_NATIVE_HASH_FIELD_INNER_SRC_MAC,
+        SAI_NATIVE_HASH_FIELD_INNER_ETHERTYPE,
+        SAI_NATIVE_HASH_FIELD_INNER_IP_PROTOCOL,
+        SAI_NATIVE_HASH_FIELD_INNER_DST_IP,
+        SAI_NATIVE_HASH_FIELD_INNER_SRC_IP,
+        SAI_NATIVE_HASH_FIELD_INNER_L4_DST_PORT,
+        SAI_NATIVE_HASH_FIELD_INNER_L4_SRC_PORT
+    };
+
+    std::set<sai_native_hash_field_t> hfSet2;
+
+    std::transform(
+        hfList.cbegin(), hfList.cend(), std::inserter(hfSet2, hfSet2.begin()),
+        [](sai_int32_t value) { return static_cast<sai_native_hash_field_t>(value); }
+    );
+    ASSERT_EQ(hfSet1, hfSet2);
+}
+
 TEST_F(SyncdMlnxTest, portBulkAddRemove)
 {
     const std::uint32_t portCount = 1;

diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws
@@ -27,6 +27,8 @@ EAPOL
 ECN
 EIO
 ETERM
+ecmp
+ECMP
 FDB
 FDBs
 FIXME

diff --git a/unittest/vslib/TestSwitchStateBase.cpp b/unittest/vslib/TestSwitchStateBase.cpp
@@ -1,12 +1,143 @@
-#include "SwitchStateBase.h"
-#include "MACsecAttr.h"
-
 #include <gtest/gtest.h>
 
+#include <cstdint>
 #include <vector>
+#include <set>
+#include <algorithm>
+
+#include "Globals.h"
+#include "sai_serialize.h"
+
+#include "MACsecAttr.h"
+#include "RealObjectIdManager.h"
+#include "ContextConfigContainer.h"
 
+#include "SwitchStateBase.h"
+
+using namespace saimeta;
 using namespace saivs;
 
+class SwitchStateBaseTest : public ::testing::Test
+{
+public:
+    SwitchStateBaseTest() = default;
+    virtual ~SwitchStateBaseTest() = default;
+
+public:
+    virtual void SetUp() override
+    {
+        m_ccc = ContextConfigContainer::getDefault();
+        m_cc = m_ccc->get(m_guid);
+        m_scc = m_cc->m_scc;
+        m_sc = m_scc->getConfig(m_scid);
+
+        m_ridmgr = std::make_shared<RealObjectIdManager>(m_cc->m_guid, m_cc->m_scc);
+        m_swid = m_ridmgr->allocateNewSwitchObjectId(Globals::getHardwareInfo(0, nullptr));
+        m_ss = std::make_shared<SwitchStateBase>(m_swid, m_ridmgr, m_sc);
+    }
+
+    virtual void TearDown() override
+    {
+        // Empty
+    }
+
+protected:
+    std::shared_ptr<ContextConfigContainer> m_ccc;
+    std::shared_ptr<ContextConfig> m_cc;
+    std::shared_ptr<SwitchConfigContainer> m_scc;
+    std::shared_ptr<SwitchConfig> m_sc;
+    std::shared_ptr<RealObjectIdManager> m_ridmgr;
+    std::shared_ptr<SwitchStateBase> m_ss;
+
+    sai_object_id_t m_swid = SAI_NULL_OBJECT_ID;
+
+    const std::uint32_t m_guid = 0; // default context config id
+    const std::uint32_t m_scid = 0; // default switch config id
+};
+
+TEST_F(SwitchStateBaseTest, switchHashGet)
+{
+    ASSERT_EQ(m_ss->create_default_hash(), SAI_STATUS_SUCCESS);
+
+    sai_attribute_t attr;
+    attr.id = SAI_SWITCH_ATTR_ECMP_HASH;
+    ASSERT_EQ(m_ss->get(SAI_OBJECT_TYPE_SWITCH, sai_serialize_object_id(m_swid), 1, &attr), SAI_STATUS_SUCCESS);
+
+    const auto ecmpHashOid = attr.value.oid;
+    ASSERT_NE(ecmpHashOid, SAI_NULL_OBJECT_ID);
+
+    attr.id = SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST;
+    attr.value.s32list.list = nullptr;
+    attr.value.s32list.count = 0;
+    ASSERT_EQ(m_ss->get(SAI_OBJECT_TYPE_HASH, sai_serialize_object_id(ecmpHashOid), 1, &attr), SAI_STATUS_SUCCESS);
+
+    std::vector<sai_int32_t> hfList(attr.value.s32list.count);
+    attr.value.s32list.list = hfList.data();
+    ASSERT_EQ(m_ss->get(SAI_OBJECT_TYPE_HASH, sai_serialize_object_id(ecmpHashOid), 1, &attr), SAI_STATUS_SUCCESS);
+
+    const std::set<sai_native_hash_field_t> hfSet1 = {
+        SAI_NATIVE_HASH_FIELD_DST_MAC,
+        SAI_NATIVE_HASH_FIELD_SRC_MAC,
+        SAI_NATIVE_HASH_FIELD_ETHERTYPE,
+        SAI_NATIVE_HASH_FIELD_IN_PORT
+    };
+
+    std::set<sai_native_hash_field_t> hfSet2;
+
+    std::transform(
+        hfList.cbegin(), hfList.cend(), std::inserter(hfSet2, hfSet2.begin()),
+        [](sai_int32_t value) { return static_cast<sai_native_hash_field_t>(value); }
+    );
+    ASSERT_EQ(hfSet1, hfSet2);
+}
+
+TEST_F(SwitchStateBaseTest, switchHashCapabilitiesGet)
+{
+    sai_s32_list_t data = { .count = 0, .list = nullptr };
+
+    auto status = m_ss->queryAttrEnumValuesCapability(
+        m_swid, SAI_OBJECT_TYPE_HASH, SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST, &data
+    );
+    ASSERT_EQ(status, SAI_STATUS_BUFFER_OVERFLOW);
+
+    std::vector<sai_int32_t> hfList(data.count);
+    data.list = hfList.data();
+
+    status = m_ss->queryAttrEnumValuesCapability(
+        m_swid, SAI_OBJECT_TYPE_HASH, SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST, &data
+    );
+    ASSERT_EQ(status, SAI_STATUS_SUCCESS);
+
+    const std::set<sai_native_hash_field_t> hfSet1 = {
+        SAI_NATIVE_HASH_FIELD_IN_PORT,
+        SAI_NATIVE_HASH_FIELD_DST_MAC,
+        SAI_NATIVE_HASH_FIELD_SRC_MAC,
+        SAI_NATIVE_HASH_FIELD_ETHERTYPE,
+        SAI_NATIVE_HASH_FIELD_VLAN_ID,
+        SAI_NATIVE_HASH_FIELD_IP_PROTOCOL,
+        SAI_NATIVE_HASH_FIELD_DST_IP,
+        SAI_NATIVE_HASH_FIELD_SRC_IP,
+        SAI_NATIVE_HASH_FIELD_L4_DST_PORT,
+        SAI_NATIVE_HASH_FIELD_L4_SRC_PORT,
+        SAI_NATIVE_HASH_FIELD_INNER_DST_MAC,
+        SAI_NATIVE_HASH_FIELD_INNER_SRC_MAC,
+        SAI_NATIVE_HASH_FIELD_INNER_ETHERTYPE,
+        SAI_NATIVE_HASH_FIELD_INNER_IP_PROTOCOL,
+        SAI_NATIVE_HASH_FIELD_INNER_DST_IP,
+        SAI_NATIVE_HASH_FIELD_INNER_SRC_IP,
+        SAI_NATIVE_HASH_FIELD_INNER_L4_DST_PORT,
+        SAI_NATIVE_HASH_FIELD_INNER_L4_SRC_PORT
+    };
+
+    std::set<sai_native_hash_field_t> hfSet2;
+
+    std::transform(
+        hfList.cbegin(), hfList.cend(), std::inserter(hfSet2, hfSet2.begin()),
+        [](sai_int32_t value) { return static_cast<sai_native_hash_field_t>(value); }
+    );
+    ASSERT_EQ(hfSet1, hfSet2);
+}
+
 //Test the following function:
 //sai_status_t initialize_voq_switch_objects(
 //             _In_ uint32_t attr_count,

diff --git a/vslib/SwitchStateBase.cpp b/vslib/SwitchStateBase.cpp
@@ -980,6 +980,48 @@ sai_status_t SwitchStateBase::set_switch_default_attributes()
     return set_switch_supported_object_types();
 }
 
+sai_status_t SwitchStateBase::create_default_hash()
+{
+    SWSS_LOG_ENTER();
+
+    SWSS_LOG_INFO("create default hash");
+
+    // Hash defaults according to SAI headers
+    std::vector<sai_native_hash_field_t> hfList = {
+        SAI_NATIVE_HASH_FIELD_DST_MAC,
+        SAI_NATIVE_HASH_FIELD_SRC_MAC,
+        SAI_NATIVE_HASH_FIELD_ETHERTYPE,
+        SAI_NATIVE_HASH_FIELD_IN_PORT
+    };
+
+    // create and populate default ecmp hash object
+    sai_attribute_t attr;
+    attr.id = SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST;
+    attr.value.s32list.list = reinterpret_cast<sai_int32_t*>(hfList.data());
+    attr.value.s32list.count = static_cast<sai_uint32_t>(hfList.size());
+
+    CHECK_STATUS(create(SAI_OBJECT_TYPE_HASH, &m_ecmp_hash_id, m_switch_id, 1, &attr));
+
+    // set default ecmp hash on switch
+    attr.id = SAI_SWITCH_ATTR_ECMP_HASH;
+    attr.value.oid = m_ecmp_hash_id;
+
+    CHECK_STATUS(set(SAI_OBJECT_TYPE_SWITCH, m_switch_id, &attr));
+
+    // create and populate default lag hash object
+    attr.id = SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST;
+    attr.value.s32list.list = reinterpret_cast<sai_int32_t*>(hfList.data());
+    attr.value.s32list.count = static_cast<sai_uint32_t>(hfList.size());
+
+    CHECK_STATUS(create(SAI_OBJECT_TYPE_HASH, &m_lag_hash_id, m_switch_id, 1, &attr));
+
+    // set default lag hash on switch
+    attr.id = SAI_SWITCH_ATTR_LAG_HASH;
+    attr.value.oid = m_lag_hash_id;
+
+    return set(SAI_OBJECT_TYPE_SWITCH, m_switch_id, &attr);
+}
+
 sai_status_t SwitchStateBase::set_static_crm_values()
 {
     SWSS_LOG_ENTER();
@@ -1608,6 +1650,7 @@ sai_status_t SwitchStateBase::initialize_default_objects(
 
     CHECK_STATUS(set_switch_mac_address());
     CHECK_STATUS(create_cpu_port());
+    CHECK_STATUS(create_default_hash());
     CHECK_STATUS(create_default_vlan());
     CHECK_STATUS(create_default_virtual_router());
     CHECK_STATUS(create_default_stp_instance());
@@ -2262,6 +2305,10 @@ sai_status_t SwitchStateBase::refresh_read_only(
             case SAI_SWITCH_ATTR_DEFAULT_1Q_BRIDGE_ID:
                 return SAI_STATUS_SUCCESS;
 
+            case SAI_SWITCH_ATTR_ECMP_HASH:
+            case SAI_SWITCH_ATTR_LAG_HASH:
+                return SAI_STATUS_SUCCESS;
+
             case SAI_SWITCH_ATTR_ACL_ENTRY_MINIMUM_PRIORITY:
             case SAI_SWITCH_ATTR_ACL_ENTRY_MAXIMUM_PRIORITY:
                 return SAI_STATUS_SUCCESS;
@@ -3632,6 +3679,39 @@ sai_status_t SwitchStateBase::queryNextHopGroupTypeCapability(
     return SAI_STATUS_SUCCESS;
 }
 
+sai_status_t SwitchStateBase::queryHashNativeHashFieldListCapability(
+                   _Inout_ sai_s32_list_t *enum_values_capability)
+{
+    SWSS_LOG_ENTER();
+
+    if (enum_values_capability->count < 18)
+    {
+        enum_values_capability->count = 18;
+        return SAI_STATUS_BUFFER_OVERFLOW;
+    }
+
+    enum_values_capability->count = 18;
+    enum_values_capability->list[0] = SAI_NATIVE_HASH_FIELD_IN_PORT;
+    enum_values_capability->list[1] = SAI_NATIVE_HASH_FIELD_DST_MAC;
+    enum_values_capability->list[2] = SAI_NATIVE_HASH_FIELD_SRC_MAC;
+    enum_values_capability->list[3] = SAI_NATIVE_HASH_FIELD_ETHERTYPE;
+    enum_values_capability->list[4] = SAI_NATIVE_HASH_FIELD_VLAN_ID;
+    enum_values_capability->list[5] = SAI_NATIVE_HASH_FIELD_IP_PROTOCOL;
+    enum_values_capability->list[6] = SAI_NATIVE_HASH_FIELD_DST_IP;
+    enum_values_capability->list[7] = SAI_NATIVE_HASH_FIELD_SRC_IP;
+    enum_values_capability->list[8] = SAI_NATIVE_HASH_FIELD_L4_DST_PORT;
+    enum_values_capability->list[9] = SAI_NATIVE_HASH_FIELD_L4_SRC_PORT;
+    enum_values_capability->list[10] = SAI_NATIVE_HASH_FIELD_INNER_DST_MAC;
+    enum_values_capability->list[11] = SAI_NATIVE_HASH_FIELD_INNER_SRC_MAC;
+    enum_values_capability->list[12] = SAI_NATIVE_HASH_FIELD_INNER_ETHERTYPE;
+    enum_values_capability->list[13] = SAI_NATIVE_HASH_FIELD_INNER_IP_PROTOCOL;
+    enum_values_capability->list[14] = SAI_NATIVE_HASH_FIELD_INNER_DST_IP;
+    enum_values_capability->list[15] = SAI_NATIVE_HASH_FIELD_INNER_SRC_IP;
+    enum_values_capability->list[16] = SAI_NATIVE_HASH_FIELD_INNER_L4_DST_PORT;
+    enum_values_capability->list[17] = SAI_NATIVE_HASH_FIELD_INNER_L4_SRC_PORT;
+
+    return SAI_STATUS_SUCCESS;
+}
 
 sai_status_t SwitchStateBase::queryAttrEnumValuesCapability(
                               _In_ sai_object_id_t switch_id,
@@ -3655,5 +3735,10 @@ sai_status_t SwitchStateBase::queryAttrEnumValuesCapability(
     {
         return queryNextHopGroupTypeCapability(enum_values_capability);
     }
+    else if (object_type == SAI_OBJECT_TYPE_HASH && attr_id == SAI_HASH_ATTR_NATIVE_HASH_FIELD_LIST)
+    {
+        return queryHashNativeHashFieldListCapability(enum_values_capability);
+    }
+
     return SAI_STATUS_NOT_SUPPORTED;
 }