From 87501b2dbd740b1f7ffa6bdffda6f1dfe816bfcb Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 11 Oct 2024 16:06:12 +0000 Subject: [PATCH 01/11] [FC] process FC after apply view Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 47 ++--------------------------- tests/mock_tests/flexcounter_ut.cpp | 11 +++++-- 2 files changed, 11 insertions(+), 47 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 705622bfa0c..b45aadd1a73 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -116,14 +116,6 @@ 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; - - if (itDelay != data.end()) - { - consumer.m_toSync.erase(it++); - continue; - } for (auto valuePair:data) { const auto &field = fvField(valuePair); @@ -245,12 +237,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()); @@ -299,39 +285,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/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index b99f9cc25d0..a17408e3425 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -536,8 +536,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); @@ -831,6 +830,14 @@ 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( From 47fb40f6675aae6ffe94ed498f30fc8de69fac9f Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 15 Oct 2024 11:11:02 +0000 Subject: [PATCH 02/11] Remove m_flexCounterConfigTable Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 1 - orchagent/flexcounterorch.h | 1 - orchagent/p4orch/tests/fake_flexcounterorch.cpp | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index b45aadd1a73..4ef8a3772c6 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -69,7 +69,6 @@ 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) diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index 4bc74dc3b81..330b8465792 100644 --- a/orchagent/flexcounterorch.h +++ b/orchagent/flexcounterorch.h @@ -63,7 +63,6 @@ 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; Table m_bufferQueueConfigTable; Table m_bufferPgConfigTable; Table m_deviceMetadataConfigTable; diff --git a/orchagent/p4orch/tests/fake_flexcounterorch.cpp b/orchagent/p4orch/tests/fake_flexcounterorch.cpp index 91d6be3d14c..476b8d69890 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) { From 017a5695eeb8eed72771ce54dc229f3da9460c3f Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 12 Nov 2024 11:31:55 +0000 Subject: [PATCH 03/11] delay counters by 60 sec Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 20 +++++++++++++++++++ orchagent/flexcounterorch.h | 4 ++++ .../p4orch/tests/fake_flexcounterorch.cpp | 4 ++++ tests/mock_tests/flexcounter_ut.cpp | 1 + tests/mock_tests/flowcounterrouteorch_ut.cpp | 1 + 5 files changed, 30 insertions(+) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 4ef8a3772c6..19dc65fb016 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -28,6 +28,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" @@ -74,6 +76,10 @@ FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector &tableNames): m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME) { SWSS_LOG_ENTER(); + m_delayTimer = new SelectableTimer(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); + auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); + Orch::addExecutor(executor); + m_delayTimer->start(); } FlexCounterOrch::~FlexCounterOrch(void) @@ -85,6 +91,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()) @@ -247,6 +258,15 @@ void FlexCounterOrch::doTask(Consumer &consumer) } } +void FlexCounterOrch::doTask(SelectableTimer &timer) +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("Processing counters"); + m_delayTimer->stop(); + m_delayTimerExpired = true; +} + bool FlexCounterOrch::getPortCountersState() const { return m_port_counter_enabled; diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index 330b8465792..0036bf628ef 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,9 +65,11 @@ class FlexCounterOrch: public Orch bool m_pg_watermark_enabled = false; bool m_hostif_trap_counter_enabled = false; bool m_route_flow_counter_enabled = false; + bool m_delayTimerExpired = false; Table m_bufferQueueConfigTable; Table m_bufferPgConfigTable; Table m_deviceMetadataConfigTable; + SelectableTimer* m_delayTimer; }; #endif diff --git a/orchagent/p4orch/tests/fake_flexcounterorch.cpp b/orchagent/p4orch/tests/fake_flexcounterorch.cpp index 476b8d69890..d26e39aea6e 100644 --- a/orchagent/p4orch/tests/fake_flexcounterorch.cpp +++ b/orchagent/p4orch/tests/fake_flexcounterorch.cpp @@ -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 a17408e3425..2a2247f91f6 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -295,6 +295,7 @@ namespace flexcounter_test CFG_FLEX_COUNTER_TABLE_NAME }; auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables); + flexCounterOrch->m_delayTimerExpired = true; gDirectory.set(flexCounterOrch); vector buffer_tables = { APP_BUFFER_POOL_TABLE_NAME, diff --git a/tests/mock_tests/flowcounterrouteorch_ut.cpp b/tests/mock_tests/flowcounterrouteorch_ut.cpp index 1b031515b30..f5184edfc15 100644 --- a/tests/mock_tests/flowcounterrouteorch_ut.cpp +++ b/tests/mock_tests/flowcounterrouteorch_ut.cpp @@ -131,6 +131,7 @@ namespace flowcounterrouteorch_test CFG_FLEX_COUNTER_TABLE_NAME }; auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables); + flexCounterOrch->m_delayTimerExpired = true; gDirectory.set(flexCounterOrch); ASSERT_EQ(gPortsOrch, nullptr); From ada9e59afcc4675adf4bcf3e84030cd4e5cf0a58 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 13 Nov 2024 16:52:41 +0000 Subject: [PATCH 04/11] Delay for warm/fast start Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 19dc65fb016..5ae5d6afacf 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; @@ -77,9 +78,16 @@ FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector &tableNames): { SWSS_LOG_ENTER(); m_delayTimer = new SelectableTimer(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); - auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); - Orch::addExecutor(executor); - m_delayTimer->start(); + if (WarmStart::isWarmStart()) + { + auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); + Orch::addExecutor(executor); + m_delayTimer->start(); + } + else + { + m_delayTimerExpired = true; + } } FlexCounterOrch::~FlexCounterOrch(void) From 956f1f6a4d39447d8fe1a47df12c6db631157ba8 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 14 Nov 2024 13:46:13 +0000 Subject: [PATCH 05/11] Handle review comment Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 5ae5d6afacf..432efa8bd8a 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -266,10 +266,15 @@ void FlexCounterOrch::doTask(Consumer &consumer) } } -void FlexCounterOrch::doTask(SelectableTimer &timer) +void FlexCounterOrch::doTask(SelectableTimer&) { SWSS_LOG_ENTER(); + if (m_delayTimerExpired) + { + return; + } + SWSS_LOG_NOTICE("Processing counters"); m_delayTimer->stop(); m_delayTimerExpired = true; From 12eea05755ab138af3e8e12b3f164478d75bd185 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 14 Nov 2024 14:50:26 +0000 Subject: [PATCH 06/11] use env variable to delay Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 13 ++++++++----- tests/conftest.py | 4 +++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 432efa8bd8a..7d59108d16f 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -77,16 +77,19 @@ FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector &tableNames): m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME) { SWSS_LOG_ENTER(); + m_delayTimer = new SelectableTimer(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); - if (WarmStart::isWarmStart()) + const string no_flex_counter_delay = getenv("NO_FLEX_COUNTER_DELAY") ?: ""; + if (no_flex_counter_delay == "1") { - auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); - Orch::addExecutor(executor); - m_delayTimer->start(); + m_delayTimerExpired = true; } else { - m_delayTimerExpired = true; + m_delayTimerExpired = false; + auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); + Orch::addExecutor(executor); + m_delayTimer->start(); } } diff --git a/tests/conftest.py b/tests/conftest.py index c94224539bf..b075d548fe5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -406,6 +406,8 @@ def __init__( vols[k] = v kwargs["volumes"] = vols + env = ["NO_FLEX_COUNTER_DELAY=1"] + env; + # create virtual switch container self.ctn = self.client.containers.run(imgname, privileged=True, @@ -1445,7 +1447,7 @@ def __init__( self.ns = namespace self.chassbr = "br4chs" self.keeptb = keeptb - self.env = env + self.env = ["NO_FLEX_COUNTER_DELAY=1"] + env self.topoFile = topoFile self.imgname = imgname self.ctninfo = {} From c8e8b41844924253323437c25da06ca859414c44 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Sat, 16 Nov 2024 10:49:14 +0000 Subject: [PATCH 07/11] Add unit test Signed-off-by: Stepan Blyschak --- tests/mock_tests/flexcounter_ut.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index 2a2247f91f6..4ca95711321 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -295,7 +295,6 @@ namespace flexcounter_test CFG_FLEX_COUNTER_TABLE_NAME }; auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables); - flexCounterOrch->m_delayTimerExpired = true; gDirectory.set(flexCounterOrch); vector buffer_tables = { APP_BUFFER_POOL_TABLE_NAME, @@ -551,6 +550,18 @@ namespace flexcounter_test flexCounterOrch->addExistingData(&flexCounterCfg); static_cast(flexCounterOrch)->doTask(); + // No port counter map created yet + ASSERT_FALSE(checkFlexCounterGroup(PORT_BUFFER_DROP_STAT_FLEX_COUNTER_GROUP, + { + {POLL_INTERVAL_FIELD, "60000"}, + {STATS_MODE_FIELD, STATS_MODE_READ}, + {FLEX_COUNTER_STATUS_FIELD, "enable"} + })); + + // 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"}, From 18b3606b992a04a5fadabb7001cc71e4fd654c01 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 19 Nov 2024 10:01:48 +0000 Subject: [PATCH 08/11] Revert "Add unit test" This reverts commit c8e8b41844924253323437c25da06ca859414c44. --- tests/mock_tests/flexcounter_ut.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index 4ca95711321..2a2247f91f6 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -295,6 +295,7 @@ namespace flexcounter_test CFG_FLEX_COUNTER_TABLE_NAME }; auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables); + flexCounterOrch->m_delayTimerExpired = true; gDirectory.set(flexCounterOrch); vector buffer_tables = { APP_BUFFER_POOL_TABLE_NAME, @@ -550,18 +551,6 @@ namespace flexcounter_test flexCounterOrch->addExistingData(&flexCounterCfg); static_cast(flexCounterOrch)->doTask(); - // No port counter map created yet - ASSERT_FALSE(checkFlexCounterGroup(PORT_BUFFER_DROP_STAT_FLEX_COUNTER_GROUP, - { - {POLL_INTERVAL_FIELD, "60000"}, - {STATS_MODE_FIELD, STATS_MODE_READ}, - {FLEX_COUNTER_STATUS_FIELD, "enable"} - })); - - // 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"}, From 71e05f53f2981c5941b952a82f225b0a2a438ad5 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 19 Nov 2024 10:05:17 +0000 Subject: [PATCH 09/11] Revert "use env variable to delay" This reverts commit 12eea05755ab138af3e8e12b3f164478d75bd185. --- orchagent/flexcounterorch.cpp | 13 +++++-------- tests/conftest.py | 4 +--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 7d59108d16f..432efa8bd8a 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -77,20 +77,17 @@ FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector &tableNames): m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME) { SWSS_LOG_ENTER(); - m_delayTimer = new SelectableTimer(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); - const string no_flex_counter_delay = getenv("NO_FLEX_COUNTER_DELAY") ?: ""; - if (no_flex_counter_delay == "1") - { - m_delayTimerExpired = true; - } - else + if (WarmStart::isWarmStart()) { - m_delayTimerExpired = false; auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); Orch::addExecutor(executor); m_delayTimer->start(); } + else + { + m_delayTimerExpired = true; + } } FlexCounterOrch::~FlexCounterOrch(void) diff --git a/tests/conftest.py b/tests/conftest.py index b075d548fe5..c94224539bf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -406,8 +406,6 @@ def __init__( vols[k] = v kwargs["volumes"] = vols - env = ["NO_FLEX_COUNTER_DELAY=1"] + env; - # create virtual switch container self.ctn = self.client.containers.run(imgname, privileged=True, @@ -1447,7 +1445,7 @@ def __init__( self.ns = namespace self.chassbr = "br4chs" self.keeptb = keeptb - self.env = ["NO_FLEX_COUNTER_DELAY=1"] + env + self.env = env self.topoFile = topoFile self.imgname = imgname self.ctninfo = {} From e2a68aee5734f412fee9dab48948e7344eda8335 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 19 Nov 2024 13:56:18 +0000 Subject: [PATCH 10/11] Update UT Signed-off-by: Stepan Blyschak --- tests/mock_tests/flexcounter_ut.cpp | 43 +++++++++++++++++--- tests/mock_tests/flowcounterrouteorch_ut.cpp | 1 - 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index 2a2247f91f6..608b1afa5d7 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -214,7 +214,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; @@ -224,6 +230,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() { @@ -250,6 +257,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(); @@ -294,8 +303,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); - flexCounterOrch->m_delayTimerExpired = true; + + if (m_start_type == StartType::Warm) + { + WarmStart::getInstance().m_enabled = false; + } + gDirectory.set(flexCounterOrch); vector buffer_tables = { APP_BUFFER_POOL_TABLE_NAME, @@ -551,6 +571,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"}, @@ -845,9 +872,13 @@ namespace flexcounter_test 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)) ); } diff --git a/tests/mock_tests/flowcounterrouteorch_ut.cpp b/tests/mock_tests/flowcounterrouteorch_ut.cpp index f5184edfc15..1b031515b30 100644 --- a/tests/mock_tests/flowcounterrouteorch_ut.cpp +++ b/tests/mock_tests/flowcounterrouteorch_ut.cpp @@ -131,7 +131,6 @@ namespace flowcounterrouteorch_test CFG_FLEX_COUNTER_TABLE_NAME }; auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables); - flexCounterOrch->m_delayTimerExpired = true; gDirectory.set(flexCounterOrch); ASSERT_EQ(gPortsOrch, nullptr); From 0d4fd746629f80ff996de4e00b9a7c1898788381 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 17 Jan 2025 10:01:10 +0000 Subject: [PATCH 11/11] Replace new with unique_ptr Signed-off-by: Stepan Blyschak --- orchagent/flexcounterorch.cpp | 6 +++--- orchagent/flexcounterorch.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 7fa7d9f0fe3..c1df9253e3f 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -77,11 +77,11 @@ FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector &tableNames): m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME) { SWSS_LOG_ENTER(); - m_delayTimer = new SelectableTimer(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); + m_delayTimer = std::make_unique(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0}); if (WarmStart::isWarmStart()) { - auto executor = new ExecutableTimer(m_delayTimer, this, "FLEX_COUNTER_DELAY"); - Orch::addExecutor(executor); + m_delayExecutor = std::make_unique(m_delayTimer.get(), this, "FLEX_COUNTER_DELAY"); + Orch::addExecutor(m_delayExecutor.get()); m_delayTimer->start(); } else diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index 0036bf628ef..aab4bd467be 100644 --- a/orchagent/flexcounterorch.h +++ b/orchagent/flexcounterorch.h @@ -69,7 +69,8 @@ class FlexCounterOrch: public Orch Table m_bufferQueueConfigTable; Table m_bufferPgConfigTable; Table m_deviceMetadataConfigTable; - SelectableTimer* m_delayTimer; + std::unique_ptr m_delayTimer; + std::unique_ptr m_delayExecutor; }; #endif