From 4d86af3474c7b4299557293c2ee4516753aff169 Mon Sep 17 00:00:00 2001 From: Nazarii Hnydyn Date: Mon, 13 Feb 2023 20:02:49 +0200 Subject: [PATCH] [hash]: Extend VS lib with ECMP/LAG hash (#1192) * [hash]: Extend VS lib with ECMP/LAG hash. Signed-off-by: Nazarii Hnydyn * [hash]: Handle review comments. Signed-off-by: Nazarii Hnydyn --------- Signed-off-by: Nazarii Hnydyn --- lib/RedisRemoteSaiInterface.cpp | 17 ++- syncd/Syncd.cpp | 9 ++ syncd/tests/TestSyncdMlnx.cpp | 49 +++++++++ tests/aspell.en.pws | 2 + unittest/vslib/TestSwitchStateBase.cpp | 137 ++++++++++++++++++++++++- vslib/SwitchStateBase.cpp | 85 +++++++++++++++ vslib/SwitchStateBase.h | 8 +- 7 files changed, 300 insertions(+), 7 deletions(-) diff --git a/lib/RedisRemoteSaiInterface.cpp b/lib/RedisRemoteSaiInterface.cpp index a43b36927..ec69c4d4f 100644 --- 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 &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 index 52ec520a5..78a1ea2d0 100644 --- 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 index 575beb490..457f621a9 100644 --- a/syncd/tests/TestSyncdMlnx.cpp +++ b/syncd/tests/TestSyncdMlnx.cpp @@ -4,7 +4,9 @@ #include #include #include +#include #include +#include #include @@ -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 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 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 hfSet2; + + std::transform( + hfList.cbegin(), hfList.cend(), std::inserter(hfSet2, hfSet2.begin()), + [](sai_int32_t value) { return static_cast(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 index 8fcf3f116..93bf80505 100644 --- 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 index 29027edcb..bb5ac18b6 100644 --- a/unittest/vslib/TestSwitchStateBase.cpp +++ b/unittest/vslib/TestSwitchStateBase.cpp @@ -1,12 +1,143 @@ -#include "SwitchStateBase.h" -#include "MACsecAttr.h" - #include +#include #include +#include +#include + +#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(m_cc->m_guid, m_cc->m_scc); + m_swid = m_ridmgr->allocateNewSwitchObjectId(Globals::getHardwareInfo(0, nullptr)); + m_ss = std::make_shared(m_swid, m_ridmgr, m_sc); + } + + virtual void TearDown() override + { + // Empty + } + +protected: + std::shared_ptr m_ccc; + std::shared_ptr m_cc; + std::shared_ptr m_scc; + std::shared_ptr m_sc; + std::shared_ptr m_ridmgr; + std::shared_ptr 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 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 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 hfSet2; + + std::transform( + hfList.cbegin(), hfList.cend(), std::inserter(hfSet2, hfSet2.begin()), + [](sai_int32_t value) { return static_cast(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 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 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 hfSet2; + + std::transform( + hfList.cbegin(), hfList.cend(), std::inserter(hfSet2, hfSet2.begin()), + [](sai_int32_t value) { return static_cast(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 index 6f419032e..9d3da0cc0 100644 --- 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 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(hfList.data()); + attr.value.s32list.count = static_cast(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(hfList.data()); + attr.value.s32list.count = static_cast(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; } diff --git a/vslib/SwitchStateBase.h b/vslib/SwitchStateBase.h index bf3ad9aac..bc262c33b 100644 --- a/vslib/SwitchStateBase.h +++ b/vslib/SwitchStateBase.h @@ -55,6 +55,8 @@ namespace saivs virtual sai_status_t set_switch_default_attributes(); + virtual sai_status_t create_default_hash(); + virtual sai_status_t create_default_vlan(); virtual sai_status_t create_cpu_port(); @@ -644,6 +646,9 @@ namespace saivs sai_object_id_t m_default_bridge_port_1q_router; sai_object_id_t m_default_vlan_id; + sai_object_id_t m_ecmp_hash_id; + sai_object_id_t m_lag_hash_id; + std::vector m_system_port_list; protected: @@ -681,7 +686,8 @@ namespace saivs virtual sai_status_t queryNextHopGroupTypeCapability( _Inout_ sai_s32_list_t *enum_values_capability); - + virtual sai_status_t queryHashNativeHashFieldListCapability( + _Inout_ sai_s32_list_t *enum_values_capability); public: // TODO private