From 128adf90763080522e329b1591d376bdc468f01a Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Tue, 17 Aug 2021 07:52:09 +0000 Subject: [PATCH 1/5] Delay the flex counters even if the tables are present in config DB Signed-off-by: Shlomi Bitton --- orchagent/flexcounterorch.cpp | 9 +++++++++ orchagent/flexcounterorch.h | 1 + 2 files changed, 10 insertions(+) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 41f024c639..1b91e277eb 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -38,6 +38,7 @@ unordered_map flexCounterGroupMap = {"RIF", RIF_STAT_COUNTER_FLEX_COUNTER_GROUP}, {"RIF_RATES", RIF_RATE_COUNTER_FLEX_COUNTER_GROUP}, {"DEBUG_COUNTER", DEBUG_COUNTER_FLEX_COUNTER_GROUP}, + {"FLEX_COUNTER_DELAY", "FLEX_COUNTER_DELAY"} }; @@ -107,6 +108,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) // which is automatically satisfied upon the creation of the orch object that requires // the syncd flex counter polling service // This postponement is introduced by design to accelerate the initialization process + if (m_delay_flex_counters) + { + continue; + } if(gPortsOrch && (value == "enable")) { if(key == PORT_KEY) @@ -144,6 +149,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) fieldValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, value); m_flexCounterGroupTable->set(flexCounterGroupMap[key], fieldValues); } + else if(field == FLEX_COUNTER_DELAY_STATUS_FIELD) + { + m_delay_flex_counters = value == "true"; + } else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index 0fb9f70e4b..d36eca8b0f 100644 --- a/orchagent/flexcounterorch.h +++ b/orchagent/flexcounterorch.h @@ -21,6 +21,7 @@ class FlexCounterOrch: public Orch private: std::shared_ptr m_flexCounterDb = nullptr; std::shared_ptr m_flexCounterGroupTable = nullptr; + bool m_delay_flex_counters = true; bool m_port_counter_enabled = false; bool m_port_buffer_drop_counter_enabled = false; }; From 7e0b82194420bd9163392649c3884f553192a7d4 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Tue, 17 Aug 2021 08:28:29 +0000 Subject: [PATCH 2/5] Adapt UT to new change Signed-off-by: Shlomi Bitton --- tests/test_flex_counters.py | 5 +++++ tests/test_pg_drop_counter.py | 4 ++++ tests/test_watermark.py | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index ecdc844572..27c6df0898 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -84,6 +84,10 @@ def enable_flex_counter_group(self, group, map): self.config_db.create_entry("FLEX_COUNTER_TABLE", group, group_stats_entry) self.wait_for_table(map) + def disable_flex_counters_delay(): + delay_indication_entry = {"FLEX_COUNTER_DELAY_STATUS": "false"} + self.config_db.create_entry("FLEX_COUNTER_TABLE", "FLEX_COUNTER_DELAY", delay_indication_entry) + @pytest.mark.parametrize("counter_type", counter_type_dict.keys()) def test_flex_counters(self, dvs, counter_type): """ @@ -92,6 +96,7 @@ def test_flex_counters(self, dvs, counter_type): For some counter types the MAPS on COUNTERS DB will be created as well after enabling the counter group, this will be also verified on this test. """ self.setup_dbs(dvs) + self.disable_flex_counters_delay() counter_key = counter_type_dict[counter_type][0] counter_stat = counter_type_dict[counter_type][1] counter_map = counter_type_dict[counter_type][2] diff --git a/tests/test_pg_drop_counter.py b/tests/test_pg_drop_counter.py index 1cdd834747..254cbf3916 100644 --- a/tests/test_pg_drop_counter.py +++ b/tests/test_pg_drop_counter.py @@ -74,9 +74,13 @@ def clear_flex_counter(self): self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_DROP") self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK") + def disable_flex_counters_delay(): + delay_indication_entry = {"FLEX_COUNTER_DELAY_STATUS": "false"} + self.config_db.create_entry("FLEX_COUNTER_TABLE", "FLEX_COUNTER_DELAY", delay_indication_entry) def test_pg_drop_counters(self, dvs): self.setup_dbs(dvs) + self.disable_flex_counters_delay() self.pgs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP") try: self.set_up_flex_counter() diff --git a/tests/test_watermark.py b/tests/test_watermark.py index 6d7c993125..78124e0429 100644 --- a/tests/test_watermark.py +++ b/tests/test_watermark.py @@ -149,7 +149,12 @@ def clear_flex_counter(self, dvs): self.config_db.delete_entry("FLEX_COUNTER_TABLE", "QUEUE_WATERMARK") self.config_db.delete_entry("FLEX_COUNTER_TABLE", "BUFFER_POOL_WATERMARK") + def disable_flex_counters_delay(): + delay_indication_entry = {"FLEX_COUNTER_DELAY_STATUS": "false"} + self.config_db.create_entry("FLEX_COUNTER_TABLE", "FLEX_COUNTER_DELAY", delay_indication_entry) + def set_up(self, dvs): + self.disable_flex_counters_delay() self.qs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_QUEUE") self.pgs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP") self.buffers = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_POOL") From 82e2ff3422363e2e9c1f5cba8456d2373a784f91 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Tue, 17 Aug 2021 14:33:51 +0000 Subject: [PATCH 3/5] comments --- orchagent/flexcounterorch.cpp | 14 ++++++-------- orchagent/flexcounterorch.h | 1 - tests/test_flex_counters.py | 5 ----- tests/test_pg_drop_counter.py | 4 ---- tests/test_watermark.py | 5 ----- 5 files changed, 6 insertions(+), 23 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 1b91e277eb..c0354a2f00 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -38,7 +38,6 @@ unordered_map flexCounterGroupMap = {"RIF", RIF_STAT_COUNTER_FLEX_COUNTER_GROUP}, {"RIF_RATES", RIF_RATE_COUNTER_FLEX_COUNTER_GROUP}, {"DEBUG_COUNTER", DEBUG_COUNTER_FLEX_COUNTER_GROUP}, - {"FLEX_COUNTER_DELAY", "FLEX_COUNTER_DELAY"} }; @@ -87,6 +86,11 @@ void FlexCounterOrch::doTask(Consumer &consumer) if (op == SET_COMMAND) { + auto it = std::find(std::begin(data), std::end(data), FieldValueTuple(FLEX_COUNTER_DELAY_STATUS_FIELD, "true")); + if (it != data.end()) + { + continue; + } for (auto valuePair:data) { const auto &field = fvField(valuePair); @@ -108,10 +112,6 @@ void FlexCounterOrch::doTask(Consumer &consumer) // which is automatically satisfied upon the creation of the orch object that requires // the syncd flex counter polling service // This postponement is introduced by design to accelerate the initialization process - if (m_delay_flex_counters) - { - continue; - } if(gPortsOrch && (value == "enable")) { if(key == PORT_KEY) @@ -150,9 +150,7 @@ void FlexCounterOrch::doTask(Consumer &consumer) m_flexCounterGroupTable->set(flexCounterGroupMap[key], fieldValues); } else if(field == FLEX_COUNTER_DELAY_STATUS_FIELD) - { - m_delay_flex_counters = value == "true"; - } + {} else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index d36eca8b0f..0fb9f70e4b 100644 --- a/orchagent/flexcounterorch.h +++ b/orchagent/flexcounterorch.h @@ -21,7 +21,6 @@ class FlexCounterOrch: public Orch private: std::shared_ptr m_flexCounterDb = nullptr; std::shared_ptr m_flexCounterGroupTable = nullptr; - bool m_delay_flex_counters = true; bool m_port_counter_enabled = false; bool m_port_buffer_drop_counter_enabled = false; }; diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index 27c6df0898..ecdc844572 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -84,10 +84,6 @@ def enable_flex_counter_group(self, group, map): self.config_db.create_entry("FLEX_COUNTER_TABLE", group, group_stats_entry) self.wait_for_table(map) - def disable_flex_counters_delay(): - delay_indication_entry = {"FLEX_COUNTER_DELAY_STATUS": "false"} - self.config_db.create_entry("FLEX_COUNTER_TABLE", "FLEX_COUNTER_DELAY", delay_indication_entry) - @pytest.mark.parametrize("counter_type", counter_type_dict.keys()) def test_flex_counters(self, dvs, counter_type): """ @@ -96,7 +92,6 @@ def test_flex_counters(self, dvs, counter_type): For some counter types the MAPS on COUNTERS DB will be created as well after enabling the counter group, this will be also verified on this test. """ self.setup_dbs(dvs) - self.disable_flex_counters_delay() counter_key = counter_type_dict[counter_type][0] counter_stat = counter_type_dict[counter_type][1] counter_map = counter_type_dict[counter_type][2] diff --git a/tests/test_pg_drop_counter.py b/tests/test_pg_drop_counter.py index 254cbf3916..1cdd834747 100644 --- a/tests/test_pg_drop_counter.py +++ b/tests/test_pg_drop_counter.py @@ -74,13 +74,9 @@ def clear_flex_counter(self): self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_DROP") self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK") - def disable_flex_counters_delay(): - delay_indication_entry = {"FLEX_COUNTER_DELAY_STATUS": "false"} - self.config_db.create_entry("FLEX_COUNTER_TABLE", "FLEX_COUNTER_DELAY", delay_indication_entry) def test_pg_drop_counters(self, dvs): self.setup_dbs(dvs) - self.disable_flex_counters_delay() self.pgs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP") try: self.set_up_flex_counter() diff --git a/tests/test_watermark.py b/tests/test_watermark.py index 78124e0429..6d7c993125 100644 --- a/tests/test_watermark.py +++ b/tests/test_watermark.py @@ -149,12 +149,7 @@ def clear_flex_counter(self, dvs): self.config_db.delete_entry("FLEX_COUNTER_TABLE", "QUEUE_WATERMARK") self.config_db.delete_entry("FLEX_COUNTER_TABLE", "BUFFER_POOL_WATERMARK") - def disable_flex_counters_delay(): - delay_indication_entry = {"FLEX_COUNTER_DELAY_STATUS": "false"} - self.config_db.create_entry("FLEX_COUNTER_TABLE", "FLEX_COUNTER_DELAY", delay_indication_entry) - def set_up(self, dvs): - self.disable_flex_counters_delay() self.qs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_QUEUE") self.pgs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP") self.buffers = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_POOL") From 0febef7d167bc7ef77bf1c8b03ee91b88cb3e213 Mon Sep 17 00:00:00 2001 From: Shlomi Bitton Date: Wed, 18 Aug 2021 08:23:36 +0000 Subject: [PATCH 4/5] Adress comments --- orchagent/flexcounterorch.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index c0354a2f00..122c648f34 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -87,6 +87,7 @@ void FlexCounterOrch::doTask(Consumer &consumer) if (op == SET_COMMAND) { auto it = std::find(std::begin(data), std::end(data), FieldValueTuple(FLEX_COUNTER_DELAY_STATUS_FIELD, "true")); + if (it != data.end()) { continue; @@ -149,6 +150,9 @@ void FlexCounterOrch::doTask(Consumer &consumer) fieldValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, value); m_flexCounterGroupTable->set(flexCounterGroupMap[key], fieldValues); } + // 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 if(field == FLEX_COUNTER_DELAY_STATUS_FIELD) {} else From fa568f783ccb1baa923aa4287b666028d955baff Mon Sep 17 00:00:00 2001 From: shlomibitton <60430976+shlomibitton@users.noreply.github.com> Date: Mon, 30 Aug 2021 15:20:47 +0300 Subject: [PATCH 5/5] Update flexcounterorch.cpp --- orchagent/flexcounterorch.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 122c648f34..51c9c9325a 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -150,11 +150,12 @@ void FlexCounterOrch::doTask(Consumer &consumer) fieldValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, value); m_flexCounterGroupTable->set(flexCounterGroupMap[key], fieldValues); } - // 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 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());