From 42469e98b099d227ce5ec7c57c77717db676b5fb Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Tue, 28 May 2019 22:03:00 +0000 Subject: [PATCH 01/13] Send clear_stats op from orchagent to syncd via Redis pipeline Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 54 +++++++++++---- meta/sai_meta.cpp | 103 ++++++++++++++++++++++++++++ meta/sai_meta.h | 15 ++++ syncd/syncd.cpp | 4 ++ 4 files changed, 162 insertions(+), 14 deletions(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 84e2d4c02..3c1fb46c4 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -339,29 +339,55 @@ 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_ERROR("generic clear stats key: %s, fields: %lu", key.c_str(), fvTuples.size()); + 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 + +} + 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 aaf281ba0..4e36d8590 100644 --- a/meta/sai_meta.cpp +++ b/meta/sai_meta.cpp @@ -5931,6 +5931,109 @@ sai_status_t meta_sai_get_stats_oid( 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(status); + + return status; +} + // NOTIFICATIONS static sai_mac_t zero_mac = { 0, 0, 0, 0, 0, 0 }; diff --git a/meta/sai_meta.h b/meta/sai_meta.h index aa6bf65ae..b68e81cff 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/syncd/syncd.cpp b/syncd/syncd.cpp index b0f204fc4..d82bfe80a 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -2616,6 +2616,10 @@ sai_status_t processEvent( { return processGetStatsEvent(kco); } + else if (op == "clear_stats") + { + return processClearStatsEvent(kco); + } else if (op == "flush") { return processFdbFlush(kco); From 7b9026eabb61cd6631c6395a7f196e9409b54f4f Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Wed, 29 May 2019 07:19:03 +0000 Subject: [PATCH 02/13] Handle clear_stats op in syncd Signed-off-by: Wenda Ni --- syncd/syncd.cpp | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index d82bfe80a..dc0cd1131 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1857,6 +1857,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()), + counter_ids.data()); +} + sai_status_t processGetStatsEvent( _In_ const swss::KeyOpFieldsValuesTuple &kco) { @@ -1927,6 +1946,56 @@ 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 = translate_vid_to_rid(object_id); + + sai_object_type_t object_type; + sai_deserialize_object_type(str_object_type, object_type); + + sai_status_t status = SAI_STATUS_FAILURE; + 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; + } + + std::vector fvTuples; + + 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, From da477f28b12e4647812d7276e5f25a868517c4f0 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Wed, 29 May 2019 19:56:26 +0000 Subject: [PATCH 03/13] Receive clear_stats op status response from sycnd in orchagent context Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 101 +++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 3c1fb46c4..6e1d54769 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -179,6 +179,54 @@ 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); + 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; + } + + sai_status_t status; + sai_deserialize_status(key, status); + if (status != SAI_STATUS_SUCCESS) + { + return status; + } + + int32_t counter_id; + for (int i = 0; i < count; i++) + { + sai_deserialize_enum(fvField(fvTuples[i]), stats_enum_metadata, counter_id); + SWSS_LOG_ERROR("Counter id received %s, expected %s", + fvField(fvTuples[i]).c_str(), + sai_serialize_enum(counter_id_list[i], stats_enum_metadata).c_str()); + 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, @@ -354,7 +402,6 @@ sai_status_t internal_redis_generic_clear_stats( 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_ERROR("generic clear stats key: %s, fields: %lu", key.c_str(), fvTuples.size()); @@ -370,7 +417,59 @@ sai_status_t internal_redis_generic_clear_stats( g_asicState->set(key, fvTuples, "clear_stats"); // wait for response + swss::Select s; + s.addSelectable(g_redisGetConsumer.get()); + while (true) + { + SWSS_LOG_ERROR("wait for clear_stats response"); + 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 &key = kfvKey(kco); + const std::string &op = kfvOp(kco); + SWSS_LOG_ERROR("response: key = %s, op = %s", key.c_str(), op.c_str()); + SWSS_LOG_DEBUG("response: key = %s, op = %s", key.c_str(), op.c_str()); + + if (op != "getresponse") // ignore non response messages + { + continue; + } + + if (g_record) + { + const auto &fvTuples = kfvFieldsValues(kco); + + // first serialized is status return by sai clear_stats + recordLine("M|" + key + "|" + joinFieldValues(fvTuples)); + } + + sai_status_t status = internal_redis_clear_stats_process( + object_type, + stats_enum, + count, + counter_id_list, + kco); + SWSS_LOG_ERROR("generic clear status: %s", sai_serialize_status(status).c_str()); + SWSS_LOG_DEBUG("generic clear status: %s", sai_serialize_status(status).c_str()); + return status; + } + + SWSS_LOG_ERROR("generic get 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( From 538817b254b0b0b01e973a43a2a0543aaee5ec3d Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Wed, 29 May 2019 20:15:01 +0000 Subject: [PATCH 04/13] Shift clear_stats to get synchronous response from ASIC Signed-off-by: Wenda Ni --- lib/inc/sai_redis_internal.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/inc/sai_redis_internal.h b/lib/inc/sai_redis_internal.h index 90ef01a3c..b2f467d53 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) \ From d9dc0bd8d32eade06e4ef3c7146f65762a2a7b21 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Thu, 30 May 2019 00:09:10 +0000 Subject: [PATCH 05/13] Fix compilation error Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 16 ++++++++-------- meta/sai_serialize.h | 5 +++++ syncd/syncd.cpp | 2 ++ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 6e1d54769..b1f1bffa0 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -208,7 +208,7 @@ sai_status_t internal_redis_clear_stats_process( } int32_t counter_id; - for (int i = 0; i < count; i++) + for (uint32_t i = 0; i < count; i++) { sai_deserialize_enum(fvField(fvTuples[i]), stats_enum_metadata, counter_id); SWSS_LOG_ERROR("Counter id received %s, expected %s", @@ -430,22 +430,22 @@ sai_status_t internal_redis_generic_clear_stats( { swss::KeyOpFieldsValuesTuple kco; g_redisGetConsumer->pop(kco); - const std::string &key = kfvKey(kco); - const std::string &op = kfvOp(kco); - SWSS_LOG_ERROR("response: key = %s, op = %s", key.c_str(), op.c_str()); - SWSS_LOG_DEBUG("response: key = %s, op = %s", key.c_str(), op.c_str()); + const std::string &respKey = kfvKey(kco); + const std::string &respOp = kfvOp(kco); + SWSS_LOG_ERROR("response: key = %s, op = %s", respKey.c_str(), respOp.c_str()); + SWSS_LOG_DEBUG("response: key = %s, op = %s", respKey.c_str(), respOp.c_str()); - if (op != "getresponse") // ignore non response messages + if (respOp != "getresponse") // ignore non response messages { continue; } if (g_record) { - const auto &fvTuples = kfvFieldsValues(kco); + const auto &respFvTuples = kfvFieldsValues(kco); // first serialized is status return by sai clear_stats - recordLine("M|" + key + "|" + joinFieldValues(fvTuples)); + recordLine("M|" + respKey + "|" + joinFieldValues(respFvTuples)); } sai_status_t status = internal_redis_clear_stats_process( diff --git a/meta/sai_serialize.h b/meta/sai_serialize.h index 4f2ee5d26..f22d5879c 100644 --- a/meta/sai_serialize.h +++ b/meta/sai_serialize.h @@ -165,6 +165,11 @@ void sai_deserialize_number( _Out_ uint32_t& number, _In_ bool hex = false); +void sai_deserialize_enum( + _In_ const std::string& s, + _In_ const sai_enum_metadata_t *meta, + _Out_ int32_t& value); + void sai_deserialize_status( _In_ const std::string& s, _Out_ sai_status_t& status); diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index dc0cd1131..258dddd06 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1966,6 +1966,7 @@ sai_status_t processClearStatsEvent( switch (object_type) { case SAI_OBJECT_TYPE_BUFFER_POOL: + SWSS_LOG_ERROR("Object type %s, about to clear its stats", str_object_type.c_str()); status = clearStatsGeneric( rid, kco, @@ -2687,6 +2688,7 @@ sai_status_t processEvent( } else if (op == "clear_stats") { + SWSS_LOG_ERROR("Receive op: %s", op.c_str()); return processClearStatsEvent(kco); } else if (op == "flush") From c7942196ddf815fe27c423b2dc99ef07c9c9ab82 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Thu, 30 May 2019 19:00:56 +0000 Subject: [PATCH 06/13] Fix log message output Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 6 +++--- syncd/syncd.cpp | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index b1f1bffa0..568477086 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -454,12 +454,12 @@ sai_status_t internal_redis_generic_clear_stats( count, counter_id_list, kco); - SWSS_LOG_ERROR("generic clear status: %s", sai_serialize_status(status).c_str()); - SWSS_LOG_DEBUG("generic clear status: %s", sai_serialize_status(status).c_str()); + SWSS_LOG_ERROR("generic clear stats status: %s", sai_serialize_status(status).c_str()); + SWSS_LOG_DEBUG("generic clear stats status: %s", sai_serialize_status(status).c_str()); return status; } - SWSS_LOG_ERROR("generic get failed due to SELECT operation result"); + SWSS_LOG_ERROR("generic clear stats failed due to SELECT operation result"); break; } diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index 258dddd06..388f68117 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -3071,6 +3071,7 @@ void processFlexCounterEvent( } else if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && field == BUFFER_POOL_COUNTER_ID_LIST) { + SWSS_LOG_ERROR("About to set buffer pool counter list"); std::vector bufferPoolCounterIds; for (const auto &str : idStrings) { From 4b6f57b81037f898b8c6c54c498f0c347695822b Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Thu, 30 May 2019 19:20:11 +0000 Subject: [PATCH 07/13] Remove debugging symbols Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 7 ------- syncd/syncd.cpp | 2 -- 2 files changed, 9 deletions(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 568477086..613304a24 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -211,9 +211,6 @@ sai_status_t internal_redis_clear_stats_process( for (uint32_t i = 0; i < count; i++) { sai_deserialize_enum(fvField(fvTuples[i]), stats_enum_metadata, counter_id); - SWSS_LOG_ERROR("Counter id received %s, expected %s", - fvField(fvTuples[i]).c_str(), - sai_serialize_enum(counter_id_list[i], stats_enum_metadata).c_str()); if (counter_id != counter_id_list[i]) { SWSS_LOG_ERROR("Counter id %s not as expected %s", @@ -404,7 +401,6 @@ sai_status_t internal_redis_generic_clear_stats( std::string str_object_type = sai_serialize_object_type(object_type); std::string key = str_object_type + ":" + serialized_object_id; - SWSS_LOG_ERROR("generic clear stats key: %s, fields: %lu", key.c_str(), fvTuples.size()); SWSS_LOG_DEBUG("generic clear stats key: %s, fields: %lu", key.c_str(), fvTuples.size()); if (g_record) @@ -421,7 +417,6 @@ sai_status_t internal_redis_generic_clear_stats( s.addSelectable(g_redisGetConsumer.get()); while (true) { - SWSS_LOG_ERROR("wait for clear_stats response"); SWSS_LOG_DEBUG("wait for clear_stats response"); swss::Selectable *sel; @@ -432,7 +427,6 @@ sai_status_t internal_redis_generic_clear_stats( g_redisGetConsumer->pop(kco); const std::string &respKey = kfvKey(kco); const std::string &respOp = kfvOp(kco); - SWSS_LOG_ERROR("response: key = %s, op = %s", respKey.c_str(), respOp.c_str()); SWSS_LOG_DEBUG("response: key = %s, op = %s", respKey.c_str(), respOp.c_str()); if (respOp != "getresponse") // ignore non response messages @@ -454,7 +448,6 @@ sai_status_t internal_redis_generic_clear_stats( count, counter_id_list, kco); - SWSS_LOG_ERROR("generic clear stats status: %s", sai_serialize_status(status).c_str()); SWSS_LOG_DEBUG("generic clear stats status: %s", sai_serialize_status(status).c_str()); return status; } diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index 388f68117..3eeec4442 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1966,7 +1966,6 @@ sai_status_t processClearStatsEvent( switch (object_type) { case SAI_OBJECT_TYPE_BUFFER_POOL: - SWSS_LOG_ERROR("Object type %s, about to clear its stats", str_object_type.c_str()); status = clearStatsGeneric( rid, kco, @@ -2688,7 +2687,6 @@ sai_status_t processEvent( } else if (op == "clear_stats") { - SWSS_LOG_ERROR("Receive op: %s", op.c_str()); return processClearStatsEvent(kco); } else if (op == "flush") From b046a9edc0aff0ad80e3d2070b30e19d8cd428c9 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Thu, 30 May 2019 19:22:27 +0000 Subject: [PATCH 08/13] Remove debugging symbols Signed-off-by: Wenda Ni --- syncd/syncd.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index 3eeec4442..dc0cd1131 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -3069,7 +3069,6 @@ void processFlexCounterEvent( } else if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && field == BUFFER_POOL_COUNTER_ID_LIST) { - SWSS_LOG_ERROR("About to set buffer pool counter list"); std::vector bufferPoolCounterIds; for (const auto &str : idStrings) { From 6d109d86c9d08e7ffb300237eeff96c156756658 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Fri, 31 May 2019 03:27:04 +0000 Subject: [PATCH 09/13] Change the validation order of KeyOpFieldsValuesTuple responded from syncd Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 613304a24..22635fade 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -192,14 +192,6 @@ sai_status_t internal_redis_clear_stats_process( // field: stat_id const auto &key = kfvKey(kco); - 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; - } - sai_status_t status; sai_deserialize_status(key, status); if (status != SAI_STATUS_SUCCESS) @@ -207,6 +199,13 @@ sai_status_t internal_redis_clear_stats_process( 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++) { From dd1d53acc254e032926ba86092b3d281943b4698 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Fri, 31 May 2019 03:30:16 +0000 Subject: [PATCH 10/13] Expand status log utility to include op type as argument Signed-off-by: Wenda Ni --- meta/sai_meta.cpp | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/meta/sai_meta.cpp b/meta/sai_meta.cpp index 4e36d8590..df6920080 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; @@ -3804,7 +3804,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) { @@ -4082,7 +4082,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) { @@ -4370,7 +4370,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) { @@ -4670,7 +4670,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) { @@ -4986,7 +4986,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) { @@ -5302,7 +5302,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) { @@ -5512,7 +5512,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) { @@ -5803,7 +5803,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) { @@ -5926,7 +5926,7 @@ 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; } @@ -6029,7 +6029,7 @@ sai_status_t meta_sai_clear_stats_oid( status = clear_stats(object_type, object_id, stats_enum, count, counter_id_list); - META_LOG_STATUS(status); + META_LOG_STATUS(clear, status); return status; } From 754a3b819e75c5f2526c9e1d0f26dc5ee665d5e0 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Mon, 3 Jun 2019 23:50:41 +0000 Subject: [PATCH 11/13] Address comments: check if object id is present in local db Signed-off-by: Wenda Ni --- syncd/syncd.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index dc0cd1131..e7ff92ee2 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1957,12 +1957,23 @@ sai_status_t processClearStatsEvent( sai_object_id_t object_id; sai_deserialize_object_id(str_object_id, object_id); - sai_object_id_t rid = translate_vid_to_rid(object_id); + sai_object_id_t rid; + sai_status_t status = SAI_STATUS_FAILURE; + std::vector fvTuples; + try + { + rid = translate_vid_to_rid(object_id); + } + catch (const std::exception &e) + { + SWSS_LOG_ERROR("VID %s to RID translation error: %s", str_object_id.c_str(), e.what()); + 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); - - sai_status_t status = SAI_STATUS_FAILURE; switch (object_type) { case SAI_OBJECT_TYPE_BUFFER_POOL: @@ -1977,8 +1988,6 @@ sai_status_t processClearStatsEvent( status = SAI_STATUS_NOT_SUPPORTED; } - std::vector fvTuples; - if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to clear stats"); @@ -1992,7 +2001,6 @@ sai_status_t processClearStatsEvent( } getResponse->set(sai_serialize_status(status), fvTuples, "getresponse"); - return status; } From fcc93e784cfaf973cd87e39febd82e67d76242cd Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Tue, 4 Jun 2019 16:57:51 +0000 Subject: [PATCH 12/13] Leverage newly merged infrastructure to check if object id is present in the local db Signed-off-by: Wenda Ni --- syncd/syncd.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index e7ff92ee2..c9fdc5382 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1960,13 +1960,9 @@ sai_status_t processClearStatsEvent( sai_object_id_t rid; sai_status_t status = SAI_STATUS_FAILURE; std::vector fvTuples; - try - { - rid = translate_vid_to_rid(object_id); - } - catch (const std::exception &e) + if (!try_translate_vid_to_rid(object_id, rid)) { - SWSS_LOG_ERROR("VID %s to RID translation error: %s", str_object_id.c_str(), e.what()); + 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; From 7b77bff2312f82aee72acf93911ae65bbba58863 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Fri, 7 Jun 2019 22:51:57 +0000 Subject: [PATCH 13/13] Fix compile error Signed-off-by: Wenda Ni --- lib/src/sai_redis_generic_stats.cpp | 2 +- meta/sai_serialize.h | 9 --------- meta/saiserialize.cpp | 9 --------- syncd/syncd.cpp | 4 ++-- 4 files changed, 3 insertions(+), 21 deletions(-) diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 22635fade..82888a8ef 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -209,7 +209,7 @@ sai_status_t internal_redis_clear_stats_process( int32_t counter_id; for (uint32_t i = 0; i < count; i++) { - sai_deserialize_enum(fvField(fvTuples[i]), stats_enum_metadata, counter_id); + 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", diff --git a/meta/sai_serialize.h b/meta/sai_serialize.h index f22d5879c..eb3bfd66a 100644 --- a/meta/sai_serialize.h +++ b/meta/sai_serialize.h @@ -165,11 +165,6 @@ void sai_deserialize_number( _Out_ uint32_t& number, _In_ bool hex = false); -void sai_deserialize_enum( - _In_ const std::string& s, - _In_ const sai_enum_metadata_t *meta, - _Out_ int32_t& value); - void sai_deserialize_status( _In_ const std::string& s, _Out_ sai_status_t& status); @@ -282,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 bd7da0e78..0ab05aefa 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 b2ad9a53a..ff83fbee7 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1891,7 +1891,7 @@ sai_status_t clearStatsGeneric( return clearStatsFn( object_id, static_cast(counter_ids.size()), - counter_ids.data()); + reinterpret_cast(counter_ids.data())); } sai_status_t processGetStatsEvent( @@ -3107,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); }