From 66645062342a276fb1cd25642c2583e5e259865a Mon Sep 17 00:00:00 2001 From: Nazarii Hnydyn Date: Fri, 11 Jul 2025 20:21:08 +0300 Subject: [PATCH 1/2] [trim]: Add Packet Trimming Drop Counters to OA Signed-off-by: Nazarii Hnydyn --- orchagent/Makefile.am | 1 + .../flex_counter/flex_counter_manager.cpp | 1 + orchagent/flex_counter/flex_counter_manager.h | 3 +- orchagent/flexcounterorch.cpp | 50 ++++++---- orchagent/main.cpp | 51 ++++++++++ orchagent/nvda_port_trim_drop.lua | 36 +++++++ orchagent/portsorch.cpp | 25 ++++- orchagent/switchorch.cpp | 61 +++++++++++- orchagent/switchorch.h | 13 +++ tests/conftest.py | 3 +- tests/dvslib/dvs_switch.py | 49 +++++++++- tests/mock_tests/flexcounter_ut.cpp | 22 +++++ tests/mock_tests/mock_orchagent_main.cpp | 1 + tests/test_trimming.py | 96 +++++++++++++++++-- 14 files changed, 376 insertions(+), 36 deletions(-) create mode 100644 orchagent/nvda_port_trim_drop.lua diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 39c8ec630b7..0eba1f2613e 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -15,6 +15,7 @@ CFLAGS_SAI = -I /usr/include/sai swssdir = $(datadir)/swss dist_swss_DATA = \ + nvda_port_trim_drop.lua \ eliminate_events.lua \ rif_rates.lua \ pfc_detect_marvell_teralynx.lua \ diff --git a/orchagent/flex_counter/flex_counter_manager.cpp b/orchagent/flex_counter/flex_counter_manager.cpp index d1e2908fa69..d4350ace089 100644 --- a/orchagent/flex_counter/flex_counter_manager.cpp +++ b/orchagent/flex_counter/flex_counter_manager.cpp @@ -52,6 +52,7 @@ const unordered_map FlexCounterManager::counter_id_field_lo { CounterType::ENI, ENI_COUNTER_ID_LIST }, { CounterType::DASH_METER, DASH_METER_COUNTER_ID_LIST }, { CounterType::SRV6, SRV6_COUNTER_ID_LIST }, + { CounterType::SWITCH, SWITCH_COUNTER_ID_LIST }, }; FlexManagerDirectory g_FlexManagerDirectory; diff --git a/orchagent/flex_counter/flex_counter_manager.h b/orchagent/flex_counter/flex_counter_manager.h index 0af1df9602e..959063b3b2d 100644 --- a/orchagent/flex_counter/flex_counter_manager.h +++ b/orchagent/flex_counter/flex_counter_manager.h @@ -40,7 +40,8 @@ enum class CounterType ROUTE, ENI, DASH_METER, - SRV6 + SRV6, + SWITCH, }; extern bool gTraditionalFlexCounter; diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 82d6e1c8c3c..a87c5efd634 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -1,22 +1,29 @@ #include -#include "portsorch.h" -#include "fabricportsorch.h" -#include "select.h" + +#include +#include +#include +#include + #include "notifier.h" -#include "sai_serialize.h" -#include "pfcwdorch.h" -#include "bufferorch.h" -#include "flexcounterorch.h" -#include "debugcounterorch.h" #include "directory.h" + +#include "bufferorch.h" #include "copporch.h" -#include -#include "routeorch.h" #include "macsecorch.h" +#include "portsorch.h" +#include "pfcwdorch.h" +#include "routeorch.h" +#include "srv6orch.h" +#include "switchorch.h" +#include "debugcounterorch.h" +#include "fabricportsorch.h" + #include "dash/dashorch.h" #include "dash/dashmeterorch.h" -#include "flowcounterrouteorch.h" -#include "warm_restart.h" +#include "flex_counter/flowcounterrouteorch.h" + +#include "flexcounterorch.h" extern sai_port_api_t *sai_port_api; extern sai_switch_api_t *sai_switch_api; @@ -29,6 +36,7 @@ extern Directory gDirectory; extern CoppOrch *gCoppOrch; extern FlowCounterRouteOrch *gFlowCounterRouteOrch; extern Srv6Orch *gSrv6Orch; +extern SwitchOrch *gSwitchOrch; extern sai_object_id_t gSwitchId; #define FLEX_COUNTER_DELAY_SEC 60 @@ -50,6 +58,7 @@ extern sai_object_id_t gSwitchId; #define WRED_QUEUE_KEY "WRED_ECN_QUEUE" #define WRED_PORT_KEY "WRED_ECN_PORT" #define SRV6_KEY "SRV6" +#define SWITCH_KEY "SWITCH" unordered_map flexCounterGroupMap = { @@ -77,6 +86,7 @@ unordered_map flexCounterGroupMap = {"WRED_ECN_PORT", WRED_PORT_STAT_COUNTER_FLEX_COUNTER_GROUP}, {"WRED_ECN_QUEUE", WRED_QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP}, {SRV6_KEY, SRV6_STAT_COUNTER_FLEX_COUNTER_GROUP}, + {SWITCH_KEY, SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP} }; @@ -219,17 +229,17 @@ void FlexCounterOrch::doTask(Consumer &consumer) m_pg_watermark_enabled = true; gPortsOrch->addPriorityGroupWatermarkFlexCounters(getPgConfigurations()); } - else if(key == WRED_PORT_KEY) - { + else if(key == WRED_PORT_KEY) + { gPortsOrch->generateWredPortCounterMap(); m_wred_port_counter_enabled = true; - } - else if(key == WRED_QUEUE_KEY) - { + } + else if(key == WRED_QUEUE_KEY) + { gPortsOrch->generateQueueMap(getQueueConfigurations()); m_wred_queue_counter_enabled = true; gPortsOrch->addWredQueueFlexCounters(getQueueConfigurations()); - } + } } if(gIntfsOrch && (key == RIF_KEY) && (value == "enable")) { @@ -285,6 +295,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) { gSrv6Orch->setCountersState((value == "enable")); } + if (gSwitchOrch && (key == SWITCH_KEY) && (value == "enable")) + { + gSwitchOrch->generateSwitchCounterIdList(); + } if (gPortsOrch) { diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 474ce1b17fa..98327c2b4aa 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -67,6 +67,7 @@ extern bool gIsNatSupported; /* orchagent heart beat message interval */ #define HEART_BEAT_INTERVAL_MSECS_DEFAULT 10 * 1000 +string gMyPlatform = ""; string gMySwitchType = ""; string gMySwitchSubType = ""; int32_t gVoqMySwitchId = -1; @@ -169,6 +170,51 @@ void init_gearbox_phys(DBConnector *applDb) delete tmpGearboxTable; } +bool parsePlatform(string &platform, Table &table) +{ + const string key = "localhost"; + const string field = "platform"; + + string value; + + if (!table.hget(key, field, value)) + { + SWSS_LOG_ERROR("Failed to get key(%s)", field.c_str()); + return false; + } + + if (value.empty()) + { + SWSS_LOG_ERROR("Platform is not configured"); + return false; + } + + platform = value; + + return true; +} + +bool initSystemInfo(const DBConnector &cfgDb) +{ + Table cfgDeviceMetaDataTable(&cfgDb, CFG_DEVICE_METADATA_TABLE_NAME); + + try + { + if (!parsePlatform(gMyPlatform, cfgDeviceMetaDataTable)) + { + SWSS_LOG_ERROR("Failed to parse platform"); + return false; + } + } + catch (const std::exception &e) + { + SWSS_LOG_ERROR("Failed to parse system info: %s", e.what()); + return false; + } + + return true; +} + void getCfgSwitchType(DBConnector *cfgDb, string &switch_type, string &switch_sub_type) { Table cfgDeviceMetaDataTable(cfgDb, CFG_DEVICE_METADATA_TABLE_NAME); @@ -539,6 +585,11 @@ int main(int argc, char **argv) zmq_server = create_zmq_server(zmq_server_address, vrf); } + if (!initSystemInfo(config_db)) + { + SWSS_LOG_ERROR("Failed to get system configuration"); + } + // Get switch_type getCfgSwitchType(&config_db, gMySwitchType, gMySwitchSubType); diff --git a/orchagent/nvda_port_trim_drop.lua b/orchagent/nvda_port_trim_drop.lua new file mode 100644 index 00000000000..32a80a355f1 --- /dev/null +++ b/orchagent/nvda_port_trim_drop.lua @@ -0,0 +1,36 @@ +-- KEYS - port IDs +-- ARGV[1] - counters db index +-- ARGV[2] - counters table name +-- ARGV[3] - poll time interval +-- return log + +local logtable = {} + +local function logit(msg) + logtable[#logtable+1] = tostring(msg) +end + +local counters_db = ARGV[1] +local counters_table_name = ARGV[2] + +-- Get configuration +redis.call('SELECT', counters_db) + +-- For each port ID in KEYS +for _, port in ipairs(KEYS) do + -- Get current values from COUNTERS DB + local trim_packets = redis.call('HGET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_TRIM_PACKETS') + local trim_sent_packets = redis.call('HGET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_TX_TRIM_PACKETS') + + if trim_packets and trim_sent_packets then + -- Calculate dropped packets + local dropped_packets = tonumber(trim_packets) - tonumber(trim_sent_packets) + -- Write result back to COUNTERS DB + redis.call('HSET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_DROPPED_TRIM_PACKETS', dropped_packets) + logit("Port " .. port .. " DROPPED_TRIM_PACKETS: " .. dropped_packets) + else + logit("Port " .. port .. " missing required counters") + end +end + +return logtable diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index a2750131f70..f849e9c3d87 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -64,6 +64,7 @@ extern string gMySwitchType; extern int32_t gVoqMySwitchId; extern string gMyHostName; extern string gMyAsicName; +extern string gMyPlatform; extern event_handle_t g_events_handle; // defines ------------------------------------------------------------------------------------------------------------ @@ -257,7 +258,9 @@ const vector port_stat_ids = SAI_PORT_STAT_IF_IN_FEC_CODEWORD_ERRORS_S14, SAI_PORT_STAT_IF_IN_FEC_CODEWORD_ERRORS_S15, SAI_PORT_STAT_IF_IN_FEC_CORRECTED_BITS, - SAI_PORT_STAT_TRIM_PACKETS + SAI_PORT_STAT_TRIM_PACKETS, + SAI_PORT_STAT_DROPPED_TRIM_PACKETS, + SAI_PORT_STAT_TX_TRIM_PACKETS }; const vector gbport_stat_ids = @@ -294,7 +297,9 @@ static const vector queue_stat_ids = SAI_QUEUE_STAT_BYTES, SAI_QUEUE_STAT_DROPPED_PACKETS, SAI_QUEUE_STAT_DROPPED_BYTES, - SAI_QUEUE_STAT_TRIM_PACKETS + SAI_QUEUE_STAT_TRIM_PACKETS, + SAI_QUEUE_STAT_DROPPED_TRIM_PACKETS, + SAI_QUEUE_STAT_TX_TRIM_PACKETS }; static const vector voq_stat_ids = { @@ -627,10 +632,11 @@ PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector switch_stat_ids = +{ + SAI_SWITCH_STAT_DROPPED_TRIM_PACKETS, + SAI_SWITCH_STAT_TX_TRIM_PACKETS +}; + const map switch_attribute_map = { {"fdb_unicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_UNICAST_MISS_PACKET_ACTION}, @@ -95,6 +107,22 @@ const std::set switch_asic_sdk_health_eve const std::set switch_non_sai_attribute_set = {"ordered_ecmp"}; +// functions ---------------------------------------------------------------------------------------------------------- + +static std::unordered_set serializeSwitchCounterStats(const std::vector statIdList) +{ + std::unordered_set stats; + + for (const auto &cit : statIdList) + { + stats.emplace(sai_serialize_switch_stat(cit)); + } + + return stats; +} + +// Switch OA ---------------------------------------------------------------------------------------------------------- + void SwitchOrch::set_switch_pfc_dlr_init_capability() { vector fvVector; @@ -124,7 +152,8 @@ SwitchOrch::SwitchOrch(DBConnector *db, vector& connectors, Tabl m_asicSensorsTable(new Table(m_stateDb.get(), ASIC_TEMPERATURE_INFO_TABLE_NAME)), m_sensorsPollerTimer (new SelectableTimer((timespec { .tv_sec = DEFAULT_ASIC_SENSORS_POLLER_INTERVAL, .tv_nsec = 0 }))), m_stateDbForNotification(new DBConnector("STATE_DB", 0)), - m_asicSdkHealthEventTable(new Table(m_stateDbForNotification.get(), STATE_ASIC_SDK_HEALTH_EVENT_TABLE_NAME)) + m_asicSdkHealthEventTable(new Table(m_stateDbForNotification.get(), STATE_ASIC_SDK_HEALTH_EVENT_TABLE_NAME)), + m_counterManager(SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, SWITCH_STAT_COUNTER_POLLING_INTERVAL_MS, false) { m_restartCheckNotificationConsumer = new NotificationConsumer(db, "RESTARTCHECK"); auto restartCheckNotifier = new Notifier(m_restartCheckNotificationConsumer, this, "RESTARTCHECK"); @@ -142,6 +171,36 @@ SwitchOrch::SwitchOrch(DBConnector *db, vector& connectors, Tabl Orch::addExecutor(executorT); } +void SwitchOrch::generateSwitchCounterNameMap() const +{ + SWSS_LOG_ENTER(); + + DBConnector db("COUNTERS_DB", 0); + Table table(&db, COUNTERS_SWITCH_NAME_MAP); + + FieldValueTuple tuple("ASIC", sai_serialize_object_id(gSwitchId)); + std::vector fvList = { tuple }; + + table.set("", fvList); + + SWSS_LOG_NOTICE("Wrote switch name mapping to Counters DB"); +} + +void SwitchOrch::generateSwitchCounterIdList() +{ + if (m_isSwitchCounterIdListGenerated) + { + return; + } + + auto switchStats = serializeSwitchCounterStats(switch_stat_ids); + m_counterManager.setCounterIdList(gSwitchId, CounterType::SWITCH, switchStats); + + generateSwitchCounterNameMap(); + + m_isSwitchCounterIdListGenerated = true; +} + void SwitchOrch::initAsicSdkHealthEventNotification() { sai_attribute_t attr; diff --git a/orchagent/switchorch.h b/orchagent/switchorch.h index 3959dddac78..67a93625c0f 100644 --- a/orchagent/switchorch.h +++ b/orchagent/switchorch.h @@ -3,6 +3,7 @@ #include "acltable.h" #include "orch.h" #include "timer.h" +#include "flex_counter/flex_counter_manager.h" #include "switch/switch_capabilities.h" #include "switch/switch_helper.h" #include "switch/trimming/capabilities.h" @@ -26,6 +27,8 @@ #define SWITCH_CAPABILITY_TABLE_REG_WARNING_ASIC_SDK_HEALTH_CATEGORY "REG_WARNING_ASIC_SDK_HEALTH_CATEGORY" #define SWITCH_CAPABILITY_TABLE_REG_NOTICE_ASIC_SDK_HEALTH_CATEGORY "REG_NOTICE_ASIC_SDK_HEALTH_CATEGORY" +#define SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP "SWITCH_STAT_COUNTER" + struct WarmRestartCheck { bool checkRestartReadyState; @@ -72,6 +75,9 @@ class SwitchOrch : public Orch bool bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id); bool unbindAclTableFromSwitch(acl_stage_type_t stage, sai_object_id_t table_id); + // Statistics + void generateSwitchCounterIdList(); + private: void doTask(Consumer &consumer); void doTask(swss::SelectableTimer &timer); @@ -84,6 +90,9 @@ class SwitchOrch : public Orch void querySwitchTpidCapability(); void querySwitchPortEgressSampleCapability(); + // Statistics + void generateSwitchCounterNameMap() const; + // Switch hash bool setSwitchHashFieldListSai(const SwitchHash &hash, bool isEcmpHash) const; bool setSwitchHashAlgorithmSai(const SwitchHash &hash, bool isEcmpHash) const; @@ -160,6 +169,10 @@ class SwitchOrch : public Orch } lagHash; } m_switchHashDefaults; + // Statistics + FlexCounterManager m_counterManager; + bool m_isSwitchCounterIdListGenerated = false; + // Information contained in the request from // external program for orchagent pre-shutdown state check WarmRestartCheck m_warmRestartCheck = {false, false, false}; diff --git a/tests/conftest.py b/tests/conftest.py index 5282f775db8..7e0f39cd378 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2047,7 +2047,8 @@ def dvs_hash_manager(request, dvs): @pytest.fixture(scope="class") def dvs_switch_manager(request, dvs): request.cls.dvs_switch = dvs_switch.DVSSwitch(dvs.get_asic_db(), - dvs.get_config_db()) + dvs.get_config_db(), + dvs.get_counters_db()) @pytest.fixture(scope="class") def dvs_twamp_manager(request, dvs): diff --git a/tests/dvslib/dvs_switch.py b/tests/dvslib/dvs_switch.py index f858e6adb1c..a76d5268e7c 100644 --- a/tests/dvslib/dvs_switch.py +++ b/tests/dvslib/dvs_switch.py @@ -1,20 +1,27 @@ """Utilities for interacting with SWITCH objects when writing VS tests.""" from typing import Dict, List +from swsscommon import swsscommon class DVSSwitch: """Manage switch objects on the virtual switch.""" - ADB_SWITCH = "ASIC_STATE:SAI_OBJECT_TYPE_SWITCH" + CHANNEL_UNITTEST = "SAI_VS_UNITTEST_CHANNEL" + + ASIC_VIDTORID = "VIDTORID" + ASIC_SWITCH = "ASIC_STATE:SAI_OBJECT_TYPE_SWITCH" CONFIG_SWITCH_TRIMMING = "SWITCH_TRIMMING" KEY_SWITCH_TRIMMING_GLOBAL = "GLOBAL" - def __init__(self, asic_db, config_db): + COUNTERS_COUNTERS = "COUNTERS" + + def __init__(self, asic_db, config_db, counters_db): """Create a new DVS switch manager.""" self.asic_db = asic_db self.config_db = config_db + self.counters_db = counters_db def update_switch_trimming( self, @@ -36,10 +43,10 @@ def get_switch_ids( The list of switch ids in ASIC DB. """ if expected is None: - return self.asic_db.get_keys(self.ADB_SWITCH) + return self.asic_db.get_keys(self.ASIC_SWITCH) num_keys = len(self.asic_db.default_switch_keys) + expected - keys = self.asic_db.wait_for_n_keys(self.ADB_SWITCH, num_keys) + keys = self.asic_db.wait_for_n_keys(self.ASIC_SWITCH, num_keys) for k in self.asic_db.default_switch_keys: assert k in keys @@ -68,4 +75,36 @@ def verify_switch( sai_switch_id: The specific switch id to check in ASIC DB. sai_qualifiers: The expected set of SAI qualifiers to be found in ASIC DB. """ - self.asic_db.wait_for_field_match(self.ADB_SWITCH, sai_switch_id, sai_qualifiers) + self.asic_db.wait_for_field_match(self.ASIC_SWITCH, sai_switch_id, sai_qualifiers) + + def set_switch_counter( + self, + sai_switch_id: str, + sai_qualifiers: Dict[str, str] + ) -> None: + """Set switch counter value in ASIC DB.""" + attr_list = [ sai_switch_id ] + fvs = self.asic_db.wait_for_fields(self.ASIC_VIDTORID, "", attr_list) + + ntf = swsscommon.NotificationProducer(self.asic_db.db_connection, self.CHANNEL_UNITTEST) + + # Enable test mode + fvp = swsscommon.FieldValuePairs() + ntf.send("enable_unittests", "true", fvp) + + # Set switch stats + key = fvs[sai_switch_id] + fvp = swsscommon.FieldValuePairs(list(sai_qualifiers.items())) + ntf.send("set_stats", str(key), fvp) + + # Disable test mode + fvp = swsscommon.FieldValuePairs() + ntf.send("enable_unittests", "false", fvp) + + def verify_switch_counter( + self, + sai_switch_id: str, + sai_qualifiers: Dict[str, str] + ) -> None: + """Verify that switch counter object has correct COUNTERS DB representation.""" + self.counters_db.wait_for_field_match(self.COUNTERS_COUNTERS, sai_switch_id, sai_qualifiers) diff --git a/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index 8af5173a2ca..651188f5913 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -498,6 +498,12 @@ namespace flexcounter_test TEST_P(FlexCounterTest, CounterTest) { // Check flex counter database after system initialization + ASSERT_TRUE(checkFlexCounterGroup(SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP, + { + {STATS_MODE_FIELD, STATS_MODE_READ}, + {POLL_INTERVAL_FIELD, "60000"}, + {FLEX_COUNTER_STATUS_FIELD, "disable"} + })); ASSERT_TRUE(checkFlexCounterGroup(QUEUE_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, { {STATS_MODE_FIELD, STATS_MODE_READ_AND_CLEAR}, @@ -621,6 +627,7 @@ namespace flexcounter_test // Enable and check counters const std::vector values({ {FLEX_COUNTER_STATUS_FIELD, "enable"} }); + flexCounterCfg.set("SWITCH", values); flexCounterCfg.set("PG_WATERMARK", values); flexCounterCfg.set("QUEUE_WATERMARK", values); flexCounterCfg.set("QUEUE", values); @@ -643,6 +650,12 @@ namespace flexcounter_test isNoPendingCounterObjects(); + ASSERT_TRUE(checkFlexCounterGroup(SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP, + { + {POLL_INTERVAL_FIELD, "60000"}, + {STATS_MODE_FIELD, STATS_MODE_READ}, + {FLEX_COUNTER_STATUS_FIELD, "enable"} + })); ASSERT_TRUE(checkFlexCounterGroup(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, { {POLL_INTERVAL_FIELD, "60000"}, @@ -696,6 +709,13 @@ namespace flexcounter_test Port firstPort; gPortsOrch->getPort(firstPortName, firstPort); auto pgOid = firstPort.m_priority_group_ids[3]; + ASSERT_TRUE(checkFlexCounter(SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP, gSwitchId, + { + {SWITCH_COUNTER_ID_LIST, + "SAI_SWITCH_STAT_TX_TRIM_PACKETS," + "SAI_SWITCH_STAT_DROPPED_TRIM_PACKETS" + } + })); ASSERT_TRUE(checkFlexCounter(PG_DROP_STAT_COUNTER_FLEX_COUNTER_GROUP, pgOid, { {PG_COUNTER_ID_LIST, @@ -719,6 +739,8 @@ namespace flexcounter_test ASSERT_TRUE(checkFlexCounter(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP, queueOid, { {QUEUE_COUNTER_ID_LIST, + "SAI_QUEUE_STAT_TX_TRIM_PACKETS," + "SAI_QUEUE_STAT_DROPPED_TRIM_PACKETS," "SAI_QUEUE_STAT_TRIM_PACKETS," "SAI_QUEUE_STAT_DROPPED_BYTES,SAI_QUEUE_STAT_DROPPED_PACKETS," "SAI_QUEUE_STAT_BYTES,SAI_QUEUE_STAT_PACKETS" diff --git a/tests/mock_tests/mock_orchagent_main.cpp b/tests/mock_tests/mock_orchagent_main.cpp index c14753c62a5..3e001ef74c8 100644 --- a/tests/mock_tests/mock_orchagent_main.cpp +++ b/tests/mock_tests/mock_orchagent_main.cpp @@ -12,6 +12,7 @@ sai_object_id_t gSwitchId = SAI_NULL_OBJECT_ID; MacAddress gMacAddress; MacAddress gVxlanMacAddress; +string gMyPlatform = "x86_64-kvm_x86_64-r0"; string gMySwitchType = "switch"; string gMySwitchSubType = "SmartSwitch"; int32_t gVoqMySwitchId = 0; diff --git a/tests/test_trimming.py b/tests/test_trimming.py index 6e6fad54c16..b3c730131f5 100644 --- a/tests/test_trimming.py +++ b/tests/test_trimming.py @@ -56,6 +56,17 @@ def dynamicModel(dvs): trimlogger.info("Disable dynamic buffer model") +@pytest.fixture(scope="class") +def switchCounters(dvs): + trimlogger.info("Initialize switch counters") + dvs.runcmd("counterpoll switch interval 1000") + dvs.runcmd("counterpoll switch enable") + yield + dvs.runcmd("counterpoll switch disable") + dvs.runcmd("counterpoll switch interval 60000") + trimlogger.info("Deinitialize switch counters") + + @pytest.fixture(scope="class") def portCounters(dvs): trimlogger.info("Initialize port counters") @@ -68,18 +79,22 @@ def portCounters(dvs): @pytest.fixture(scope="class") def pgCounters(dvs): trimlogger.info("Initialize priority group counters") + dvs.runcmd("counterpoll watermark interval 10000") dvs.runcmd("counterpoll watermark enable") yield dvs.runcmd("counterpoll watermark disable") + dvs.runcmd("counterpoll watermark interval 60000") trimlogger.info("Deinitialize priority group counters") @pytest.fixture(scope="class") def queueCounters(dvs): trimlogger.info("Initialize queue counters") + dvs.runcmd("counterpoll queue interval 1000") dvs.runcmd("counterpoll queue enable") yield dvs.runcmd("counterpoll queue disable") + dvs.runcmd("counterpoll queue interval 10000") trimlogger.info("Deinitialize queue counters") @@ -1320,6 +1335,7 @@ def test_TrimNegDynamicBufferProfileEdit(self, bufferData, portData, pgData, tar self.verifyProfileListBufferEditConfiguration(portData, bufferData, False) +@pytest.mark.usefixtures("dvs_switch_manager") @pytest.mark.usefixtures("dvs_port_manager") @pytest.mark.usefixtures("dvs_queue_manager") @pytest.mark.usefixtures("testlog") @@ -1327,6 +1343,33 @@ class TestTrimmingStats: PORT = "Ethernet4" QUEUE = "1" + @pytest.fixture(scope="class") + def switchData(self, switchCounters): + trimlogger.info("Initialize switch data") + + trimlogger.info("Verify switch count") + self.dvs_switch.verify_switch_count(0) + + trimlogger.info("Get switch id") + switchIdList = self.dvs_switch.get_switch_ids() + + # Assumption: VS has only one switch object + meta_dict = { + "id": switchIdList[0] + } + + yield meta_dict + + sai_attr_dict = { + "SAI_SWITCH_STAT_TX_TRIM_PACKETS": "0", + "SAI_SWITCH_STAT_DROPPED_TRIM_PACKETS": "0", + } + + trimlogger.info("Reset switch trimming counters") + self.dvs_switch.set_switch_counter(switchIdList[0], sai_attr_dict) + + trimlogger.info("Deinitialize switch data") + @pytest.fixture(scope="class") def portData(self, portCounters): trimlogger.info("Initialize port data") @@ -1341,7 +1384,9 @@ def portData(self, portCounters): yield meta_dict sai_attr_dict = { - "SAI_PORT_STAT_TRIM_PACKETS": "0" + "SAI_PORT_STAT_TRIM_PACKETS": "0", + "SAI_PORT_STAT_TX_TRIM_PACKETS": "0", + "SAI_PORT_STAT_DROPPED_TRIM_PACKETS": "0" } trimlogger.info("Reset port trimming counters: port={}".format(self.PORT)) @@ -1363,7 +1408,9 @@ def queueData(self, queueCounters): yield meta_dict sai_attr_dict = { - "SAI_QUEUE_STAT_TRIM_PACKETS": "0" + "SAI_QUEUE_STAT_TRIM_PACKETS": "0", + "SAI_QUEUE_STAT_TX_TRIM_PACKETS": "0", + "SAI_QUEUE_STAT_DROPPED_TRIM_PACKETS": "0" } trimlogger.info("Reset queue trimming counters: port={}, queue={}".format(self.PORT, self.QUEUE)) @@ -1371,9 +1418,39 @@ def queueData(self, queueCounters): trimlogger.info("Deinitialize queue data") - def test_TrimPortStats(self, portData): + @pytest.mark.parametrize( + "attr, value", [ + pytest.param("SAI_SWITCH_STAT_TX_TRIM_PACKETS", "1000", id="tx-packet"), + pytest.param("SAI_SWITCH_STAT_DROPPED_TRIM_PACKETS", "2000", id="drop-packet") + ] + ) + def test_TrimSwitchStats(self, switchData, attr, value): + sai_attr_dict = { + attr: value + } + + trimlogger.info("Update switch counter") + self.dvs_switch.set_switch_counter( + sai_switch_id=switchData["id"], + sai_qualifiers=sai_attr_dict + ) + + trimlogger.info("Validate switch counter") + self.dvs_switch.verify_switch_counter( + sai_switch_id=switchData["id"], + sai_qualifiers=sai_attr_dict + ) + + @pytest.mark.parametrize( + "attr, value", [ + pytest.param("SAI_PORT_STAT_TRIM_PACKETS", "1000", id="trim-packet"), + pytest.param("SAI_PORT_STAT_TX_TRIM_PACKETS", "2000", id="tx-packet"), + pytest.param("SAI_PORT_STAT_DROPPED_TRIM_PACKETS", "3000", id="drop-packet") + ] + ) + def test_TrimPortStats(self, portData, attr, value): sai_attr_dict = { - "SAI_PORT_STAT_TRIM_PACKETS": "1000" + attr: value } trimlogger.info("Update port counters: port={}".format(self.PORT)) @@ -1388,9 +1465,16 @@ def test_TrimPortStats(self, portData): sai_qualifiers=sai_attr_dict ) - def test_TrimQueueStats(self, queueData): + @pytest.mark.parametrize( + "attr, value", [ + pytest.param("SAI_QUEUE_STAT_TRIM_PACKETS", "1000", id="trim-packet"), + pytest.param("SAI_QUEUE_STAT_TX_TRIM_PACKETS", "2000", id="tx-packet"), + pytest.param("SAI_QUEUE_STAT_DROPPED_TRIM_PACKETS", "3000", id="drop-packet") + ] + ) + def test_TrimQueueStats(self, queueData, attr, value): sai_attr_dict = { - "SAI_QUEUE_STAT_TRIM_PACKETS": "1000" + attr: value } trimlogger.info("Update queue counters: port={}, queue={}".format(self.PORT, self.QUEUE)) From 09f8827a7752ee247ebe8a5b5a5b505a6176ccfb Mon Sep 17 00:00:00 2001 From: Nazarii Hnydyn Date: Tue, 5 Aug 2025 20:31:09 +0300 Subject: [PATCH 2/2] [trim]: Handle review comments Signed-off-by: Nazarii Hnydyn --- orchagent/main.cpp | 51 --------- orchagent/portsorch.cpp | 60 ++++++++++- tests/conftest.py | 6 ++ tests/dvslib/dvs_flex_counter.py | 57 ++++++++++ tests/mock_tests/mock_orchagent_main.cpp | 1 - tests/test_trimming.py | 131 +++++++++++++++++++---- 6 files changed, 234 insertions(+), 72 deletions(-) diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 98327c2b4aa..474ce1b17fa 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -67,7 +67,6 @@ extern bool gIsNatSupported; /* orchagent heart beat message interval */ #define HEART_BEAT_INTERVAL_MSECS_DEFAULT 10 * 1000 -string gMyPlatform = ""; string gMySwitchType = ""; string gMySwitchSubType = ""; int32_t gVoqMySwitchId = -1; @@ -170,51 +169,6 @@ void init_gearbox_phys(DBConnector *applDb) delete tmpGearboxTable; } -bool parsePlatform(string &platform, Table &table) -{ - const string key = "localhost"; - const string field = "platform"; - - string value; - - if (!table.hget(key, field, value)) - { - SWSS_LOG_ERROR("Failed to get key(%s)", field.c_str()); - return false; - } - - if (value.empty()) - { - SWSS_LOG_ERROR("Platform is not configured"); - return false; - } - - platform = value; - - return true; -} - -bool initSystemInfo(const DBConnector &cfgDb) -{ - Table cfgDeviceMetaDataTable(&cfgDb, CFG_DEVICE_METADATA_TABLE_NAME); - - try - { - if (!parsePlatform(gMyPlatform, cfgDeviceMetaDataTable)) - { - SWSS_LOG_ERROR("Failed to parse platform"); - return false; - } - } - catch (const std::exception &e) - { - SWSS_LOG_ERROR("Failed to parse system info: %s", e.what()); - return false; - } - - return true; -} - void getCfgSwitchType(DBConnector *cfgDb, string &switch_type, string &switch_sub_type) { Table cfgDeviceMetaDataTable(cfgDb, CFG_DEVICE_METADATA_TABLE_NAME); @@ -585,11 +539,6 @@ int main(int argc, char **argv) zmq_server = create_zmq_server(zmq_server_address, vrf); } - if (!initSystemInfo(config_db)) - { - SWSS_LOG_ERROR("Failed to get system configuration"); - } - // Get switch_type getCfgSwitchType(&config_db, gMySwitchType, gMySwitchSubType); diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index f849e9c3d87..e2a382dcb92 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1,3 +1,7 @@ +#include +#include +#include + #include "portsorch.h" #include "intfsorch.h" #include "bufferorch.h" @@ -64,7 +68,6 @@ extern string gMySwitchType; extern int32_t gVoqMySwitchId; extern string gMyHostName; extern string gMyAsicName; -extern string gMyPlatform; extern event_handle_t g_events_handle; // defines ------------------------------------------------------------------------------------------------------------ @@ -545,6 +548,56 @@ bool PortsOrch::checkPathTracingCapability() return m_isPathTracingSupported; } +static bool isPortStatSupported(sai_port_stat_t stat) +{ + static std::vector statList; + + if (statList.empty()) + { + sai_stat_capability_list_t capList = { .count = 0, .list = nullptr }; + + auto status = sai_query_stats_capability(gSwitchId, SAI_OBJECT_TYPE_PORT, &capList); + if ((status != SAI_STATUS_SUCCESS) && (status != SAI_STATUS_BUFFER_OVERFLOW)) + { + return false; + } + + statList.resize(capList.count); + capList.list = statList.data(); + + status = sai_query_stats_capability(gSwitchId, SAI_OBJECT_TYPE_PORT, &capList); + if (status != SAI_STATUS_SUCCESS) + { + return false; + } + } + + return std::any_of( + statList.cbegin(), + statList.cend(), + [stat](const sai_stat_capability_t &cap) { + return static_cast(cap.stat_enum) == stat; + } + ); +} + +static bool isMlnxPlatform() +{ + const auto *platform = std::getenv("platform"); + if (platform == nullptr) + { + return false; + } + + const auto *result = std::strstr(platform, MLNX_PLATFORM_SUBSTRING); + if (result == nullptr) + { + return false; + } + + return true; +} + // Port OA ------------------------------------------------------------------------------------------------------------ /* @@ -660,7 +713,10 @@ PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector None: + """Update flex counter in CONFIG DB.""" + self.config_db.update_entry(self.CONFIG_FLEX_COUNTER, group_name, attr_dict) + + def verify_flex_counter( + self, + stat_name: str, + qualifiers: Dict[str, str] + ) -> None: + """Verify that flex counter object has correct FLEX_COUNTER DB representation.""" + self.flex_db.wait_for_field_match(self.FLEX_FLEX_COUNTER_GROUP, stat_name, qualifiers) + + def set_interval( + self, + group_name: str, + interval: str + ) -> None: + """Set flex counter poll interval in CONFIG DB.""" + attr_dict = { + swsscommon.POLL_INTERVAL_FIELD: interval + } + self.update_flex_counter(group_name, attr_dict) + + def set_status( + self, + group_name: str, + status: str + ) -> None: + """Set flex counter status in CONFIG DB.""" + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: status + } + self.update_flex_counter(group_name, attr_dict) + + class TestFlexCountersBase(object): def setup_dbs(self, dvs): diff --git a/tests/mock_tests/mock_orchagent_main.cpp b/tests/mock_tests/mock_orchagent_main.cpp index 3e001ef74c8..c14753c62a5 100644 --- a/tests/mock_tests/mock_orchagent_main.cpp +++ b/tests/mock_tests/mock_orchagent_main.cpp @@ -12,7 +12,6 @@ sai_object_id_t gSwitchId = SAI_NULL_OBJECT_ID; MacAddress gMacAddress; MacAddress gVxlanMacAddress; -string gMyPlatform = "x86_64-kvm_x86_64-r0"; string gMySwitchType = "switch"; string gMySwitchSubType = "SmartSwitch"; int32_t gVoqMySwitchId = 0; diff --git a/tests/test_trimming.py b/tests/test_trimming.py index b3c730131f5..e34ddb7556b 100644 --- a/tests/test_trimming.py +++ b/tests/test_trimming.py @@ -3,6 +3,7 @@ import logging from typing import NamedTuple +from swsscommon import swsscommon import buffer_model @@ -57,44 +58,138 @@ def dynamicModel(dvs): @pytest.fixture(scope="class") -def switchCounters(dvs): +def switchCounters(request, dvs_flex_counter_manager): trimlogger.info("Initialize switch counters") - dvs.runcmd("counterpoll switch interval 1000") - dvs.runcmd("counterpoll switch enable") + + request.cls.dvs_flex_counter.set_interval("SWITCH", "1000") + request.cls.dvs_flex_counter.set_status("SWITCH", "enable") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "enable", + swsscommon.POLL_INTERVAL_FIELD: "1000", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="SWITCH_STAT_COUNTER", + qualifiers=attr_dict + ) + yield - dvs.runcmd("counterpoll switch disable") - dvs.runcmd("counterpoll switch interval 60000") + + request.cls.dvs_flex_counter.set_status("SWITCH", "disable") + request.cls.dvs_flex_counter.set_interval("SWITCH", "60000") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "disable", + swsscommon.POLL_INTERVAL_FIELD: "60000", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="SWITCH_STAT_COUNTER", + qualifiers=attr_dict + ) + trimlogger.info("Deinitialize switch counters") @pytest.fixture(scope="class") -def portCounters(dvs): +def portCounters(request, dvs_flex_counter_manager): trimlogger.info("Initialize port counters") - dvs.runcmd("counterpoll port enable") + + request.cls.dvs_flex_counter.set_status("PORT", "enable") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "enable" + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="PORT_STAT_COUNTER", + qualifiers=attr_dict + ) + yield - dvs.runcmd("counterpoll port disable") + + request.cls.dvs_flex_counter.set_status("PORT", "disable") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "disable", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="PORT_STAT_COUNTER", + qualifiers=attr_dict + ) + trimlogger.info("Deinitialize port counters") @pytest.fixture(scope="class") -def pgCounters(dvs): +def pgCounters(request, dvs_flex_counter_manager): trimlogger.info("Initialize priority group counters") - dvs.runcmd("counterpoll watermark interval 10000") - dvs.runcmd("counterpoll watermark enable") + + request.cls.dvs_flex_counter.set_interval("PG_WATERMARK", "1000") + request.cls.dvs_flex_counter.set_status("PG_WATERMARK", "enable") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "enable", + swsscommon.POLL_INTERVAL_FIELD: "1000", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="PG_WATERMARK_STAT_COUNTER", + qualifiers=attr_dict + ) + yield - dvs.runcmd("counterpoll watermark disable") - dvs.runcmd("counterpoll watermark interval 60000") + + request.cls.dvs_flex_counter.set_status("PG_WATERMARK", "disable") + request.cls.dvs_flex_counter.set_interval("PG_WATERMARK", "60000") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "disable", + swsscommon.POLL_INTERVAL_FIELD: "60000", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="PG_WATERMARK_STAT_COUNTER", + qualifiers=attr_dict + ) + trimlogger.info("Deinitialize priority group counters") @pytest.fixture(scope="class") -def queueCounters(dvs): +def queueCounters(request, dvs_flex_counter_manager): trimlogger.info("Initialize queue counters") - dvs.runcmd("counterpoll queue interval 1000") - dvs.runcmd("counterpoll queue enable") + + request.cls.dvs_flex_counter.set_interval("QUEUE", "1000") + request.cls.dvs_flex_counter.set_status("QUEUE", "enable") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "enable", + swsscommon.POLL_INTERVAL_FIELD: "1000", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="QUEUE_STAT_COUNTER", + qualifiers=attr_dict + ) + yield - dvs.runcmd("counterpoll queue disable") - dvs.runcmd("counterpoll queue interval 10000") + + request.cls.dvs_flex_counter.set_status("QUEUE", "disable") + request.cls.dvs_flex_counter.set_interval("QUEUE", "10000") + + attr_dict = { + swsscommon.FLEX_COUNTER_STATUS_FIELD: "disable", + swsscommon.POLL_INTERVAL_FIELD: "10000", + } + + request.cls.dvs_flex_counter.verify_flex_counter( + stat_name="QUEUE_STAT_COUNTER", + qualifiers=attr_dict + ) + trimlogger.info("Deinitialize queue counters")