Merge pull request #3519 from stepanblyschak/202411-fc-after-apply-view

kperumalbfn · web-flow · commit 5071becf1257 · 2025-02-18T10:27:23.000-08:00
[202411][FC] process FC after apply view
diff --git 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<string, string> flexCounterGroupMap =
 
 FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector<string> &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<SelectableTimer>(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0});
+    if (WarmStart::isWarmStart())
+    {
+        m_delayExecutor = std::make_unique<ExecutableTimer>(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<VxlanTunnelOrch*>();
     DashOrch* dash_orch = gDirectory.get<DashOrch*>();
     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<KeyOpFieldsValuesTuple> entries;
-    vector<string> 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<Consumer *>(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
@@ -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<std::string> &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<std::string> m_groupsWithBulkChunkSize;
+    std::unique_ptr<SelectableTimer> m_delayTimer;
+    std::unique_ptr<Executor> m_delayExecutor;
 };
 
 #endif
diff --git 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<std::string> &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
@@ -220,7 +220,13 @@ namespace flexcounter_test
         sai_switch_api = pold_sai_switch_api;
     }
 
-    struct FlexCounterTest : public ::testing::TestWithParam<std::tuple<bool, bool>>
+    enum class StartType
+    {
+        Cold,
+        Warm,
+    };
+
+    struct FlexCounterTest : public ::testing::TestWithParam<std::tuple<bool, bool, StartType>>
     {
         shared_ptr<swss::DBConnector> m_app_db;
         shared_ptr<swss::DBConnector> m_config_db;
@@ -230,6 +236,7 @@ namespace flexcounter_test
         shared_ptr<swss::DBConnector> m_asic_db;
         shared_ptr<swss::DBConnector> 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<string> 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<string> 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<FieldValueTuple> values({ {FLEX_COUNTER_DELAY_STATUS_FIELD, "false"},
-                                                    {FLEX_COUNTER_STATUS_FIELD, "enable"} });
+        const std::vector<FieldValueTuple> 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<Orch *>(flexCounterOrch)->doTask();
 
+        if (m_start_type == StartType::Warm)
+        {
+            // Expire timer
+            flexCounterOrch->doTask(*flexCounterOrch->m_delayTimer);
+            static_cast<Orch *>(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<Orch *>(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<std::string> ts;
+
+        gDirectory.get<FlexCounterOrch*>()->bake();
+        gDirectory.get<FlexCounterOrch*>()->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;

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`	`#include "flexcounterorch.h"`
`3`	`3`
`4`	`4`	`FlexCounterOrch::FlexCounterOrch(swss::DBConnector *db, std::vector<std::string> &tableNames)`
`5`		`- : Orch(db, tableNames), m_flexCounterConfigTable(db, CFG_FLEX_COUNTER_TABLE_NAME),`
	`5`	`+ : Orch(db, tableNames),`
`6`	`6`	`m_bufferQueueConfigTable(db, CFG_BUFFER_QUEUE_TABLE_NAME), m_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME),`
`7`	`7`	`m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME)`
`8`	`8`	`{`
`@@ -16,6 +16,10 @@ void FlexCounterOrch::doTask(Consumer &consumer)`
`16`	`16`	`{`
`17`	`17`	`}`
`18`	`18`
	`19`	`+void FlexCounterOrch::doTask(SelectableTimer &timer)`
	`20`	`+{`
	`21`	`+}`
	`22`	`+`
`19`	`23`	`bool FlexCounterOrch::getPortCountersState() const`
`20`	`24`	`{`
`21`	`25`	`return true;`