From 78306254e36538d9967ca42c86e5a70a23c3521c Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Fri, 7 Jun 2019 17:35:30 -0700 Subject: [PATCH] Add synchronous clear_stats operation path (#463) * Send clear_stats op from orchagent to syncd via Redis pipeline Signed-off-by: Wenda Ni * Handle clear_stats op in syncd Signed-off-by: Wenda Ni * Receive clear_stats op status response from sycnd in orchagent context Signed-off-by: Wenda Ni * Shift clear_stats to get synchronous response from ASIC Signed-off-by: Wenda Ni * Fix compilation error Signed-off-by: Wenda Ni * Fix log message output Signed-off-by: Wenda Ni * Remove debugging symbols Signed-off-by: Wenda Ni * Remove debugging symbols Signed-off-by: Wenda Ni * Change the validation order of KeyOpFieldsValuesTuple responded from syncd Signed-off-by: Wenda Ni * Expand status log utility to include op type as argument Signed-off-by: Wenda Ni * Address comments: check if object id is present in local db Signed-off-by: Wenda Ni * Leverage newly merged infrastructure to check if object id is present in the local db Signed-off-by: Wenda Ni * Fix compile error Signed-off-by: Wenda Ni --- lib/inc/sai_redis_internal.h | 7 +- lib/src/sai_redis_generic_stats.cpp | 145 +++++++++++++++++++++++++--- meta/sai_meta.cpp | 129 ++++++++++++++++++++++--- meta/sai_meta.h | 15 +++ meta/sai_serialize.h | 4 - meta/saiserialize.cpp | 9 -- syncd/syncd.cpp | 79 ++++++++++++++- 7 files changed, 344 insertions(+), 44 deletions(-) diff --git a/lib/inc/sai_redis_internal.h b/lib/inc/sai_redis_internal.h index 90ef01a3cb8e..b2f467d53dbd 100644 --- a/lib/inc/sai_redis_internal.h +++ b/lib/inc/sai_redis_internal.h @@ -150,7 +150,7 @@ { \ MUTEX(); \ SWSS_LOG_ENTER(); \ - return meta_sai_get_stats_oid( \ + return meta_sai_get_stats_oid( \ SAI_OBJECT_TYPE_ ## OBJECT_TYPE, \ object_type ## _id, \ &sai_metadata_enum_sai_ ## object_type ## _stat_t, \ @@ -188,12 +188,13 @@ { \ MUTEX(); \ SWSS_LOG_ENTER(); \ - return redis_generic_clear_stats( \ + return meta_sai_clear_stats_oid( \ SAI_OBJECT_TYPE_ ## OBJECT_TYPE, \ object_type ## _id, \ &sai_metadata_enum_sai_ ## object_type ## _stat_t, \ number_of_counters, \ - (const int32_t*)counter_ids); \ + (const int32_t*)counter_ids, \ + &redis_generic_clear_stats); \ } #define REDIS_GENERIC_STATS(OT, ot) \ diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 84e2d4c022b0..82888a8ef3df 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -179,6 +179,50 @@ sai_status_t internal_redis_get_stats_process( return status; } +sai_status_t internal_redis_clear_stats_process( + _In_ sai_object_type_t object_type, + _In_ const sai_enum_metadata_t *stats_enum_metadata, + _In_ uint32_t count, + _In_ const int32_t *counter_id_list, + _In_ swss::KeyOpFieldsValuesTuple &kco) +{ + SWSS_LOG_ENTER(); + + // key: sai_status + // field: stat_id + + const auto &key = kfvKey(kco); + sai_status_t status; + sai_deserialize_status(key, status); + if (status != SAI_STATUS_SUCCESS) + { + return status; + } + + const auto &fvTuples = kfvFieldsValues(kco); + if (fvTuples.size() != count) + { + SWSS_LOG_ERROR("Field count %zu not as expected %u", fvTuples.size(), count); + return SAI_STATUS_FAILURE; + } + + int32_t counter_id; + for (uint32_t i = 0; i < count; i++) + { + sai_deserialize_enum(fvField(fvTuples[i]).c_str(), stats_enum_metadata, &counter_id); + if (counter_id != counter_id_list[i]) + { + SWSS_LOG_ERROR("Counter id %s not as expected %s", + fvField(fvTuples[i]).c_str(), + sai_serialize_enum(counter_id_list[i], stats_enum_metadata).c_str()); + status = SAI_STATUS_FAILURE; + break; + } + } + + return status; +} + std::vector serialize_counter_id_list( _In_ const sai_enum_metadata_t *stats_enum, _In_ uint32_t count, @@ -339,29 +383,102 @@ sai_status_t redis_generic_get_stats_ext( counters); } +sai_status_t internal_redis_generic_clear_stats( + _In_ sai_object_type_t object_type, + _In_ const std::string &serialized_object_id, + _In_ const sai_enum_metadata_t *stats_enum, + _In_ uint32_t count, + _In_ const int32_t *counter_id_list) +{ + SWSS_LOG_ENTER(); + + std::vector fvTuples = serialize_counter_id_list( + stats_enum, + count, + counter_id_list); + + std::string str_object_type = sai_serialize_object_type(object_type); + std::string key = str_object_type + ":" + serialized_object_id; + + SWSS_LOG_DEBUG("generic clear stats key: %s, fields: %lu", key.c_str(), fvTuples.size()); + + if (g_record) + { + recordLine("m|" + key + "|" + joinFieldValues(fvTuples)); + } + + // clear is special, it will not put data + // into asic view, only to message queue + g_asicState->set(key, fvTuples, "clear_stats"); + + // wait for response + swss::Select s; + s.addSelectable(g_redisGetConsumer.get()); + while (true) + { + SWSS_LOG_DEBUG("wait for clear_stats response"); + + swss::Selectable *sel; + int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + if (result == swss::Select::OBJECT) + { + swss::KeyOpFieldsValuesTuple kco; + g_redisGetConsumer->pop(kco); + const std::string &respKey = kfvKey(kco); + const std::string &respOp = kfvOp(kco); + SWSS_LOG_DEBUG("response: key = %s, op = %s", respKey.c_str(), respOp.c_str()); + + if (respOp != "getresponse") // ignore non response messages + { + continue; + } + + if (g_record) + { + const auto &respFvTuples = kfvFieldsValues(kco); + + // first serialized is status return by sai clear_stats + recordLine("M|" + respKey + "|" + joinFieldValues(respFvTuples)); + } + + sai_status_t status = internal_redis_clear_stats_process( + object_type, + stats_enum, + count, + counter_id_list, + kco); + SWSS_LOG_DEBUG("generic clear stats status: %s", sai_serialize_status(status).c_str()); + return status; + } + + SWSS_LOG_ERROR("generic clear stats failed due to SELECT operation result"); + break; + } + + if (g_record) + { + recordLine("M|SAI_STATUS_FAILURE"); + } + + SWSS_LOG_ERROR("generic clear stats failed to get response"); + return SAI_STATUS_FAILURE; +} + sai_status_t redis_generic_clear_stats( _In_ sai_object_type_t object_type, _In_ sai_object_id_t object_id, - _In_ const sai_enum_metadata_t *enum_metadata, + _In_ const sai_enum_metadata_t *enum_metadata_stats, _In_ uint32_t number_of_counters, _In_ const int32_t *counter_ids) { SWSS_LOG_ENTER(); - /* - * Clear stats is the same as get stats ext with mode == - * SAI_STATS_MODE_READ_AND_CLEAR and we just read counters locally and - * discard them, in that way. - */ - - uint64_t counters[REDIS_MAX_COUNTERS]; + std::string str_object_id = sai_serialize_object_id(object_id); - return redis_generic_stats_function( + return internal_redis_generic_clear_stats( object_type, - object_id, - enum_metadata, + str_object_id, + enum_metadata_stats, number_of_counters, - counter_ids, - SAI_STATS_MODE_READ_AND_CLEAR, - counters); + counter_ids); } diff --git a/meta/sai_meta.cpp b/meta/sai_meta.cpp index 9ad05543151a..2049f3d35fb5 100644 --- a/meta/sai_meta.cpp +++ b/meta/sai_meta.cpp @@ -14,15 +14,15 @@ #include #include -#define META_LOG_STATUS(s)\ +#define META_LOG_STATUS(op, s)\ if (s == SAI_STATUS_SUCCESS) \ - SWSS_LOG_DEBUG("get status: %s", sai_serialize_status(s).c_str()); \ + SWSS_LOG_DEBUG(#op " status: %s", sai_serialize_status(s).c_str()); \ else if (s == SAI_STATUS_BUFFER_OVERFLOW \ || SAI_STATUS_IS_ATTR_NOT_IMPLEMENTED(s) \ || SAI_STATUS_IS_ATTR_NOT_SUPPORTED(s)) \ - SWSS_LOG_INFO("get status: %s", sai_serialize_status(s).c_str()); \ + SWSS_LOG_INFO(#op " status: %s", sai_serialize_status(s).c_str()); \ else \ - SWSS_LOG_ERROR("get status: %s", sai_serialize_status(s).c_str()); + SWSS_LOG_ERROR(#op " status: %s", sai_serialize_status(s).c_str()); static volatile bool unittests_enabled = false; @@ -3808,7 +3808,7 @@ sai_status_t meta_sai_get_fdb_entry( status = get(fdb_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -4086,7 +4086,7 @@ sai_status_t meta_sai_get_mcast_fdb_entry( status = get(mcast_fdb_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -4374,7 +4374,7 @@ sai_status_t meta_sai_get_neighbor_entry( status = get(neighbor_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -4674,7 +4674,7 @@ sai_status_t meta_sai_get_route_entry( status = get(route_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -4990,7 +4990,7 @@ sai_status_t meta_sai_get_l2mc_entry( status = get(l2mc_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -5306,7 +5306,7 @@ sai_status_t meta_sai_get_ipmc_entry( status = get(ipmc_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -5516,7 +5516,7 @@ sai_status_t meta_sai_get_inseg_entry( status = get(inseg_entry, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -5807,7 +5807,7 @@ sai_status_t meta_sai_get_oid( status = get(object_type, object_id, attr_count, attr_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); if (status == SAI_STATUS_SUCCESS) { @@ -5930,7 +5930,110 @@ sai_status_t meta_sai_get_stats_oid( status = get_stats(object_type, object_id, stats_enum, count, counter_id_list, counter_list); - META_LOG_STATUS(status); + META_LOG_STATUS(get, status); + + return status; +} + +sai_status_t meta_generic_validation_clear_stats( + _In_ const sai_object_meta_key_t& meta_key, + _In_ const sai_enum_metadata_t* stats_enum, + _In_ uint32_t count, + _In_ const int32_t *counter_id_list) +{ + SWSS_LOG_ENTER(); + + if (meta_unittests_enabled() && (count & 0x80000000)) + { + /* + * If last bit of counters count is set to high, and unittests are enabled, + * then this api can be used to SET counter values by user for debugging purposes. + */ + count = count & ~0x80000000; + } + + if (count < 1) + { + SWSS_LOG_ERROR("expected at least 1 stat when calling clear_stats, zero given"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (count > MAX_LIST_COUNT) + { + SWSS_LOG_ERROR("clear_stats count %u > max list count %u", count, MAX_LIST_COUNT); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (counter_id_list == NULL) + { + SWSS_LOG_ERROR("counter id list pointer is NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (stats_enum == NULL) + { + SWSS_LOG_ERROR("enum metadata pointer is NULL, bug?"); + + return SAI_STATUS_FAILURE; + } + + for (uint32_t i = 0; i < count; i++) + { + if (sai_metadata_get_enum_value_name(stats_enum, counter_id_list[i]) == NULL) + { + SWSS_LOG_ERROR("counter id %u is not allowed on %s", counter_id_list[i], stats_enum->name); + + return SAI_STATUS_INVALID_PARAMETER; + } + } + + return SAI_STATUS_SUCCESS; +} + +sai_status_t meta_sai_clear_stats_oid( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t object_id, + _In_ const sai_enum_metadata_t* stats_enum, + _In_ uint32_t count, + _In_ const int32_t *counter_id_list, + _In_ sai_clear_generic_stats_fn clear_stats) +{ + SWSS_LOG_ENTER(); + + sai_object_id_t switch_id = sai_switch_id_query(object_id); + + sai_status_t status = meta_sai_validate_oid(object_type, &object_id, switch_id, false); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("oid validation failed"); + return status; + } + + sai_object_meta_key_t meta_key = { .objecttype = object_type, .objectkey = { .key = { .object_id = object_id } } }; + + // This ensures that all counter ids in counter id list are valid + // with regards to stats_enum before calling clear_stats() + status = meta_generic_validation_clear_stats(meta_key, stats_enum, count, counter_id_list); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("clear_stats generic validation failed"); + return status; + } + + if (clear_stats == NULL) + { + SWSS_LOG_ERROR("clear_stats function pointer is NULL"); + return SAI_STATUS_INVALID_PARAMETER; + } + + status = clear_stats(object_type, object_id, stats_enum, count, counter_id_list); + + META_LOG_STATUS(clear, status); return status; } diff --git a/meta/sai_meta.h b/meta/sai_meta.h index aa6bf65aebf3..b68e81cffcc6 100644 --- a/meta/sai_meta.h +++ b/meta/sai_meta.h @@ -127,6 +127,21 @@ sai_status_t meta_sai_get_stats_oid( _Out_ uint64_t *counter_list, _In_ sai_get_generic_stats_fn get_stats); +typedef sai_status_t (*sai_clear_generic_stats_fn)( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t object_id, + _In_ const sai_enum_metadata_t *enum_metadata, + _In_ uint32_t number_of_counters, + _In_ const int32_t *counter_ids); + +sai_status_t meta_sai_clear_stats_oid( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t object_id, + _In_ const sai_enum_metadata_t* stats_enum, + _In_ uint32_t count, + _In_ const int32_t *counter_id_list, + _In_ sai_clear_generic_stats_fn clear_stats); + // NOTIFICATIONS extern void meta_sai_on_fdb_event( diff --git a/meta/sai_serialize.h b/meta/sai_serialize.h index 4f2ee5d261e0..eb3bfd66a768 100644 --- a/meta/sai_serialize.h +++ b/meta/sai_serialize.h @@ -277,10 +277,6 @@ void sai_deserialize_ingress_priority_group_attr( _In_ const std::string& s, _Out_ sai_ingress_priority_group_attr_t& attr); -void sai_deserialize_buffer_pool_stat( - _In_ const std::string& s, - _Out_ sai_buffer_pool_stat_t& stat); - void sai_deserialize_queue_attr( _In_ const std::string& s, _Out_ sai_queue_attr_t& attr); diff --git a/meta/saiserialize.cpp b/meta/saiserialize.cpp index bd7da0e78d68..0ab05aefa63a 100644 --- a/meta/saiserialize.cpp +++ b/meta/saiserialize.cpp @@ -3000,15 +3000,6 @@ void sai_deserialize_ingress_priority_group_attr( sai_deserialize_enum(s, &sai_metadata_enum_sai_ingress_priority_group_attr_t, (int32_t&)attr); } -void sai_deserialize_buffer_pool_stat( - _In_ const std::string& s, - _Out_ sai_buffer_pool_stat_t& stat) -{ - SWSS_LOG_ENTER(); - - sai_deserialize_enum(s, &sai_metadata_enum_sai_buffer_pool_stat_t, (int32_t&)stat); -} - void sai_deserialize_queue_attr( _In_ const std::string& s, _Out_ sai_queue_attr_t& attr) diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index 5acbd12bc55c..ff83fbee74f9 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1875,6 +1875,25 @@ sai_status_t getStatsGeneric( counters.data()); } +template +sai_status_t clearStatsGeneric( + _In_ sai_object_id_t object_id, + _In_ const swss::KeyOpFieldsValuesTuple &kco, + _In_ F clearStatsFn, + _In_ G deserializeIdFn) +{ + SWSS_LOG_ENTER(); + + const std::vector counter_ids = extractCounterIdsGeneric( + kco, + deserializeIdFn); + + return clearStatsFn( + object_id, + static_cast(counter_ids.size()), + reinterpret_cast(counter_ids.data())); +} + sai_status_t processGetStatsEvent( _In_ const swss::KeyOpFieldsValuesTuple &kco) { @@ -1945,6 +1964,60 @@ sai_status_t processGetStatsEvent( return status; } +sai_status_t processClearStatsEvent( + _In_ const swss::KeyOpFieldsValuesTuple &kco) +{ + SWSS_LOG_ENTER(); + + const std::string &key = kfvKey(kco); + const std::string &str_object_type = key.substr(0, key.find(":")); + const std::string &str_object_id = key.substr(key.find(":") + 1); + + sai_object_id_t object_id; + sai_deserialize_object_id(str_object_id, object_id); + sai_object_id_t rid; + sai_status_t status = SAI_STATUS_FAILURE; + std::vector fvTuples; + if (!try_translate_vid_to_rid(object_id, rid)) + { + SWSS_LOG_ERROR("VID %s to RID translation error", str_object_id.c_str()); + status = SAI_STATUS_INVALID_OBJECT_ID; + getResponse->set(sai_serialize_status(status), fvTuples, "getresponse"); + return status; + } + + sai_object_type_t object_type; + sai_deserialize_object_type(str_object_type, object_type); + switch (object_type) + { + case SAI_OBJECT_TYPE_BUFFER_POOL: + status = clearStatsGeneric( + rid, + kco, + sai_metadata_sai_buffer_api->clear_buffer_pool_stats, + sai_deserialize_buffer_pool_stat); + break; + default: + SWSS_LOG_ERROR("SAI object type %s not supported", str_object_type.c_str()); + status = SAI_STATUS_NOT_SUPPORTED; + } + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to clear stats"); + } + else + { + for (const auto &fv : kfvFieldsValues(kco)) + { + fvTuples.emplace_back(fvField(fv), ""); + } + } + + getResponse->set(sai_serialize_status(status), fvTuples, "getresponse"); + return status; +} + void on_switch_create_in_init_view( _In_ sai_object_id_t switch_vid, _In_ uint32_t attr_count, @@ -2634,6 +2707,10 @@ sai_status_t processEvent( { return processGetStatsEvent(kco); } + else if (op == "clear_stats") + { + return processClearStatsEvent(kco); + } else if (op == "flush") { return processFdbFlush(kco); @@ -3030,7 +3107,7 @@ void processFlexCounterEvent( for (const auto &str : idStrings) { sai_buffer_pool_stat_t stat; - sai_deserialize_buffer_pool_stat(str, stat); + sai_deserialize_buffer_pool_stat(str.c_str(), &stat); bufferPoolCounterIds.push_back(stat); }