From 46206316645ebfa3bd3a5623d39f0af7073a6be8 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Sat, 16 Jul 2022 12:34:16 -0700 Subject: [PATCH 01/12] Add support of mdio IPC server class using sai switch api and unix socket Signed-off-by: Jiahua Wang --- configure.ac | 1 + syncd/Makefile.am | 5 + syncd/MdioIpcServer.cpp | 463 ++++++++++++++++++++++++++++++++++++++++ syncd/MdioIpcServer.h | 74 +++++++ syncd/SaiSwitch.cpp | 8 + syncd/Syncd.cpp | 16 ++ syncd/VendorSai.cpp | 8 + 7 files changed, 575 insertions(+) create mode 100644 syncd/MdioIpcServer.cpp create mode 100644 syncd/MdioIpcServer.h diff --git a/configure.ac b/configure.ac index 659dce57d..6dc404010 100644 --- a/configure.ac +++ b/configure.ac @@ -16,6 +16,7 @@ AX_CODE_COVERAGE AX_ADD_AM_MACRO_STATIC([]) AM_CONDITIONAL(SONIC_ASIC_PLATFORM_BAREFOOT, test x$CONFIGURED_PLATFORM = xbarefoot) +AM_CONDITIONAL(SONIC_ASIC_PLATFORM_BROADCOM, test x$CONFIGURED_PLATFORM = xbroadcom) AC_ARG_ENABLE(debug, [ --enable-debug turn on debugging], diff --git a/syncd/Makefile.am b/syncd/Makefile.am index ddcdd04fc..05859c0fd 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -71,6 +71,11 @@ syncd_CXXFLAGS += -DSAITHRIFT=yes syncd_LDADD += -lrpcserver -lthrift endif +if SONIC_ASIC_PLATFORM_BROADCOM +libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU +libSyncd_a_SOURCES += MdioIpcServer.cpp +endif + libSyncdRequestShutdown_a_SOURCES = \ RequestShutdown.cpp \ RequestShutdownCommandLineOptions.cpp \ diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp new file mode 100644 index 000000000..7a920c75c --- /dev/null +++ b/syncd/MdioIpcServer.cpp @@ -0,0 +1,463 @@ +#include "MdioIpcServer.h" + +#include "meta/sai_serialize.h" + +#include "swss/logger.h" + +#include +#include +#include + +#define SYNCD_IPC_SOCK_SYNCD "/var/run/sswsyncd" +#define SYNCD_IPC_SOCK_HOST "/var/run/docker-syncd" +#define SYNCD_IPC_SOCK_FILE "mdio-ipc" +#define SYNCD_IPC_BUFF_SIZE 256 /* buffer size */ + +#define CONN_TIMEOUT 30 /* sec, connection timeout */ +#define CONN_MAX 18 /* max. number of connections */ + +#ifndef COUNTOF +#define COUNTOF(x) ((int)(sizeof((x)) / sizeof((x)[0]))) +#endif + +using namespace syncd; + +sai_object_id_t MdioIpcServer::mdioSwitchId = SAI_NULL_OBJECT_ID; +bool MdioIpcServer::syncdContext = true; +bool MdioIpcServer::accessInitialized = false; +sai_switch_api_t* MdioIpcServer::switch_mdio_api = NULL; + +typedef struct syncd_mdio_ipc_conn_s +{ + int fd; + time_t timeout; +} syncd_mdio_ipc_conn_t; + +MdioIpcServer::MdioIpcServer(int globalContext) +{ + SWSS_LOG_ENTER(); + + MdioIpcServer::taskAlive = 0; + + /* globalContext == 0 for syncd, globalContext > 0 for gbsyncd */ + MdioIpcServer::syncdContext = (globalContext == 0); +} + +MdioIpcServer::~MdioIpcServer() +{ + SWSS_LOG_ENTER(); + + // empty +} + +void MdioIpcServer::setSwitchId( + _In_ sai_object_id_t switchRid) +{ + SWSS_LOG_ENTER(); + + /* MDIO switch id is only relevent in syncd but not in gbsyncd */ + if (!MdioIpcServer::syncdContext) + { + return; + } + + MdioIpcServer::mdioSwitchId = switchRid; + + SWSS_LOG_NOTICE("Initialize mdio switch id with RID = %s", + sai_serialize_object_id(MdioIpcServer::mdioSwitchId).c_str()); +} + +void MdioIpcServer::setSwitchMdioApi(sai_switch_api_t *switch_api) +{ + SWSS_LOG_ENTER(); + + /* Switch mdio api is only relevent in syncd but not in gbsyncd */ + if (!MdioIpcServer::syncdContext) + { + return; + } + + MdioIpcServer::switch_mdio_api = switch_api; + MdioIpcServer::accessInitialized = true; +} + +/* + * mdio reg: Read from the PHY register + * mdio reg val: Write to the PHY register + */ +sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common(char *resp, int argc, char *argv[]) +{ + int ret = 0; + uint32_t mdio_addr = 0, reg_addr = 0, val = 0; + + if (argc < 3) + { + return SAI_STATUS_INVALID_PARAMETER; + } + + mdio_addr = (uint32_t)strtoul(argv[1], NULL, 0); + reg_addr = (uint32_t)strtoul(argv[2], NULL, 0); + + if (MdioIpcServer::mdioSwitchId == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("mdio switch id not initialized"); + return SAI_STATUS_FAILURE; + } + + if (!MdioIpcServer::accessInitialized) + { + SWSS_LOG_ERROR("switch mdio access not initialized"); + return SAI_STATUS_FAILURE; + } + + if (argc > 3) + { + val = (uint32_t)strtoul(argv[3], NULL, 0); + ret = MdioIpcServer::switch_mdio_api->switch_mdio_write(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + sprintf(resp, "%d\n", ret); + } + else + { + ret = MdioIpcServer::switch_mdio_api->switch_mdio_read(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + sprintf(resp, "%d 0x%x\n", ret, val); + } + + return (ret == 0) ? SAI_STATUS_SUCCESS : SAI_STATUS_FAILURE; +} + +#if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) +sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common_cl22(char *resp, int argc, char *argv[]) +{ + int ret = 0; + uint32_t mdio_addr = 0, reg_addr = 0, val = 0; + + if (argc < 3) + { + return SAI_STATUS_INVALID_PARAMETER; + } + + mdio_addr = (uint32_t)strtoul(argv[1], NULL, 0); + reg_addr = (uint32_t)strtoul(argv[2], NULL, 0); + + if (MdioIpcServer::mdioSwitchId == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("mdio switch id not initialized"); + return SAI_STATUS_FAILURE; + } + + if (!MdioIpcServer::accessInitialized) + { + SWSS_LOG_ERROR("switch mdio cl22 access not initialized"); + return SAI_STATUS_FAILURE; + } + + if (argc > 3) + { + val = (uint32_t)strtoul(argv[3], NULL, 0); + ret = MdioIpcServer::switch_mdio_api->switch_mdio_cl22_read(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + sprintf(resp, "%d\n", ret); + } + else + { + ret = MdioIpcServer::switch_mdio_api->switch_mdio_cl22_read(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + sprintf(resp, "%d 0x%x\n", ret, val); + } + + return (ret == 0) ? SAI_STATUS_SUCCESS : SAI_STATUS_FAILURE; +} +#endif + +sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio(char *resp, int argc, char *argv[]) +{ + return syncd_ipc_cmd_mdio_common(resp, argc, argv); +} + +sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_cl22(char *resp, int argc, char *argv[]) +{ +#if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) + return MdioIpcServer::syncd_ipc_cmd_mdio_common_cl22(resp, argc, argv); +#else + /* In this case, sai configuration should take care of mdio clause 22 */ + return MdioIpcServer::syncd_ipc_cmd_mdio_common(resp, argc, argv); +#endif +} + +void *MdioIpcServer::syncd_ipc_task_main() +{ + int i; + int fd; + int len; + int ret; + int sock_srv; + int sock_cli; + int sock_max; + syncd_mdio_ipc_conn_t conn[CONN_MAX]; + struct sockaddr_un addr; + char path[64]; + fd_set rfds; + char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE], *argv[64], *save; + int argc = 0; + + SWSS_LOG_ENTER(); + + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + fd = open(path, O_DIRECTORY); + if (fd < 0) + { + SWSS_LOG_ERROR("Unable to open the directory %s for IPC\n", path); + return &errno; + } + + sock_srv = socket(AF_UNIX, SOCK_STREAM, 0); + if (sock_srv < 0) + { + SWSS_LOG_ERROR("socket() returns %d", errno); + return &errno; + } + + /***************************************/ + /* Set up the UNIX sockaddr structure */ + /* by using AF_UNIX for the family and */ + /* giving it a filepath to bind to. */ + /* */ + /* Unlink the file so the bind will */ + /* succeed, then bind to that file. */ + /***************************************/ + memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + sprintf(addr.sun_path, "%s/%s.srv", path, SYNCD_IPC_SOCK_FILE); + unlink(addr.sun_path); + if (bind(sock_srv, (struct sockaddr *)&addr, sizeof(addr)) < 0) + { + SWSS_LOG_ERROR("bind() returns %d", errno); + close(sock_srv); + return &errno; + } + + /* Listen for the upcoming client sockets */ + if (listen(sock_srv, CONN_MAX) < 0) + { + SWSS_LOG_ERROR("listen() returns %d", errno); + unlink(addr.sun_path); + close(sock_srv); + return &errno; + } + + SWSS_LOG_NOTICE("IPC service is online\n"); + + memset(conn, 0, sizeof(conn)); + while (taskAlive) + { + time_t now; + struct timeval timeout; + + /* garbage collection */ + now = time(NULL); + for (i = 0; i < CONN_MAX; ++i) + { + if ((conn[i].fd > 0) && (conn[i].timeout < now)) + { + SWSS_LOG_NOTICE("socket %d: connection timeout\n", conn[i].fd); + close(conn[i].fd); + conn[i].fd = 0; + conn[i].timeout = 0; + } + } + + /* reset readfds */ + FD_ZERO(&rfds); + FD_SET(sock_srv, &rfds); + sock_max = sock_srv; + for (i = 0; i < CONN_MAX; ++i) + { + if (conn[i].fd <= 0) + { + continue; + } + FD_SET(conn[i].fd, &rfds); + if (sock_max < conn[i].fd) + { + sock_max = conn[i].fd; + } + } + + /* monitor the socket descriptors */ + timeout.tv_sec = 1; + timeout.tv_usec = 0; + ret = select(sock_max + 1, &rfds, NULL, NULL, &timeout); + if (ret == 0) + { + continue; + } + else if (ret < 0) + { + if (errno == EINTR) + { + continue; + } + else + { + SWSS_LOG_ERROR("select() returns %d", errno); + break; + } + } + + /* Accept the new connection */ + now = time(NULL); + if (FD_ISSET(sock_srv, &rfds)) + { + sock_cli = accept(sock_srv, NULL, NULL); + if (sock_cli <= 0) + { + SWSS_LOG_ERROR("accept() returns %d", errno); + continue; + } + + for (i = 0; i < CONN_MAX; ++i) + { + if (conn[i].fd <= 0) + { + break; + } + } + if (i < CONN_MAX) + { + conn[i].fd = sock_cli; + conn[i].timeout = now + CONN_TIMEOUT; + } + else + { + SWSS_LOG_ERROR("too many connections!"); + close(sock_cli); + } + } + + /* Handle the client requests */ + for (i = 0; i < CONN_MAX; ++i) + { + sai_status_t rc = SAI_STATUS_NOT_SUPPORTED; + + sock_cli = conn[i].fd; + if ((sock_cli <= 0) || !FD_ISSET(sock_cli, &rfds)) + { + continue; + } + + /* get the command message */ + len = (int)recv(sock_cli, (void *)cmd, sizeof(cmd) - 1, 0); + if (len <= 0) + { + close(sock_cli); + conn[i].fd = 0; + conn[i].timeout = 0; + continue; + } + cmd[len] = 0; + + /* tokenize the command string */ + argc = 0; + std::string str(cmd); + boost::algorithm::trim(str); + boost::algorithm::to_lower(str); + std::vectorv(str.size()+1); + memcpy( &v.front(), str.c_str(), str.size() + 1 ); + argv[argc++] = strtok_r(v.data(), " \t\r\n", &save); + while (argc < COUNTOF(argv)) + { + argv[argc] = strtok_r(NULL, " \t\r\n", &save); + if (argv[argc] == NULL) + break; + ++argc; + } + + /* command dispatch */ + resp[0] = 0; + rc = SAI_STATUS_NOT_SUPPORTED; + if (strcmp("mdio", argv[0]) == 0) + { + rc = MdioIpcServer::syncd_ipc_cmd_mdio(resp, argc, argv); + } + else if (strcmp("mdio-cl22", argv[0]) == 0) + { + rc = MdioIpcServer::syncd_ipc_cmd_mdio_cl22(resp, argc, argv); + } + + /* build the error message */ + if (rc != SAI_STATUS_SUCCESS) + { + sprintf(resp, "%d\n", rc); + } + + /* send out the response */ + len = (int)strlen(resp); + if (send(sock_cli, resp, len, 0) < len) + { + SWSS_LOG_ERROR("send() returns %d", errno); + } + + /* update the connection timeout counter */ + conn[i].timeout = time(NULL) + CONN_TIMEOUT; + } + } + + /* close socket descriptors */ + for (i = 0; i < CONN_MAX; ++i) + { + if (conn[i].fd <= 0) + { + continue; + } + close(conn[i].fd); + } + close(sock_srv); + unlink(addr.sun_path); + return &errno; +} + +void *MdioIpcServer::syncd_ipc_task_enter(void *ctx) +{ + MdioIpcServer *mdioServer = (MdioIpcServer *)ctx; + return mdioServer->syncd_ipc_task_main(); +} + +void MdioIpcServer::stopMdioThread(void) +{ + int *err = NULL; + + SWSS_LOG_ENTER(); + + /* MDIO IPC server thread is only relevent in syncd but not in gbsyncd */ + if (!MdioIpcServer::syncdContext) + { + return; + } + + taskAlive = 0; + pthread_join(taskId, (void **)&err); + SWSS_LOG_NOTICE("IPC task thread is stopped\n"); +} + +int MdioIpcServer::startMdioThread() +{ + int err = 0; + + SWSS_LOG_ENTER(); + + /* MDIO IPC server thread is only relevent in syncd but not in gbsyncd */ + if (!MdioIpcServer::syncdContext) + { + return 0; + } + + if (!taskAlive) + { + taskAlive = 1; + err = pthread_create(&taskId, NULL, MdioIpcServer::syncd_ipc_task_enter, this); + if (err != 0) + { + MdioIpcServer::taskAlive = 0; + SWSS_LOG_ERROR("Unable to create IPC task thread"); + } + } + return err; +} diff --git a/syncd/MdioIpcServer.h b/syncd/MdioIpcServer.h new file mode 100644 index 000000000..b742dfa03 --- /dev/null +++ b/syncd/MdioIpcServer.h @@ -0,0 +1,74 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +extern "C" { +#include +#include +} + +namespace syncd +{ + class MdioIpcServer + { + public: + + MdioIpcServer(int globalContext); + + virtual ~MdioIpcServer(); + + public: + + static void setSwitchId(_In_ sai_object_id_t switchRid); + + static void setSwitchMdioApi(sai_switch_api_t *switch_api); + + int startMdioThread(); + + void stopMdioThread(); + + public: + + static sai_object_id_t mdioSwitchId; + + static bool syncdContext; + + static bool accessInitialized; + + static sai_switch_api_t *switch_mdio_api; + + static void *syncd_ipc_task_enter(void*); + + private: + + void *syncd_ipc_task_main(); + + sai_status_t syncd_ipc_cmd_mdio_common(char *resp, int argc, char *argv[]); + +#if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) + sai_status_t syncd_ipc_cmd_mdio_common_cl22(char *resp, int argc, char *argv[]); +#endif + + sai_status_t syncd_ipc_cmd_mdio(char *resp, int argc, char *argv[]); + + sai_status_t syncd_ipc_cmd_mdio_cl22(char *resp, int argc, char *argv[]); + + pthread_t taskId; + + int taskAlive; + + }; +} diff --git a/syncd/SaiSwitch.cpp b/syncd/SaiSwitch.cpp index 839969f3b..68f4c2bc2 100644 --- a/syncd/SaiSwitch.cpp +++ b/syncd/SaiSwitch.cpp @@ -8,6 +8,10 @@ #include "meta/sai_serialize.h" #include "swss/logger.h" +#ifdef MDIO_ACCESS_USE_NPU +#include "MdioIpcServer.h" +#endif + using namespace syncd; #define MAX_OBJLIST_LEN 128 @@ -39,6 +43,10 @@ SaiSwitch::SaiSwitch( GlobalSwitchId::setSwitchId(m_switch_rid); +#ifdef MDIO_ACCESS_USE_NPU + MdioIpcServer::setSwitchId(m_switch_rid); +#endif + m_hardware_info = saiGetHardwareInfo(); /* diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 583ab893b..3ffeb0d3d 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -33,6 +33,10 @@ #include #include +#ifdef MDIO_ACCESS_USE_NPU +#include "MdioIpcServer.h" +#endif + #define DEF_SAI_WARM_BOOT_DATA_FILE "/var/warmboot/sai-warmboot.bin" using namespace syncd; @@ -4484,6 +4488,10 @@ void Syncd::run() std::shared_ptr s = std::make_shared(); +#ifdef MDIO_ACCESS_USE_NPU + MdioIpcServer mdioServer(m_commandLineOptions->m_globalContext); +#endif + try { onSyncdStart(m_commandLineOptions->m_startType == SAI_START_TYPE_WARM_BOOT); @@ -4494,6 +4502,10 @@ void Syncd::run() // notification queue is created before we create switch m_processor->startNotificationsProcessingThread(); +#ifdef MDIO_ACCESS_USE_NPU + mdioServer.startMdioThread(); +#endif + SWSS_LOG_NOTICE("syncd listening for events"); s->addSelectable(m_selectableChannel.get()); @@ -4678,6 +4690,10 @@ void Syncd::run() m_manager->removeAllCounters(); +#ifdef MDIO_ACCESS_USE_NPU + mdioServer.stopMdioThread(); +#endif + sai_status_t status = removeAllSwitches(); // Stop notification thread after removing switch diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index f917a33cb..07dcc2311 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -4,6 +4,10 @@ #include "swss/logger.h" +#ifdef MDIO_ACCESS_USE_NPU +#include "MdioIpcServer.h" +#endif + #include #include @@ -95,6 +99,10 @@ sai_status_t VendorSai::initialize( } m_apiInitialized = true; + +#ifdef MDIO_ACCESS_USE_NPU + MdioIpcServer::setSwitchMdioApi(m_apis.switch_api); +#endif } return status; From 73c94cf83f82c7d38ba8691d8584a58d3cf2fcdb Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Sat, 16 Jul 2022 20:33:35 -0700 Subject: [PATCH 02/12] mdio IPC server checks NULL message input Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 7a920c75c..e17e0f884 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -373,7 +373,11 @@ void *MdioIpcServer::syncd_ipc_task_main() /* command dispatch */ resp[0] = 0; rc = SAI_STATUS_NOT_SUPPORTED; - if (strcmp("mdio", argv[0]) == 0) + if (argv[0] == NULL) + { + rc = SAI_STATUS_NOT_SUPPORTED; + } + else if (strcmp("mdio", argv[0]) == 0) { rc = MdioIpcServer::syncd_ipc_cmd_mdio(resp, argc, argv); } From 94417e62538ead87caea79d950cf0c480e8d75f5 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 18 Jul 2022 12:00:33 -0700 Subject: [PATCH 03/12] Fix SWSS_LOG_ENTER() Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index e17e0f884..ad40265d9 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -55,7 +55,7 @@ void MdioIpcServer::setSwitchId( { SWSS_LOG_ENTER(); - /* MDIO switch id is only relevent in syncd but not in gbsyncd */ + /* MDIO switch id is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::syncdContext) { return; @@ -184,6 +184,8 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_cl22(char *resp, int argc, char * void *MdioIpcServer::syncd_ipc_task_main() { + SWSS_LOG_ENTER(); + int i; int fd; int len; @@ -198,8 +200,6 @@ void *MdioIpcServer::syncd_ipc_task_main() char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE], *argv[64], *save; int argc = 0; - SWSS_LOG_ENTER(); - strcpy(path, SYNCD_IPC_SOCK_SYNCD); fd = open(path, O_DIRECTORY); if (fd < 0) @@ -420,16 +420,18 @@ void *MdioIpcServer::syncd_ipc_task_main() void *MdioIpcServer::syncd_ipc_task_enter(void *ctx) { + SWSS_LOG_ENTER(); + MdioIpcServer *mdioServer = (MdioIpcServer *)ctx; return mdioServer->syncd_ipc_task_main(); } void MdioIpcServer::stopMdioThread(void) { - int *err = NULL; - SWSS_LOG_ENTER(); + int *err = NULL; + /* MDIO IPC server thread is only relevent in syncd but not in gbsyncd */ if (!MdioIpcServer::syncdContext) { @@ -443,10 +445,10 @@ void MdioIpcServer::stopMdioThread(void) int MdioIpcServer::startMdioThread() { - int err = 0; - SWSS_LOG_ENTER(); + int err = 0; + /* MDIO IPC server thread is only relevent in syncd but not in gbsyncd */ if (!MdioIpcServer::syncdContext) { From dc4b53ff04987e85a38c03f588f9137af4bee91e Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 18 Jul 2022 12:48:49 -0700 Subject: [PATCH 04/12] Fix some word spelling Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index ad40265d9..8d1c1e079 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -71,7 +71,7 @@ void MdioIpcServer::setSwitchMdioApi(sai_switch_api_t *switch_api) { SWSS_LOG_ENTER(); - /* Switch mdio api is only relevent in syncd but not in gbsyncd */ + /* Switch mdio api is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::syncdContext) { return; @@ -220,7 +220,7 @@ void *MdioIpcServer::syncd_ipc_task_main() /* by using AF_UNIX for the family and */ /* giving it a filepath to bind to. */ /* */ - /* Unlink the file so the bind will */ + /* Un-link the file so the bind will */ /* succeed, then bind to that file. */ /***************************************/ memset(&addr, 0, sizeof(addr)); @@ -264,7 +264,7 @@ void *MdioIpcServer::syncd_ipc_task_main() } } - /* reset readfds */ + /* reset read file descriptors */ FD_ZERO(&rfds); FD_SET(sock_srv, &rfds); sock_max = sock_srv; @@ -432,7 +432,7 @@ void MdioIpcServer::stopMdioThread(void) int *err = NULL; - /* MDIO IPC server thread is only relevent in syncd but not in gbsyncd */ + /* MDIO IPC server thread is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::syncdContext) { return; @@ -449,7 +449,7 @@ int MdioIpcServer::startMdioThread() int err = 0; - /* MDIO IPC server thread is only relevent in syncd but not in gbsyncd */ + /* MDIO IPC server thread is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::syncdContext) { return 0; From 70209490b5adbe2131c2f05374a11dc45060aba3 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 18 Jul 2022 13:30:11 -0700 Subject: [PATCH 05/12] Fix some word spelling Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 8d1c1e079..7931ba07f 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -216,11 +216,11 @@ void *MdioIpcServer::syncd_ipc_task_main() } /***************************************/ - /* Set up the UNIX sockaddr structure */ + /* Set up the UNIX "sockaddr" structure*/ /* by using AF_UNIX for the family and */ - /* giving it a filepath to bind to. */ + /* giving it a file path to bind to. */ /* */ - /* Un-link the file so the bind will */ + /* Unlink the file so the bind will */ /* succeed, then bind to that file. */ /***************************************/ memset(&addr, 0, sizeof(addr)); From ae87c599dd6da9f3c63e5c3495d19be0a98ba216 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 18 Jul 2022 14:10:09 -0700 Subject: [PATCH 06/12] Fix some word spelling Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 7931ba07f..7a16aea6d 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -216,11 +216,11 @@ void *MdioIpcServer::syncd_ipc_task_main() } /***************************************/ - /* Set up the UNIX "sockaddr" structure*/ + /* Set up the UNIX socket address */ /* by using AF_UNIX for the family and */ /* giving it a file path to bind to. */ /* */ - /* Unlink the file so the bind will */ + /* Delete the file so the bind will */ /* succeed, then bind to that file. */ /***************************************/ memset(&addr, 0, sizeof(addr)); From b67a15eb110dc1c241f94949fb0e9e4e1dd50fc0 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Tue, 19 Jul 2022 17:07:54 -0700 Subject: [PATCH 07/12] Add MdioIpcServer::clearSwitchMdioApi() Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 23 +++++++++++++++++++---- syncd/MdioIpcServer.h | 12 ++++++++---- syncd/VendorSai.cpp | 4 ++++ 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 7a16aea6d..81712019d 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -33,12 +33,12 @@ typedef struct syncd_mdio_ipc_conn_s time_t timeout; } syncd_mdio_ipc_conn_t; -MdioIpcServer::MdioIpcServer(int globalContext) +MdioIpcServer::MdioIpcServer( + _In_ int globalContext): + taskAlive(0) { SWSS_LOG_ENTER(); - MdioIpcServer::taskAlive = 0; - /* globalContext == 0 for syncd, globalContext > 0 for gbsyncd */ MdioIpcServer::syncdContext = (globalContext == 0); } @@ -67,7 +67,8 @@ void MdioIpcServer::setSwitchId( sai_serialize_object_id(MdioIpcServer::mdioSwitchId).c_str()); } -void MdioIpcServer::setSwitchMdioApi(sai_switch_api_t *switch_api) +void MdioIpcServer::setSwitchMdioApi( + _In_ sai_switch_api_t *switch_api) { SWSS_LOG_ENTER(); @@ -81,6 +82,20 @@ void MdioIpcServer::setSwitchMdioApi(sai_switch_api_t *switch_api) MdioIpcServer::accessInitialized = true; } +void MdioIpcServer::clearSwitchMdioApi() +{ + SWSS_LOG_ENTER(); + + /* Switch mdio api is only relevant in syncd but not in gbsyncd */ + if (!MdioIpcServer::syncdContext) + { + return; + } + + MdioIpcServer::switch_mdio_api = NULL; + MdioIpcServer::accessInitialized = false; +} + /* * mdio reg: Read from the PHY register * mdio reg val: Write to the PHY register diff --git a/syncd/MdioIpcServer.h b/syncd/MdioIpcServer.h index b742dfa03..cfe7e29cd 100644 --- a/syncd/MdioIpcServer.h +++ b/syncd/MdioIpcServer.h @@ -26,15 +26,20 @@ namespace syncd { public: - MdioIpcServer(int globalContext); + MdioIpcServer( + _In_ int globalContext); virtual ~MdioIpcServer(); public: - static void setSwitchId(_In_ sai_object_id_t switchRid); + static void setSwitchId( + _In_ sai_object_id_t switchRid); - static void setSwitchMdioApi(sai_switch_api_t *switch_api); + static void setSwitchMdioApi( + _In_ sai_switch_api_t *switch_api); + + static void clearSwitchMdioApi(); int startMdioThread(); @@ -69,6 +74,5 @@ namespace syncd pthread_t taskId; int taskAlive; - }; } diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index 07dcc2311..c38f526c9 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -113,6 +113,10 @@ sai_status_t VendorSai::uninitialize(void) SWSS_LOG_ENTER(); VENDOR_CHECK_API_INITIALIZED(); +#ifdef MDIO_ACCESS_USE_NPU + MdioIpcServer::clearSwitchMdioApi(); +#endif + auto status = sai_api_uninitialize(); if (status == SAI_STATUS_SUCCESS) From c96b56242131ade0b9f26ee6d2215f7791625bf3 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 20 Jul 2022 21:50:29 -0700 Subject: [PATCH 08/12] Add mdioRegRead/mdioRegWrite to SaiInterface class Signed-off-by: Jiahua Wang --- meta/SaiInterface.h | 22 +++++++++++++++ syncd/MdioIpcServer.cpp | 53 +++++------------------------------- syncd/MdioIpcServer.h | 14 ++++------ syncd/Syncd.cpp | 2 +- syncd/VendorSai.cpp | 60 ++++++++++++++++++++++++++++++++--------- syncd/VendorSai.h | 16 +++++++++++ 6 files changed, 98 insertions(+), 69 deletions(-) diff --git a/meta/SaiInterface.h b/meta/SaiInterface.h index fb8cdf0c4..3ba792917 100644 --- a/meta/SaiInterface.h +++ b/meta/SaiInterface.h @@ -222,6 +222,28 @@ namespace sairedis _In_ uint32_t attrCount, _In_ const sai_attribute_t *attrList) = 0; + virtual sai_status_t mdioRegRead( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _Out_ uint32_t *reg_val) + { + return SAI_STATUS_FAILURE; + } + + virtual sai_status_t mdioRegWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _In_ const uint32_t *reg_val) + { + return SAI_STATUS_FAILURE; + } + public: // SAI API virtual sai_status_t objectTypeGetAvailability( diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 81712019d..643cd0f11 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -24,8 +24,6 @@ using namespace syncd; sai_object_id_t MdioIpcServer::mdioSwitchId = SAI_NULL_OBJECT_ID; bool MdioIpcServer::syncdContext = true; -bool MdioIpcServer::accessInitialized = false; -sai_switch_api_t* MdioIpcServer::switch_mdio_api = NULL; typedef struct syncd_mdio_ipc_conn_s { @@ -34,7 +32,9 @@ typedef struct syncd_mdio_ipc_conn_s } syncd_mdio_ipc_conn_t; MdioIpcServer::MdioIpcServer( + _In_ std::shared_ptr vendorSai, _In_ int globalContext): + m_vendorSai(vendorSai), taskAlive(0) { SWSS_LOG_ENTER(); @@ -67,35 +67,6 @@ void MdioIpcServer::setSwitchId( sai_serialize_object_id(MdioIpcServer::mdioSwitchId).c_str()); } -void MdioIpcServer::setSwitchMdioApi( - _In_ sai_switch_api_t *switch_api) -{ - SWSS_LOG_ENTER(); - - /* Switch mdio api is only relevant in syncd but not in gbsyncd */ - if (!MdioIpcServer::syncdContext) - { - return; - } - - MdioIpcServer::switch_mdio_api = switch_api; - MdioIpcServer::accessInitialized = true; -} - -void MdioIpcServer::clearSwitchMdioApi() -{ - SWSS_LOG_ENTER(); - - /* Switch mdio api is only relevant in syncd but not in gbsyncd */ - if (!MdioIpcServer::syncdContext) - { - return; - } - - MdioIpcServer::switch_mdio_api = NULL; - MdioIpcServer::accessInitialized = false; -} - /* * mdio reg: Read from the PHY register * mdio reg val: Write to the PHY register @@ -119,21 +90,15 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common(char *resp, int argc, char return SAI_STATUS_FAILURE; } - if (!MdioIpcServer::accessInitialized) - { - SWSS_LOG_ERROR("switch mdio access not initialized"); - return SAI_STATUS_FAILURE; - } - if (argc > 3) { val = (uint32_t)strtoul(argv[3], NULL, 0); - ret = MdioIpcServer::switch_mdio_api->switch_mdio_write(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + ret = m_vendorSai->mdioRegWrite(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, false, &val); sprintf(resp, "%d\n", ret); } else { - ret = MdioIpcServer::switch_mdio_api->switch_mdio_read(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + ret = m_vendorSai->mdioRegRead(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, false, &val); sprintf(resp, "%d 0x%x\n", ret, val); } @@ -160,21 +125,15 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common_cl22(char *resp, int argc, return SAI_STATUS_FAILURE; } - if (!MdioIpcServer::accessInitialized) - { - SWSS_LOG_ERROR("switch mdio cl22 access not initialized"); - return SAI_STATUS_FAILURE; - } - if (argc > 3) { val = (uint32_t)strtoul(argv[3], NULL, 0); - ret = MdioIpcServer::switch_mdio_api->switch_mdio_cl22_read(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + ret = m_vendorSai->mdioRegWrite(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, true, &val); sprintf(resp, "%d\n", ret); } else { - ret = MdioIpcServer::switch_mdio_api->switch_mdio_cl22_read(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, &val); + ret = m_vendorSai->mdioRegRead(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, true, &val); sprintf(resp, "%d 0x%x\n", ret, val); } diff --git a/syncd/MdioIpcServer.h b/syncd/MdioIpcServer.h index cfe7e29cd..34bda5b7e 100644 --- a/syncd/MdioIpcServer.h +++ b/syncd/MdioIpcServer.h @@ -15,6 +15,8 @@ #include #include +#include "VendorSai.h" + extern "C" { #include #include @@ -27,6 +29,7 @@ namespace syncd public: MdioIpcServer( + _In_ std::shared_ptr vendorSai, _In_ int globalContext); virtual ~MdioIpcServer(); @@ -36,11 +39,6 @@ namespace syncd static void setSwitchId( _In_ sai_object_id_t switchRid); - static void setSwitchMdioApi( - _In_ sai_switch_api_t *switch_api); - - static void clearSwitchMdioApi(); - int startMdioThread(); void stopMdioThread(); @@ -51,10 +49,6 @@ namespace syncd static bool syncdContext; - static bool accessInitialized; - - static sai_switch_api_t *switch_mdio_api; - static void *syncd_ipc_task_enter(void*); private: @@ -71,6 +65,8 @@ namespace syncd sai_status_t syncd_ipc_cmd_mdio_cl22(char *resp, int argc, char *argv[]); + std::shared_ptr m_vendorSai; + pthread_t taskId; int taskAlive; diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 3ffeb0d3d..1d64ff5df 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -4489,7 +4489,7 @@ void Syncd::run() std::shared_ptr s = std::make_shared(); #ifdef MDIO_ACCESS_USE_NPU - MdioIpcServer mdioServer(m_commandLineOptions->m_globalContext); + MdioIpcServer mdioServer(m_vendorSai, m_commandLineOptions->m_globalContext); #endif try diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index c38f526c9..17b0b304a 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -4,10 +4,6 @@ #include "swss/logger.h" -#ifdef MDIO_ACCESS_USE_NPU -#include "MdioIpcServer.h" -#endif - #include #include @@ -99,10 +95,6 @@ sai_status_t VendorSai::initialize( } m_apiInitialized = true; - -#ifdef MDIO_ACCESS_USE_NPU - MdioIpcServer::setSwitchMdioApi(m_apis.switch_api); -#endif } return status; @@ -113,10 +105,6 @@ sai_status_t VendorSai::uninitialize(void) SWSS_LOG_ENTER(); VENDOR_CHECK_API_INITIALIZED(); -#ifdef MDIO_ACCESS_USE_NPU - MdioIpcServer::clearSwitchMdioApi(); -#endif - auto status = sai_api_uninitialize(); if (status == SAI_STATUS_SUCCESS) @@ -1189,6 +1177,54 @@ sai_status_t VendorSai::flushFdbEntries( return m_apis.fdb_api->flush_fdb_entries(switch_id, attr_count, attr_list); } +sai_status_t VendorSai::mdioRegRead( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _Out_ uint32_t *reg_val) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + +#if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) + if (clause_22) + { + return m_apis.switch_api->switch_mdio_cl22_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); + } + else +#endif + { + return m_apis.switch_api->switch_mdio_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); + } +} + +sai_status_t VendorSai::mdioRegWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _In_ const uint32_t *reg_val) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + +#if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) + if (clause_22) + { + return m_apis.switch_api->switch_mdio_cl22_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); + } + else +#endif + { + return m_apis.switch_api->switch_mdio_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); + } +} + // SAI API sai_status_t VendorSai::objectTypeGetAvailability( diff --git a/syncd/VendorSai.h b/syncd/VendorSai.h index 091a2afda..1c620ee72 100644 --- a/syncd/VendorSai.h +++ b/syncd/VendorSai.h @@ -121,6 +121,22 @@ namespace syncd _In_ uint32_t attrCount, _In_ const sai_attribute_t *attrList) override; + virtual sai_status_t mdioRegRead( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _Out_ uint32_t *reg_val) override; + + virtual sai_status_t mdioRegWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _In_ const uint32_t *reg_val) override; + public: // SAI API virtual sai_status_t objectTypeGetAvailability( From d9160baf8d94d0a64d0ba7a8f7f541877ab18548 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 20 Jul 2022 23:00:51 -0700 Subject: [PATCH 09/12] Refactor SaiInterface::mdioRegRead/SaiInterface::mdioRegWrite Signed-off-by: Jiahua Wang --- meta/SaiInterface.cpp | 26 ++++++++++++++++++++++++++ meta/SaiInterface.h | 10 ++-------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/meta/SaiInterface.cpp b/meta/SaiInterface.cpp index 82e37242d..7390f894b 100644 --- a/meta/SaiInterface.cpp +++ b/meta/SaiInterface.cpp @@ -197,3 +197,29 @@ sai_status_t SaiInterface::get( return SAI_STATUS_FAILURE; } } + +sai_status_t SaiInterface::mdioRegRead( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _Out_ uint32_t *reg_val) +{ + SWSS_LOG_ENTER(); + + return SAI_STATUS_FAILURE; +} + +sai_status_t SaiInterface::mdioRegWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ bool clause_22, + _In_ const uint32_t *reg_val) +{ + SWSS_LOG_ENTER(); + + return SAI_STATUS_FAILURE; +} diff --git a/meta/SaiInterface.h b/meta/SaiInterface.h index 3ba792917..6211c5319 100644 --- a/meta/SaiInterface.h +++ b/meta/SaiInterface.h @@ -228,10 +228,7 @@ namespace sairedis _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, _In_ bool clause_22, - _Out_ uint32_t *reg_val) - { - return SAI_STATUS_FAILURE; - } + _Out_ uint32_t *reg_val); virtual sai_status_t mdioRegWrite( _In_ sai_object_id_t switch_id, @@ -239,10 +236,7 @@ namespace sairedis _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, _In_ bool clause_22, - _In_ const uint32_t *reg_val) - { - return SAI_STATUS_FAILURE; - } + _In_ const uint32_t *reg_val); public: // SAI API From 1960244dbc8b9092bb529ba5bc743277fb66110e Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Fri, 22 Jul 2022 14:02:37 -0700 Subject: [PATCH 10/12] Add SaiInterface::switchMdioRead/switchMdioWrite/switchMdioCl22Read/switchMdioCl22Write Signed-off-by: Jiahua Wang --- meta/SaiInterface.cpp | 30 ++++++++++-- meta/SaiInterface.h | 20 ++++++-- syncd/Makefile.am | 2 +- syncd/MdioIpcServer.cpp | 104 +++++++++++++++++++++++++--------------- syncd/MdioIpcServer.h | 34 ++++--------- syncd/SaiSwitch.cpp | 8 ---- syncd/Syncd.cpp | 26 +++++----- syncd/Syncd.h | 3 ++ syncd/VendorSai.cpp | 56 ++++++++++++++-------- syncd/VendorSai.h | 20 ++++++-- 10 files changed, 185 insertions(+), 118 deletions(-) diff --git a/meta/SaiInterface.cpp b/meta/SaiInterface.cpp index 7390f894b..b268f7890 100644 --- a/meta/SaiInterface.cpp +++ b/meta/SaiInterface.cpp @@ -198,12 +198,11 @@ sai_status_t SaiInterface::get( } } -sai_status_t SaiInterface::mdioRegRead( +sai_status_t SaiInterface::switchMdioRead( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _Out_ uint32_t *reg_val) { SWSS_LOG_ENTER(); @@ -211,12 +210,35 @@ sai_status_t SaiInterface::mdioRegRead( return SAI_STATUS_FAILURE; } -sai_status_t SaiInterface::mdioRegWrite( +sai_status_t SaiInterface::switchMdioWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ const uint32_t *reg_val) +{ + SWSS_LOG_ENTER(); + + return SAI_STATUS_FAILURE; +} + +sai_status_t SaiInterface::switchMdioCl22Read( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _Out_ uint32_t *reg_val) +{ + SWSS_LOG_ENTER(); + + return SAI_STATUS_FAILURE; +} + +sai_status_t SaiInterface::switchMdioCl22Write( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _In_ const uint32_t *reg_val) { SWSS_LOG_ENTER(); diff --git a/meta/SaiInterface.h b/meta/SaiInterface.h index 6211c5319..8acf4983c 100644 --- a/meta/SaiInterface.h +++ b/meta/SaiInterface.h @@ -222,20 +222,32 @@ namespace sairedis _In_ uint32_t attrCount, _In_ const sai_attribute_t *attrList) = 0; - virtual sai_status_t mdioRegRead( + virtual sai_status_t switchMdioRead( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _Out_ uint32_t *reg_val); - virtual sai_status_t mdioRegWrite( + virtual sai_status_t switchMdioWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ const uint32_t *reg_val); + + virtual sai_status_t switchMdioCl22Read( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _Out_ uint32_t *reg_val); + + virtual sai_status_t switchMdioCl22Write( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _In_ const uint32_t *reg_val); public: // SAI API diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 05859c0fd..14519ff99 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -23,6 +23,7 @@ libSyncd_a_SOURCES = \ FlexCounterManager.cpp \ GlobalSwitchId.cpp \ HardReiniter.cpp \ + MdioIpcServer.cpp \ MetadataLogger.cpp \ NotificationHandler.cpp \ NotificationProcessor.cpp \ @@ -73,7 +74,6 @@ endif if SONIC_ASIC_PLATFORM_BROADCOM libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU -libSyncd_a_SOURCES += MdioIpcServer.cpp endif libSyncdRequestShutdown_a_SOURCES = \ diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 643cd0f11..55e22e5d3 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -1,3 +1,17 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + #include "MdioIpcServer.h" #include "meta/sai_serialize.h" @@ -22,8 +36,7 @@ using namespace syncd; -sai_object_id_t MdioIpcServer::mdioSwitchId = SAI_NULL_OBJECT_ID; -bool MdioIpcServer::syncdContext = true; +bool MdioIpcServer::m_syncdContext = true; typedef struct syncd_mdio_ipc_conn_s { @@ -35,19 +48,22 @@ MdioIpcServer::MdioIpcServer( _In_ std::shared_ptr vendorSai, _In_ int globalContext): m_vendorSai(vendorSai), - taskAlive(0) + m_switchRid(SAI_NULL_OBJECT_ID), + m_taskThread(), + m_taskAlive(0) { SWSS_LOG_ENTER(); /* globalContext == 0 for syncd, globalContext > 0 for gbsyncd */ - MdioIpcServer::syncdContext = (globalContext == 0); + MdioIpcServer::m_syncdContext = (globalContext == 0); } MdioIpcServer::~MdioIpcServer() { SWSS_LOG_ENTER(); - // empty + m_taskAlive = 0; + if(m_taskThread.joinable()) m_taskThread.join(); } void MdioIpcServer::setSwitchId( @@ -55,16 +71,24 @@ void MdioIpcServer::setSwitchId( { SWSS_LOG_ENTER(); +#ifdef MDIO_ACCESS_USE_NPU /* MDIO switch id is only relevant in syncd but not in gbsyncd */ - if (!MdioIpcServer::syncdContext) + if (!MdioIpcServer::m_syncdContext) + { + return; + } + + if (m_switchRid != SAI_NULL_OBJECT_ID) { + SWSS_LOG_ERROR("mdio switch id already initialized"); return; } - MdioIpcServer::mdioSwitchId = switchRid; + m_switchRid = switchRid; SWSS_LOG_NOTICE("Initialize mdio switch id with RID = %s", - sai_serialize_object_id(MdioIpcServer::mdioSwitchId).c_str()); + sai_serialize_object_id(m_switchRid).c_str()); +#endif } /* @@ -84,7 +108,7 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common(char *resp, int argc, char mdio_addr = (uint32_t)strtoul(argv[1], NULL, 0); reg_addr = (uint32_t)strtoul(argv[2], NULL, 0); - if (MdioIpcServer::mdioSwitchId == SAI_NULL_OBJECT_ID) + if (m_switchRid == SAI_NULL_OBJECT_ID) { SWSS_LOG_ERROR("mdio switch id not initialized"); return SAI_STATUS_FAILURE; @@ -93,12 +117,12 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common(char *resp, int argc, char if (argc > 3) { val = (uint32_t)strtoul(argv[3], NULL, 0); - ret = m_vendorSai->mdioRegWrite(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, false, &val); + ret = m_vendorSai->switchMdioWrite(m_switchRid, mdio_addr, reg_addr, 1, &val); sprintf(resp, "%d\n", ret); } else { - ret = m_vendorSai->mdioRegRead(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, false, &val); + ret = m_vendorSai->switchMdioRead(m_switchRid, mdio_addr, reg_addr, 1, &val); sprintf(resp, "%d 0x%x\n", ret, val); } @@ -119,7 +143,7 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common_cl22(char *resp, int argc, mdio_addr = (uint32_t)strtoul(argv[1], NULL, 0); reg_addr = (uint32_t)strtoul(argv[2], NULL, 0); - if (MdioIpcServer::mdioSwitchId == SAI_NULL_OBJECT_ID) + if (m_switchRid == SAI_NULL_OBJECT_ID) { SWSS_LOG_ERROR("mdio switch id not initialized"); return SAI_STATUS_FAILURE; @@ -128,12 +152,12 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_common_cl22(char *resp, int argc, if (argc > 3) { val = (uint32_t)strtoul(argv[3], NULL, 0); - ret = m_vendorSai->mdioRegWrite(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, true, &val); + ret = m_vendorSai->switchMdioCl22Write(m_switchRid, mdio_addr, reg_addr, 1, &val); sprintf(resp, "%d\n", ret); } else { - ret = m_vendorSai->mdioRegRead(MdioIpcServer::mdioSwitchId, mdio_addr, reg_addr, 1, true, &val); + ret = m_vendorSai->switchMdioCl22Read(m_switchRid, mdio_addr, reg_addr, 1, &val); sprintf(resp, "%d 0x%x\n", ret, val); } @@ -156,7 +180,7 @@ sai_status_t MdioIpcServer::syncd_ipc_cmd_mdio_cl22(char *resp, int argc, char * #endif } -void *MdioIpcServer::syncd_ipc_task_main() +int MdioIpcServer::syncd_ipc_task_main() { SWSS_LOG_ENTER(); @@ -179,14 +203,14 @@ void *MdioIpcServer::syncd_ipc_task_main() if (fd < 0) { SWSS_LOG_ERROR("Unable to open the directory %s for IPC\n", path); - return &errno; + return errno; } sock_srv = socket(AF_UNIX, SOCK_STREAM, 0); if (sock_srv < 0) { SWSS_LOG_ERROR("socket() returns %d", errno); - return &errno; + return errno; } /***************************************/ @@ -205,7 +229,7 @@ void *MdioIpcServer::syncd_ipc_task_main() { SWSS_LOG_ERROR("bind() returns %d", errno); close(sock_srv); - return &errno; + return errno; } /* Listen for the upcoming client sockets */ @@ -214,13 +238,13 @@ void *MdioIpcServer::syncd_ipc_task_main() SWSS_LOG_ERROR("listen() returns %d", errno); unlink(addr.sun_path); close(sock_srv); - return &errno; + return errno; } SWSS_LOG_NOTICE("IPC service is online\n"); memset(conn, 0, sizeof(conn)); - while (taskAlive) + while (m_taskAlive) { time_t now; struct timeval timeout; @@ -389,55 +413,57 @@ void *MdioIpcServer::syncd_ipc_task_main() } close(sock_srv); unlink(addr.sun_path); - return &errno; + return errno; } -void *MdioIpcServer::syncd_ipc_task_enter(void *ctx) +void MdioIpcServer::syncd_ipc_task_enter(void *ctx) { SWSS_LOG_ENTER(); MdioIpcServer *mdioServer = (MdioIpcServer *)ctx; - return mdioServer->syncd_ipc_task_main(); + mdioServer->syncd_ipc_task_main(); } void MdioIpcServer::stopMdioThread(void) { SWSS_LOG_ENTER(); - int *err = NULL; - +#ifdef MDIO_ACCESS_USE_NPU /* MDIO IPC server thread is only relevant in syncd but not in gbsyncd */ - if (!MdioIpcServer::syncdContext) + if (!MdioIpcServer::m_syncdContext) { return; } - taskAlive = 0; - pthread_join(taskId, (void **)&err); + m_taskAlive = 0; + m_taskThread.join(); SWSS_LOG_NOTICE("IPC task thread is stopped\n"); +#endif } int MdioIpcServer::startMdioThread() { SWSS_LOG_ENTER(); - int err = 0; - +#ifdef MDIO_ACCESS_USE_NPU /* MDIO IPC server thread is only relevant in syncd but not in gbsyncd */ - if (!MdioIpcServer::syncdContext) + if (!MdioIpcServer::m_syncdContext) { return 0; } - if (!taskAlive) + if (!m_taskAlive) { - taskAlive = 1; - err = pthread_create(&taskId, NULL, MdioIpcServer::syncd_ipc_task_enter, this); - if (err != 0) - { - MdioIpcServer::taskAlive = 0; - SWSS_LOG_ERROR("Unable to create IPC task thread"); + m_taskAlive = 1; + try { + m_taskThread = std::thread(&MdioIpcServer::syncd_ipc_task_enter, this); + } + catch(const std::exception& e) { + MdioIpcServer::m_taskAlive = 0; + SWSS_LOG_ERROR("Unable to create IPC task thread: %s", e.what()); + return SAI_STATUS_FAILURE; } } - return err; +#endif + return SAI_STATUS_SUCCESS; } diff --git a/syncd/MdioIpcServer.h b/syncd/MdioIpcServer.h index 34bda5b7e..2da4e6c64 100644 --- a/syncd/MdioIpcServer.h +++ b/syncd/MdioIpcServer.h @@ -1,20 +1,6 @@ #pragma once -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - +#include #include "VendorSai.h" extern "C" { @@ -36,7 +22,7 @@ namespace syncd public: - static void setSwitchId( + void setSwitchId( _In_ sai_object_id_t switchRid); int startMdioThread(); @@ -45,15 +31,11 @@ namespace syncd public: - static sai_object_id_t mdioSwitchId; - - static bool syncdContext; - - static void *syncd_ipc_task_enter(void*); + static void syncd_ipc_task_enter(void*); private: - void *syncd_ipc_task_main(); + int syncd_ipc_task_main(); sai_status_t syncd_ipc_cmd_mdio_common(char *resp, int argc, char *argv[]); @@ -65,10 +47,14 @@ namespace syncd sai_status_t syncd_ipc_cmd_mdio_cl22(char *resp, int argc, char *argv[]); + static bool m_syncdContext; + std::shared_ptr m_vendorSai; - pthread_t taskId; + sai_object_id_t m_switchRid; + + std::thread m_taskThread; - int taskAlive; + int m_taskAlive; }; } diff --git a/syncd/SaiSwitch.cpp b/syncd/SaiSwitch.cpp index 68f4c2bc2..839969f3b 100644 --- a/syncd/SaiSwitch.cpp +++ b/syncd/SaiSwitch.cpp @@ -8,10 +8,6 @@ #include "meta/sai_serialize.h" #include "swss/logger.h" -#ifdef MDIO_ACCESS_USE_NPU -#include "MdioIpcServer.h" -#endif - using namespace syncd; #define MAX_OBJLIST_LEN 128 @@ -43,10 +39,6 @@ SaiSwitch::SaiSwitch( GlobalSwitchId::setSwitchId(m_switch_rid); -#ifdef MDIO_ACCESS_USE_NPU - MdioIpcServer::setSwitchId(m_switch_rid); -#endif - m_hardware_info = saiGetHardwareInfo(); /* diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 1d64ff5df..2c256576b 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -33,10 +33,6 @@ #include #include -#ifdef MDIO_ACCESS_USE_NPU -#include "MdioIpcServer.h" -#endif - #define DEF_SAI_WARM_BOOT_DATA_FILE "/var/warmboot/sai-warmboot.bin" using namespace syncd; @@ -110,6 +106,7 @@ Syncd::Syncd( // we need STATE_DB ASIC_DB and COUNTERS_DB m_dbAsic = std::make_shared(m_contextConfig->m_dbAsic, 0); + m_mdioIpcServer = std::make_shared(m_vendorSai, m_commandLineOptions->m_globalContext); if (m_contextConfig->m_zmqEnable) { @@ -2616,6 +2613,8 @@ sai_status_t Syncd::processOidCreate( m_switches[switchVid] = std::make_shared(switchVid, objectRid, m_client, m_translator, m_vendorSai); + m_mdioIpcServer->setSwitchId(objectRid); + startDiagShell(objectRid); } @@ -3900,6 +3899,8 @@ void Syncd::onSwitchCreateInInitViewMode( m_switches[switchVid] = std::make_shared(switchVid, switchRid, m_client, m_translator, m_vendorSai); + m_mdioIpcServer->setSwitchId(switchRid); + startDiagShell(switchRid); } else @@ -4488,10 +4489,6 @@ void Syncd::run() std::shared_ptr s = std::make_shared(); -#ifdef MDIO_ACCESS_USE_NPU - MdioIpcServer mdioServer(m_vendorSai, m_commandLineOptions->m_globalContext); -#endif - try { onSyncdStart(m_commandLineOptions->m_startType == SAI_START_TYPE_WARM_BOOT); @@ -4502,9 +4499,12 @@ void Syncd::run() // notification queue is created before we create switch m_processor->startNotificationsProcessingThread(); -#ifdef MDIO_ACCESS_USE_NPU - mdioServer.startMdioThread(); -#endif + for (auto& sw: m_switches) + { + m_mdioIpcServer->setSwitchId(sw.second->getRid()); + } + + m_mdioIpcServer->startMdioThread(); SWSS_LOG_NOTICE("syncd listening for events"); @@ -4690,9 +4690,7 @@ void Syncd::run() m_manager->removeAllCounters(); -#ifdef MDIO_ACCESS_USE_NPU - mdioServer.stopMdioThread(); -#endif + m_mdioIpcServer->stopMdioThread(); sai_status_t status = removeAllSwitches(); diff --git a/syncd/Syncd.h b/syncd/Syncd.h index d84153703..b7622bf01 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -17,6 +17,7 @@ #include "BreakConfig.h" #include "NotificationProducerBase.h" #include "TimerWatchdog.h" +#include "MdioIpcServer.h" #include "meta/SaiAttributeList.h" #include "meta/SelectableChannel.h" @@ -443,6 +444,8 @@ namespace syncd std::shared_ptr m_selectableChannel; + std::shared_ptr m_mdioIpcServer; + bool m_enableSyncMode; private: diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index 17b0b304a..303cfde66 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -1177,12 +1177,39 @@ sai_status_t VendorSai::flushFdbEntries( return m_apis.fdb_api->flush_fdb_entries(switch_id, attr_count, attr_list); } -sai_status_t VendorSai::mdioRegRead( +sai_status_t VendorSai::switchMdioRead( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _Out_ uint32_t *reg_val) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + + return m_apis.switch_api->switch_mdio_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); +} + +sai_status_t VendorSai::switchMdioWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ const uint32_t *reg_val) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + + return m_apis.switch_api->switch_mdio_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); +} + +sai_status_t VendorSai::switchMdioCl22Read( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _Out_ uint32_t *reg_val) { MUTEX(); @@ -1190,23 +1217,17 @@ sai_status_t VendorSai::mdioRegRead( VENDOR_CHECK_API_INITIALIZED(); #if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) - if (clause_22) - { - return m_apis.switch_api->switch_mdio_cl22_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); - } - else + return m_apis.switch_api->switch_mdio_cl22_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); +#else + return m_apis.switch_api->switch_mdio_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); #endif - { - return m_apis.switch_api->switch_mdio_read(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); - } } -sai_status_t VendorSai::mdioRegWrite( +sai_status_t VendorSai::switchMdioCl22Write( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _In_ const uint32_t *reg_val) { MUTEX(); @@ -1214,15 +1235,10 @@ sai_status_t VendorSai::mdioRegWrite( VENDOR_CHECK_API_INITIALIZED(); #if (SAI_API_VERSION >= SAI_VERSION(1, 11, 0)) - if (clause_22) - { - return m_apis.switch_api->switch_mdio_cl22_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); - } - else + return m_apis.switch_api->switch_mdio_cl22_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); +#else + return m_apis.switch_api->switch_mdio_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); #endif - { - return m_apis.switch_api->switch_mdio_write(switch_id, device_addr, start_reg_addr, number_of_registers, reg_val); - } } // SAI API diff --git a/syncd/VendorSai.h b/syncd/VendorSai.h index 1c620ee72..3ef2f7694 100644 --- a/syncd/VendorSai.h +++ b/syncd/VendorSai.h @@ -121,20 +121,32 @@ namespace syncd _In_ uint32_t attrCount, _In_ const sai_attribute_t *attrList) override; - virtual sai_status_t mdioRegRead( + virtual sai_status_t switchMdioRead( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _Out_ uint32_t *reg_val) override; - virtual sai_status_t mdioRegWrite( + virtual sai_status_t switchMdioWrite( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ const uint32_t *reg_val) override; + + virtual sai_status_t switchMdioCl22Read( + _In_ sai_object_id_t switch_id, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _Out_ uint32_t *reg_val) override; + + virtual sai_status_t switchMdioCl22Write( _In_ sai_object_id_t switch_id, _In_ uint32_t device_addr, _In_ uint32_t start_reg_addr, _In_ uint32_t number_of_registers, - _In_ bool clause_22, _In_ const uint32_t *reg_val) override; public: // SAI API From 6f226fbf777cebb29e46e4a2fc562913a8396777 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Sat, 23 Jul 2022 21:00:07 -0700 Subject: [PATCH 11/12] Change single line if statement Signed-off-by: Jiahua Wang --- syncd/MdioIpcServer.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 55e22e5d3..452719055 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -63,7 +63,10 @@ MdioIpcServer::~MdioIpcServer() SWSS_LOG_ENTER(); m_taskAlive = 0; - if(m_taskThread.joinable()) m_taskThread.join(); + if(m_taskThread.joinable()) + { + m_taskThread.join(); + } } void MdioIpcServer::setSwitchId( From 8e22a48fd5ed1065f53b91ebf64ae9c867b0546e Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Sun, 24 Jul 2022 02:57:32 -0700 Subject: [PATCH 12/12] Update aspell dict Signed-off-by: Jiahua Wang --- tests/aspell.en.pws | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index 05c891b08..1ceca3ecd 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -31,6 +31,7 @@ FDB FDBs FIXME FlexCounter +gbsyncd GCM GRE GUID @@ -87,6 +88,7 @@ TODO TPID TXSC TestCase +tokenize VID VIDCOUNTER VIDTORID @@ -261,6 +263,8 @@ macsec MACsec MCAST md +mdio +MDIO Mellanox memcpy metadata @@ -295,6 +299,7 @@ param params performTransition pfc +PHY plaintext pn PN