diff --git a/orchagent/flex_counter/flex_counter_manager.cpp b/orchagent/flex_counter/flex_counter_manager.cpp index 89084e51de8..6c0833fe708 100644 --- a/orchagent/flex_counter/flex_counter_manager.cpp +++ b/orchagent/flex_counter/flex_counter_manager.cpp @@ -56,6 +56,8 @@ const unordered_map FlexCounterManager::counter_id_field_lo { CounterType::SRV6, SRV6_COUNTER_ID_LIST }, { CounterType::SWITCH, SWITCH_COUNTER_ID_LIST }, { CounterType::HA_SET, HA_SET_COUNTER_ID_LIST }, + { CounterType::OFFLOAD_SESSION, FLOW_COUNTER_ID_LIST }, + { CounterType::ICMP_ECHO_SESSION, ICMP_ECHO_SESSION_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 160e7690d3e..1b03ded59df 100644 --- a/orchagent/flex_counter/flex_counter_manager.h +++ b/orchagent/flex_counter/flex_counter_manager.h @@ -46,6 +46,8 @@ enum class CounterType SRV6, SWITCH, HA_SET, + OFFLOAD_SESSION, + ICMP_ECHO_SESSION, }; extern bool gTraditionalFlexCounter; diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index e31fad6af24..751e0cb08f9 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -15,6 +15,7 @@ #include "pfcwdorch.h" #include "routeorch.h" #include "srv6orch.h" +#include "icmporch.h" #include "switchorch.h" #include "debugcounterorch.h" #include "fabricportsorch.h" @@ -37,6 +38,7 @@ extern Directory gDirectory; extern CoppOrch *gCoppOrch; extern FlowCounterRouteOrch *gFlowCounterRouteOrch; extern Srv6Orch *gSrv6Orch; +extern IcmpOrch *gIcmpOrch; extern SwitchOrch *gSwitchOrch; extern sai_object_id_t gSwitchId; extern string gMySwitchType; @@ -62,6 +64,7 @@ extern string gMySwitchType; #define WRED_QUEUE_KEY "WRED_ECN_QUEUE" #define WRED_PORT_KEY "WRED_ECN_PORT" #define SRV6_KEY "SRV6" +#define ICMP_SESSION_KEY "ICMP_SESSION" #define SWITCH_KEY "SWITCH" #define HA_SET_KEY "HA_SET" @@ -94,6 +97,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}, + {ICMP_SESSION_KEY, ICMP_SESSION_STAT_COUNTER_FLEX_COUNTER_GROUP}, {SWITCH_KEY, SWITCH_STAT_COUNTER_FLEX_COUNTER_GROUP}, {HA_SET_KEY, HA_SET_STAT_COUNTER_FLEX_COUNTER_GROUP} }; @@ -338,6 +342,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) { gSrv6Orch->setCountersState((value == "enable")); } + if (gIcmpOrch && (key == ICMP_SESSION_KEY)) + { + gIcmpOrch->setCountersState((value == "enable")); + } if (gPortsOrch && (key == PORT_PHY_ATTR_KEY)) { if(value == "enable") diff --git a/orchagent/icmporch.cpp b/orchagent/icmporch.cpp index eaa5b6c51f2..609585a612f 100644 --- a/orchagent/icmporch.cpp +++ b/orchagent/icmporch.cpp @@ -51,11 +51,6 @@ IcmpOrch::IcmpOrch(DBConnector *db, string tableName, TableConnector stateDbIcmp return; } - if (!resolve_stats_count_mode()) - { - SWSS_LOG_WARN("ICMP stats count mode resolution failed, will try to create ICMP session without explicit stats count mode"); - } - DBConnector *notificationsDb = new DBConnector("ASIC_DB", 0); m_icmpStateNotificationConsumer = new swss::NotificationConsumer(notificationsDb, "NOTIFICATIONS"); @@ -71,64 +66,71 @@ IcmpOrch::IcmpOrch(DBConnector *db, string tableName, TableConnector stateDbIcmp auto icmpStateNotifier = new Notifier(m_icmpStateNotificationConsumer, this, "ICMP_STATE_NOTIFICATIONS"); Orch::addExecutor(icmpStateNotifier); + + initializeCounters(); } -IcmpOrch::~IcmpOrch(void) +void IcmpOrch::initializeCounters() { - // do nothing, just log SWSS_LOG_ENTER(); + + m_stats_handler = std::make_unique>( + ICMP_SESSION_STAT_COUNTER_FLEX_COUNTER_GROUP, + COUNTERS_ICMP_ECHO_SESSION_NAME_MAP, + ICMP_SESSION_STAT_COUNTER_POLLING_INTERVAL_MS, + ICMP_SESSION_FLEX_COUNTER_UPDATE_TIMER_SEC); + + if (!m_stats_handler->initialize()) + { + return; + } + + // Modern mode registers counters at create time; only traditional + // mode needs the retry timer for VID->RID resolution. + if (gTraditionalFlexCounter) + { + auto timer = m_stats_handler->createUpdateTimer(); + auto et = new ExecutableTimer(timer, this, "ICMP_SESSION_FLEX_COUNTER_UPDATE_TIMER"); + Orch::addExecutor(et); + } } -bool IcmpOrch::resolve_stats_count_mode() +std::map IcmpOrch::get_existing_session_map() const { - m_stats_count_mode_initialized = false; - - const auto *meta = sai_metadata_get_attr_metadata( - SAI_OBJECT_TYPE_ICMP_ECHO_SESSION, - SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE); - if (!meta || !meta->isenum) + std::map existing; + for (const auto& kv : m_icmp_session_map) { - SWSS_LOG_WARN("sai_metadata_get_attr_metadata for SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE failed"); - return false; + existing[kv.first] = kv.second.session_id; } + return existing; +} - std::vector values_list(meta->enummetadata->valuescount); - sai_s32_list_t values; - values.count = static_cast(values_list.size()); - values.list = values_list.data(); +void IcmpOrch::setCountersState(bool enable) +{ + SWSS_LOG_ENTER(); - auto status = sai_query_attribute_enum_values_capability( - gSwitchId, - SAI_OBJECT_TYPE_ICMP_ECHO_SESSION, - SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE, - &values); - if (status != SAI_STATUS_SUCCESS) + if (!m_stats_handler) { - SWSS_LOG_WARN("sai_query_attribute_enum_values_capability for SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE failed"); - return false; + return; } - auto *end = values.list + values.count; + m_stats_handler->setState(enable, get_existing_session_map(), sai_icmp_echo_api); +} - static const sai_stats_count_mode_t preferred_modes[] = { - SAI_STATS_COUNT_MODE_PACKET_AND_BYTE, - SAI_STATS_COUNT_MODE_PACKET, - SAI_STATS_COUNT_MODE_BYTE, - SAI_STATS_COUNT_MODE_NONE, - }; +void IcmpOrch::doTask(swss::SelectableTimer &timer) +{ + SWSS_LOG_ENTER(); - for (auto mode : preferred_modes) + if (m_stats_handler) { - if (std::find(values.list, end, static_cast(mode)) != end) - { - m_stats_count_mode = mode; - m_stats_count_mode_initialized = true; - return true; - } + m_stats_handler->processPending(); } +} - SWSS_LOG_WARN("No supported stats count mode found"); - return false; +IcmpOrch::~IcmpOrch(void) +{ + // do nothing, just log + SWSS_LOG_ENTER(); } void IcmpOrch::doTask(Consumer &consumer) @@ -288,6 +290,11 @@ bool IcmpOrch::create_icmp_session(const string& key, const vectorisEnabled()) + { + m_stats_handler->addSession(key, session_id, sai_icmp_echo_api); + } + SWSS_LOG_NOTICE("Created ICMP offload session key(%s)", key.c_str()); return true; } @@ -351,6 +358,13 @@ bool IcmpOrch::remove_icmp_session(const string& key) } sai_object_id_t icmp_session_id = m_icmp_session_map[key].session_id; + + // Detach counters before removing the session. + if (m_stats_handler) + { + m_stats_handler->removeSession(key, icmp_session_id, sai_icmp_echo_api); + } + auto remove_status = sai_session_handler.remove(icmp_session_id); if ( remove_status != SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY) { @@ -374,6 +388,34 @@ bool IcmpOrch::remove_icmp_session(const string& key) const std::string IcmpSaiSessionHandler::m_name = "IcmpOffload"; +const std::vector +IcmpSaiSessionHandler::m_selective_counter_variants = { + { "IN", { SAI_ICMP_ECHO_SESSION_STAT_IN_PACKETS } }, + { "OUT", { SAI_ICMP_ECHO_SESSION_STAT_OUT_PACKETS } }, +}; + +CounterType IcmpSaiSessionHandler::native_counter_type() +{ + return CounterType::ICMP_ECHO_SESSION; +} + +std::unordered_set IcmpSaiSessionHandler::native_counter_stats() +{ + // Must match the serializer registered in sairedis for + // COUNTER_TYPE_ICMP_ECHO_SESSION (FlexCounter::createCounterContext). + return { + sai_serialize_icmp_echo_session_stat(SAI_ICMP_ECHO_SESSION_STAT_IN_PACKETS), + sai_serialize_icmp_echo_session_stat(SAI_ICMP_ECHO_SESSION_STAT_OUT_PACKETS), + }; +} + +sai_status_t IcmpSaiSessionHandler::set_session_attribute(sai_icmp_echo_api_t* api, + sai_object_id_t session_id, + const sai_attribute_t* attr) +{ + return api->set_icmp_echo_session_attribute(session_id, attr); +} + const std::string IcmpSaiSessionHandler::m_tx_interval_fname = "tx_interval"; const std::string IcmpSaiSessionHandler::m_rx_interval_fname = "rx_interval"; const std::string IcmpSaiSessionHandler::m_src_ip_fname = "src_ip"; @@ -617,15 +659,9 @@ SaiOffloadHandlerStatus IcmpSaiSessionHandler::do_create() m_fv_map[m_tx_interval_fname] = "0"; } - if (m_orch.m_stats_count_mode_initialized) - { - sai_attribute_value_t statsCountModeAttr{}; - statsCountModeAttr.s32 = m_orch.m_stats_count_mode; - m_attr_val_map[SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE] = statsCountModeAttr; - } - else + if (m_orch.m_stats_handler) { - SWSS_LOG_WARN("%s, Stats count mode capability unresolved", m_name.c_str()); + m_orch.m_stats_handler->applyStatsCountMode(m_attr_val_map); } // update the hw_lookup parameter in fv_vector diff --git a/orchagent/icmporch.h b/orchagent/icmporch.h index fd685ded35a..c1c4af94727 100644 --- a/orchagent/icmporch.h +++ b/orchagent/icmporch.h @@ -12,9 +12,16 @@ #include "saioffloadsession.h" #include #include +#include +#include extern sai_icmp_echo_api_t* sai_icmp_echo_api; +#define ICMP_SESSION_STAT_COUNTER_FLEX_COUNTER_GROUP "ICMP_SESSION_STAT_COUNTER" +#define COUNTERS_ICMP_ECHO_SESSION_NAME_MAP "COUNTERS_ICMP_ECHO_SESSION_NAME_MAP" +#define ICMP_SESSION_STAT_COUNTER_POLLING_INTERVAL_MS 10000 +#define ICMP_SESSION_FLEX_COUNTER_UPDATE_TIMER_SEC 1 + extern void sai_deserialize_icmp_session_state_ntf( const std::string& s, uint32_t &count, sai_icmp_echo_session_state_notification_t** bfd_session_state); @@ -98,6 +105,20 @@ class IcmpOrch: public Orch, public Subject */ void doTask(swss::NotificationConsumer &consumer) override; + /** + *@method doTask + * + *@brief Drives pending counter registration in traditional flex-counter mode. + */ + void doTask(swss::SelectableTimer &timer) override; + + /** + *@method setCountersState + * + *@brief Enable/disable ICMP echo session counters from FLEX_COUNTER_GROUP state. + */ + void setCountersState(bool enable); + // friend handler have access to IcmpOrch friend struct IcmpSaiSessionHandler; @@ -109,16 +130,6 @@ class IcmpOrch: public Orch, public Subject private: - /** - *@method resolve_stats_count_mode - * - *@brief resolves supported stats count mode for ICMP sessions once - * - *@return true on success, false on failure - */ - bool resolve_stats_count_mode(); - - /** *@method create_icmp_session * @@ -159,6 +170,20 @@ class IcmpOrch: public Orch, public Subject */ bool update_icmp_session(const string& key, const vector& data); + /** + *@method initializeCounters + * + *@brief Construct the stats handler and query platform capability. + */ + void initializeCounters(); + + /** + *@method get_existing_session_map + * + *@brief session_key -> SAI session OID snapshot for setState(). + */ + std::map get_existing_session_map() const; + // map of session key to session data cache std::map m_icmp_session_map; // map of session object id to update data for handling notification from asic db @@ -175,11 +200,6 @@ class IcmpOrch: public Orch, public Subject // keeps track of number of sessions uint32_t m_num_sessions = 0; - // resolved ICMP stats count mode reused across session creates - sai_stats_count_mode_t m_stats_count_mode = SAI_STATS_COUNT_MODE_PACKET; - // set to true only if ICMP stats count mode is resolved - bool m_stats_count_mode_initialized = false; - // max number of sessions static const uint32_t m_max_sessions; @@ -187,6 +207,10 @@ class IcmpOrch: public Orch, public Subject static const std::map m_session_state_lkup; // map of icmp session state string to sai icmp session state static const std::map m_session_state_str_lkup; + + // Manages per-session SAI selective counters and FlexCounterManager + // registration; instantiated once capability has been queried. + std::unique_ptr> m_stats_handler; }; /** @@ -255,6 +279,55 @@ struct IcmpSaiSessionHandler : public SaiOffloadSessionHandler(session_key[i + 2]))) continue; + + // Token must start at a non-alnum boundary to avoid matching + // inside arbitrary identifiers. + if (i > 0) + { + unsigned char prev = static_cast(session_key[i - 1]); + if (std::isalnum(prev)) continue; + } + + size_t end = i + 2; + while (end < session_key.size() && + std::isxdigit(static_cast(session_key[end]))) + { + ++end; + } + + std::string guid = session_key.substr(i, end - i); + if (guid.size() > kMaxLabelLen) + { + guid.resize(kMaxLabelLen); + } + return guid; + } + + return SaiOffloadStatsHandler::default_counter_label(session_key); + } + /** *@method do_init * @@ -356,6 +429,23 @@ struct IcmpSaiSessionHandler : public SaiOffloadSessionHandler m_selective_counter_variants; + + // Native FlexCounter fallback (required by SaiOffloadStatsHandler). + // Used when SAI_ICMP_ECHO_SESSION_ATTR_SELECTIVE_COUNTER_LIST is not + // implemented; syncd polls sai_get_icmp_echo_session_stats_ext() into + // COUNTERS_DB directly. + static CounterType native_counter_type(); + static std::unordered_set native_counter_stats(); + + // Thin wrapper so SaiOffloadStatsHandler stays API-agnostic. + static sai_status_t set_session_attribute(sai_icmp_echo_api_t* api, + sai_object_id_t session_id, + const sai_attribute_t* attr); }; #endif /* SWSS_ICMPORCH_H */ diff --git a/orchagent/saioffloadsession.h b/orchagent/saioffloadsession.h index f038cc29194..fc5cec1ff1f 100644 --- a/orchagent/saioffloadsession.h +++ b/orchagent/saioffloadsession.h @@ -10,9 +10,20 @@ #include #include #include +#include +#include #include +#include +#include +#include #include "portsorch.h" #include "vrforch.h" +#include "dbconnector.h" +#include "table.h" +#include "select.h" +#include "timer.h" +#include "sai_serialize.h" +#include "flex_counter/flex_counter_manager.h" using namespace std; using namespace swss; @@ -31,7 +42,24 @@ extern sai_object_id_t gSwitchId; extern sai_object_id_t gVirtualRouterId; extern PortsOrch* gPortsOrch; extern sai_switch_api_t* sai_switch_api; +extern sai_counter_api_t* sai_counter_api; extern Directory gDirectory; +extern bool gTraditionalFlexCounter; + +/** + *@struct SelectiveCounterVariant + * + *@brief One direction/aspect of a per-session selective counter. Each + * variant becomes a distinct SAI counter aggregating stat_ids via + * SAI_COUNTER_ATTR_STAT_ID_LIST. The suffix (e.g. "IN", "OUT") is + * appended to the SAI counter label and to the COUNTERS_DB name-map + * key; "-" must fit in SAI_HOSTIF_NAME_SIZE - 1. + */ +struct SelectiveCounterVariant +{ + std::string suffix; + std::vector stat_ids; +}; // saioffload handler types for BFD and ICMP template @@ -575,4 +603,751 @@ bool SaiOffloadSessionHandler::register_state_change_not return true; } +/** + *@class SaiOffloadStatsHandler + * + *@brief Manages per-session SAI counters for offload sessions (ICMP Echo, + * BFD, ...) and registers them with FlexCounterManager so syncd + * polls them into COUNTERS_DB. + * + * Mode is selected at initialize() based on platform capability: + * - Selective-counter mode: creates one SAI counter per + * Derived::m_selective_counter_variants, attaches via + * SAI_*_ATTR_SELECTIVE_COUNTER_LIST, polls SAI_COUNTER_STAT_*. + * - Native mode (fallback): registers the session OID itself with + * FlexCounterManager using Derived::native_counter_{type,stats}(). + * + * Derived must provide: + * - session_object_type, SAI_ATTR_ID::COUNTER_LIST_ID, m_name + * - m_selective_counter_variants (>= 1 entry) + * - make_counter_label() returning a printable ASCII base label + * (<= SAI_HOSTIF_NAME_SIZE - 1 - len("-")) + * - native_counter_type() / native_counter_stats() + * - set_session_attribute() wrapper over the object-type SAI API + */ +template +class SaiOffloadStatsHandler +{ +public: + using Traits = SaiOffloadHandlerTraits; + using api_t = typename Traits::api_t; + + SaiOffloadStatsHandler(const std::string& flex_counter_group, + const std::string& counter_name_map, + uint32_t polling_interval_ms, + uint32_t update_timer_interval_sec = 1) + : m_group_name(flex_counter_group), + m_name_map_name(counter_name_map), + m_polling_interval_ms(polling_interval_ms), + m_update_timer_interval_sec(update_timer_interval_sec), + m_counter_manager(flex_counter_group, StatsMode::READ, polling_interval_ms, false) + { + } + + /** + *@method default_counter_label + * + *@brief Generic SAI counter label builder; returns the trailing + * SAI_HOSTIF_NAME_SIZE-1 chars of session_key. Deriveds with + * embedded GUIDs should override make_counter_label() since + * trailing truncation is not guaranteed to be unique. + */ + static std::string default_counter_label(const std::string& session_key) + { + constexpr size_t kMaxLabelLen = SAI_HOSTIF_NAME_SIZE - 1; + if (session_key.size() <= kMaxLabelLen) + { + return session_key; + } + return session_key.substr(session_key.size() - kMaxLabelLen); + } + + /** + *@method initialize + * + *@brief Query capability, pick mode, set up DB connectors. Call once + * before adding counters. Returns true if counters are usable. + */ + bool initialize() + { + if (m_initialized) + { + return m_supported; + } + m_initialized = true; + + if (!resolve_stats_count_mode()) + { + SWSS_LOG_WARN("%s, stats count mode resolution failed; sessions will be " + "created without explicit stats count mode", + Derived::m_name.c_str()); + } + + if (query_capability()) + { + m_supported = true; + } + else + { + // Selective counter unsupported; fall back to native FlexCounter. + m_native_mode = true; + m_supported = true; + SWSS_LOG_NOTICE("%s using native FlexCounter fallback " + "(selective counter unsupported on this platform)", + Derived::m_name.c_str()); + } + + m_asic_db = std::make_shared("ASIC_DB", 0); + m_counter_db = std::make_shared("COUNTERS_DB", 0); + m_counters_name_map = std::make_unique(m_counter_db.get(), m_name_map_name); + + if (gTraditionalFlexCounter) + { + m_vid_to_rid_table = std::make_unique
(m_asic_db.get(), "VIDTORID"); + } + + return true; + } + + /** + *@method applyStatsCountMode + * + *@brief If the platform reported a supported stats count mode at + * initialize() time, inject it into the session create attribute + * map under Derived::SAI_ATTR_ID::COUNT_MODE_ID. Logs a warning + * and is a no-op when the capability was not resolved. + */ + void applyStatsCountMode(sai_attr_id_val_map_t& attr_val_map) const + { + if (!m_stats_count_mode_initialized) + { + SWSS_LOG_WARN("%s, Stats count mode capability unresolved", + Derived::m_name.c_str()); + return; + } + + constexpr sai_attr_id_t count_mode_attr = + static_cast(Derived::SAI_ATTR_ID::COUNT_MODE_ID); + + sai_attribute_value_t val{}; + val.s32 = m_stats_count_mode; + attr_val_map[count_mode_attr] = val; + } + + /** + *@method createUpdateTimer + * + *@brief Returns a SelectableTimer used by the owning orch to drive + * processPending() in traditional flex counter mode. Caller + * registers it via Orch::addExecutor. + */ + SelectableTimer* createUpdateTimer() + { + if (m_counter_update_timer == nullptr) + { + m_counter_update_timer = new SelectableTimer( + timespec{ .tv_sec = m_update_timer_interval_sec, .tv_nsec = 0 }); + } + return m_counter_update_timer; + } + + /** + *@method addSession + * + *@brief Create one SAI selective counter per variant, attach them via + * SAI_*_ATTR_SELECTIVE_COUNTER_LIST, record name-map entries, + * and register with FlexCounterManager (deferred in traditional + * mode until VID->RID maps). Rolls back on failure. + */ + bool addSession(const std::string& session_key, sai_object_id_t session_id, api_t* api) + { + SWSS_LOG_ENTER(); + + if (!m_enabled) + { + return false; + } + + if (m_native_mode) + { + return addSessionNative(session_key, session_id); + } + + const auto& variants = Derived::m_selective_counter_variants; + if (variants.empty()) + { + SWSS_LOG_ERROR("%s, no selective counter variants declared by Derived; " + "refusing to addSession(%s)", + Derived::m_name.c_str(), session_key.c_str()); + return false; + } + + std::vector oids; + oids.reserve(variants.size()); + + for (const auto& v : variants) + { + sai_object_id_t oid = SAI_NULL_OBJECT_ID; + if (!create_selective_counter(oid, session_key, v)) + { + SWSS_LOG_ERROR("%s, Failed to create selective counter for %s " + "variant '%s'", + Derived::m_name.c_str(), session_key.c_str(), + v.suffix.c_str()); + for (auto created : oids) + { + remove_selective_counter(created); + } + return false; + } + oids.push_back(oid); + } + + if (!attach_counters_to_session(api, session_id, oids)) + { + for (auto oid : oids) + { + remove_selective_counter(oid); + } + return false; + } + + // Persist | -> oid; '|' avoids colliding with + // the ':' tokens already present in session_key. + std::vector fvs; + fvs.reserve(variants.size()); + for (size_t i = 0; i < variants.size(); ++i) + { + fvs.emplace_back(make_name_map_key(session_key, variants[i].suffix), + sai_serialize_object_id(oids[i])); + } + m_counters_name_map->set("", fvs); + + m_session_counters[session_key] = oids; + + if (!gTraditionalFlexCounter) + { + std::unordered_set counter_stats; + get_counter_stat_id_list(counter_stats); + for (auto oid : oids) + { + m_counter_manager.setCounterIdList(oid, CounterType::OFFLOAD_SESSION, counter_stats); + } + } + else + { + bool was_empty = m_pending_counters.empty(); + for (auto oid : oids) + { + m_pending_counters[oid] = session_key; + } + if (was_empty && !m_pending_counters.empty() && + m_counter_update_timer != nullptr) + { + m_counter_update_timer->start(); + } + } + + return true; + } + + /** + *@method removeSession + * + *@brief Detach counters from the session, clear name-map entries, + * unregister from FlexCounterManager, and destroy SAI counters. + */ + void removeSession(const std::string& session_key, sai_object_id_t session_id, api_t* api) + { + SWSS_LOG_ENTER(); + + auto it = m_session_counters.find(session_key); + if (it == m_session_counters.end()) + { + return; + } + const auto oids = it->second; // copy: we erase below + + if (m_native_mode) + { + // oids[0] is the session OID itself; nothing to detach/destroy. + m_counters_name_map->hdel("", session_key); + for (auto oid : oids) + { + bool was_pending = m_pending_counters.erase(oid) == 1; + if (!was_pending) + { + m_counter_manager.clearCounterIdList(oid); + } + } + m_session_counters.erase(it); + return; + } + + detach_counter_from_session(api, session_id); + + const auto& variants = Derived::m_selective_counter_variants; + for (const auto& v : variants) + { + m_counters_name_map->hdel("", + make_name_map_key(session_key, v.suffix)); + } + + for (auto oid : oids) + { + bool was_pending = m_pending_counters.erase(oid) == 1; + if (!was_pending) + { + m_counter_manager.clearCounterIdList(oid); + } + remove_selective_counter(oid); + } + m_session_counters.erase(it); + } + + /** + *@method processPending + * + *@brief Register pending counters with FlexCounterManager once their + * VID->RID mapping resolves. Driven by the update timer. + */ + void processPending() + { + SWSS_LOG_ENTER(); + + const CounterType counter_type = m_native_mode + ? Derived::native_counter_type() + : CounterType::OFFLOAD_SESSION; + std::unordered_set counter_stats; + if (m_native_mode) + { + counter_stats = Derived::native_counter_stats(); + } + else + { + get_counter_stat_id_list(counter_stats); + } + + std::string value; + for (auto it = m_pending_counters.begin(); it != m_pending_counters.end();) + { + const auto oid_str = sai_serialize_object_id(it->first); + if (!gTraditionalFlexCounter || + (m_vid_to_rid_table && m_vid_to_rid_table->hget("", oid_str, value))) + { + m_counter_manager.setCounterIdList(it->first, counter_type, counter_stats); + it = m_pending_counters.erase(it); + } + else + { + ++it; + } + } + + if (m_pending_counters.empty() && m_counter_update_timer != nullptr) + { + m_counter_update_timer->stop(); + } + } + + /** + *@method setState + * + *@brief Enable/disable counter collection across existing_sessions. + * Idempotent. + */ + void setState(bool enable, + const std::map& existing_sessions, + api_t* api) + { + SWSS_LOG_ENTER(); + + if (!m_supported) + { + SWSS_LOG_WARN("%s selective counters not supported; ignoring state change", + Derived::m_name.c_str()); + return; + } + + if (enable == m_enabled) + { + return; + } + + SWSS_LOG_NOTICE("%s selective counters state -> %s", + Derived::m_name.c_str(), enable ? "enabled" : "disabled"); + + m_enabled = enable; + + if (enable) + { + for (const auto& kv : existing_sessions) + { + addSession(kv.first, kv.second, api); + } + } + else + { + auto to_remove = m_session_counters; + for (const auto& kv : to_remove) + { + auto sit = existing_sessions.find(kv.first); + sai_object_id_t session_id = (sit != existing_sessions.end()) ? sit->second : SAI_NULL_OBJECT_ID; + removeSession(kv.first, session_id, api); + } + } + } + + bool isEnabled() const { return m_enabled; } + bool isSupported() const { return m_supported; } + +private: + bool query_capability() const + { + sai_attr_capability_t capability; + constexpr sai_attr_id_t counter_list_attr = + static_cast(Derived::SAI_ATTR_ID::COUNTER_LIST_ID); + sai_status_t status = sai_query_attribute_capability( + gSwitchId, Derived::session_object_type, counter_list_attr, &capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("%s, Failed to query selective counter capability: %d", + Derived::m_name.c_str(), status); + return false; + } + return capability.set_implemented && capability.create_implemented; + } + + /** + *@method resolve_stats_count_mode + * + *@brief Query the platform for the supported enum values of + * Derived::SAI_ATTR_ID::COUNT_MODE_ID on Derived::session_object_type + * and pick the most informative supported mode. Result is stored + * in m_stats_count_mode for later use by applyStatsCountMode(). + */ + bool resolve_stats_count_mode() + { + m_stats_count_mode_initialized = false; + + constexpr sai_attr_id_t count_mode_attr = + static_cast(Derived::SAI_ATTR_ID::COUNT_MODE_ID); + + const auto *meta = sai_metadata_get_attr_metadata( + Derived::session_object_type, count_mode_attr); + if (!meta || !meta->isenum) + { + SWSS_LOG_WARN("%s, sai_metadata_get_attr_metadata for stats count mode failed", + Derived::m_name.c_str()); + return false; + } + + std::vector values_list(meta->enummetadata->valuescount); + sai_s32_list_t values; + values.count = static_cast(values_list.size()); + values.list = values_list.data(); + + sai_status_t status = sai_query_attribute_enum_values_capability( + gSwitchId, + Derived::session_object_type, + count_mode_attr, + &values); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("%s, sai_query_attribute_enum_values_capability for stats count mode failed", + Derived::m_name.c_str()); + return false; + } + + auto *end = values.list + values.count; + + static const sai_stats_count_mode_t preferred_modes[] = { + SAI_STATS_COUNT_MODE_PACKET_AND_BYTE, + SAI_STATS_COUNT_MODE_PACKET, + SAI_STATS_COUNT_MODE_BYTE, + SAI_STATS_COUNT_MODE_NONE, + }; + + for (auto mode : preferred_modes) + { + if (std::find(values.list, end, static_cast(mode)) != end) + { + m_stats_count_mode = mode; + m_stats_count_mode_initialized = true; + return true; + } + } + + SWSS_LOG_WARN("%s, No supported stats count mode found", + Derived::m_name.c_str()); + return false; + } + + /** + *@method make_name_map_key + * + *@brief Build "|" name-map field. '|' + * avoids colliding with the ':' tokens inside session_key. + */ + static std::string make_name_map_key(const std::string& session_key, + const std::string& variant_suffix) + { + return session_key + "|" + variant_suffix; + } + + bool create_selective_counter(sai_object_id_t& counter_oid, + const std::string& session_key, + const SelectiveCounterVariant& variant) + { + std::vector attrs; + + sai_attribute_t type_attr{}; + type_attr.id = SAI_COUNTER_ATTR_TYPE; + type_attr.value.s32 = SAI_COUNTER_TYPE_SELECTIVE; + attrs.push_back(type_attr); + + sai_attribute_t obj_type_attr{}; + obj_type_attr.id = SAI_COUNTER_ATTR_OBJECT_TYPE; + obj_type_attr.value.s32 = Derived::session_object_type; + attrs.push_back(obj_type_attr); + + std::vector id_list; + id_list.reserve(variant.stat_ids.size()); + for (auto id : variant.stat_ids) + { + id_list.push_back(static_cast(id)); + } + + sai_attribute_t stat_ids_attr{}; + stat_ids_attr.id = SAI_COUNTER_ATTR_STAT_ID_LIST; + stat_ids_attr.value.s32list.count = static_cast(id_list.size()); + stat_ids_attr.value.s32list.list = id_list.data(); + attrs.push_back(stat_ids_attr); + + sai_attribute_t label_attr{}; + label_attr.id = SAI_COUNTER_ATTR_LABEL; + + // SAI Meta caps every CHARDATA attribute at SAI_HOSTIF_NAME_SIZE-1 + // printable ASCII chars; longer values are rejected with the + // misleading "host interface name is too long" error. The full + // session_key is still recorded in COUNTERS_DB name map. + constexpr size_t kMaxLabelLen = SAI_HOSTIF_NAME_SIZE - 1; + std::string base_label = Derived::make_counter_label(session_key); + + // Truncate the base, not the suffix, so the direction stays visible. + const std::string suffix_part = variant.suffix.empty() + ? std::string() + : ("-" + variant.suffix); + if (base_label.size() + suffix_part.size() > kMaxLabelLen) + { + const size_t budget = (suffix_part.size() < kMaxLabelLen) + ? kMaxLabelLen - suffix_part.size() + : 0; + base_label.resize(budget); + } + const std::string compact_label = base_label + suffix_part; + const size_t copy_len = std::min(compact_label.size(), kMaxLabelLen); + + // Reject non-printable bytes early; Meta would otherwise fail the + // create with a generic invalid-parameter. + for (size_t i = 0; i < copy_len; ++i) + { + char c = compact_label[i]; + if (c < 0x20 || c > 0x7e) + { + SWSS_LOG_ERROR( + "%s, counter label '%s' for session '%s' contains " + "non-printable character 0x%02x; aborting create", + Derived::m_name.c_str(), + compact_label.c_str(), session_key.c_str(), + static_cast(c)); + return false; + } + } + + std::memcpy(label_attr.value.chardata, compact_label.data(), copy_len); + label_attr.value.chardata[copy_len] = '\0'; + + if (compact_label.size() > kMaxLabelLen) + { + SWSS_LOG_WARN( + "%s, counter label '%s' (from session '%s') truncated " + "to %zu chars to satisfy SAI Meta CHARDATA limit", + Derived::m_name.c_str(), + compact_label.c_str(), session_key.c_str(), kMaxLabelLen); + } + attrs.push_back(label_attr); + + sai_status_t status = sai_counter_api->create_counter( + &counter_oid, gSwitchId, + static_cast(attrs.size()), attrs.data()); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("%s, Failed to create SAI selective counter " + "(session=%s variant=%s): %d", + Derived::m_name.c_str(), + session_key.c_str(), variant.suffix.c_str(), status); + return false; + } + return true; + } + + bool remove_selective_counter(sai_object_id_t counter_oid) + { + if (counter_oid == SAI_NULL_OBJECT_ID) + { + return true; + } + sai_status_t status = sai_counter_api->remove_counter(counter_oid); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("%s, Failed to remove SAI selective counter %s: %d", + Derived::m_name.c_str(), + sai_serialize_object_id(counter_oid).c_str(), status); + return false; + } + return true; + } + + bool attach_counters_to_session(api_t* api, sai_object_id_t session_id, + const std::vector& counter_oids) + { + constexpr sai_attr_id_t counter_list_attr = + static_cast(Derived::SAI_ATTR_ID::COUNTER_LIST_ID); + + sai_attribute_t attr{}; + attr.id = counter_list_attr; + // const_cast is safe: SAI only reads objlist.list during the set. + attr.value.objlist.count = static_cast(counter_oids.size()); + attr.value.objlist.list = const_cast(counter_oids.data()); + + sai_status_t status = Derived::set_session_attribute(api, session_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("%s, Failed to attach %zu counter(s) to session %s: %d", + Derived::m_name.c_str(), counter_oids.size(), + sai_serialize_object_id(session_id).c_str(), status); + return false; + } + return true; + } + + bool detach_counter_from_session(api_t* api, sai_object_id_t session_id) + { + if (session_id == SAI_NULL_OBJECT_ID) + { + return true; + } + + constexpr sai_attr_id_t counter_list_attr = + static_cast(Derived::SAI_ATTR_ID::COUNTER_LIST_ID); + + sai_attribute_t attr{}; + attr.id = counter_list_attr; + attr.value.objlist.count = 0; + attr.value.objlist.list = nullptr; + + sai_status_t status = Derived::set_session_attribute(api, session_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("%s, Failed to detach counter from session %s: %d", + Derived::m_name.c_str(), + sai_serialize_object_id(session_id).c_str(), status); + return false; + } + return true; + } + + static void get_counter_stat_id_list(std::unordered_set& counter_stats) + { + static const sai_counter_stat_t s_ids[] = { + SAI_COUNTER_STAT_PACKETS, + SAI_COUNTER_STAT_BYTES, + }; + for (auto id : s_ids) + { + counter_stats.emplace(sai_serialize_counter_stat(id)); + } + } + + /** + *@method addSessionNative + * + *@brief Native FlexCounter path: register the session OID itself with + * Derived::native_counter_{type,stats}(). syncd polls via + * sai_get__session_stats_ext() into COUNTERS_DB. + */ + bool addSessionNative(const std::string& session_key, sai_object_id_t session_id) + { + if (session_id == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("%s, addSessionNative(%s) called with NULL session OID", + Derived::m_name.c_str(), session_key.c_str()); + return false; + } + + auto counter_stats = Derived::native_counter_stats(); + if (counter_stats.empty()) + { + SWSS_LOG_ERROR("%s, Derived::native_counter_stats() returned empty; " + "refusing to addSession(%s)", + Derived::m_name.c_str(), session_key.c_str()); + return false; + } + + std::vector fvs; + fvs.emplace_back(session_key, sai_serialize_object_id(session_id)); + m_counters_name_map->set("", fvs); + + m_session_counters[session_key] = { session_id }; + + if (!gTraditionalFlexCounter) + { + m_counter_manager.setCounterIdList(session_id, + Derived::native_counter_type(), + counter_stats); + } + else + { + bool was_empty = m_pending_counters.empty(); + m_pending_counters[session_id] = session_key; + if (was_empty && !m_pending_counters.empty() && + m_counter_update_timer != nullptr) + { + m_counter_update_timer->start(); + } + } + + return true; + } + + std::string m_group_name; + std::string m_name_map_name; + uint32_t m_polling_interval_ms; + uint32_t m_update_timer_interval_sec; + + FlexCounterManager m_counter_manager; + std::unique_ptr
m_counters_name_map; + std::unique_ptr
m_vid_to_rid_table; + std::shared_ptr m_counter_db; + std::shared_ptr m_asic_db; + + // session_key -> counter OIDs (one per variant, same order). + std::map> m_session_counters; + // counter OID -> owning session_key while awaiting VID->RID resolution. + std::map m_pending_counters; + SelectableTimer* m_counter_update_timer = nullptr; + + bool m_enabled = false; + bool m_supported = false; + bool m_initialized = false; + // Selective counter unsupported; register session OIDs directly with + // FlexCounterManager and skip SAI counter create/attach. + bool m_native_mode = false; + + // Resolved per-platform stats count mode reused across session creates. + sai_stats_count_mode_t m_stats_count_mode = SAI_STATS_COUNT_MODE_PACKET; + bool m_stats_count_mode_initialized = false; +}; + #endif diff --git a/tests/mock_tests/icmporch_ut.cpp b/tests/mock_tests/icmporch_ut.cpp index 7d642f1d646..18a560a0d39 100644 --- a/tests/mock_tests/icmporch_ut.cpp +++ b/tests/mock_tests/icmporch_ut.cpp @@ -1,7 +1,13 @@ #include "icmporch_sai_wrap.h" +#include "hftelorch_is_supported_sai_wrap.h" #include "mock_orch_test.h" #include "schema.h" +#define private public #include "icmporch.h" +#undef private +#include "table.h" +#include "dbconnector.h" +#include "sai_serialize.h" #include #include @@ -17,6 +23,8 @@ namespace icmporch_test void TearDown() override { icmporch_sai_wrap_ut::setIcmpSaiHookNone(); + hftel_is_supported_ut::setSaiHookNone(); + gTraditionalFlexCounter = false; MockOrchTest::TearDown(); } }; @@ -32,70 +40,117 @@ namespace icmporch_test }; } - /** - * IcmpOrch::resolve_stats_count_mode() (constructor): metadata unavailable. - */ - TEST_F(IcmpOrchStatsCountModeTest, DoCreate_continuesWhenStatsCountModeMetadataNull) + static bool nameMapHasField(const std::string& field, std::string& value) + { + DBConnector countersDb("COUNTERS_DB", 0); + Table nameMapTable(&countersDb, COUNTERS_ICMP_ECHO_SESSION_NAME_MAP); + return nameMapTable.hget("", field, value); + } + + static void addVidtOridMap(sai_object_id_t oid) + { + DBConnector asicDb("ASIC_DB", 0); + Table vidToRidTable(&asicDb, "VIDTORID"); + std::vector fvs = { + {sai_serialize_object_id(oid), "0x1234"}, + }; + vidToRidTable.set("", fvs); + } + + struct TestableIcmpSaiSessionHandler : public IcmpSaiSessionHandler + { + explicit TestableIcmpSaiSessionHandler(IcmpOrch& orch) : IcmpSaiSessionHandler(orch) {} + + bool hasStatsCountModeAttr() const + { + return m_attr_val_map.find(SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE) != m_attr_val_map.end(); + } + + int32_t getStatsCountModeAttr() const + { + auto it = m_attr_val_map.find(SAI_ICMP_ECHO_SESSION_ATTR_STATS_COUNT_MODE); + return (it == m_attr_val_map.end()) ? -1 : it->second.s32; + } + }; + + TEST_F(IcmpOrchStatsCountModeTest, DoCreate_doesNotApplyStatsCountModeWhenCapabilityUnresolved) { icmporch_sai_wrap_ut::IcmpSaiHookGuard g(icmporch_sai_wrap_ut::setIcmpSaiHookMetadataNull); IcmpOrch icmpOrch(m_app_db.get(), APP_ICMP_ECHO_SESSION_TABLE_NAME, TableConnector(m_state_db.get(), STATE_ICMP_ECHO_SESSION_TABLE_NAME)); - IcmpSaiSessionHandler h(icmpOrch); - ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + TestableIcmpSaiSessionHandler h(icmpOrch); + ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), + SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); EXPECT_EQ(h.create(makeMinimalIcmpSessionFvs()), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + EXPECT_FALSE(h.hasStatsCountModeAttr()); } - /** - * resolve_stats_count_mode: metadata not marked enum false. - */ - TEST_F(IcmpOrchStatsCountModeTest, DoCreate_continuesWhenStatsCountModeMetadataNotEnum) + TEST_F(IcmpOrchStatsCountModeTest, DoCreate_appliesPacketAndByteStatsCountModeWhenReported) { - icmporch_sai_wrap_ut::IcmpSaiHookGuard g(icmporch_sai_wrap_ut::setIcmpSaiHookMetadataNotEnum); + icmporch_sai_wrap_ut::IcmpSaiHookGuard g( + icmporch_sai_wrap_ut::setIcmpSaiHookQueryEnumPacketAndByteOnly); IcmpOrch icmpOrch(m_app_db.get(), APP_ICMP_ECHO_SESSION_TABLE_NAME, TableConnector(m_state_db.get(), STATE_ICMP_ECHO_SESSION_TABLE_NAME)); - IcmpSaiSessionHandler h(icmpOrch); - ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + TestableIcmpSaiSessionHandler h(icmpOrch); + ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), + SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); EXPECT_EQ(h.create(makeMinimalIcmpSessionFvs()), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + EXPECT_TRUE(h.hasStatsCountModeAttr()); + EXPECT_EQ(h.getStatsCountModeAttr(), SAI_STATS_COUNT_MODE_PACKET_AND_BYTE); } - /** - * resolve_stats_count_mode: sai_query_attribute_enum_values_capability fails false; - */ - TEST_F(IcmpOrchStatsCountModeTest, DoCreate_continuesWhenQueryAttributeEnumValuesCapabilityFails) + TEST_F(IcmpOrchStatsCountModeTest, CountersState_SelectiveModeUsesInOutNameMapFields) { - icmporch_sai_wrap_ut::IcmpSaiHookGuard g(icmporch_sai_wrap_ut::setIcmpSaiHookQueryEnumFail); IcmpOrch icmpOrch(m_app_db.get(), APP_ICMP_ECHO_SESSION_TABLE_NAME, TableConnector(m_state_db.get(), STATE_ICMP_ECHO_SESSION_TABLE_NAME)); - IcmpSaiSessionHandler h(icmpOrch); - ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); - EXPECT_EQ(h.create(makeMinimalIcmpSessionFvs()), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + ASSERT_TRUE(icmpOrch.create_icmp_session("default:default:5100:NORMAL", + makeMinimalIcmpSessionFvs())); + + icmpOrch.setCountersState(true); + + std::string value; + EXPECT_TRUE(nameMapHasField("default:default:5100:NORMAL|IN", value)); + EXPECT_TRUE(nameMapHasField("default:default:5100:NORMAL|OUT", value)); + EXPECT_FALSE(nameMapHasField("default:default:5100:NORMAL", value)); + + icmpOrch.setCountersState(false); + EXPECT_FALSE(nameMapHasField("default:default:5100:NORMAL|IN", value)); + EXPECT_FALSE(nameMapHasField("default:default:5100:NORMAL|OUT", value)); } - /** - * resolve_stats_count_mode: sai_query_attribute_enum_values_capability success with an empty list. - */ - TEST_F(IcmpOrchStatsCountModeTest, DoCreate_continuesWhenCapabilityEnumListEmpty) + TEST_F(IcmpOrchStatsCountModeTest, CountersState_NativeFallbackUsesSessionKeyNameMapField) { - icmporch_sai_wrap_ut::IcmpSaiHookGuard g(icmporch_sai_wrap_ut::setIcmpSaiHookQueryEnumEmptyList); + hftel_is_supported_ut::SaiHookGuard g( + hftel_is_supported_ut::setSaiHookAttributeCapabilityQueryFail); IcmpOrch icmpOrch(m_app_db.get(), APP_ICMP_ECHO_SESSION_TABLE_NAME, TableConnector(m_state_db.get(), STATE_ICMP_ECHO_SESSION_TABLE_NAME)); - IcmpSaiSessionHandler h(icmpOrch); - ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); - EXPECT_EQ(h.create(makeMinimalIcmpSessionFvs()), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + ASSERT_TRUE(icmpOrch.create_icmp_session("default:default:5200:NORMAL", + makeMinimalIcmpSessionFvs())); + + icmpOrch.setCountersState(true); + + std::string value; + EXPECT_TRUE(nameMapHasField("default:default:5200:NORMAL", value)); + EXPECT_FALSE(nameMapHasField("default:default:5200:NORMAL|IN", value)); + EXPECT_FALSE(nameMapHasField("default:default:5200:NORMAL|OUT", value)); } - /** - * resolve_stats_count_mode: capability includes PACKET_AND_BYTE (first entry in preferred modes) and create succeeds. - */ - TEST_F(IcmpOrchStatsCountModeTest, DoCreate_resolvesPacketAndByteWhenReported) + TEST_F(IcmpOrchStatsCountModeTest, CountersState_TraditionalModePendingCountersDrainAfterVidToRid) { - icmporch_sai_wrap_ut::IcmpSaiHookGuard g( - icmporch_sai_wrap_ut::setIcmpSaiHookQueryEnumPacketAndByteOnly); + gTraditionalFlexCounter = true; + IcmpOrch icmpOrch(m_app_db.get(), APP_ICMP_ECHO_SESSION_TABLE_NAME, TableConnector(m_state_db.get(), STATE_ICMP_ECHO_SESSION_TABLE_NAME)); - IcmpSaiSessionHandler h(icmpOrch); - ASSERT_EQ(h.init(sai_icmp_echo_api, "default:default:5000:NORMAL"), - SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); - EXPECT_EQ(h.create(makeMinimalIcmpSessionFvs()), SaiOffloadHandlerStatus::SUCCESS_VALID_ENTRY); + ASSERT_TRUE(icmpOrch.create_icmp_session("default:default:5300:NORMAL", + makeMinimalIcmpSessionFvs())); + + icmpOrch.setCountersState(true); + ASSERT_FALSE(icmpOrch.m_stats_handler->m_pending_counters.empty()); + + auto pending_oid = icmpOrch.m_stats_handler->m_pending_counters.begin()->first; + addVidtOridMap(pending_oid); + icmpOrch.m_stats_handler->processPending(); + + EXPECT_TRUE(icmpOrch.m_stats_handler->m_pending_counters.empty()); } } // namespace icmporch_test diff --git a/tests/test_icmp_echo.py b/tests/test_icmp_echo.py index 3efe81e9785..1adea288533 100644 --- a/tests/test_icmp_echo.py +++ b/tests/test_icmp_echo.py @@ -5,12 +5,17 @@ class TestIcmpEcho(object): + ICMP_COUNTER_GROUP_KEY = "ICMP_SESSION" + ICMP_COUNTER_NAME_MAP = "COUNTERS_ICMP_ECHO_SESSION_NAME_MAP" + def setup_db(self, dvs): dvs.setup_db() self.pdb = dvs.get_app_db() self.adb = dvs.get_asic_db() self.sdb = dvs.get_state_db() self.cdb = dvs.get_config_db() + self.fdb = dvs.get_flex_db() + self.cntdb = dvs.get_counters_db() # Set switch icmp offload capability dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'ICMP_OFFLOAD_CAPABLE', 'true') @@ -48,6 +53,27 @@ def update_icmp_echo_session_state(self, dvs, session, state): def set_admin_status(self, interface, status): self.cdb.update_entry("PORT", interface, {"admin_status": status}) + def set_icmp_counter_group_status(self, status): + self.cdb.create_entry("FLEX_COUNTER_TABLE", self.ICMP_COUNTER_GROUP_KEY, + {"FLEX_COUNTER_STATUS": status}) + + def wait_for_icmp_counter_name_map_fields(self, expected_fields, present=True, timeout=10): + for _ in range(timeout): + name_map = self.cntdb.db_connection.hgetall(self.ICMP_COUNTER_NAME_MAP) + if present: + if all(field in name_map and name_map[field] for field in expected_fields): + return + else: + if all(field not in name_map for field in expected_fields): + return + time.sleep(1) + + assert False, "ICMP counter name-map fields {} not {} in {}".format( + expected_fields, "present" if present else "removed", self.ICMP_COUNTER_NAME_MAP) + + def _counter_map_fields_for_session(self, session_key): + return [session_key + "|IN", session_key + "|OUT"] + def create_vrf(self, vrf_name): initial_entries = set(self.adb.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_VIRTUAL_ROUTER")) @@ -76,6 +102,62 @@ def add_ip_address(self, interface, ip): def remove_ip_address(self, interface, ip): self.cdb.delete_entry("INTERFACE", interface + "|" + ip) + def test_icmp_counter_name_map_enable_disable(self, dvs): + self.setup_db(dvs) + + session_key = "default:default:7001:NORMAL" + fieldValues = { + "session_cookie": "701", + "src_ip": "10.0.0.1", + "dst_ip": "10.0.0.2", + "tx_interval": "10", + "rx_interval": "10", + } + + before = self.get_exist_icmp_echo_session() + self.create_icmp_echo_session(session_key, fieldValues) + self.adb.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ICMP_ECHO_SESSION", len(before) + 1) + + expected_fields = self._counter_map_fields_for_session(session_key) + self.set_icmp_counter_group_status("enable") + self.wait_for_icmp_counter_name_map_fields(expected_fields, present=True) + + self.set_icmp_counter_group_status("disable") + self.wait_for_icmp_counter_name_map_fields(expected_fields, present=False) + + created = self.get_exist_icmp_echo_session() - before + assert len(created) == 1 + self.remove_icmp_echo_session(session_key) + self.adb.wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_ICMP_ECHO_SESSION", created.pop()) + + def test_icmp_counter_name_map_backfill_existing_session(self, dvs): + self.setup_db(dvs) + + session_key = "default:default:7002:NORMAL" + fieldValues = { + "session_cookie": "702", + "src_ip": "10.0.0.1", + "dst_ip": "10.0.0.3", + "tx_interval": "10", + "rx_interval": "10", + } + + before = self.get_exist_icmp_echo_session() + self.create_icmp_echo_session(session_key, fieldValues) + self.adb.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ICMP_ECHO_SESSION", len(before) + 1) + + expected_fields = self._counter_map_fields_for_session(session_key) + self.wait_for_icmp_counter_name_map_fields(expected_fields, present=False, timeout=2) + + self.set_icmp_counter_group_status("enable") + self.wait_for_icmp_counter_name_map_fields(expected_fields, present=True) + + created = self.get_exist_icmp_echo_session() - before + assert len(created) == 1 + self.remove_icmp_echo_session(session_key) + self.adb.wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_ICMP_ECHO_SESSION", created.pop()) + self.wait_for_icmp_counter_name_map_fields(expected_fields, present=False) + @pytest.mark.skip(reason="This test is flaky") def test_addUpdateRemoveIcmpEchoSession(self, dvs): self.setup_db(dvs)