From c3437bd224a7acd600e1229fa94ba12d405c1e1f Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 17 Feb 2025 10:36:06 +0200 Subject: [PATCH] [202411][FC] process FC after apply view Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 80 ++++++++----------- orchagent/flexcounterorch.h | 6 +- .../p4orch/tests/fake_flexcounterorch.cpp | 6 +- tests/mock_tests/flexcounter_ut.cpp | 53 ++++++++++-- 4 files changed, 91 insertions(+), 54 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 6fa37f49508..efcbe242f5e 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -15,6 +15,7 @@ #include "macsecorch.h" #include "dash/dashorch.h" #include "flowcounterrouteorch.h" +#include "warm_restart.h" extern sai_port_api_t *sai_port_api; extern sai_switch_api_t *sai_switch_api; @@ -28,6 +29,8 @@ extern CoppOrch *gCoppOrch; extern FlowCounterRouteOrch *gFlowCounterRouteOrch; extern sai_object_id_t gSwitchId; +#define FLEX_COUNTER_DELAY_SEC 60 + #define BUFFER_POOL_WATERMARK_KEY "BUFFER_POOL_WATERMARK" #define PORT_KEY "PORT" #define PORT_BUFFER_DROP_KEY "PORT_BUFFER_DROP" @@ -69,12 +72,22 @@ unordered_map flexCounterGroupMap = FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector &tableNames): Orch(db, tableNames), - m_flexCounterConfigTable(db, CFG_FLEX_COUNTER_TABLE_NAME), m_bufferQueueConfigTable(db, CFG_BUFFER_QUEUE_TABLE_NAME), m_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME), m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME) { SWSS_LOG_ENTER(); + m_delayTimer = std::make_unique(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); + if (WarmStart::isWarmStart()) + { + m_delayExecutor = std::make_unique(m_delayTimer.get(), this, "FLEX_COUNTER_DELAY"); + Orch::addExecutor(m_delayExecutor.get()); + m_delayTimer->start(); + } + else + { + m_delayTimerExpired = true; + } } FlexCounterOrch::~FlexCounterOrch(void) @@ -86,6 +99,11 @@ void FlexCounterOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); + if (!m_delayTimerExpired) + { + return; + } + VxlanTunnelOrch* vxlan_tunnel_orch = gDirectory.get(); DashOrch* dash_orch = gDirectory.get(); if (gPortsOrch && !gPortsOrch->allPortsReady()) @@ -116,16 +134,9 @@ void FlexCounterOrch::doTask(Consumer &consumer) if (op == SET_COMMAND) { - auto itDelay = std::find(std::begin(data), std::end(data), FieldValueTuple(FLEX_COUNTER_DELAY_STATUS_FIELD, "true")); - string poll_interval; string bulk_chunk_size; string bulk_chunk_size_per_counter; - if (itDelay != data.end()) - { - consumer.m_toSync.erase(it++); - continue; - } for (auto valuePair:data) { const auto &field = fvField(valuePair); @@ -255,12 +266,6 @@ void FlexCounterOrch::doTask(Consumer &consumer) } } } - else if(field == FLEX_COUNTER_DELAY_STATUS_FIELD) - { - // This field is ignored since it is being used before getting into this loop. - // If it is exist and the value is 'true' we need to skip the iteration in order to delay the counter creation. - // The field will clear out and counter will be created when enable_counters script is called. - } else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); @@ -285,6 +290,20 @@ void FlexCounterOrch::doTask(Consumer &consumer) } } +void FlexCounterOrch::doTask(SelectableTimer&) +{ + SWSS_LOG_ENTER(); + + if (m_delayTimerExpired) + { + return; + } + + SWSS_LOG_NOTICE("Processing counters"); + m_delayTimer->stop(); + m_delayTimerExpired = true; +} + bool FlexCounterOrch::getPortCountersState() const { return m_port_counter_enabled; @@ -322,39 +341,10 @@ bool FlexCounterOrch::bake() * By default, it should fetch items from the tables the sub agents listen to, * and then push them into m_toSync of each sub agent. * The motivation is to make sub agents handle the saved entries first and then handle the upcoming entries. + * The FCs are not data plane configuration required during reconciling process, hence don't do anything in bake. */ - std::deque entries; - vector keys; - m_flexCounterConfigTable.getKeys(keys); - for (const auto &key: keys) - { - if (!flexCounterGroupMap.count(key)) - { - SWSS_LOG_NOTICE("FlexCounterOrch: Invalid flex counter group intput %s is skipped during reconciling", key.c_str()); - continue; - } - - if (key == BUFFER_POOL_WATERMARK_KEY) - { - SWSS_LOG_NOTICE("FlexCounterOrch: Do not handle any FLEX_COUNTER table for %s update during reconciling", - BUFFER_POOL_WATERMARK_KEY); - continue; - } - - KeyOpFieldsValuesTuple kco; - - kfvKey(kco) = key; - kfvOp(kco) = SET_COMMAND; - - if (!m_flexCounterConfigTable.get(key, kfvFieldsValues(kco))) - { - continue; - } - entries.push_back(kco); - } - Consumer* consumer = dynamic_cast(getExecutor(CFG_FLEX_COUNTER_TABLE_NAME)); - return consumer->addToSync(entries); + return true; } static bool isCreateOnlyConfigDbBuffers(Table& deviceMetadataConfigTable) diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index f3aa03e6c0e..75eb4cb6522 100644 --- a/orchagent/flexcounterorch.h +++ b/orchagent/flexcounterorch.h @@ -4,6 +4,7 @@ #include "orch.h" #include "port.h" #include "producertable.h" +#include "selectabletimer.h" #include "table.h" extern "C" { @@ -40,6 +41,7 @@ class FlexCounterOrch: public Orch { public: void doTask(Consumer &consumer); + void doTask(SelectableTimer &timer); FlexCounterOrch(swss::DBConnector *db, std::vector &tableNames); virtual ~FlexCounterOrch(void); bool getPortCountersState() const; @@ -63,11 +65,13 @@ class FlexCounterOrch: public Orch bool m_pg_watermark_enabled = false; bool m_hostif_trap_counter_enabled = false; bool m_route_flow_counter_enabled = false; - Table m_flexCounterConfigTable; + bool m_delayTimerExpired = false; Table m_bufferQueueConfigTable; Table m_bufferPgConfigTable; Table m_deviceMetadataConfigTable; std::unordered_set m_groupsWithBulkChunkSize; + std::unique_ptr m_delayTimer; + std::unique_ptr m_delayExecutor; }; #endif diff --git a/orchagent/p4orch/tests/fake_flexcounterorch.cpp b/orchagent/p4orch/tests/fake_flexcounterorch.cpp index 91d6be3d14c..d26e39aea6e 100644 --- a/orchagent/p4orch/tests/fake_flexcounterorch.cpp +++ b/orchagent/p4orch/tests/fake_flexcounterorch.cpp @@ -2,7 +2,7 @@ #include "flexcounterorch.h" FlexCounterOrch::FlexCounterOrch(swss::DBConnector *db, std::vector &tableNames) - : Orch(db, tableNames), m_flexCounterConfigTable(db, CFG_FLEX_COUNTER_TABLE_NAME), + : Orch(db, tableNames), m_bufferQueueConfigTable(db, CFG_BUFFER_QUEUE_TABLE_NAME), m_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME), m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME) { @@ -16,6 +16,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) { } +void FlexCounterOrch::doTask(SelectableTimer &timer) +{ +} + bool FlexCounterOrch::getPortCountersState() const { return true; diff --git a/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index 22e3bdab656..686b818984e 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -220,7 +220,13 @@ namespace flexcounter_test sai_switch_api = pold_sai_switch_api; } - struct FlexCounterTest : public ::testing::TestWithParam> + enum class StartType + { + Cold, + Warm, + }; + + struct FlexCounterTest : public ::testing::TestWithParam> { shared_ptr m_app_db; shared_ptr m_config_db; @@ -230,6 +236,7 @@ namespace flexcounter_test shared_ptr m_asic_db; shared_ptr m_flex_counter_db; bool create_only_config_db_buffers; + StartType m_start_type; FlexCounterTest() { @@ -256,6 +263,8 @@ namespace flexcounter_test gTraditionalFlexCounter = get<0>(GetParam()); create_only_config_db_buffers = get<1>(GetParam()); + m_start_type = get<2>(GetParam()); + if (gTraditionalFlexCounter) { initFlexCounterTables(); @@ -300,7 +309,19 @@ namespace flexcounter_test vector flex_counter_tables = { CFG_FLEX_COUNTER_TABLE_NAME }; + + if (m_start_type == StartType::Warm) + { + WarmStart::getInstance().m_enabled = true; + } + auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables); + + if (m_start_type == StartType::Warm) + { + WarmStart::getInstance().m_enabled = false; + } + gDirectory.set(flexCounterOrch); vector buffer_tables = { APP_BUFFER_POOL_TABLE_NAME, @@ -542,8 +563,7 @@ namespace flexcounter_test ASSERT_TRUE(gPortsOrch->allPortsReady()); // Enable and check counters - const std::vector values({ {FLEX_COUNTER_DELAY_STATUS_FIELD, "false"}, - {FLEX_COUNTER_STATUS_FIELD, "enable"} }); + const std::vector values({ {FLEX_COUNTER_STATUS_FIELD, "enable"} }); flexCounterCfg.set("PG_WATERMARK", values); flexCounterCfg.set("QUEUE_WATERMARK", values); flexCounterCfg.set("QUEUE", values); @@ -557,6 +577,13 @@ namespace flexcounter_test flexCounterOrch->addExistingData(&flexCounterCfg); static_cast(flexCounterOrch)->doTask(); + if (m_start_type == StartType::Warm) + { + // Expire timer + flexCounterOrch->doTask(*flexCounterOrch->m_delayTimer); + static_cast(flexCounterOrch)->doTask(); + } + ASSERT_TRUE(checkFlexCounterGroup(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, { {POLL_INTERVAL_FIELD, "60000"}, @@ -878,16 +905,28 @@ namespace flexcounter_test entries.clear(); static_cast(gBufferOrch)->doTask(); ASSERT_TRUE(checkFlexCounter(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, pool_oid)); + + // Warm/fast-boot case - no FC processing done before APPLY_VIEW + std::vector ts; + + gDirectory.get()->bake(); + gDirectory.get()->dumpPendingTasks(ts); + + ASSERT_TRUE(ts.empty()); } INSTANTIATE_TEST_CASE_P( FlexCounterTests, FlexCounterTest, ::testing::Values( - std::make_tuple(false, true), - std::make_tuple(false, false), - std::make_tuple(true, true), - std::make_tuple(true, false)) + std::make_tuple(false, true, StartType::Cold), + std::make_tuple(false, false, StartType::Cold), + std::make_tuple(true, true, StartType::Cold), + std::make_tuple(true, false, StartType::Cold), + std::make_tuple(false, true, StartType::Warm), + std::make_tuple(false, false, StartType::Warm), + std::make_tuple(true, true, StartType::Warm), + std::make_tuple(true, false, StartType::Warm)) ); using namespace mock_orch_test;