From ad65b0a5bbe77055888ff5dd19ed239505cb4b25 Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Wed, 16 Mar 2022 23:59:51 +0800 Subject: [PATCH] Fix issue: sometimes PFC WD unable to create zero buffer pool (#2164) What I did Fix issue: sometimes PFC WD is unable to create zero buffer pool. On some platforms, an ingress/egress zero buffer profile will be applied on the PG and queue which are under PFC storm. The zero buffer profile is created based on zero buffer pool. However, sometimes it fails to create zero buffer pool due to too many buffer pools existing in the system. Sometimes, there is a zero buffer pool existing on the system for reclaiming buffer. In that case, we can leverage it to create zero buffer profile for PFC WD. Why I did it Fix the issue via sharing the zero buffer pool between PFC WD and buffer orchagent How I verified it Manually test Run PFC WD test and PFC WD warm reboot test Run unit test Details if related The detailed flow is like this: PFC Storm detected: If there is a zero pool in PFC WD's cache, just create the zero buffer profile based on it Otherwise, fetching the zero pool from buffer orchagent If got one, create the zero buffer profile based on it Otherwise, create a zero buffer pool notify the zero buffer pool about the buffer orch In both cases, PFC WD should notify buffer orch to increase the reference number of the zero buffer pool. Buffer orchagent: When creating the zero buffer pool, check whether there is one. if yes, skip the SAI API create_buffer_pool increase the reference number. Before removing the zero buffer pool, decrease and check the reference number. if it is zero (after decreased), skip SAI API destroy_buffer_pool. When PFC WD decrease reference number: remove the zero buffer pool if the reference number becomes zero Notes We do not leverage the object_reference_map infrastructure to track the dependency because: it assumes the dependency will eventually be removed if an object is removed. that's NOT true in this scenario because the PFC storm can last for a relatively long time and even cross warm reboot. the interfaces differ. Signed-off-by: Stephen Sun --- orchagent/bufferorch.cpp | 168 ++++++++++++-- orchagent/bufferorch.h | 13 ++ orchagent/pfcactionhandler.cpp | 86 ++++--- orchagent/pfcactionhandler.h | 5 +- tests/mock_tests/mock_orchagent_main.h | 2 + tests/mock_tests/portsorch_ut.cpp | 309 ++++++++++++++++++++++++- 6 files changed, 531 insertions(+), 52 deletions(-) diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index e7204344d5a3..41101480d590 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -48,7 +48,11 @@ BufferOrch::BufferOrch(DBConnector *applDb, DBConnector *confDb, DBConnector *st m_flexCounterTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_TABLE)), m_flexCounterGroupTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_GROUP_TABLE)), m_countersDb(new DBConnector("COUNTERS_DB", 0)), - m_stateBufferMaximumValueTable(stateDb, STATE_BUFFER_MAXIMUM_VALUE_TABLE) + m_stateBufferMaximumValueTable(stateDb, STATE_BUFFER_MAXIMUM_VALUE_TABLE), + m_ingressZeroBufferPool(SAI_NULL_OBJECT_ID), + m_egressZeroBufferPool(SAI_NULL_OBJECT_ID), + m_ingressZeroPoolRefCount(0), + m_egressZeroPoolRefCount(0) { SWSS_LOG_ENTER(); initTableHandlers(); @@ -310,6 +314,65 @@ const object_reference_map &BufferOrch::getBufferPoolNameOidMap(void) return *m_buffer_type_maps[APP_BUFFER_POOL_TABLE_NAME]; } +void BufferOrch::lockZeroBufferPool(bool ingress) +{ + if (ingress) + m_ingressZeroPoolRefCount++; + else + m_egressZeroPoolRefCount++; +} + +void BufferOrch::unlockZeroBufferPool(bool ingress) +{ + sai_object_id_t pool = SAI_NULL_OBJECT_ID; + if (ingress) + { + if (--m_ingressZeroPoolRefCount <= 0) + { + pool = m_ingressZeroBufferPool; + m_ingressZeroBufferPool = SAI_NULL_OBJECT_ID; + } + } + else + { + if (--m_egressZeroPoolRefCount <= 0) + { + pool = m_egressZeroBufferPool; + m_egressZeroBufferPool = SAI_NULL_OBJECT_ID; + } + } + + if (pool != SAI_NULL_OBJECT_ID) + { + auto sai_status = sai_buffer_api->remove_buffer_pool(pool); + if (SAI_STATUS_SUCCESS != sai_status) + { + SWSS_LOG_ERROR("Failed to remove buffer pool, rv:%d", sai_status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_BUFFER, sai_status); + if (handle_status != task_process_status::task_success) + { + return; + } + } + else + { + SWSS_LOG_NOTICE("Zero buffer pool has been successfully removed"); + } + } +} + +void BufferOrch::setZeroBufferPool(bool ingress, sai_object_id_t pool) +{ + if (ingress) + { + m_ingressZeroBufferPool = pool; + } + else + { + m_egressZeroBufferPool = pool; + } +} + task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) { SWSS_LOG_ENTER(); @@ -318,6 +381,8 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) string map_type_name = APP_BUFFER_POOL_TABLE_NAME; string object_name = kfvKey(tuple); string op = kfvOp(tuple); + sai_buffer_pool_type_t pool_direction = SAI_BUFFER_POOL_TYPE_INGRESS; + bool creating_zero_pool = false; SWSS_LOG_DEBUG("object name:%s", object_name.c_str()); if (m_buffer_type_maps[map_type_name]->find(object_name) != m_buffer_type_maps[map_type_name]->end()) @@ -326,6 +391,17 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) SWSS_LOG_DEBUG("found existing object:%s of type:%s", object_name.c_str(), map_type_name.c_str()); } SWSS_LOG_DEBUG("processing command:%s", op.c_str()); + if (object_name == "ingress_zero_pool") + { + creating_zero_pool = true; + pool_direction = SAI_BUFFER_POOL_TYPE_INGRESS; + } + else if (object_name == "egress_zero_pool") + { + creating_zero_pool = true; + pool_direction = SAI_BUFFER_POOL_TYPE_EGRESS; + } + if (op == SET_COMMAND) { vector attribs; @@ -372,6 +448,11 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) return task_process_status::task_invalid_entry; } attr.id = SAI_BUFFER_POOL_ATTR_TYPE; + if (creating_zero_pool && pool_direction != static_cast(attr.value.u32)) + { + SWSS_LOG_ERROR("Wrong pool direction for pool %s", object_name.c_str()); + return task_process_status::task_invalid_entry; + } attribs.push_back(attr); } else if (field == buffer_pool_mode_field_name) @@ -437,18 +518,53 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) } else { - sai_status = sai_buffer_api->create_buffer_pool(&sai_object, gSwitchId, (uint32_t)attribs.size(), attribs.data()); - if (SAI_STATUS_SUCCESS != sai_status) + if (creating_zero_pool) { - SWSS_LOG_ERROR("Failed to create buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status); - task_process_status handle_status = handleSaiCreateStatus(SAI_API_BUFFER, sai_status); - if (handle_status != task_process_status::task_success) + if (pool_direction == SAI_BUFFER_POOL_TYPE_INGRESS) { - return handle_status; + sai_object = m_ingressZeroBufferPool; + } + else if (pool_direction == SAI_BUFFER_POOL_TYPE_EGRESS) + { + sai_object = m_egressZeroBufferPool; + } + } + + if (SAI_NULL_OBJECT_ID == sai_object) + { + sai_status = sai_buffer_api->create_buffer_pool(&sai_object, gSwitchId, (uint32_t)attribs.size(), attribs.data()); + if (SAI_STATUS_SUCCESS != sai_status) + { + SWSS_LOG_ERROR("Failed to create buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status); + task_process_status handle_status = handleSaiCreateStatus(SAI_API_BUFFER, sai_status); + if (handle_status != task_process_status::task_success) + { + return handle_status; + } + } + + SWSS_LOG_NOTICE("Created buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str()); + } + else + { + SWSS_LOG_NOTICE("No need to create buffer pool %s since it has been created", object_name.c_str()); + } + + if (creating_zero_pool) + { + if (pool_direction == SAI_BUFFER_POOL_TYPE_INGRESS) + { + m_ingressZeroPoolRefCount++; + m_ingressZeroBufferPool = sai_object; + } + else if (pool_direction == SAI_BUFFER_POOL_TYPE_EGRESS) + { + m_egressZeroPoolRefCount++; + m_egressZeroBufferPool = sai_object; } } + (*(m_buffer_type_maps[map_type_name]))[object_name].m_saiObjectId = sai_object; - SWSS_LOG_NOTICE("Created buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str()); // Here we take the PFC watchdog approach to update the COUNTERS_DB metadata (e.g., PFC_WD_DETECTION_TIME per queue) // at initialization (creation and registration phase) // Specifically, we push the buffer pool name to oid mapping upon the creation of the oid @@ -470,18 +586,40 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) if (SAI_NULL_OBJECT_ID != sai_object) { clearBufferPoolWatermarkCounterIdList(sai_object); - sai_status = sai_buffer_api->remove_buffer_pool(sai_object); - if (SAI_STATUS_SUCCESS != sai_status) + bool remove = true; + if (sai_object == m_ingressZeroBufferPool) { - SWSS_LOG_ERROR("Failed to remove buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status); - task_process_status handle_status = handleSaiRemoveStatus(SAI_API_BUFFER, sai_status); - if (handle_status != task_process_status::task_success) + if (--m_ingressZeroPoolRefCount > 0) + remove = false; + else + m_ingressZeroBufferPool = SAI_NULL_OBJECT_ID; + } + else if (sai_object == m_egressZeroBufferPool) + { + if (--m_egressZeroPoolRefCount > 0) + remove = false; + else + m_egressZeroBufferPool = SAI_NULL_OBJECT_ID; + } + if (remove) + { + sai_status = sai_buffer_api->remove_buffer_pool(sai_object); + if (SAI_STATUS_SUCCESS != sai_status) { - return handle_status; + SWSS_LOG_ERROR("Failed to remove buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_BUFFER, sai_status); + if (handle_status != task_process_status::task_success) + { + return handle_status; + } } + SWSS_LOG_NOTICE("Removed buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str()); + } + else + { + SWSS_LOG_NOTICE("Will not remove buffer pool %s since it is still referenced", object_name.c_str()); } } - SWSS_LOG_NOTICE("Removed buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str()); auto it_to_delete = (m_buffer_type_maps[map_type_name])->find(object_name); (m_buffer_type_maps[map_type_name])->erase(it_to_delete); m_countersDb->hdel(COUNTERS_BUFFER_POOL_NAME_MAP, object_name); diff --git a/orchagent/bufferorch.h b/orchagent/bufferorch.h index 05fdd7917fdc..24af140b4a28 100644 --- a/orchagent/bufferorch.h +++ b/orchagent/bufferorch.h @@ -37,6 +37,14 @@ class BufferOrch : public Orch static type_map m_buffer_type_maps; void generateBufferPoolWatermarkCounterIdList(void); const object_reference_map &getBufferPoolNameOidMap(void); + sai_object_id_t getZeroBufferPool(bool ingress) + { + return ingress ? m_ingressZeroBufferPool : m_egressZeroBufferPool; + } + + void lockZeroBufferPool(bool ingress); + void unlockZeroBufferPool(bool ingress); + void setZeroBufferPool(bool direction, sai_object_id_t pool); private: typedef task_process_status (BufferOrch::*buffer_table_handler)(KeyOpFieldsValuesTuple &tuple); @@ -71,6 +79,11 @@ class BufferOrch : public Orch unique_ptr m_countersDb; bool m_isBufferPoolWatermarkCounterIdListGenerated = false; + + sai_object_id_t m_ingressZeroBufferPool; + sai_object_id_t m_egressZeroBufferPool; + int m_ingressZeroPoolRefCount; + int m_egressZeroPoolRefCount; }; #endif /* SWSS_BUFFORCH_H */ diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp index 05f2d6ef5600..6fb497812df9 100644 --- a/orchagent/pfcactionhandler.cpp +++ b/orchagent/pfcactionhandler.cpp @@ -3,6 +3,7 @@ #include "logger.h" #include "sai_serialize.h" #include "portsorch.h" +#include "bufferorch.h" #include #include @@ -26,6 +27,7 @@ extern sai_object_id_t gSwitchId; extern PortsOrch *gPortsOrch; extern AclOrch * gAclOrch; +extern BufferOrch *gBufferOrch; extern sai_port_api_t *sai_port_api; extern sai_queue_api_t *sai_queue_api; extern sai_buffer_api_t *sai_buffer_api; @@ -715,6 +717,25 @@ PfcWdZeroBufferHandler::ZeroBufferProfile &PfcWdZeroBufferHandler::ZeroBufferPro return instance; } +sai_object_id_t& PfcWdZeroBufferHandler::ZeroBufferProfile::getPool(bool ingress) +{ + // If there is a cached zero buffer pool, just use it + // else fetch zero buffer pool from buffer orch + // If there is one, use it and increase the reference number. + // otherwise, just return NULL OID + // PfcWdZeroBufferHandler will create it later and notify buffer orch later + auto &poolId = ingress ? m_zeroIngressBufferPool : m_zeroEgressBufferPool; + if (poolId == SAI_NULL_OBJECT_ID) + { + poolId = gBufferOrch->getZeroBufferPool(ingress); + if (poolId != SAI_NULL_OBJECT_ID) + { + gBufferOrch->lockZeroBufferPool(ingress); + } + } + return poolId; +} + sai_object_id_t PfcWdZeroBufferHandler::ZeroBufferProfile::getZeroBufferProfile(bool ingress) { SWSS_LOG_ENTER(); @@ -733,29 +754,39 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ing sai_attribute_t attr; vector attribs; + sai_status_t status; - // Create zero pool - attr.id = SAI_BUFFER_POOL_ATTR_SIZE; - attr.value.u64 = 0; - attribs.push_back(attr); + auto &poolId = getPool(ingress); - attr.id = SAI_BUFFER_POOL_ATTR_TYPE; - attr.value.u32 = ingress ? SAI_BUFFER_POOL_TYPE_INGRESS : SAI_BUFFER_POOL_TYPE_EGRESS; - attribs.push_back(attr); + if (SAI_NULL_OBJECT_ID == poolId) + { + // Create zero pool + attr.id = SAI_BUFFER_POOL_ATTR_SIZE; + attr.value.u64 = 0; + attribs.push_back(attr); - attr.id = SAI_BUFFER_POOL_ATTR_THRESHOLD_MODE; - attr.value.u32 = SAI_BUFFER_POOL_THRESHOLD_MODE_DYNAMIC; - attribs.push_back(attr); + attr.id = SAI_BUFFER_POOL_ATTR_TYPE; + attr.value.u32 = ingress ? SAI_BUFFER_POOL_TYPE_INGRESS : SAI_BUFFER_POOL_TYPE_EGRESS; + attribs.push_back(attr); + + attr.id = SAI_BUFFER_POOL_ATTR_THRESHOLD_MODE; + attr.value.u32 = SAI_BUFFER_POOL_THRESHOLD_MODE_STATIC; + attribs.push_back(attr); - sai_status_t status = sai_buffer_api->create_buffer_pool( - &getPool(ingress), + status = sai_buffer_api->create_buffer_pool( + &poolId, gSwitchId, static_cast(attribs.size()), attribs.data()); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to create dynamic zero buffer pool for PFC WD: %d", status); - return; + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create dynamic zero buffer pool for PFC WD: %d", status); + return; + } + + // Pass the ownership to BufferOrch + gBufferOrch->setZeroBufferPool(ingress, poolId); + gBufferOrch->lockZeroBufferPool(ingress); } // Create zero profile @@ -766,15 +797,15 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ing attribs.push_back(attr); attr.id = SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE; - attr.value.u32 = SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC; + attr.value.u32 = SAI_BUFFER_PROFILE_THRESHOLD_MODE_STATIC; attribs.push_back(attr); attr.id = SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE; attr.value.u64 = 0; attribs.push_back(attr); - attr.id = SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH; - attr.value.s8 = -8; // ALPHA_0 + attr.id = SAI_BUFFER_PROFILE_ATTR_SHARED_STATIC_TH; + attr.value.s8 = 0; attribs.push_back(attr); status = sai_buffer_api->create_buffer_profile( @@ -793,16 +824,19 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::destroyZeroBufferProfile(bool in { SWSS_LOG_ENTER(); - sai_status_t status = sai_buffer_api->remove_buffer_profile(getProfile(ingress)); - if (status != SAI_STATUS_SUCCESS) + if (getProfile(ingress) != SAI_NULL_OBJECT_ID) { - SWSS_LOG_ERROR("Failed to remove static zero buffer profile for PFC WD: %d", status); - return; + sai_status_t status = sai_buffer_api->remove_buffer_profile(getProfile(ingress)); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove static zero buffer profile for PFC WD: %d", status); + return; + } } - status = sai_buffer_api->remove_buffer_pool(getPool(ingress)); - if (status != SAI_STATUS_SUCCESS) + auto &pool = ingress ? m_zeroIngressBufferPool : m_zeroEgressBufferPool; + if (pool != SAI_NULL_OBJECT_ID) { - SWSS_LOG_ERROR("Failed to remove static zero buffer pool for PFC WD: %d", status); + gBufferOrch->unlockZeroBufferPool(ingress); } } diff --git a/orchagent/pfcactionhandler.h b/orchagent/pfcactionhandler.h index a55ac3b4a435..22908fbe0850 100644 --- a/orchagent/pfcactionhandler.h +++ b/orchagent/pfcactionhandler.h @@ -148,10 +148,7 @@ class PfcWdZeroBufferHandler: public PfcWdLossyHandler return ingress ? m_zeroIngressBufferProfile : m_zeroEgressBufferProfile; } - sai_object_id_t& getPool(bool ingress) - { - return ingress ? m_zeroIngressBufferPool : m_zeroEgressBufferPool; - } + sai_object_id_t& getPool(bool ingress); sai_object_id_t m_zeroIngressBufferPool = SAI_NULL_OBJECT_ID; sai_object_id_t m_zeroEgressBufferPool = SAI_NULL_OBJECT_ID; diff --git a/tests/mock_tests/mock_orchagent_main.h b/tests/mock_tests/mock_orchagent_main.h index a194d3ea0aa5..3166f3d9624f 100644 --- a/tests/mock_tests/mock_orchagent_main.h +++ b/tests/mock_tests/mock_orchagent_main.h @@ -9,7 +9,9 @@ #include "neighorch.h" #include "fdborch.h" #include "mirrororch.h" +#define private public #include "bufferorch.h" +#undef private #include "qosorch.h" #include "vrforch.h" #include "vnetorch.h" diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index 853fdbfb6988..28df6610fdf6 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -7,7 +7,9 @@ #include "mock_orchagent_main.h" #include "mock_table.h" #include "notifier.h" +#define private public #include "pfcactionhandler.h" +#undef private #include @@ -18,6 +20,105 @@ namespace portsorch_test using namespace std; + sai_queue_api_t ut_sai_queue_api; + sai_queue_api_t *pold_sai_queue_api; + sai_buffer_api_t ut_sai_buffer_api; + sai_buffer_api_t *pold_sai_buffer_api; + + string _ut_stub_queue_key; + sai_status_t _ut_stub_sai_get_queue_attribute( + _In_ sai_object_id_t queue_id, + _In_ uint32_t attr_count, + _Inout_ sai_attribute_t *attr_list) + { + if (attr_count == 1 && attr_list[0].id == SAI_QUEUE_ATTR_BUFFER_PROFILE_ID) + { + auto &typemapQueue = (*gBufferOrch->m_buffer_type_maps[APP_BUFFER_QUEUE_TABLE_NAME]); + auto &profileName = typemapQueue["Ethernet0:3-4"].m_objsReferencingByMe["profile"]; + auto profileNameVec = tokenize(profileName, ':'); + auto &typemapProfile = (*gBufferOrch->m_buffer_type_maps[APP_BUFFER_PROFILE_TABLE_NAME]); + attr_list[0].value.oid = typemapProfile[profileNameVec[1]].m_saiObjectId; + return SAI_STATUS_SUCCESS; + } + else + { + return pold_sai_queue_api->get_queue_attribute(queue_id, attr_count, attr_list); + } + } + + sai_status_t _ut_stub_sai_get_ingress_priority_group_attribute( + _In_ sai_object_id_t ingress_priority_group_id, + _In_ uint32_t attr_count, + _Inout_ sai_attribute_t *attr_list) + { + if (attr_count == 1 && attr_list[0].id == SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE) + { + auto &typemapPg = (*gBufferOrch->m_buffer_type_maps[APP_BUFFER_PG_TABLE_NAME]); + auto &profileName = typemapPg["Ethernet0:3-4"].m_objsReferencingByMe["profile"]; + auto profileNameVec = tokenize(profileName, ':'); + auto &typemapProfile = (*gBufferOrch->m_buffer_type_maps[APP_BUFFER_PROFILE_TABLE_NAME]); + attr_list[0].value.oid = typemapProfile[profileNameVec[1]].m_saiObjectId; + return SAI_STATUS_SUCCESS; + } + else + { + return pold_sai_buffer_api->get_ingress_priority_group_attribute(ingress_priority_group_id, attr_count, attr_list); + } + } + + int _sai_create_buffer_pool_count = 0; + sai_status_t _ut_stub_sai_create_buffer_pool( + _Out_ sai_object_id_t *buffer_pool_id, + _In_ sai_object_id_t switch_id, + _In_ uint32_t attr_count, + _In_ const sai_attribute_t *attr_list) + { + auto status = pold_sai_buffer_api->create_buffer_pool(buffer_pool_id, switch_id, attr_count, attr_list); + if (SAI_STATUS_SUCCESS == status) + _sai_create_buffer_pool_count++; + return status; + } + + int _sai_remove_buffer_pool_count = 0; + sai_status_t _ut_stub_sai_remove_buffer_pool( + _In_ sai_object_id_t buffer_pool_id) + { + auto status = pold_sai_buffer_api->remove_buffer_pool(buffer_pool_id); + if (SAI_STATUS_SUCCESS == status) + _sai_remove_buffer_pool_count++; + return status; + } + + void _hook_sai_buffer_and_queue_api() + { + ut_sai_buffer_api = *sai_buffer_api; + pold_sai_buffer_api = sai_buffer_api; + ut_sai_buffer_api.create_buffer_pool = _ut_stub_sai_create_buffer_pool; + ut_sai_buffer_api.remove_buffer_pool = _ut_stub_sai_remove_buffer_pool; + ut_sai_buffer_api.get_ingress_priority_group_attribute = _ut_stub_sai_get_ingress_priority_group_attribute; + sai_buffer_api = &ut_sai_buffer_api; + + ut_sai_queue_api = *sai_queue_api; + pold_sai_queue_api = sai_queue_api; + ut_sai_queue_api.get_queue_attribute = _ut_stub_sai_get_queue_attribute; + sai_queue_api = &ut_sai_queue_api; + } + + void _unhook_sai_buffer_and_queue_api() + { + sai_buffer_api = pold_sai_buffer_api; + sai_queue_api = pold_sai_queue_api; + } + + void clear_pfcwd_zero_buffer_handler() + { + auto &zeroProfile = PfcWdZeroBufferHandler::ZeroBufferProfile::getInstance(); + zeroProfile.m_zeroIngressBufferPool = SAI_NULL_OBJECT_ID; + zeroProfile.m_zeroEgressBufferPool = SAI_NULL_OBJECT_ID; + zeroProfile.m_zeroIngressBufferProfile = SAI_NULL_OBJECT_ID; + zeroProfile.m_zeroEgressBufferProfile = SAI_NULL_OBJECT_ID; + } + struct PortsOrchTest : public ::testing::Test { shared_ptr m_app_db; @@ -103,6 +204,12 @@ namespace portsorch_test { ::testing_db::reset(); + auto buffer_maps = BufferOrch::m_buffer_type_maps; + for (auto &i : buffer_maps) + { + i.second->clear(); + } + delete gNeighOrch; gNeighOrch = nullptr; delete gFdbOrch; @@ -355,10 +462,12 @@ namespace portsorch_test TEST_F(PortsOrchTest, PfcZeroBufferHandlerLocksPortPgAndQueue) { + _hook_sai_buffer_and_queue_api(); Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); Table pgTable = Table(m_app_db.get(), APP_BUFFER_PG_TABLE_NAME); Table profileTable = Table(m_app_db.get(), APP_BUFFER_PROFILE_TABLE_NAME); Table poolTable = Table(m_app_db.get(), APP_BUFFER_POOL_TABLE_NAME); + Table queueTable = Table(m_app_db.get(), APP_BUFFER_QUEUE_TABLE_NAME); // Get SAI default ports to populate DB auto ports = ut_helper::getInitialSaiPorts(); @@ -397,39 +506,71 @@ namespace portsorch_test Port port; gPortsOrch->getPort("Ethernet0", port); - auto countersTable = make_shared(m_counters_db.get(), COUNTERS_TABLE); - auto dropHandler = make_unique(port.m_port_id, port.m_queue_ids[3], 3, countersTable); - // Create test buffer pool poolTable.set( - "test_pool", + "ingress_pool", { { "type", "ingress" }, { "mode", "dynamic" }, { "size", "4200000" }, }); + poolTable.set( + "egress_pool", + { + { "type", "egress" }, + { "mode", "dynamic" }, + { "size", "4200000" }, + }); // Create test buffer profile - profileTable.set("test_profile", { { "pool", "test_pool" }, + profileTable.set("test_profile", { { "pool", "ingress_pool" }, { "xon", "14832" }, { "xoff", "14832" }, { "size", "35000" }, { "dynamic_th", "0" } }); + profileTable.set("ingress_profile", { { "pool", "ingress_pool" }, + { "xon", "14832" }, + { "xoff", "14832" }, + { "size", "35000" }, + { "dynamic_th", "0" } }); + profileTable.set("egress_profile", { { "pool", "egress_pool" }, + { "size", "0" }, + { "dynamic_th", "0" } }); // Apply profile on PGs 3-4 all ports for (const auto &it : ports) { std::ostringstream oss; oss << it.first << ":3-4"; - pgTable.set(oss.str(), { { "profile", "test_profile" } }); + pgTable.set(oss.str(), { { "profile", "ingress_profile" } }); + queueTable.set(oss.str(), { {"profile", "egress_profile" } }); } gBufferOrch->addExistingData(&pgTable); gBufferOrch->addExistingData(&poolTable); gBufferOrch->addExistingData(&profileTable); + gBufferOrch->addExistingData(&queueTable); // process pool, profile and PGs static_cast(gBufferOrch)->doTask(); + auto countersTable = make_shared
(m_counters_db.get(), COUNTERS_TABLE); + auto current_create_buffer_pool_count = _sai_create_buffer_pool_count; + auto dropHandler = make_unique(port.m_port_id, port.m_queue_ids[3], 3, countersTable); + + current_create_buffer_pool_count += 2; + ASSERT_TRUE(current_create_buffer_pool_count == _sai_create_buffer_pool_count); + ASSERT_TRUE(PfcWdZeroBufferHandler::ZeroBufferProfile::getInstance().getPool(true) == gBufferOrch->m_ingressZeroBufferPool); + ASSERT_TRUE(PfcWdZeroBufferHandler::ZeroBufferProfile::getInstance().getPool(false) == gBufferOrch->m_egressZeroBufferPool); + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 1); + ASSERT_TRUE(gBufferOrch->m_egressZeroPoolRefCount == 1); + + std::deque entries; + entries.push_back({"Ethernet0:3-4", "SET", {{ "profile", "test_profile"}}}); + auto pgConsumer = static_cast(gBufferOrch->getExecutor(APP_BUFFER_PG_TABLE_NAME)); + pgConsumer->addToSync(entries); + entries.clear(); + static_cast(gBufferOrch)->doTask(); + // Port should have been updated by BufferOrch->doTask gPortsOrch->getPort("Ethernet0", port); auto profile_id = (*BufferOrch::m_buffer_type_maps["BUFFER_PROFILE_TABLE"])[string("test_profile")].m_saiObjectId; @@ -437,11 +578,32 @@ namespace portsorch_test ASSERT_TRUE(port.m_priority_group_pending_profile[3] == profile_id); ASSERT_TRUE(port.m_priority_group_pending_profile[4] == SAI_NULL_OBJECT_ID); - auto pgConsumer = static_cast(gBufferOrch->getExecutor(APP_BUFFER_PG_TABLE_NAME)); + pgConsumer = static_cast(gBufferOrch->getExecutor(APP_BUFFER_PG_TABLE_NAME)); pgConsumer->dumpPendingTasks(ts); ASSERT_TRUE(ts.empty()); // PG is stored in m_priority_group_pending_profile ts.clear(); + // Create a zero buffer pool after PFC storm + entries.push_back({"ingress_zero_pool", "SET", {{ "type", "ingress" }, + { "mode", "static" }, + { "size", "0" }}}); + auto poolConsumer = static_cast(gBufferOrch->getExecutor(APP_BUFFER_POOL_TABLE_NAME)); + poolConsumer->addToSync(entries); + entries.clear(); + static_cast(gBufferOrch)->doTask(); + // Reference increased + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 2); + // Didn't create buffer pool again + ASSERT_TRUE(_sai_create_buffer_pool_count == current_create_buffer_pool_count); + + entries.push_back({"ingress_zero_pool", "DEL", {}}); + poolConsumer->addToSync(entries); + entries.clear(); + auto current_remove_buffer_pool_count = _sai_remove_buffer_pool_count; + static_cast(gBufferOrch)->doTask(); + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 1); + ASSERT_TRUE(_sai_remove_buffer_pool_count == current_remove_buffer_pool_count); + // release zero buffer drop handler dropHandler.reset(); @@ -459,6 +621,139 @@ namespace portsorch_test pgConsumer->dumpPendingTasks(ts); ASSERT_TRUE(ts.empty()); // PG should be processed now ts.clear(); + clear_pfcwd_zero_buffer_handler(); + _unhook_sai_buffer_and_queue_api(); + } + + TEST_F(PortsOrchTest, PfcZeroBufferHandlerLocksPortWithZeroPoolCreated) + { + _hook_sai_buffer_and_queue_api(); + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + Table pgTable = Table(m_app_db.get(), APP_BUFFER_PG_TABLE_NAME); + Table profileTable = Table(m_app_db.get(), APP_BUFFER_PROFILE_TABLE_NAME); + Table poolTable = Table(m_app_db.get(), APP_BUFFER_POOL_TABLE_NAME); + Table queueTable = Table(m_app_db.get(), APP_BUFFER_QUEUE_TABLE_NAME); + + // Get SAI default ports to populate DB + auto ports = ut_helper::getInitialSaiPorts(); + + // Populate port table with SAI ports + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone, PortInitDone + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + portTable.set("PortInitDone", { { "lanes", "0" } }); + + // refill consumer + gPortsOrch->addExistingData(&portTable); + + // Apply configuration : + // create ports + + static_cast(gPortsOrch)->doTask(); + + // Apply configuration + // ports + static_cast(gPortsOrch)->doTask(); + + ASSERT_TRUE(gPortsOrch->allPortsReady()); + + // No more tasks + vector ts; + gPortsOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + ts.clear(); + + // Simulate storm drop handler started on Ethernet0 TC 3 + Port port; + gPortsOrch->getPort("Ethernet0", port); + + // Create test buffer pool + poolTable.set("ingress_pool", + { + { "type", "ingress" }, + { "mode", "dynamic" }, + { "size", "4200000" }, + }); + poolTable.set("egress_pool", + { + { "type", "egress" }, + { "mode", "dynamic" }, + { "size", "4200000" }, + }); + poolTable.set("ingress_zero_pool", + { + { "type", "ingress" }, + { "mode", "static" }, + { "size", "0" } + }); + auto poolConsumer = static_cast(gBufferOrch->getExecutor(APP_BUFFER_POOL_TABLE_NAME)); + + // Create test buffer profile + profileTable.set("ingress_profile", { { "pool", "ingress_pool" }, + { "xon", "14832" }, + { "xoff", "14832" }, + { "size", "35000" }, + { "dynamic_th", "0" } }); + profileTable.set("egress_profile", { { "pool", "egress_pool" }, + { "size", "0" }, + { "dynamic_th", "0" } }); + + // Apply profile on PGs 3-4 all ports + for (const auto &it : ports) + { + std::ostringstream oss; + oss << it.first << ":3-4"; + pgTable.set(oss.str(), { { "profile", "ingress_profile" } }); + queueTable.set(oss.str(), { {"profile", "egress_profile" } }); + } + + gBufferOrch->addExistingData(&poolTable); + gBufferOrch->addExistingData(&profileTable); + gBufferOrch->addExistingData(&pgTable); + gBufferOrch->addExistingData(&queueTable); + + auto current_create_buffer_pool_count = _sai_create_buffer_pool_count + 3; // call SAI API create_buffer_pool for each pool + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 0); + ASSERT_TRUE(gBufferOrch->m_egressZeroPoolRefCount == 0); + ASSERT_TRUE(gBufferOrch->m_ingressZeroBufferPool == SAI_NULL_OBJECT_ID); + ASSERT_TRUE(gBufferOrch->m_egressZeroBufferPool == SAI_NULL_OBJECT_ID); + + // process pool, profile and PGs + static_cast(gBufferOrch)->doTask(); + + ASSERT_TRUE(current_create_buffer_pool_count == _sai_create_buffer_pool_count); + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 1); + ASSERT_TRUE(gBufferOrch->m_egressZeroPoolRefCount == 0); + ASSERT_TRUE(gBufferOrch->m_ingressZeroBufferPool != SAI_NULL_OBJECT_ID); + ASSERT_TRUE(gBufferOrch->m_egressZeroBufferPool == SAI_NULL_OBJECT_ID); + + auto countersTable = make_shared
(m_counters_db.get(), COUNTERS_TABLE); + auto dropHandler = make_unique(port.m_port_id, port.m_queue_ids[3], 3, countersTable); + + current_create_buffer_pool_count++; // Increased for egress zero pool + ASSERT_TRUE(current_create_buffer_pool_count == _sai_create_buffer_pool_count); + ASSERT_TRUE(PfcWdZeroBufferHandler::ZeroBufferProfile::getInstance().getPool(true) == gBufferOrch->m_ingressZeroBufferPool); + ASSERT_TRUE(PfcWdZeroBufferHandler::ZeroBufferProfile::getInstance().getPool(false) == gBufferOrch->m_egressZeroBufferPool); + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 2); + ASSERT_TRUE(gBufferOrch->m_egressZeroPoolRefCount == 1); + + std::deque entries; + entries.push_back({"ingress_zero_pool", "DEL", {}}); + poolConsumer->addToSync(entries); + entries.clear(); + auto current_remove_buffer_pool_count = _sai_remove_buffer_pool_count; + static_cast(gBufferOrch)->doTask(); + ASSERT_TRUE(gBufferOrch->m_ingressZeroPoolRefCount == 1); + ASSERT_TRUE(_sai_remove_buffer_pool_count == current_remove_buffer_pool_count); + + // release zero buffer drop handler + dropHandler.reset(); + clear_pfcwd_zero_buffer_handler(); + _unhook_sai_buffer_and_queue_api(); } /* This test checks that a LAG member validation happens on orchagent level