diff --git a/lib/inc/sai_redis.h b/lib/inc/sai_redis.h index 928c16454842..26bdbf5581a0 100644 --- a/lib/inc/sai_redis.h +++ b/lib/inc/sai_redis.h @@ -23,6 +23,10 @@ extern "C" { #include "swss/logger.h" #include "meta/sai_meta.h" +// if we don't receive response from syncd in 60 seconds +// there is something wrong and we should fail +#define GET_RESPONSE_TIMEOUT (60*1000) + extern volatile bool g_record; extern void setRecording(bool record); extern void recordLine(std::string s); @@ -36,10 +40,8 @@ extern service_method_table_t g_services; extern swss::DBConnector *g_db; extern swss::ProducerTable *g_asicState; -extern swss::NotificationProducer *g_notifySyncdProducer; extern swss::ConsumerTable *g_redisGetConsumer; extern swss::NotificationConsumer *g_redisNotifications; -extern swss::NotificationConsumer *g_notifySyncdConsumer; extern swss::Table *g_vidToRid; extern swss::Table *g_ridToVid; diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h new file mode 100644 index 000000000000..2934d280d0d6 --- /dev/null +++ b/lib/inc/sairedis.h @@ -0,0 +1,41 @@ +#ifndef __SAIREDIS__ +#define __SAIREDIS__ + +extern "C" { +#include "sai.h" +} + +#define SYNCD_INIT_VIEW "INIT_VIEW" +#define SYNCD_APPLY_VIEW "APPLY_VIEW" + +typedef enum _sai_redis_notify_syncd_t +{ + SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW, + + SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW + +} sai_redis_notify_syncd_t; + +typedef enum _sai_redis_switch_attr_t +{ + /** + * @brief Will start or stop recording history file for player + * + * @type bool + * @flags CREATE_AND_SET + * @default true + */ + SAI_REDIS_SWITCH_ATTR_RECORD = SAI_SWITCH_ATTR_CUSTOM_RANGE_BASE, + + /** + * @brief Will notify syncd whether to init or apply view + * + * @type sai_redis_notify_syncd_t + * @flags CREATE_AND_SET + * @default SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW + */ + SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD, + +} sai_redis_switch_attr_t; + +#endif // __SAIREDIS__ diff --git a/lib/src/sai_redis_generic_get.cpp b/lib/src/sai_redis_generic_get.cpp index 580040121b63..fa6908112d79 100644 --- a/lib/src/sai_redis_generic_get.cpp +++ b/lib/src/sai_redis_generic_get.cpp @@ -2,10 +2,6 @@ #include "meta/saiserialize.h" #include "meta/saiattributelist.h" -// if we don't receive response from syncd in 60 seconds -// there is something wrong and we should fail -#define GET_RESPONSE_TIMEOUT (60*1000) - sai_status_t internal_redis_get_process( _In_ sai_object_type_t object_type, _In_ uint32_t attr_count, diff --git a/lib/src/sai_redis_interfacequery.cpp b/lib/src/sai_redis_interfacequery.cpp index 51588afe1b35..95c293e78cdd 100644 --- a/lib/src/sai_redis_interfacequery.cpp +++ b/lib/src/sai_redis_interfacequery.cpp @@ -13,10 +13,8 @@ swss::DBConnector *g_dbNtf = NULL; swss::ProducerTable *g_asicState = NULL; // we probably don't need those to tables to access GET requests -swss::NotificationProducer *g_notifySyncdProducer = NULL; swss::ConsumerTable *g_redisGetConsumer = NULL; swss::NotificationConsumer *g_redisNotifications = NULL; -swss::NotificationConsumer *g_notifySyncdConsumer = NULL; swss::RedisClient *g_redisClient = NULL; @@ -57,16 +55,6 @@ sai_status_t sai_api_initialize( g_asicState = new swss::ProducerTable(g_db, "ASIC_STATE"); - if (g_notifySyncdProducer != NULL) - delete g_notifySyncdProducer; - - g_notifySyncdProducer = new swss::NotificationProducer(g_db, "NOTIFYSYNCDREQUERY"); - - if (g_notifySyncdConsumer != NULL) - delete g_notifySyncdConsumer; - - g_notifySyncdConsumer = new swss::NotificationConsumer(g_db, "NOTIFYSYNCRESPONSE"); - if (g_redisGetConsumer != NULL) delete g_redisGetConsumer; diff --git a/lib/src/sai_redis_switch.cpp b/lib/src/sai_redis_switch.cpp index f73e3ab5eaea..282e947c5676 100644 --- a/lib/src/sai_redis_switch.cpp +++ b/lib/src/sai_redis_switch.cpp @@ -1,4 +1,5 @@ #include "sai_redis.h" +#include "sairedis.h" #include #include "swss/selectableevent.h" @@ -6,14 +7,6 @@ // TODO it may be needed to obtain SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP object id -// if we will not get response in 60 seconds when -// notify syncd to compile new state or to switch -// to compiled state, then there is something wrong -#define NOTIFY_SYNCD_TIMEOUT (60*1000) - -#define NOTIFY_SAI_INIT_VIEW "SAI_INIT_VIEW" -#define NOTIFY_SAI_APPLY_VIEW "SAI_APPLY_VIEW" - sai_switch_notification_t redis_switch_notifications; bool g_switchInitialized = false; @@ -64,52 +57,6 @@ void ntf_thread() } } -sai_status_t notify_syncd(const std::string &operation) -{ - SWSS_LOG_ENTER(); - - std::vector entry; - - g_notifySyncdProducer->send(operation, "", entry); - - swss::Select s; - - s.addSelectable(g_notifySyncdConsumer); - - SWSS_LOG_DEBUG("wait for response after: %s", operation.c_str()); - - swss::Selectable *sel; - - int fd; - - int result = s.select(&sel, &fd, NOTIFY_SYNCD_TIMEOUT); - - if (result == swss::Select::OBJECT) - { - swss::KeyOpFieldsValuesTuple kco; - - std::string op; - std::string data; - std::vector values; - - g_notifySyncdConsumer->pop(op, data, values); - - const std::string &strStatus = op; - - sai_status_t status; - - sai_deserialize_status(strStatus, status); - - SWSS_LOG_NOTICE("%s status: %d", op.c_str(), status); - - return status; - } - - SWSS_LOG_ERROR("%s get response failed, result: %d", operation.c_str(), result); - - return SAI_STATUS_FAILURE; -} - void clear_local_state() { SWSS_LOG_ENTER(); @@ -119,7 +66,7 @@ void clear_local_state() if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("failed to init meta db FIXME"); - throw; + throw std::runtime_error("failed to init meta db"); } } @@ -150,53 +97,6 @@ sai_status_t redis_initialize_switch( SWSS_LOG_ENTER(); - if (firmware_path_name == NULL) - { - SWSS_LOG_ERROR("firmware path name is NULL"); - - return SAI_STATUS_FAILURE; - } - - std::string op = std::string(firmware_path_name); - - SWSS_LOG_NOTICE("operation: '%s'", op.c_str()); - - if (op == NOTIFY_SAI_INIT_VIEW || op == NOTIFY_SAI_APPLY_VIEW) - { - sai_status_t status = notify_syncd(op); - - if (status == SAI_STATUS_SUCCESS) - { - SWSS_LOG_NOTICE("sending %s to syncd succeeded", op.c_str()); - - if (g_switchInitialized) - { - if (op == NOTIFY_SAI_INIT_VIEW) - { - SWSS_LOG_NOTICE("clearing current local state sinice init view is called on initialised switch"); - - // TODO since we clear all defaults here and we are compiling new view - // there may be some problems with GET - clear_local_state(); - } - - return status; - } - - // proceed with proper initialization - } - else - { - SWSS_LOG_ERROR("sending %s to syncd failed: %d", op.c_str(), status); - - return status; - } - } - else - { - SWSS_LOG_WARN("unknown operation: '%s'", op.c_str()); - } - if (g_switchInitialized) { SWSS_LOG_ERROR("switch is already initialized"); @@ -321,6 +221,114 @@ void redis_disconnect_switch(void) SWSS_LOG_ERROR("not implemented"); } +sai_status_t sai_redis_internal_notify_syncd( + _In_ const std::string& key) +{ + SWSS_LOG_ENTER(); + + std::vector entry; + + g_asicState->set(key, entry, "notify"); + + swss::Select s; + + s.addSelectable(g_redisGetConsumer); + + while (true) + { + SWSS_LOG_NOTICE("wait for notify response"); + + swss::Selectable *sel; + + int fd; + + int result = s.select(&sel, &fd, GET_RESPONSE_TIMEOUT); + + if (result == swss::Select::OBJECT) + { + swss::KeyOpFieldsValuesTuple kco; + + g_redisGetConsumer->pop(kco); + + const std::string &op = kfvOp(kco); + const std::string &opkey = kfvKey(kco); + + SWSS_LOG_NOTICE("notify response: %s", opkey.c_str()); + + if (op != "notify") + { + continue; + } + + sai_status_t status; + sai_deserialize_status(opkey, status); + + return status; + } + + SWSS_LOG_ERROR("notify syncd failed to get response result from select: %d", result); + break; + } + + SWSS_LOG_ERROR("notify syncd failed to get response"); + + return SAI_STATUS_FAILURE; +} + +sai_status_t sai_redis_notify_syncd( + _In_ const sai_attribute_t *attr) +{ + SWSS_LOG_ENTER(); + + // we need to use "GET" channel to be sure that + // all previous operations were applied, if we don't + // use GET channel then we may hit race condition + // on syncd side where syncd will start compare view + // when there are still objects in op queue + // + // other solution can be to use notify event + // and then on syncd side read all the asic state queue + // and apply changes before switching to init/apply mode + + std::string op; + + switch (attr->value.s32) + { + case SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW: + SWSS_LOG_NOTICE("sending syncd INIT view"); + op = SYNCD_INIT_VIEW; + break; + + case SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW: + SWSS_LOG_NOTICE("sending syncd APPLY view"); + op = SYNCD_APPLY_VIEW; + break; + + default: + SWSS_LOG_ERROR("invalid notify syncd attr value %d", attr->value.s32); + return SAI_STATUS_FAILURE; + } + + sai_status_t status = sai_redis_internal_notify_syncd(op); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("notify syncd failed: %s", sai_serialize_status(status).c_str()); + return status; + } + + SWSS_LOG_NOTICE("notify syncd sycceeded"); + + if (attr->value.s32 == SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW) + { + SWSS_LOG_NOTICE("clearing current local state sinice init view is called on initialised switch"); + + clear_local_state(); + } + + return SAI_STATUS_SUCCESS; +} + /** * Routine Description: * @brief Set switch attribute value @@ -341,10 +349,17 @@ sai_status_t redis_set_switch_attribute( if (attr != NULL) { - if (attr->id == SAI_SWITCH_ATTR_CUSTOM_RANGE_BASE + 1) + switch (attr->id) { - setRecording(attr->value.booldata); - return SAI_STATUS_SUCCESS; + case SAI_REDIS_SWITCH_ATTR_RECORD: + setRecording(attr->value.booldata); + return SAI_STATUS_SUCCESS; + + case SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD: + return sai_redis_notify_syncd(attr); + + default: + break; } } diff --git a/player/Makefile.am b/player/Makefile.am index 2fd3dd8965e2..fc1ef0175909 100644 --- a/player/Makefile.am +++ b/player/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I/usr/include/sai +AM_CPPFLAGS = -I/usr/include/sai -I$(top_srcdir)/lib/inc bin_PROGRAMS = player diff --git a/player/player.cpp b/player/player.cpp index 59ce43322a9e..efb835d56360 100644 --- a/player/player.cpp +++ b/player/player.cpp @@ -14,6 +14,7 @@ extern "C" { #include "meta/saiattributelist.h" #include "swss/logger.h" #include "swss/tokenize.h" +#include "sairedis.h" std::map profile_map; @@ -943,6 +944,8 @@ void handle_get_response( match_redis_with_rec(object_type, get_attr_count, get_attr_list, attr_count, attr_list); } +bool g_notifySyncd = true; + int replay(int argc, char **argv) { //swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG); @@ -956,18 +959,29 @@ int replay(int argc, char **argv) return -1; } - SWSS_LOG_NOTICE("using file: %s", argv[1]); + char* filename = argv[argc - 1]; + + SWSS_LOG_NOTICE("using file: %s", filename); - std::ifstream infile(argv[1]); + std::ifstream infile(filename); if (!infile.is_open()) { - SWSS_LOG_ERROR("failed to open file %s", argv[1]); + SWSS_LOG_ERROR("failed to open file %s", filename); return -1; } std::string line; + if (g_notifySyncd) + { + // tell syncd that we are compiling new view + sai_attribute_t attr; + attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + EXIT_ON_ERROR(sai_switch_api->set_switch_attribute(&attr)); + } + while (std::getline(infile, line)) { // std::cout << "processing " << line << std::endl; @@ -1084,14 +1098,27 @@ int replay(int argc, char **argv) infile.close(); + if (g_notifySyncd) + { + // tell syncd that we want to apply this view + sai_attribute_t attr; + attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW; + EXIT_ON_ERROR(sai_switch_api->set_switch_attribute(&attr)); + } + return 0; } void printUsage() { - std::cout << "Usage: player [-h] recordfile" << std::endl; + std::cout << "Usage: player [-h] recordfile" << std::endl << std::endl; + std::cout << " -C --skipNotifySyncd:" << std::endl; + std::cout << " Will not send notify init/apply view to syncd" << std::endl << std::endl; + std::cout << " -d --enableDebug:" << std::endl; + std::cout << " Enable syslog debug messages" << std::endl << std::endl; std::cout << " -h --help:" << std::endl; - std::cout << " Print out this message" << std::endl; + std::cout << " Print out this message" << std::endl << std::endl; } void handleCmdLine(int argc, char **argv) @@ -1103,12 +1130,14 @@ void handleCmdLine(int argc, char **argv) static struct option long_options[] = { { "help", no_argument, 0, 'h' }, + { "skipNotifySyncd", no_argument, 0, 'C' }, + { "enableDebug", no_argument, 0, 'd' }, { 0, 0, 0, 0 } }; - int option_index = 0; + const char* const optstring = "hCd"; - const char* const optstring = "h"; + int option_index; int c = getopt_long(argc, argv, optstring, long_options, &option_index); @@ -1117,6 +1146,14 @@ void handleCmdLine(int argc, char **argv) switch (c) { + case 'd': + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG); + break; + + case 'C': + g_notifySyncd = false; + break; + case 'h': printUsage(); exit(EXIT_SUCCESS); @@ -1135,10 +1172,12 @@ void handleCmdLine(int argc, char **argv) int main(int argc, char **argv) { - swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE); + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG); SWSS_LOG_ENTER(); + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE); + handleCmdLine(argc, argv); EXIT_ON_ERROR(sai_api_initialize(0, (const service_method_table_t *)&test_services)); @@ -1151,7 +1190,7 @@ int main(int argc, char **argv) // Disable recording sai_attribute_t attr; - attr.id = SAI_SWITCH_ATTR_CUSTOM_RANGE_BASE + 1; + attr.id = SAI_REDIS_SWITCH_ATTR_RECORD; attr.value.booldata = false; EXIT_ON_ERROR(sai_switch_api->set_switch_attribute(&attr)); diff --git a/syncd/Makefile.am b/syncd/Makefile.am index ef5026fa0693..0b72c44c903e 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I/usr/include/sai -I$(top_srcdir)/vslib/inc +AM_CPPFLAGS = -I/usr/include/sai -I$(top_srcdir)/vslib/inc -I$(top_srcdir)/lib/inc bin_PROGRAMS = syncd syncd_request_shutdown diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index 717de3b418f1..6bed08962afe 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -1,11 +1,11 @@ #include "syncd.h" +#include "sairedis.h" #include std::mutex g_mutex; swss::RedisClient *g_redisClient = NULL; -swss::NotificationProducer *notifySyncdResponse = NULL; std::map gProfileMap; @@ -757,6 +757,63 @@ sai_status_t handle_trap( } } +void sendResponse(sai_status_t status) +{ + SWSS_LOG_ENTER(); + + std::string str_status = sai_serialize_status(status); + + std::vector entry; + + SWSS_LOG_NOTICE("sending response: %s", str_status.c_str()); + + getResponse->set(str_status, entry, "notify"); +} + +sai_status_t notifySyncd(const std::string& op) +{ + SWSS_LOG_ENTER(); + + if (g_veryFirstRun) + { + SWSS_LOG_NOTICE("very first run is TRUE, op = %s", op.c_str()); + + // on the very first start of syncd, "compile" view is directly + // applied on device, since it will make it easier to switch + // to new asic state later on when we restart orch agent + + if (op == SYNCD_APPLY_VIEW) + { + g_veryFirstRun = false; + + SWSS_LOG_NOTICE("setting very first run to FALSE, op = %s", op.c_str()); + } + + sendResponse(SAI_STATUS_SUCCESS); + return SAI_STATUS_SUCCESS; + } + + if (op == SYNCD_INIT_VIEW) + { + SWSS_LOG_ERROR("op = %s - not implemented", op.c_str()); + exit_and_notify(EXIT_FAILURE); + } + else if (op == SYNCD_APPLY_VIEW) + { + SWSS_LOG_ERROR("op = %s - not implemented", op.c_str()); + sendResponse(SAI_STATUS_NOT_IMPLEMENTED); + exit_and_notify(EXIT_FAILURE); + } + else + { + SWSS_LOG_ERROR("unknown operation: %s", op.c_str()); + sendResponse(SAI_STATUS_NOT_IMPLEMENTED); + exit_and_notify(EXIT_FAILURE); + } + + return SAI_STATUS_SUCCESS; +} + sai_status_t processEvent(swss::ConsumerTable &consumer) { std::lock_guard lock(g_mutex); @@ -784,6 +841,8 @@ sai_status_t processEvent(swss::ConsumerTable &consumer) api = SAI_COMMON_API_SET; else if (op == "get") api = SAI_COMMON_API_GET; + else if (op == "notify") + return notifySyncd(key); else { if (op != "delget") @@ -902,75 +961,6 @@ void updateLogLevel() } } -void sendResponse(sai_status_t status) -{ - SWSS_LOG_ENTER(); - - std::string str_status = sai_serialize_status(status); - - std::vector entry; - - SWSS_LOG_NOTICE("sending response: %s", str_status.c_str()); - - notifySyncdResponse->send(str_status, "", entry); -} - -void notifySyncd(swss::NotificationConsumer &consumer) -{ - std::lock_guard lock(g_mutex); - - SWSS_LOG_ENTER(); - - std::string op; - std::string data; - std::vector values; - - consumer.pop(op, data, values); - - if (g_veryFirstRun) - { - SWSS_LOG_NOTICE("very first run is TRUE, op = %s", op.c_str()); - - // on the very first start of syncd, "compile" view is directly - // applied on device, since it will make it easier to switch - // to new asic state later on when we restart orch agent - - if (op == NOTIFY_SAI_APPLY_VIEW) - { - g_veryFirstRun = false; - - SWSS_LOG_NOTICE("setting very first run to FALSE, op = %s", op.c_str()); - } - - sendResponse(SAI_STATUS_SUCCESS); - return; - } - - sai_status_t status = SAI_STATUS_FAILURE; - - if (op == NOTIFY_SAI_INIT_VIEW) - { - // TODO - SWSS_LOG_ERROR("op = %s - not implemented", op.c_str()); - - status = SAI_STATUS_NOT_IMPLEMENTED; - } - - if (op == NOTIFY_SAI_APPLY_VIEW) - { - // TODO - SWSS_LOG_ERROR("op = %s - not implemented", op.c_str()); - - status = SAI_STATUS_NOT_IMPLEMENTED; - } - else - { - SWSS_LOG_ERROR("unknown operation: %s", op.c_str()); - } - - sendResponse(status); -} - struct cmdOptions { int countersThreadIntervalInSeconds; @@ -1300,7 +1290,6 @@ int main(int argc, char **argv) updateLogLevel(); swss::ConsumerTable *asicState = new swss::ConsumerTable(db, "ASIC_STATE"); - swss::NotificationConsumer *notifySyncdQuery = new swss::NotificationConsumer(db, "NOTIFYSYNCDREQUERY"); swss::NotificationConsumer *restartQuery = new swss::NotificationConsumer(db, "RESTARTQUERY"); // at the end we cant use producer consumer concept since @@ -1308,7 +1297,6 @@ int main(int argc, char **argv) // also "remove" from response queue will also trigger another "response" getResponse = new swss::ProducerTable(db, "GETRESPONSE"); notifications = new swss::NotificationProducer(dbNtf, "NOTIFICATIONS"); - notifySyncdResponse = new swss::NotificationProducer(db, "NOTIFYSYNCDRESPONSE"); g_veryFirstRun = isVeryFirstRun(); @@ -1392,7 +1380,6 @@ int main(int argc, char **argv) swss::Select s; s.addSelectable(asicState); - s.addSelectable(notifySyncdQuery); s.addSelectable(restartQuery); while(true) @@ -1409,12 +1396,6 @@ int main(int argc, char **argv) break; } - if (sel == notifySyncdQuery) - { - notifySyncd(*notifySyncdQuery); - continue; - } - if (result == swss::Select::OBJECT) { processEvent(*(swss::ConsumerTable*)sel); diff --git a/syncd/syncd.h b/syncd/syncd.h index 05d0779597aa..a5ee01328002 100644 --- a/syncd/syncd.h +++ b/syncd/syncd.h @@ -51,9 +51,6 @@ extern "C" { #define DEFAULT_TRAP_GROUP_ID "DEFAULT_TRAP_GROUP_ID" #define CPU_PORT_ID "CPU_PORT_ID" -#define NOTIFY_SAI_INIT_VIEW "SAI_INIT_VIEW" -#define NOTIFY_SAI_APPLY_VIEW "SAI_APPLY_VIEW" - #define SAI_COLD_BOOT 0 #define SAI_WARM_BOOT 1 #define SAI_FAST_BOOT 2