From 1fd8a92690845bd4b13ac75959a0e1f11dced2ba Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 28 Sep 2022 16:49:39 -0700 Subject: [PATCH 01/30] Add mdio IPC client library Signed-off-by: Jiahua Wang --- debian/syncd.dirs | 1 + debian/syncd.install | 1 + syncd/Makefile.am | 15 ++- syncd/MdioIpcClient.cpp | 224 ++++++++++++++++++++++++++++++++++++++++ syncd/MdioIpcClient.h | 21 ++++ 5 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 syncd/MdioIpcClient.cpp create mode 100644 syncd/MdioIpcClient.h diff --git a/debian/syncd.dirs b/debian/syncd.dirs index e772481755..527b78f2d8 100644 --- a/debian/syncd.dirs +++ b/debian/syncd.dirs @@ -1 +1,2 @@ usr/bin +usr/lib diff --git a/debian/syncd.install b/debian/syncd.install index bdf0f4c8d1..c559f1fea8 100644 --- a/debian/syncd.install +++ b/debian/syncd.install @@ -5,3 +5,4 @@ usr/bin/saidiscovery usr/bin/saiasiccmp usr/bin/syncd* syncd/scripts/* usr/bin +usr/lib/*/libMdioIpcClient.so.* diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 14519ff997..a4bf32bb44 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -8,7 +8,9 @@ endif bin_PROGRAMS = syncd syncd_request_shutdown tests -noinst_LIBRARIES = libSyncd.a libSyncdRequestShutdown.a +lib_LTLIBRARIES = libMdioIpcClient.la + +noinst_LIBRARIES = libSyncd.a libSyncdRequestShutdown.a libMdioIpcClient.a libSyncd_a_SOURCES = \ AsicOperation.cpp \ @@ -89,6 +91,17 @@ syncd_request_shutdown_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) syncd_request_shutdown_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) syncd_request_shutdown_LDADD = libSyncdRequestShutdown.a $(top_srcdir)/lib/libSaiRedis.a -lhiredis -lswsscommon -lpthread $(CODE_COVERAGE_LIBS) +libMdioIpcClient_a_SOURCES = MdioIpcClient.cpp + +libMdioIpcClient_la_SOURCES = MdioIpcClient.cpp + +libMdioIpcClient_a_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) +libMdioIpcClient_a_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) + +libMdioIpcClient_la_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) +libMdioIpcClient_la_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) +libMdioIpcClient_la_LIBADD = $(CODE_COVERAGE_LIBS) + tests_SOURCES = tests.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) tests_LDFLAGS = -Wl,-rpath,$(top_srcdir)/lib/.libs -Wl,-rpath,$(top_srcdir)/meta/.libs diff --git a/syncd/MdioIpcClient.cpp b/syncd/MdioIpcClient.cpp new file mode 100644 index 0000000000..93bcb67426 --- /dev/null +++ b/syncd/MdioIpcClient.cpp @@ -0,0 +1,224 @@ +/* Includes */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "MdioIpcClient.h" + +#define SAI_IPC_SOCK_DIR_SYNCD "/var/run/sswsyncd" +#define SAI_IPC_SOCK_DIR_HOST "/var/run/docker-syncd" +#define SAI_IPC_SOCK_FILE "mdio-ipc" +#define SAI_IPC_BUFF_SIZE 256 + +#define CONN_TIMEOUT 25 /* shorter than 30 sec on server side */ + +static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; + +/* Global variables */ + +static int syncd_mdio_ipc_command(char *cmd, char *resp) +{ + int fd; + ssize_t ret; + size_t len; + struct sockaddr_un saddr, caddr; + static int sock = 0; + static char path[128] = { 0 }; + static time_t timeout = 0; + + if (timeout < time(NULL)) + { + /* It might already be timed out at the server side, reconnect ... */ + if (sock > 0) + { + close(sock); + } + sock = 0; + } + + if (strlen(path) == 0) + { + strcpy(path, SAI_IPC_SOCK_DIR_SYNCD); + fd = open(path, O_DIRECTORY); + if (fd < 0) + { + printf("%s: Program is not run on host\n", __func__); + strcpy(path, SAI_IPC_SOCK_DIR_HOST); + fd = open(path, O_DIRECTORY); + if (fd < 0) + { + printf("%s: Unable to open the directory for IPC\n", __func__); + return errno; + } + } + } + + if (sock <= 0) + { + sock = socket(AF_UNIX, SOCK_STREAM, 0); + if (sock < 0) + { + perror("socket()"); + return errno; + } + + caddr.sun_family = AF_UNIX; + snprintf(caddr.sun_path, sizeof(caddr.sun_path), "%s/%s.cli.%d", path, SAI_IPC_SOCK_FILE, getpid()); + unlink(caddr.sun_path); + if (bind(sock, (struct sockaddr *)&caddr, sizeof(caddr)) < 0) + { + perror("bind()"); + close(sock); + sock = 0; + return errno; + } + + saddr.sun_family = AF_UNIX; + snprintf(saddr.sun_path, sizeof(saddr.sun_path), "%s/%s.srv", path, SAI_IPC_SOCK_FILE); + if (connect(sock, (struct sockaddr *) &saddr, sizeof(saddr)) < 0) + { + perror("connect()"); + close(sock); + sock = 0; + unlink(caddr.sun_path); + return errno; + } + } + + len = strlen(cmd); + ret = send(sock, cmd, len, 0); + if (ret < (ssize_t)len) + { + printf("send failed, ret=%ld, expected=%ld\n", ret, len); + close(sock); + sock = 0; + unlink(caddr.sun_path); + return -EIO; + } + + ret = recv(sock, resp, SAI_IPC_BUFF_SIZE - 1, 0); + if (ret <= 0) + { + printf("recv failed, ret=%ld\n", ret); + close(sock); + sock = 0; + unlink(caddr.sun_path); + return -EIO; + } + + timeout = time(NULL) + CONN_TIMEOUT; + return (int)strtol(resp, NULL, 0); +} + + +/* Function to read data from MDIO interface */ +extern "C" sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, + uint32_t number_of_registers, uint32_t *data) +{ + int rc = SAI_STATUS_FAILURE; + char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + + if (number_of_registers > 1) + { + printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + return SAI_STATUS_FAILURE; + } + + pthread_mutex_lock(&mutex); + + sprintf(cmd, "mdio 0x%x 0x%x\n", mdio_addr, reg_addr); + if (syncd_mdio_ipc_command(cmd, resp) == 0) + { + *data = (uint32_t)strtoul(strchrnul(resp, ' ') + 1, NULL, 0); + rc = SAI_STATUS_SUCCESS; + } + + pthread_mutex_unlock(&mutex); + return rc; +} + +/* Function to write data to MDIO interface */ +extern "C" sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, + uint32_t number_of_registers, const uint32_t *data) +{ + int rc = SAI_STATUS_FAILURE; + char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + + if (number_of_registers > 1) + { + printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + return SAI_STATUS_FAILURE; + } + + pthread_mutex_lock(&mutex); + + sprintf(cmd, "mdio 0x%x 0x%x 0x%x\n", mdio_addr, reg_addr, *data); + if (syncd_mdio_ipc_command(cmd, resp) == 0) + { + rc = SAI_STATUS_SUCCESS; + } + + pthread_mutex_unlock(&mutex); + return rc; +} + +/* Function to read data using clause 22 from MDIO interface */ +extern "C" sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, + uint32_t number_of_registers, uint32_t *data) +{ + int rc = SAI_STATUS_FAILURE; + char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + + if (number_of_registers > 1) + { + printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + return SAI_STATUS_FAILURE; + } + + pthread_mutex_lock(&mutex); + + sprintf(cmd, "mdio-cl22 0x%x 0x%x\n", mdio_addr, reg_addr); + if (syncd_mdio_ipc_command(cmd, resp) == 0) + { + *data = (uint32_t)strtoul(strchrnul(resp, ' ') + 1, NULL, 0); + rc = SAI_STATUS_SUCCESS; + } + + pthread_mutex_unlock(&mutex); + return rc; +} + +/* Function to write data using clause 22 to MDIO interface */ +extern "C" sai_status_t mdio_write_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, + uint32_t number_of_registers, const uint32_t *data) +{ + int rc = SAI_STATUS_FAILURE; + char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + + if (number_of_registers > 1) + { + printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + return SAI_STATUS_FAILURE; + } + + pthread_mutex_lock(&mutex); + + sprintf(cmd, "mdio-cl22 0x%x 0x%x 0x%x\n", mdio_addr, reg_addr, *data); + if (syncd_mdio_ipc_command(cmd, resp) == 0) + { + rc = SAI_STATUS_SUCCESS; + } + + pthread_mutex_unlock(&mutex); + return rc; +} diff --git a/syncd/MdioIpcClient.h b/syncd/MdioIpcClient.h new file mode 100644 index 0000000000..9081562030 --- /dev/null +++ b/syncd/MdioIpcClient.h @@ -0,0 +1,21 @@ +#pragma once + +#include +#include + +extern "C" { +#include "sai.h" +} + +/* Function declarations */ +extern "C" { +sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, + uint32_t number_of_registers, uint32_t *data); +sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr, + uint32_t reg_addr, uint32_t number_of_registers, const uint32_t *data); + +sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, + uint32_t number_of_registers, uint32_t *data); +sai_status_t mdio_write_cl22(uint64_t platform_context, uint32_t mdio_addr, + uint32_t reg_addr, uint32_t number_of_registers, const uint32_t *data); +} From 1db84cf6cffea6e4cbc12a81c33a043106879715 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 28 Sep 2022 17:19:51 -0700 Subject: [PATCH 02/30] Add -Wno-format-truncation to CPPFLAGS Signed-off-by: Jiahua Wang --- syncd/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/syncd/Makefile.am b/syncd/Makefile.am index a4bf32bb44..0b00c90e8e 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -95,10 +95,10 @@ libMdioIpcClient_a_SOURCES = MdioIpcClient.cpp libMdioIpcClient_la_SOURCES = MdioIpcClient.cpp -libMdioIpcClient_a_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) +libMdioIpcClient_a_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) -Wno-format-truncation libMdioIpcClient_a_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) -libMdioIpcClient_la_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) +libMdioIpcClient_la_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) -Wno-format-truncation libMdioIpcClient_la_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) libMdioIpcClient_la_LIBADD = $(CODE_COVERAGE_LIBS) From 69f9d23e6b6baae8f1cf9bcde90d9205225605c8 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Fri, 14 Oct 2022 10:33:26 -0700 Subject: [PATCH 03/30] Add MdioIpcCommon.h and syncd.links Signed-off-by: Jiahua Wang --- debian/syncd.links | 2 + syncd/Makefile.am | 2 +- syncd/MdioIpcClient.cpp | 97 ++++++++++++++++++++++++----------------- syncd/MdioIpcCommon.h | 9 ++++ syncd/MdioIpcServer.cpp | 20 +++++---- 5 files changed, 80 insertions(+), 50 deletions(-) create mode 100755 debian/syncd.links create mode 100644 syncd/MdioIpcCommon.h diff --git a/debian/syncd.links b/debian/syncd.links new file mode 100755 index 0000000000..6f5af02f61 --- /dev/null +++ b/debian/syncd.links @@ -0,0 +1,2 @@ +#! /usr/bin/dh-exec +/usr/lib/${DEB_HOST_MULTIARCH}/libMdioIpcClient.so.0 /usr/lib/${DEB_HOST_MULTIARCH}/libMdioIpcClient.so diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 0b00c90e8e..93d3740a00 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -100,7 +100,7 @@ libMdioIpcClient_a_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CO libMdioIpcClient_la_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) -Wno-format-truncation libMdioIpcClient_la_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS) -libMdioIpcClient_la_LIBADD = $(CODE_COVERAGE_LIBS) +libMdioIpcClient_la_LIBADD = -lswsscommon $(CODE_COVERAGE_LIBS) tests_SOURCES = tests.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) diff --git a/syncd/MdioIpcClient.cpp b/syncd/MdioIpcClient.cpp index 93bcb67426..ed4065a574 100644 --- a/syncd/MdioIpcClient.cpp +++ b/syncd/MdioIpcClient.cpp @@ -12,17 +12,14 @@ #include #include #include +#include #include "MdioIpcClient.h" +#include "MdioIpcCommon.h" -#define SAI_IPC_SOCK_DIR_SYNCD "/var/run/sswsyncd" -#define SAI_IPC_SOCK_DIR_HOST "/var/run/docker-syncd" -#define SAI_IPC_SOCK_FILE "mdio-ipc" -#define SAI_IPC_BUFF_SIZE 256 +#include "swss/logger.h" -#define CONN_TIMEOUT 25 /* shorter than 30 sec on server side */ - -static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; +static std::mutex ipcMutex; /* Global variables */ @@ -48,16 +45,16 @@ static int syncd_mdio_ipc_command(char *cmd, char *resp) if (strlen(path) == 0) { - strcpy(path, SAI_IPC_SOCK_DIR_SYNCD); + strcpy(path, SYNCD_IPC_SOCK_SYNCD); fd = open(path, O_DIRECTORY); if (fd < 0) { - printf("%s: Program is not run on host\n", __func__); - strcpy(path, SAI_IPC_SOCK_DIR_HOST); + SWSS_LOG_INFO("Program is not run on host\n"); + strcpy(path, SYNCD_IPC_SOCK_HOST); fd = open(path, O_DIRECTORY); if (fd < 0) { - printf("%s: Unable to open the directory for IPC\n", __func__); + SWSS_LOG_ERROR("Unable to open the directory for IPC\n"); return errno; } } @@ -68,26 +65,26 @@ static int syncd_mdio_ipc_command(char *cmd, char *resp) sock = socket(AF_UNIX, SOCK_STREAM, 0); if (sock < 0) { - perror("socket()"); + SWSS_LOG_ERROR("socket() :%s", strerror(errno)); return errno; } caddr.sun_family = AF_UNIX; - snprintf(caddr.sun_path, sizeof(caddr.sun_path), "%s/%s.cli.%d", path, SAI_IPC_SOCK_FILE, getpid()); + snprintf(caddr.sun_path, sizeof(caddr.sun_path), "%s/%s.cli.%d", path, SYNCD_IPC_SOCK_FILE, getpid()); unlink(caddr.sun_path); if (bind(sock, (struct sockaddr *)&caddr, sizeof(caddr)) < 0) { - perror("bind()"); + SWSS_LOG_ERROR("bind() :%s", strerror(errno)); close(sock); sock = 0; return errno; } saddr.sun_family = AF_UNIX; - snprintf(saddr.sun_path, sizeof(saddr.sun_path), "%s/%s.srv", path, SAI_IPC_SOCK_FILE); + snprintf(saddr.sun_path, sizeof(saddr.sun_path), "%s/%s.srv", path, SYNCD_IPC_SOCK_FILE); if (connect(sock, (struct sockaddr *) &saddr, sizeof(saddr)) < 0) { - perror("connect()"); + SWSS_LOG_ERROR("connect() :%s", strerror(errno)); close(sock); sock = 0; unlink(caddr.sun_path); @@ -99,24 +96,24 @@ static int syncd_mdio_ipc_command(char *cmd, char *resp) ret = send(sock, cmd, len, 0); if (ret < (ssize_t)len) { - printf("send failed, ret=%ld, expected=%ld\n", ret, len); + SWSS_LOG_ERROR("send failed, ret=%ld, expected=%ld\n", ret, len); close(sock); sock = 0; unlink(caddr.sun_path); return -EIO; } - ret = recv(sock, resp, SAI_IPC_BUFF_SIZE - 1, 0); + ret = recv(sock, resp, SYNCD_IPC_BUFF_SIZE - 1, 0); if (ret <= 0) { - printf("recv failed, ret=%ld\n", ret); + SWSS_LOG_ERROR("recv failed, ret=%ld\n", ret); close(sock); sock = 0; unlink(caddr.sun_path); return -EIO; } - timeout = time(NULL) + CONN_TIMEOUT; + timeout = time(NULL) + MDIO_CLIENT_TIMEOUT; return (int)strtol(resp, NULL, 0); } @@ -126,24 +123,29 @@ extern "C" sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, uint32_t number_of_registers, uint32_t *data) { int rc = SAI_STATUS_FAILURE; - char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; if (number_of_registers > 1) { - printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); return SAI_STATUS_FAILURE; } - pthread_mutex_lock(&mutex); + ipcMutex.lock(); sprintf(cmd, "mdio 0x%x 0x%x\n", mdio_addr, reg_addr); - if (syncd_mdio_ipc_command(cmd, resp) == 0) + rc = syncd_mdio_ipc_command(cmd, resp); + if (rc == 0) { *data = (uint32_t)strtoul(strchrnul(resp, ' ') + 1, NULL, 0); rc = SAI_STATUS_SUCCESS; } + else + { + SWSS_LOG_ERROR("syncd_mdio_ipc_command returns : %d\n", rc); + } - pthread_mutex_unlock(&mutex); + ipcMutex.unlock(); return rc; } @@ -152,23 +154,28 @@ extern "C" sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr uint32_t number_of_registers, const uint32_t *data) { int rc = SAI_STATUS_FAILURE; - char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; if (number_of_registers > 1) { - printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); return SAI_STATUS_FAILURE; } - pthread_mutex_lock(&mutex); + ipcMutex.lock(); sprintf(cmd, "mdio 0x%x 0x%x 0x%x\n", mdio_addr, reg_addr, *data); - if (syncd_mdio_ipc_command(cmd, resp) == 0) + rc = syncd_mdio_ipc_command(cmd, resp); + if (rc == 0) { rc = SAI_STATUS_SUCCESS; } + else + { + SWSS_LOG_ERROR("syncd_mdio_ipc_command returns : %d\n", rc); + } - pthread_mutex_unlock(&mutex); + ipcMutex.unlock(); return rc; } @@ -177,24 +184,29 @@ extern "C" sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_ uint32_t number_of_registers, uint32_t *data) { int rc = SAI_STATUS_FAILURE; - char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; if (number_of_registers > 1) { - printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); return SAI_STATUS_FAILURE; } - pthread_mutex_lock(&mutex); + ipcMutex.lock(); sprintf(cmd, "mdio-cl22 0x%x 0x%x\n", mdio_addr, reg_addr); - if (syncd_mdio_ipc_command(cmd, resp) == 0) + rc = syncd_mdio_ipc_command(cmd, resp); + if (rc == 0) { *data = (uint32_t)strtoul(strchrnul(resp, ' ') + 1, NULL, 0); rc = SAI_STATUS_SUCCESS; } + else + { + SWSS_LOG_ERROR("syncd_mdio_ipc_command returns : %d\n", rc); + } - pthread_mutex_unlock(&mutex); + ipcMutex.unlock(); return rc; } @@ -203,22 +215,27 @@ extern "C" sai_status_t mdio_write_cl22(uint64_t platform_context, uint32_t mdio uint32_t number_of_registers, const uint32_t *data) { int rc = SAI_STATUS_FAILURE; - char cmd[SAI_IPC_BUFF_SIZE], resp[SAI_IPC_BUFF_SIZE]; + char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; if (number_of_registers > 1) { - printf("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); + SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers); return SAI_STATUS_FAILURE; } - pthread_mutex_lock(&mutex); + ipcMutex.lock(); sprintf(cmd, "mdio-cl22 0x%x 0x%x 0x%x\n", mdio_addr, reg_addr, *data); - if (syncd_mdio_ipc_command(cmd, resp) == 0) + rc = syncd_mdio_ipc_command(cmd, resp); + if (rc == 0) { rc = SAI_STATUS_SUCCESS; } + else + { + SWSS_LOG_ERROR("syncd_mdio_ipc_command returns : %d\n", rc); + } - pthread_mutex_unlock(&mutex); + ipcMutex.unlock(); return rc; } diff --git a/syncd/MdioIpcCommon.h b/syncd/MdioIpcCommon.h new file mode 100644 index 0000000000..c42bd237ca --- /dev/null +++ b/syncd/MdioIpcCommon.h @@ -0,0 +1,9 @@ +#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 MDIO_SERVER_TIMEOUT 30 /* sec, connection timeout */ +#define MDIO_CLIENT_TIMEOUT 25 /* shorter than 30 sec on server side */ + +#define CONN_MAX 18 /* max. number of connections */ diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 452719055e..796a9e2f9d 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -13,6 +13,7 @@ #include #include "MdioIpcServer.h" +#include "MdioIpcCommon.h" #include "meta/sai_serialize.h" @@ -22,13 +23,6 @@ #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]))) @@ -324,7 +318,7 @@ int MdioIpcServer::syncd_ipc_task_main() if (i < CONN_MAX) { conn[i].fd = sock_cli; - conn[i].timeout = now + CONN_TIMEOUT; + conn[i].timeout = now + MDIO_SERVER_TIMEOUT; } else { @@ -381,10 +375,18 @@ int MdioIpcServer::syncd_ipc_task_main() else if (strcmp("mdio", argv[0]) == 0) { rc = MdioIpcServer::syncd_ipc_cmd_mdio(resp, argc, argv); + if (rc != 0) + { + SWSS_LOG_ERROR("command %s returns %d", cmd, rc); + } } else if (strcmp("mdio-cl22", argv[0]) == 0) { rc = MdioIpcServer::syncd_ipc_cmd_mdio_cl22(resp, argc, argv); + if (rc != 0) + { + SWSS_LOG_ERROR("command %s returns %d", cmd, rc); + } } /* build the error message */ @@ -401,7 +403,7 @@ int MdioIpcServer::syncd_ipc_task_main() } /* update the connection timeout counter */ - conn[i].timeout = time(NULL) + CONN_TIMEOUT; + conn[i].timeout = time(NULL) + MDIO_SERVER_TIMEOUT; } } From d95c2bb4bd4381fb894c20d38a012db2e7cb8594 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Fri, 21 Oct 2022 11:23:08 -0700 Subject: [PATCH 04/30] Add SWSS_LOG_ENTER() but disabled to pass swsslogentercheck.sh Signed-off-by: Jiahua Wang --- syncd/MdioIpcClient.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/syncd/MdioIpcClient.cpp b/syncd/MdioIpcClient.cpp index ed4065a574..503a73fb1c 100644 --- a/syncd/MdioIpcClient.cpp +++ b/syncd/MdioIpcClient.cpp @@ -122,6 +122,8 @@ static int syncd_mdio_ipc_command(char *cmd, char *resp) extern "C" sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, uint32_t *data) { + // SWSS_LOG_ENTER(); // disabled + int rc = SAI_STATUS_FAILURE; char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; @@ -153,6 +155,8 @@ extern "C" sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, extern "C" sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, const uint32_t *data) { + // SWSS_LOG_ENTER(); // disabled + int rc = SAI_STATUS_FAILURE; char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; @@ -183,6 +187,8 @@ extern "C" sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr extern "C" sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, uint32_t *data) { + // SWSS_LOG_ENTER(); // disabled + int rc = SAI_STATUS_FAILURE; char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; @@ -214,6 +220,8 @@ extern "C" sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_ extern "C" sai_status_t mdio_write_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, const uint32_t *data) { + // SWSS_LOG_ENTER(); // disabled + int rc = SAI_STATUS_FAILURE; char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE]; From 70462c7ecfdd9891f727733573233457e68e8328 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Sun, 23 Oct 2022 13:30:57 -0700 Subject: [PATCH 05/30] Add SWSS_LOG_ENTER() but disabled to pass swsslogentercheck.sh Signed-off-by: Jiahua Wang --- syncd/MdioIpcClient.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/syncd/MdioIpcClient.cpp b/syncd/MdioIpcClient.cpp index 503a73fb1c..1b85279a15 100644 --- a/syncd/MdioIpcClient.cpp +++ b/syncd/MdioIpcClient.cpp @@ -25,6 +25,8 @@ static std::mutex ipcMutex; static int syncd_mdio_ipc_command(char *cmd, char *resp) { + // SWSS_LOG_ENTER(); // disabled + int fd; ssize_t ret; size_t len; From fcde33bacca36f755e5176eab2e0f1799912dd67 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 16 Mar 2023 04:55:50 +0000 Subject: [PATCH 06/30] Add unit test for MdioIpcServer class and MdioIpcClient code Signed-off-by: Jiahua Wang --- meta/DummySaiInterface.cpp | 48 ++++++ meta/DummySaiInterface.h | 28 ++++ unittest/syncd/Makefile.am | 3 +- unittest/syncd/MockableSaiInterface.cpp | 65 +++++++- unittest/syncd/MockableSaiInterface.h | 36 +++++ unittest/syncd/TestMdioIpcServer.cpp | 203 ++++++++++++++++++++++++ 6 files changed, 381 insertions(+), 2 deletions(-) create mode 100644 unittest/syncd/TestMdioIpcServer.cpp diff --git a/meta/DummySaiInterface.cpp b/meta/DummySaiInterface.cpp index d09d8d2338..8c86349ea4 100644 --- a/meta/DummySaiInterface.cpp +++ b/meta/DummySaiInterface.cpp @@ -131,6 +131,54 @@ sai_status_t DummySaiInterface::flushFdbEntries( return m_status; } +sai_status_t DummySaiInterface::switchMdioRead( + _In_ sai_object_id_t switchId, + _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 m_status; +} + +sai_status_t DummySaiInterface::switchMdioWrite( + _In_ sai_object_id_t switchId, + _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 m_status; +} + +sai_status_t DummySaiInterface::switchMdioCl22Read( + _In_ sai_object_id_t switchId, + _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 m_status; +} + +sai_status_t DummySaiInterface::switchMdioCl22Write( + _In_ sai_object_id_t switchId, + _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 m_status; +} + sai_status_t DummySaiInterface::objectTypeGetAvailability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/meta/DummySaiInterface.h b/meta/DummySaiInterface.h index 2cb9db43ee..8cef2ecffd 100644 --- a/meta/DummySaiInterface.h +++ b/meta/DummySaiInterface.h @@ -145,6 +145,34 @@ namespace saimeta _In_ uint32_t attrCount, _In_ const sai_attribute_t *attrList) override; + virtual sai_status_t switchMdioRead( + _In_ sai_object_id_t switchId, + _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 switchMdioWrite( + _In_ sai_object_id_t switchId, + _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 switchId, + _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_ const uint32_t *reg_val) override; + public: // SAI API virtual sai_status_t objectTypeGetAvailability( diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 69ebd53f06..85fea2b9a2 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -14,9 +14,10 @@ tests_SOURCES = main.cpp \ TestNotificationQueue.cpp \ TestNotificationProcessor.cpp \ TestNotificationHandler.cpp \ + TestMdioIpcServer.cpp \ TestVendorSai.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) -tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a $(top_srcdir)/vslib/libSaiVS.a -lhiredis -lswsscommon -lnl-genl-3 -lnl-nf-3 -lnl-route-3 -lnl-3 -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) +tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a $(top_srcdir)/vslib/libSaiVS.a $(top_srcdir)/syncd/libMdioIpcClient.a -lhiredis -lswsscommon -lnl-genl-3 -lnl-nf-3 -lnl-route-3 -lnl-3 -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) TESTS = tests diff --git a/unittest/syncd/MockableSaiInterface.cpp b/unittest/syncd/MockableSaiInterface.cpp index e52b1dc58e..79dac43b1f 100644 --- a/unittest/syncd/MockableSaiInterface.cpp +++ b/unittest/syncd/MockableSaiInterface.cpp @@ -1,7 +1,6 @@ #include "MockableSaiInterface.h" #include "swss/logger.h" - MockableSaiInterface::MockableSaiInterface() { SWSS_LOG_ENTER(); @@ -253,6 +252,70 @@ sai_status_t MockableSaiInterface::flushFdbEntries( return SAI_STATUS_SUCCESS; } +sai_status_t MockableSaiInterface::switchMdioRead( + _In_ sai_object_id_t switchId, + _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(); + if (mock_switchMdioRead) + { + return mock_switchMdioRead(switchId, device_addr, start_reg_addr, number_of_registers, reg_val); + } + + return SAI_STATUS_SUCCESS; +} + +sai_status_t MockableSaiInterface::switchMdioWrite( + _In_ sai_object_id_t switchId, + _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(); + if (mock_switchMdioWrite) + { + return mock_switchMdioWrite(switchId, device_addr, start_reg_addr, number_of_registers, reg_val); + } + + return SAI_STATUS_SUCCESS; +} + +sai_status_t MockableSaiInterface::switchMdioCl22Read( + _In_ sai_object_id_t switchId, + _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(); + if (mock_switchMdioCl22Read) + { + return mock_switchMdioCl22Read(switchId, device_addr, start_reg_addr, number_of_registers, reg_val); + } + + return SAI_STATUS_SUCCESS; +} + +sai_status_t MockableSaiInterface::switchMdioCl22Write( + _In_ sai_object_id_t switchId, + _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(); + if (mock_switchMdioCl22Write) + { + return mock_switchMdioCl22Write(switchId, device_addr, start_reg_addr, number_of_registers, reg_val); + } + + return SAI_STATUS_SUCCESS; +} + sai_status_t MockableSaiInterface::objectTypeGetAvailability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/unittest/syncd/MockableSaiInterface.h b/unittest/syncd/MockableSaiInterface.h index 0a17e4b22f..c01b512068 100644 --- a/unittest/syncd/MockableSaiInterface.h +++ b/unittest/syncd/MockableSaiInterface.h @@ -154,6 +154,42 @@ class MockableSaiInterface: public saimeta::DummySaiInterface std::function mock_flushFdbEntries; + virtual sai_status_t switchMdioRead( + _In_ sai_object_id_t switchId, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _Out_ uint32_t *reg_val) override; + + std::function mock_switchMdioRead; + + virtual sai_status_t switchMdioWrite( + _In_ sai_object_id_t switchId, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ const uint32_t *reg_val) override; + + std::function mock_switchMdioWrite; + + virtual sai_status_t switchMdioCl22Read( + _In_ sai_object_id_t switchId, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _Out_ uint32_t *reg_val) override; + + std::function mock_switchMdioCl22Read; + + virtual sai_status_t switchMdioCl22Write( + _In_ sai_object_id_t switchId, + _In_ uint32_t device_addr, + _In_ uint32_t start_reg_addr, + _In_ uint32_t number_of_registers, + _In_ const uint32_t *reg_val) override; + + std::function mock_switchMdioCl22Write; + public: // SAI API virtual sai_status_t objectTypeGetAvailability( diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp new file mode 100644 index 0000000000..d5d097a34b --- /dev/null +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -0,0 +1,203 @@ +#include "MdioIpcServer.h" +#include "MdioIpcClient.h" +#include "MdioIpcCommon.h" +#include "MockableSaiInterface.h" +#include "swss/logger.h" + +#include +#include +#include + +#include + + +using namespace syncd; +using namespace std; + +static std::map mdioDevRegValMap; +static std::map mdioDevCl22RegValMap; + + +static sai_status_t MockMdioRead( + _In_ sai_object_id_t switchId, + _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(); + uint64_t key = device_addr; + key <<= 32; + key |= start_reg_addr; + auto it = mdioDevRegValMap.find(key); + if (it == mdioDevRegValMap.end()) + { + *reg_val = 0; + return SAI_STATUS_FAILURE; + } + else + { + *reg_val = it->second; + } + return SAI_STATUS_SUCCESS; +} + +static sai_status_t MockMdioWrite( + _In_ sai_object_id_t switchId, + _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(); + uint64_t key = device_addr; + key <<= 32; + key |= start_reg_addr; + mdioDevRegValMap[key] = *reg_val; + return SAI_STATUS_SUCCESS; +} + +static sai_status_t MockMdioCl22Read( + _In_ sai_object_id_t switchId, + _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(); + uint64_t key = device_addr; + key <<= 32; + key |= start_reg_addr; + auto it = mdioDevCl22RegValMap.find(key); + if (it == mdioDevCl22RegValMap.end()) + { + *reg_val = 0; + return SAI_STATUS_FAILURE; + } + else + { + *reg_val = it->second; + } + return SAI_STATUS_SUCCESS; +} + +static sai_status_t MockMdioCl22Write( + _In_ sai_object_id_t switchId, + _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(); + uint64_t key = device_addr; + key <<= 32; + key |= start_reg_addr; + mdioDevCl22RegValMap[key] = *reg_val; + return SAI_STATUS_SUCCESS; +} + +TEST(MdioIpcServer, mdioCl22Write) +{ + std::shared_ptr mdio_sai(new MockableSaiInterface()); + mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + } + mdioDevCl22RegValMap.clear(); + std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); + sai_status_t rc; + uint32_t data = 0xCAFE; + rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + uint64_t key = 0x1; + key <<= 32; + key |= 0x1D; + EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); + mdio_server->stopMdioThread(); + sleep(10); +} + +TEST(MdioIpcServer, mdioCl22Read) +{ + std::shared_ptr mdio_sai(new MockableSaiInterface()); + mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + } + mdioDevCl22RegValMap.clear(); + std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); + sai_status_t rc; + uint32_t data = 0x0; + uint64_t key = 0x2; + key <<= 32; + key |= 0x1C; + mdioDevCl22RegValMap[key] = 0xFEED; + rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + EXPECT_EQ(data, 0xFEED); + mdio_server->stopMdioThread(); + sleep(10); +} + +TEST(MdioIpcServer, mdioWrite) +{ + std::shared_ptr mdio_sai(new MockableSaiInterface()); + mdio_sai->mock_switchMdioWrite = MockMdioWrite; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + } + mdioDevRegValMap.clear(); + std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); + sai_status_t rc; + uint32_t data = 0xBEEF; + rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + uint64_t key = 0x3; + key <<= 32; + key |= 0x1B; + EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); + mdio_server->stopMdioThread(); + sleep(10); +} + +TEST(MdioIpcServer, mdioRead) +{ + std::shared_ptr mdio_sai(new MockableSaiInterface()); + mdio_sai->mock_switchMdioRead = MockMdioRead; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + } + mdioDevRegValMap.clear(); + std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); + sai_status_t rc; + uint32_t data = 0; + uint64_t key = 0x4; + key <<= 32; + key |= 0x1A; + mdioDevRegValMap[key] = 0xC0DE; + rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + EXPECT_EQ(data, 0xC0DE); + mdio_server->stopMdioThread(); + sleep(10); +} From 042b5d6f9921f4167fdac9dfd5eb79e307dcc93a Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 16 Mar 2023 21:01:02 +0000 Subject: [PATCH 07/30] Add log message to check existence of directory /var/run/sswsyncd Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index d5d097a34b..122da62bbb 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -104,7 +104,11 @@ TEST(MdioIpcServer, mdioCl22Write) strcpy(path, SYNCD_IPC_SOCK_SYNCD); if (open(path, O_DIRECTORY) < 0) { - mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } } mdioDevCl22RegValMap.clear(); std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); @@ -130,7 +134,11 @@ TEST(MdioIpcServer, mdioCl22Read) strcpy(path, SYNCD_IPC_SOCK_SYNCD); if (open(path, O_DIRECTORY) < 0) { - mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } } mdioDevCl22RegValMap.clear(); std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); @@ -157,7 +165,11 @@ TEST(MdioIpcServer, mdioWrite) strcpy(path, SYNCD_IPC_SOCK_SYNCD); if (open(path, O_DIRECTORY) < 0) { - mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } } mdioDevRegValMap.clear(); std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); @@ -183,7 +195,11 @@ TEST(MdioIpcServer, mdioRead) strcpy(path, SYNCD_IPC_SOCK_SYNCD); if (open(path, O_DIRECTORY) < 0) { - mkdir(SYNCD_IPC_SOCK_SYNCD, 0755); + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } } mdioDevRegValMap.clear(); std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); From 5f4e7bc469e9b38d69a39e11df0fc30aaca43f54 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Fri, 17 Mar 2023 21:16:47 +0000 Subject: [PATCH 08/30] Change from TEST() to TEST_F() Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 108 ++++++++++----------------- 1 file changed, 40 insertions(+), 68 deletions(-) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 122da62bbb..5c74a051cf 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -96,24 +96,49 @@ static sai_status_t MockMdioCl22Write( return SAI_STATUS_SUCCESS; } -TEST(MdioIpcServer, mdioCl22Write) +class MdioIpcServerTest : public ::testing::Test { - std::shared_ptr mdio_sai(new MockableSaiInterface()); - mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) +public: + MdioIpcServerTest() = default; + virtual ~MdioIpcServerTest() = default; + +public: + virtual void SetUp() override { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } } + mdio_sai = std::make_shared(); + mdio_sai->mock_switchMdioRead = MockMdioRead; + mdio_sai->mock_switchMdioWrite = MockMdioWrite; + mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; + mdio_server = std::make_shared(mdio_sai, 0); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); } + + virtual void TearDown() override + { + mdio_server->stopMdioThread(); + } + +protected: + std::shared_ptr mdio_sai; + std::shared_ptr mdio_server; + +}; + +TEST_F(MdioIpcServerTest, mdioCl22Write) +{ mdioDevCl22RegValMap.clear(); - std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); - mdio_server->setSwitchId(0x21000000000000); - mdio_server->startMdioThread(); sai_status_t rc; uint32_t data = 0xCAFE; rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); @@ -122,28 +147,11 @@ TEST(MdioIpcServer, mdioCl22Write) key <<= 32; key |= 0x1D; EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); - mdio_server->stopMdioThread(); - sleep(10); } -TEST(MdioIpcServer, mdioCl22Read) +TEST_F(MdioIpcServerTest, mdioCl22Read) { - std::shared_ptr mdio_sai(new MockableSaiInterface()); - mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } mdioDevCl22RegValMap.clear(); - std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); - mdio_server->setSwitchId(0x21000000000000); - mdio_server->startMdioThread(); sai_status_t rc; uint32_t data = 0x0; uint64_t key = 0x2; @@ -153,28 +161,11 @@ TEST(MdioIpcServer, mdioCl22Read) rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 1, &data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xFEED); - mdio_server->stopMdioThread(); - sleep(10); } -TEST(MdioIpcServer, mdioWrite) +TEST_F(MdioIpcServerTest, mdioWrite) { - std::shared_ptr mdio_sai(new MockableSaiInterface()); - mdio_sai->mock_switchMdioWrite = MockMdioWrite; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } mdioDevRegValMap.clear(); - std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); - mdio_server->setSwitchId(0x21000000000000); - mdio_server->startMdioThread(); sai_status_t rc; uint32_t data = 0xBEEF; rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); @@ -183,28 +174,11 @@ TEST(MdioIpcServer, mdioWrite) key <<= 32; key |= 0x1B; EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); - mdio_server->stopMdioThread(); - sleep(10); } -TEST(MdioIpcServer, mdioRead) +TEST_F(MdioIpcServerTest, mdioRead) { - std::shared_ptr mdio_sai(new MockableSaiInterface()); - mdio_sai->mock_switchMdioRead = MockMdioRead; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } mdioDevRegValMap.clear(); - std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); - mdio_server->setSwitchId(0x21000000000000); - mdio_server->startMdioThread(); sai_status_t rc; uint32_t data = 0; uint64_t key = 0x4; @@ -214,6 +188,4 @@ TEST(MdioIpcServer, mdioRead) rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xC0DE); - mdio_server->stopMdioThread(); - sleep(10); } From e9f648366c25b3306f76c826ae603d988b2c0cd3 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 23 Mar 2023 17:26:43 +0000 Subject: [PATCH 09/30] Create directory /var/run/sswsyncd with mode 777 Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index a243bb1790..e2f70c00d3 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -98,6 +98,7 @@ jobs: sudo sed -ri 's/^unixsocketperm .../unixsocketperm 777/' /etc/redis/redis.conf sudo sed -ri 's/redis-server.sock/redis.sock/' /etc/redis/redis.conf sudo service redis-server start + sudo mkdir -m 777 /var/run/sswsyncd sudo apt-get install -y rsyslog sudo service rsyslog start From cc08e1977aa9a696d84ae613e720d577eaf4e414 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 23 Mar 2023 18:11:41 +0000 Subject: [PATCH 10/30] Add CAP_DAC_OVERRIDE capability for system directory creation in syncd unittest Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index e2f70c00d3..b13c57ccf0 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -184,6 +184,8 @@ jobs: set -ex # Add SYS_TIME capability for settimeofday ok in syncd test sudo setcap "cap_sys_time=eip" syncd/.libs/syncd_tests + # Add CAP_DAC_OVERRIDE capability for system directory creation in syncd unittest + sudo setcap "cap_dac_override=eip" unittest/syncd/.libs/tests make check gcovr --version find SAI/meta -name "*.gc*" | xargs rm -vf From 519cb6be835a619cf22f2f287dd837145a02e62b Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 23 Mar 2023 21:55:22 +0000 Subject: [PATCH 11/30] Not to create directory /var/run/sswsyncd Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index b13c57ccf0..537707e5a2 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -98,7 +98,6 @@ jobs: sudo sed -ri 's/^unixsocketperm .../unixsocketperm 777/' /etc/redis/redis.conf sudo sed -ri 's/redis-server.sock/redis.sock/' /etc/redis/redis.conf sudo service redis-server start - sudo mkdir -m 777 /var/run/sswsyncd sudo apt-get install -y rsyslog sudo service rsyslog start From 3f820d7888b4c678970302504fbc4df1dfd76b9b Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 27 Mar 2023 18:04:56 +0000 Subject: [PATCH 12/30] Create directory /var/run/sswsyncd with mode 755 Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 537707e5a2..f42e6d7d03 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -98,6 +98,7 @@ jobs: sudo sed -ri 's/^unixsocketperm .../unixsocketperm 777/' /etc/redis/redis.conf sudo sed -ri 's/redis-server.sock/redis.sock/' /etc/redis/redis.conf sudo service redis-server start + sudo mkdir -m 755 /var/run/sswsyncd sudo apt-get install -y rsyslog sudo service rsyslog start From 11d9882725b803ceb4eb1399665bf896aef677e2 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 29 Mar 2023 20:46:32 +0000 Subject: [PATCH 13/30] Add tests_LDFLAGS, add IPC capabilities Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 2 +- unittest/syncd/Makefile.am | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index f42e6d7d03..1bc2237d28 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -185,7 +185,7 @@ jobs: # Add SYS_TIME capability for settimeofday ok in syncd test sudo setcap "cap_sys_time=eip" syncd/.libs/syncd_tests # Add CAP_DAC_OVERRIDE capability for system directory creation in syncd unittest - sudo setcap "cap_dac_override=eip" unittest/syncd/.libs/tests + sudo setcap "cap_dac_override,cap_ipc_lock,cap_ipc_owner,cap_sys_time=eip" unittest/syncd/.libs/tests make check gcovr --version find SAI/meta -name "*.gc*" | xargs rm -vf diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 85fea2b9a2..577614a19e 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -18,6 +18,7 @@ tests_SOURCES = main.cpp \ TestVendorSai.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) +tests_LDFLAGS = -Wl,-rpath,$(top_srcdir)/lib/.libs -Wl,-rpath,$(top_srcdir)/meta/.libs tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a $(top_srcdir)/vslib/libSaiVS.a $(top_srcdir)/syncd/libMdioIpcClient.a -lhiredis -lswsscommon -lnl-genl-3 -lnl-nf-3 -lnl-route-3 -lnl-3 -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) TESTS = tests From f0d52f94da36a82f70b69dae0ecad1269d270105 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 30 Mar 2023 19:01:45 +0000 Subject: [PATCH 14/30] Give enough time for the server thread to finish setting up socket Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 5c74a051cf..4048ecefbb 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include @@ -123,6 +124,9 @@ class MdioIpcServerTest : public ::testing::Test mdio_server = std::make_shared(mdio_sai, 0); mdio_server->setSwitchId(0x21000000000000); mdio_server->startMdioThread(); + + /* enough time for the server thread to finish setting up socket */ + sleep(3); } virtual void TearDown() override From fce22a914f56653e901208200957d973b3c09f0e Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 30 Mar 2023 21:17:31 +0000 Subject: [PATCH 15/30] Call setSwitchId() Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 4048ecefbb..946d6a80e1 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -142,6 +142,7 @@ class MdioIpcServerTest : public ::testing::Test TEST_F(MdioIpcServerTest, mdioCl22Write) { + mdio_server->setSwitchId(0x21000000000000); mdioDevCl22RegValMap.clear(); sai_status_t rc; uint32_t data = 0xCAFE; @@ -155,6 +156,7 @@ TEST_F(MdioIpcServerTest, mdioCl22Write) TEST_F(MdioIpcServerTest, mdioCl22Read) { + mdio_server->setSwitchId(0x21000000000000); mdioDevCl22RegValMap.clear(); sai_status_t rc; uint32_t data = 0x0; @@ -169,6 +171,7 @@ TEST_F(MdioIpcServerTest, mdioCl22Read) TEST_F(MdioIpcServerTest, mdioWrite) { + mdio_server->setSwitchId(0x21000000000000); mdioDevRegValMap.clear(); sai_status_t rc; uint32_t data = 0xBEEF; @@ -182,6 +185,7 @@ TEST_F(MdioIpcServerTest, mdioWrite) TEST_F(MdioIpcServerTest, mdioRead) { + mdio_server->setSwitchId(0x21000000000000); mdioDevRegValMap.clear(); sai_status_t rc; uint32_t data = 0; From 47666722764e9dcb034f83e7062288afd2724865 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 30 Mar 2023 23:00:53 +0000 Subject: [PATCH 16/30] Use TEST() Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 946d6a80e1..386c9563e9 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -97,6 +97,41 @@ static sai_status_t MockMdioCl22Write( return SAI_STATUS_SUCCESS; } +TEST(MdioIpcServer1, mdioRead1) +{ + std::shared_ptr mdio_sai1(new MockableSaiInterface()); + mdio_sai1->mock_switchMdioRead = MockMdioRead; + mdio_sai1->mock_switchMdioWrite = MockMdioWrite; + mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } + } + mdioDevRegValMap.clear(); + std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); + mdio_server1->setSwitchId(0x21000000000000); + mdio_server1->startMdioThread(); + sleep(3); + sai_status_t rc; + uint32_t data = 0; + uint64_t key = 0x4; + key <<= 32; + key |= 0x1A; + mdioDevRegValMap[key] = 0xC0DE; + rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + EXPECT_EQ(data, 0xC0DE); + mdio_server1->stopMdioThread(); + sleep(3); +} + class MdioIpcServerTest : public ::testing::Test { public: From 12c271842e0464868626fc9ef6459b01860bcf81 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Thu, 6 Apr 2023 04:19:19 +0000 Subject: [PATCH 17/30] Add conditional flag SONIC_ASIC_PLATFORM_VS Signed-off-by: Jiahua Wang --- configure.ac | 1 + syncd/Makefile.am | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/configure.ac b/configure.ac index e7ccb4ee2c..63d83404ab 100644 --- a/configure.ac +++ b/configure.ac @@ -18,6 +18,7 @@ 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) AM_CONDITIONAL(SONIC_ASIC_PLATFORM_MELLANOX, test x$CONFIGURED_PLATFORM = xmellanox) +AM_CONDITIONAL(SONIC_ASIC_PLATFORM_VS, test x$CONFIGURED_PLATFORM = xvs) AM_COND_IF([SONIC_ASIC_PLATFORM_MELLANOX], AC_DEFINE([MELLANOX], [1], [Define to 1 on Mellanox Platform])) diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 62d5d8be0e..9345cdea10 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -80,6 +80,10 @@ if SONIC_ASIC_PLATFORM_BROADCOM libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU endif +if SONIC_ASIC_PLATFORM_VS +libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU +endif + libSyncdRequestShutdown_a_SOURCES = \ RequestShutdown.cpp \ RequestShutdownCommandLineOptions.cpp \ From fe907f775cd912bdf7ec76a2a81006761807d97e Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 10 Apr 2023 22:11:21 +0000 Subject: [PATCH 18/30] Add conditional flag SONIC_ASIC_PLATFORM_GENERIC and SONIC_ASIC_PLATFORM_UNDEFINED, remove SONIC_ASIC_PLATFORM_VS Signed-off-by: Jiahua Wang --- configure.ac | 3 ++- syncd/Makefile.am | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 63d83404ab..c237e1bcc1 100644 --- a/configure.ac +++ b/configure.ac @@ -18,7 +18,8 @@ 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) AM_CONDITIONAL(SONIC_ASIC_PLATFORM_MELLANOX, test x$CONFIGURED_PLATFORM = xmellanox) -AM_CONDITIONAL(SONIC_ASIC_PLATFORM_VS, test x$CONFIGURED_PLATFORM = xvs) +AM_CONDITIONAL(SONIC_ASIC_PLATFORM_GENERIC, test x$CONFIGURED_PLATFORM = xgeneric) +AM_CONDITIONAL(SONIC_ASIC_PLATFORM_UNDEFINED, test x$CONFIGURED_PLATFORM = xundefined) AM_COND_IF([SONIC_ASIC_PLATFORM_MELLANOX], AC_DEFINE([MELLANOX], [1], [Define to 1 on Mellanox Platform])) diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 9345cdea10..8b5254694f 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -80,7 +80,11 @@ if SONIC_ASIC_PLATFORM_BROADCOM libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU endif -if SONIC_ASIC_PLATFORM_VS +if SONIC_ASIC_PLATFORM_GENERIC +libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU +endif + +if SONIC_ASIC_PLATFORM_UNDEFINED libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU endif From 4c5a6e44b2c52f3e557a14ae24943d36f3f08fbd Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Mon, 10 Apr 2023 23:23:32 +0000 Subject: [PATCH 19/30] Remove SONIC_ASIC_PLATFORM_GENERIC and SONIC_ASIC_PLATFORM_UNDEFINED, add -DMDIO_ACCESS_USE_NPU to CXXFLAGS in azure_pipeline Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 2 +- configure.ac | 2 -- syncd/Makefile.am | 8 -------- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 1bc2237d28..efff2bdcf3 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -162,7 +162,7 @@ jobs: if [ '${{ parameters.asan }}' == True ]; then extraflags='--enable-asan' fi - DEB_BUILD_OPTIONS=nocheck fakeroot debian/rules DEB_CONFIGURE_EXTRA_FLAGS=$extraflags CFLAGS="" CXXFLAGS="" binary-syncd-vs + DEB_BUILD_OPTIONS=nocheck fakeroot debian/rules DEB_CONFIGURE_EXTRA_FLAGS=$extraflags CFLAGS="" CXXFLAGS="-DMDIO_ACCESS_USE_NPU" binary-syncd-vs mv ../*.deb . displayName: "Compile sonic sairedis with coverage enabled" - script: | diff --git a/configure.ac b/configure.ac index c237e1bcc1..e7ccb4ee2c 100644 --- a/configure.ac +++ b/configure.ac @@ -18,8 +18,6 @@ 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) AM_CONDITIONAL(SONIC_ASIC_PLATFORM_MELLANOX, test x$CONFIGURED_PLATFORM = xmellanox) -AM_CONDITIONAL(SONIC_ASIC_PLATFORM_GENERIC, test x$CONFIGURED_PLATFORM = xgeneric) -AM_CONDITIONAL(SONIC_ASIC_PLATFORM_UNDEFINED, test x$CONFIGURED_PLATFORM = xundefined) AM_COND_IF([SONIC_ASIC_PLATFORM_MELLANOX], AC_DEFINE([MELLANOX], [1], [Define to 1 on Mellanox Platform])) diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 8b5254694f..62d5d8be0e 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -80,14 +80,6 @@ if SONIC_ASIC_PLATFORM_BROADCOM libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU endif -if SONIC_ASIC_PLATFORM_GENERIC -libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU -endif - -if SONIC_ASIC_PLATFORM_UNDEFINED -libSyncd_a_CXXFLAGS += -DMDIO_ACCESS_USE_NPU -endif - libSyncdRequestShutdown_a_SOURCES = \ RequestShutdown.cpp \ RequestShutdownCommandLineOptions.cpp \ From 118e10de652f7fc45e4250447595a02fa7af3f89 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 00:16:14 +0000 Subject: [PATCH 20/30] Remove -DMDIO_ACCESS_USE_NPU in azure_pipeline, add MdioIpcServer::setIpcTestMode() Signed-off-by: Jiahua Wang --- .azure-pipelines/build-template.yml | 2 +- syncd/MdioIpcCommon.h | 2 +- syncd/MdioIpcServer.cpp | 53 +++++++++--- syncd/MdioIpcServer.h | 4 + unittest/syncd/TestMdioIpcServer.cpp | 115 ++++++++++++++++++++++++++- 5 files changed, 157 insertions(+), 19 deletions(-) diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index f884ab3eb4..addf514373 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -159,7 +159,7 @@ jobs: if [ '${{ parameters.asan }}' == True ]; then extraflags='--enable-asan' fi - DEB_BUILD_OPTIONS=nocheck fakeroot debian/rules DEB_CONFIGURE_EXTRA_FLAGS=$extraflags DEB_BUILD_PROFILES=nopython2 CFLAGS="" CXXFLAGS="-DMDIO_ACCESS_USE_NPU" binary-syncd-vs + DEB_BUILD_OPTIONS=nocheck fakeroot debian/rules DEB_CONFIGURE_EXTRA_FLAGS=$extraflags DEB_BUILD_PROFILES=nopython2 CFLAGS="" CXXFLAGS="" binary-syncd-vs mv ../*.deb . displayName: "Compile sonic sairedis with coverage enabled" - script: | diff --git a/syncd/MdioIpcCommon.h b/syncd/MdioIpcCommon.h index c42bd237ca..788de0e5e3 100644 --- a/syncd/MdioIpcCommon.h +++ b/syncd/MdioIpcCommon.h @@ -6,4 +6,4 @@ #define MDIO_SERVER_TIMEOUT 30 /* sec, connection timeout */ #define MDIO_CLIENT_TIMEOUT 25 /* shorter than 30 sec on server side */ -#define CONN_MAX 18 /* max. number of connections */ +#define MDIO_CONN_MAX 18 /* max. number of connections */ diff --git a/syncd/MdioIpcServer.cpp b/syncd/MdioIpcServer.cpp index 796a9e2f9d..7194332fb3 100644 --- a/syncd/MdioIpcServer.cpp +++ b/syncd/MdioIpcServer.cpp @@ -50,6 +50,11 @@ MdioIpcServer::MdioIpcServer( /* globalContext == 0 for syncd, globalContext > 0 for gbsyncd */ MdioIpcServer::m_syncdContext = (globalContext == 0); +#ifdef MDIO_ACCESS_USE_NPU + MdioIpcServer::m_accessUseNPU = true; +#else + MdioIpcServer::m_accessUseNPU = false; +#endif } MdioIpcServer::~MdioIpcServer() @@ -68,7 +73,12 @@ void MdioIpcServer::setSwitchId( { SWSS_LOG_ENTER(); -#ifdef MDIO_ACCESS_USE_NPU + /* Skip on any platform where MDIO access not using NPU */ + if (!m_accessUseNPU) + { + return; + } + /* MDIO switch id is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::m_syncdContext) { @@ -85,6 +95,15 @@ void MdioIpcServer::setSwitchId( SWSS_LOG_NOTICE("Initialize mdio switch id with RID = %s", sai_serialize_object_id(m_switchRid).c_str()); +} + +void MdioIpcServer::setIpcTestMode() +{ + SWSS_LOG_ENTER(); + +#ifndef MDIO_ACCESS_USE_NPU + /* Allow unit test to start IPC server */ + MdioIpcServer::m_accessUseNPU = true; #endif } @@ -188,7 +207,7 @@ int MdioIpcServer::syncd_ipc_task_main() int sock_srv; int sock_cli; int sock_max; - syncd_mdio_ipc_conn_t conn[CONN_MAX]; + syncd_mdio_ipc_conn_t conn[MDIO_CONN_MAX]; struct sockaddr_un addr; char path[64]; fd_set rfds; @@ -230,7 +249,7 @@ int MdioIpcServer::syncd_ipc_task_main() } /* Listen for the upcoming client sockets */ - if (listen(sock_srv, CONN_MAX) < 0) + if (listen(sock_srv, MDIO_CONN_MAX) < 0) { SWSS_LOG_ERROR("listen() returns %d", errno); unlink(addr.sun_path); @@ -248,7 +267,7 @@ int MdioIpcServer::syncd_ipc_task_main() /* garbage collection */ now = time(NULL); - for (i = 0; i < CONN_MAX; ++i) + for (i = 0; i < MDIO_CONN_MAX; ++i) { if ((conn[i].fd > 0) && (conn[i].timeout < now)) { @@ -263,7 +282,7 @@ int MdioIpcServer::syncd_ipc_task_main() FD_ZERO(&rfds); FD_SET(sock_srv, &rfds); sock_max = sock_srv; - for (i = 0; i < CONN_MAX; ++i) + for (i = 0; i < MDIO_CONN_MAX; ++i) { if (conn[i].fd <= 0) { @@ -308,14 +327,14 @@ int MdioIpcServer::syncd_ipc_task_main() continue; } - for (i = 0; i < CONN_MAX; ++i) + for (i = 0; i < MDIO_CONN_MAX; ++i) { if (conn[i].fd <= 0) { break; } } - if (i < CONN_MAX) + if (i < MDIO_CONN_MAX) { conn[i].fd = sock_cli; conn[i].timeout = now + MDIO_SERVER_TIMEOUT; @@ -328,7 +347,7 @@ int MdioIpcServer::syncd_ipc_task_main() } /* Handle the client requests */ - for (i = 0; i < CONN_MAX; ++i) + for (i = 0; i < MDIO_CONN_MAX; ++i) { sai_status_t rc = SAI_STATUS_NOT_SUPPORTED; @@ -408,7 +427,7 @@ int MdioIpcServer::syncd_ipc_task_main() } /* close socket descriptors */ - for (i = 0; i < CONN_MAX; ++i) + for (i = 0; i < MDIO_CONN_MAX; ++i) { if (conn[i].fd <= 0) { @@ -433,7 +452,12 @@ void MdioIpcServer::stopMdioThread(void) { SWSS_LOG_ENTER(); -#ifdef MDIO_ACCESS_USE_NPU + /* Skip on any platform where MDIO access not using NPU */ + if (!m_accessUseNPU) + { + return; + } + /* MDIO IPC server thread is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::m_syncdContext) { @@ -443,14 +467,18 @@ void MdioIpcServer::stopMdioThread(void) m_taskAlive = 0; m_taskThread.join(); SWSS_LOG_NOTICE("IPC task thread is stopped\n"); -#endif } int MdioIpcServer::startMdioThread() { SWSS_LOG_ENTER(); -#ifdef MDIO_ACCESS_USE_NPU + /* Skip on any platform where MDIO access not using NPU */ + if (!m_accessUseNPU) + { + return SAI_STATUS_SUCCESS; + } + /* MDIO IPC server thread is only relevant in syncd but not in gbsyncd */ if (!MdioIpcServer::m_syncdContext) { @@ -469,6 +497,5 @@ int MdioIpcServer::startMdioThread() return SAI_STATUS_FAILURE; } } -#endif return SAI_STATUS_SUCCESS; } diff --git a/syncd/MdioIpcServer.h b/syncd/MdioIpcServer.h index 2da4e6c64a..3e1dcac1a7 100644 --- a/syncd/MdioIpcServer.h +++ b/syncd/MdioIpcServer.h @@ -25,6 +25,8 @@ namespace syncd void setSwitchId( _In_ sai_object_id_t switchRid); + void setIpcTestMode(); + int startMdioThread(); void stopMdioThread(); @@ -49,6 +51,8 @@ namespace syncd static bool m_syncdContext; + bool m_accessUseNPU; + std::shared_ptr m_vendorSai; sai_object_id_t m_switchRid; diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 386c9563e9..11581e2c2d 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -116,6 +116,7 @@ TEST(MdioIpcServer1, mdioRead1) } mdioDevRegValMap.clear(); std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); + mdio_server1->setIpcTestMode(); mdio_server1->setSwitchId(0x21000000000000); mdio_server1->startMdioThread(); sleep(3); @@ -132,6 +133,112 @@ TEST(MdioIpcServer1, mdioRead1) sleep(3); } +TEST(MdioIpcServer1, mdioWrite1) +{ + std::shared_ptr mdio_sai1(new MockableSaiInterface()); + mdio_sai1->mock_switchMdioRead = MockMdioRead; + mdio_sai1->mock_switchMdioWrite = MockMdioWrite; + mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } + } + mdioDevRegValMap.clear(); + std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); + mdio_server1->setIpcTestMode(); + mdio_server1->setSwitchId(0x21000000000000); + mdio_server1->startMdioThread(); + sleep(3); + sai_status_t rc; + uint32_t data = 0xBEEF; + rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + uint64_t key = 0x3; + key <<= 32; + key |= 0x1B; + EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); + mdio_server1->stopMdioThread(); + sleep(3); +} + +TEST(MdioIpcServer1, mdioCl22Read1) +{ + std::shared_ptr mdio_sai1(new MockableSaiInterface()); + mdio_sai1->mock_switchMdioRead = MockMdioRead; + mdio_sai1->mock_switchMdioWrite = MockMdioWrite; + mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } + } + mdioDevCl22RegValMap.clear(); + std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); + mdio_server1->setIpcTestMode(); + mdio_server1->setSwitchId(0x21000000000000); + mdio_server1->startMdioThread(); + sleep(3); + sai_status_t rc; + uint32_t data = 0x0; + uint64_t key = 0x2; + key <<= 32; + key |= 0x1C; + mdioDevCl22RegValMap[key] = 0xFEED; + rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + EXPECT_EQ(data, 0xFEED); + mdio_server1->stopMdioThread(); + sleep(3); +} + +TEST(MdioIpcServer1, mdioCl22Write1) +{ + std::shared_ptr mdio_sai1(new MockableSaiInterface()); + mdio_sai1->mock_switchMdioRead = MockMdioRead; + mdio_sai1->mock_switchMdioWrite = MockMdioWrite; + mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } + } + mdioDevCl22RegValMap.clear(); + std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); + mdio_server1->setIpcTestMode(); + mdio_server1->setSwitchId(0x21000000000000); + mdio_server1->startMdioThread(); + sleep(3); + sai_status_t rc; + uint32_t data = 0xCAFE; + rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + uint64_t key = 0x1; + key <<= 32; + key |= 0x1D; + EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); + mdio_server1->stopMdioThread(); + sleep(3); +} + class MdioIpcServerTest : public ::testing::Test { public: @@ -157,6 +264,7 @@ class MdioIpcServerTest : public ::testing::Test mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; mdio_server = std::make_shared(mdio_sai, 0); + mdio_server->setIpcTestMode(); mdio_server->setSwitchId(0x21000000000000); mdio_server->startMdioThread(); @@ -167,6 +275,9 @@ class MdioIpcServerTest : public ::testing::Test virtual void TearDown() override { mdio_server->stopMdioThread(); + + /* enough time stop the server thread*/ + sleep(3); } protected: @@ -177,7 +288,6 @@ class MdioIpcServerTest : public ::testing::Test TEST_F(MdioIpcServerTest, mdioCl22Write) { - mdio_server->setSwitchId(0x21000000000000); mdioDevCl22RegValMap.clear(); sai_status_t rc; uint32_t data = 0xCAFE; @@ -191,7 +301,6 @@ TEST_F(MdioIpcServerTest, mdioCl22Write) TEST_F(MdioIpcServerTest, mdioCl22Read) { - mdio_server->setSwitchId(0x21000000000000); mdioDevCl22RegValMap.clear(); sai_status_t rc; uint32_t data = 0x0; @@ -206,7 +315,6 @@ TEST_F(MdioIpcServerTest, mdioCl22Read) TEST_F(MdioIpcServerTest, mdioWrite) { - mdio_server->setSwitchId(0x21000000000000); mdioDevRegValMap.clear(); sai_status_t rc; uint32_t data = 0xBEEF; @@ -220,7 +328,6 @@ TEST_F(MdioIpcServerTest, mdioWrite) TEST_F(MdioIpcServerTest, mdioRead) { - mdio_server->setSwitchId(0x21000000000000); mdioDevRegValMap.clear(); sai_status_t rc; uint32_t data = 0; From d34f7ff48de93fbcc217541e4b9c8691db542d14 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 00:55:16 +0000 Subject: [PATCH 21/30] Add NPU to dictionary Signed-off-by: Jiahua Wang --- tests/aspell.en.pws | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index 93bf805057..8368e24307 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -55,6 +55,7 @@ MCAST MTU Mellanox NHG +NPU OA OIDs ObjectAttrHash From abfd2172fee4a98467b4fa1e7fa14eaf7d436514 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 02:14:00 +0000 Subject: [PATCH 22/30] Change sleep 3 to sleep 1 Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 11581e2c2d..088797290d 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -119,7 +119,7 @@ TEST(MdioIpcServer1, mdioRead1) mdio_server1->setIpcTestMode(); mdio_server1->setSwitchId(0x21000000000000); mdio_server1->startMdioThread(); - sleep(3); + sleep(1); sai_status_t rc; uint32_t data = 0; uint64_t key = 0x4; @@ -130,7 +130,7 @@ TEST(MdioIpcServer1, mdioRead1) EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xC0DE); mdio_server1->stopMdioThread(); - sleep(3); + sleep(1); } TEST(MdioIpcServer1, mdioWrite1) @@ -155,7 +155,7 @@ TEST(MdioIpcServer1, mdioWrite1) mdio_server1->setIpcTestMode(); mdio_server1->setSwitchId(0x21000000000000); mdio_server1->startMdioThread(); - sleep(3); + sleep(1); sai_status_t rc; uint32_t data = 0xBEEF; rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); @@ -165,7 +165,7 @@ TEST(MdioIpcServer1, mdioWrite1) key |= 0x1B; EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); mdio_server1->stopMdioThread(); - sleep(3); + sleep(1); } TEST(MdioIpcServer1, mdioCl22Read1) @@ -190,7 +190,7 @@ TEST(MdioIpcServer1, mdioCl22Read1) mdio_server1->setIpcTestMode(); mdio_server1->setSwitchId(0x21000000000000); mdio_server1->startMdioThread(); - sleep(3); + sleep(1); sai_status_t rc; uint32_t data = 0x0; uint64_t key = 0x2; @@ -201,7 +201,7 @@ TEST(MdioIpcServer1, mdioCl22Read1) EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xFEED); mdio_server1->stopMdioThread(); - sleep(3); + sleep(1); } TEST(MdioIpcServer1, mdioCl22Write1) @@ -226,7 +226,7 @@ TEST(MdioIpcServer1, mdioCl22Write1) mdio_server1->setIpcTestMode(); mdio_server1->setSwitchId(0x21000000000000); mdio_server1->startMdioThread(); - sleep(3); + sleep(1); sai_status_t rc; uint32_t data = 0xCAFE; rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); @@ -236,7 +236,7 @@ TEST(MdioIpcServer1, mdioCl22Write1) key |= 0x1D; EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); mdio_server1->stopMdioThread(); - sleep(3); + sleep(1); } class MdioIpcServerTest : public ::testing::Test @@ -269,7 +269,7 @@ class MdioIpcServerTest : public ::testing::Test mdio_server->startMdioThread(); /* enough time for the server thread to finish setting up socket */ - sleep(3); + sleep(1); } virtual void TearDown() override @@ -277,7 +277,7 @@ class MdioIpcServerTest : public ::testing::Test mdio_server->stopMdioThread(); /* enough time stop the server thread*/ - sleep(3); + sleep(1); } protected: From 84b8dbd41eebdf70a4ca66e8203460facb1ca7eb Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 05:48:50 +0000 Subject: [PATCH 23/30] only use TEST_F Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 088797290d..b749a0e106 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -97,6 +97,7 @@ static sai_status_t MockMdioCl22Write( return SAI_STATUS_SUCCESS; } +#if 0 TEST(MdioIpcServer1, mdioRead1) { std::shared_ptr mdio_sai1(new MockableSaiInterface()); @@ -127,6 +128,7 @@ TEST(MdioIpcServer1, mdioRead1) key |= 0x1A; mdioDevRegValMap[key] = 0xC0DE; rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); + SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xC0DE); mdio_server1->stopMdioThread(); @@ -159,10 +161,12 @@ TEST(MdioIpcServer1, mdioWrite1) sai_status_t rc; uint32_t data = 0xBEEF; rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); + SWSS_LOG_NOTICE("rc = %d", rc); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); uint64_t key = 0x3; key <<= 32; key |= 0x1B; + SWSS_LOG_NOTICE("reg = %x", mdioDevRegValMap[key]); EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); mdio_server1->stopMdioThread(); sleep(1); @@ -198,6 +202,7 @@ TEST(MdioIpcServer1, mdioCl22Read1) key |= 0x1C; mdioDevCl22RegValMap[key] = 0xFEED; rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 1, &data); + SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xFEED); mdio_server1->stopMdioThread(); @@ -230,14 +235,17 @@ TEST(MdioIpcServer1, mdioCl22Write1) sai_status_t rc; uint32_t data = 0xCAFE; rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); + SWSS_LOG_NOTICE("rc = %d", rc); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); uint64_t key = 0x1; key <<= 32; key |= 0x1D; + SWSS_LOG_NOTICE("cl22 reg = %x", mdioDevCl22RegValMap[key]); EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); mdio_server1->stopMdioThread(); sleep(1); } +#endif class MdioIpcServerTest : public ::testing::Test { @@ -292,10 +300,12 @@ TEST_F(MdioIpcServerTest, mdioCl22Write) sai_status_t rc; uint32_t data = 0xCAFE; rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); + SWSS_LOG_NOTICE("rc = %d", rc); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); uint64_t key = 0x1; key <<= 32; key |= 0x1D; + SWSS_LOG_NOTICE("cl22 reg = %x", mdioDevCl22RegValMap[key]); EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); } @@ -309,6 +319,7 @@ TEST_F(MdioIpcServerTest, mdioCl22Read) key |= 0x1C; mdioDevCl22RegValMap[key] = 0xFEED; rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 1, &data); + SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xFEED); } @@ -319,10 +330,12 @@ TEST_F(MdioIpcServerTest, mdioWrite) sai_status_t rc; uint32_t data = 0xBEEF; rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); + SWSS_LOG_NOTICE("rc = %d", rc); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); uint64_t key = 0x3; key <<= 32; key |= 0x1B; + SWSS_LOG_NOTICE("reg = %x", mdioDevRegValMap[key]); EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); } @@ -336,6 +349,7 @@ TEST_F(MdioIpcServerTest, mdioRead) key |= 0x1A; mdioDevRegValMap[key] = 0xC0DE; rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); + SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xC0DE); } From 4e18bb3ec5fc62ddd907d12a5f68f1147eb826c1 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 17:47:28 +0000 Subject: [PATCH 24/30] Combine 4 TEST() into 1 Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 236 ++++----------------------- 1 file changed, 29 insertions(+), 207 deletions(-) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index b749a0e106..7a10eafb7f 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -97,14 +97,14 @@ static sai_status_t MockMdioCl22Write( return SAI_STATUS_SUCCESS; } -#if 0 -TEST(MdioIpcServer1, mdioRead1) +TEST(MdioIpcServer, mdioAccess) { - std::shared_ptr mdio_sai1(new MockableSaiInterface()); - mdio_sai1->mock_switchMdioRead = MockMdioRead; - mdio_sai1->mock_switchMdioWrite = MockMdioWrite; - mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; - mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; + SWSS_LOG_ENTER(); + std::shared_ptr mdio_sai(new MockableSaiInterface()); + mdio_sai->mock_switchMdioRead = MockMdioRead; + mdio_sai->mock_switchMdioWrite = MockMdioWrite; + mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; char path[64]; strcpy(path, SYNCD_IPC_SOCK_SYNCD); if (open(path, O_DIRECTORY) < 0) @@ -115,15 +115,20 @@ TEST(MdioIpcServer1, mdioRead1) SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); } } - mdioDevRegValMap.clear(); - std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); - mdio_server1->setIpcTestMode(); - mdio_server1->setSwitchId(0x21000000000000); - mdio_server1->startMdioThread(); + std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); + mdio_server->setIpcTestMode(); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); sleep(1); + + uint32_t data; + uint64_t key; sai_status_t rc; - uint32_t data = 0; - uint64_t key = 0x4; + + /* MDIO read */ + mdioDevRegValMap.clear(); + data = 0; + key = 0x4; key <<= 32; key |= 0x1A; mdioDevRegValMap[key] = 0xC0DE; @@ -131,73 +136,23 @@ TEST(MdioIpcServer1, mdioRead1) SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xC0DE); - mdio_server1->stopMdioThread(); - sleep(1); -} -TEST(MdioIpcServer1, mdioWrite1) -{ - std::shared_ptr mdio_sai1(new MockableSaiInterface()); - mdio_sai1->mock_switchMdioRead = MockMdioRead; - mdio_sai1->mock_switchMdioWrite = MockMdioWrite; - mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; - mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } + /* MDIO write */ mdioDevRegValMap.clear(); - std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); - mdio_server1->setIpcTestMode(); - mdio_server1->setSwitchId(0x21000000000000); - mdio_server1->startMdioThread(); - sleep(1); - sai_status_t rc; - uint32_t data = 0xBEEF; + data = 0xBEEF; rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); SWSS_LOG_NOTICE("rc = %d", rc); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); - uint64_t key = 0x3; + key = 0x3; key <<= 32; key |= 0x1B; SWSS_LOG_NOTICE("reg = %x", mdioDevRegValMap[key]); EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); - mdio_server1->stopMdioThread(); - sleep(1); -} -TEST(MdioIpcServer1, mdioCl22Read1) -{ - std::shared_ptr mdio_sai1(new MockableSaiInterface()); - mdio_sai1->mock_switchMdioRead = MockMdioRead; - mdio_sai1->mock_switchMdioWrite = MockMdioWrite; - mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; - mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } + /* MDIO CL22 read */ mdioDevCl22RegValMap.clear(); - std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); - mdio_server1->setIpcTestMode(); - mdio_server1->setSwitchId(0x21000000000000); - mdio_server1->startMdioThread(); - sleep(1); - sai_status_t rc; - uint32_t data = 0x0; - uint64_t key = 0x2; + data = 0x0; + key = 0x2; key <<= 32; key |= 0x1C; mdioDevCl22RegValMap[key] = 0xFEED; @@ -205,151 +160,18 @@ TEST(MdioIpcServer1, mdioCl22Read1) SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xFEED); - mdio_server1->stopMdioThread(); - sleep(1); -} - -TEST(MdioIpcServer1, mdioCl22Write1) -{ - std::shared_ptr mdio_sai1(new MockableSaiInterface()); - mdio_sai1->mock_switchMdioRead = MockMdioRead; - mdio_sai1->mock_switchMdioWrite = MockMdioWrite; - mdio_sai1->mock_switchMdioCl22Read = MockMdioCl22Read; - mdio_sai1->mock_switchMdioCl22Write = MockMdioCl22Write; - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } - mdioDevCl22RegValMap.clear(); - std::shared_ptr mdio_server1(new MdioIpcServer(mdio_sai1, 0)); - mdio_server1->setIpcTestMode(); - mdio_server1->setSwitchId(0x21000000000000); - mdio_server1->startMdioThread(); - sleep(1); - sai_status_t rc; - uint32_t data = 0xCAFE; - rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); - SWSS_LOG_NOTICE("rc = %d", rc); - EXPECT_EQ(rc, SAI_STATUS_SUCCESS); - uint64_t key = 0x1; - key <<= 32; - key |= 0x1D; - SWSS_LOG_NOTICE("cl22 reg = %x", mdioDevCl22RegValMap[key]); - EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); - mdio_server1->stopMdioThread(); - sleep(1); -} -#endif - -class MdioIpcServerTest : public ::testing::Test -{ -public: - MdioIpcServerTest() = default; - virtual ~MdioIpcServerTest() = default; - -public: - virtual void SetUp() override - { - char path[64]; - strcpy(path, SYNCD_IPC_SOCK_SYNCD); - if (open(path, O_DIRECTORY) < 0) - { - SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); - if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) - { - SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); - } - } - mdio_sai = std::make_shared(); - mdio_sai->mock_switchMdioRead = MockMdioRead; - mdio_sai->mock_switchMdioWrite = MockMdioWrite; - mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; - mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; - mdio_server = std::make_shared(mdio_sai, 0); - mdio_server->setIpcTestMode(); - mdio_server->setSwitchId(0x21000000000000); - mdio_server->startMdioThread(); - - /* enough time for the server thread to finish setting up socket */ - sleep(1); - } - - virtual void TearDown() override - { - mdio_server->stopMdioThread(); - - /* enough time stop the server thread*/ - sleep(1); - } - -protected: - std::shared_ptr mdio_sai; - std::shared_ptr mdio_server; -}; - -TEST_F(MdioIpcServerTest, mdioCl22Write) -{ + /* MDIO CL22 write */ mdioDevCl22RegValMap.clear(); - sai_status_t rc; - uint32_t data = 0xCAFE; + data = 0xCAFE; rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); SWSS_LOG_NOTICE("rc = %d", rc); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); - uint64_t key = 0x1; + key = 0x1; key <<= 32; key |= 0x1D; SWSS_LOG_NOTICE("cl22 reg = %x", mdioDevCl22RegValMap[key]); EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); -} - -TEST_F(MdioIpcServerTest, mdioCl22Read) -{ - mdioDevCl22RegValMap.clear(); - sai_status_t rc; - uint32_t data = 0x0; - uint64_t key = 0x2; - key <<= 32; - key |= 0x1C; - mdioDevCl22RegValMap[key] = 0xFEED; - rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 1, &data); - SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); - EXPECT_EQ(rc, SAI_STATUS_SUCCESS); - EXPECT_EQ(data, 0xFEED); -} - -TEST_F(MdioIpcServerTest, mdioWrite) -{ - mdioDevRegValMap.clear(); - sai_status_t rc; - uint32_t data = 0xBEEF; - rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 1, &data); - SWSS_LOG_NOTICE("rc = %d", rc); - EXPECT_EQ(rc, SAI_STATUS_SUCCESS); - uint64_t key = 0x3; - key <<= 32; - key |= 0x1B; - SWSS_LOG_NOTICE("reg = %x", mdioDevRegValMap[key]); - EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); -} -TEST_F(MdioIpcServerTest, mdioRead) -{ - mdioDevRegValMap.clear(); - sai_status_t rc; - uint32_t data = 0; - uint64_t key = 0x4; - key <<= 32; - key |= 0x1A; - mdioDevRegValMap[key] = 0xC0DE; - rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); - SWSS_LOG_NOTICE("rc = %d, data = %x", rc, data); - EXPECT_EQ(rc, SAI_STATUS_SUCCESS); - EXPECT_EQ(data, 0xC0DE); + mdio_server->stopMdioThread(); } From 51065d74b9c39c6d83eaff3db4e188b8b7bc3d9b Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 18:51:00 +0000 Subject: [PATCH 25/30] Add negtive test Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 7a10eafb7f..c82634bafb 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -11,6 +11,8 @@ #include +#define MDIO_MISSING_DEV_ADDR 0x1F +#define MDIO_MISSING_REG_ADDR 0xFFFF using namespace syncd; using namespace std; @@ -51,6 +53,10 @@ static sai_status_t MockMdioWrite( _In_ const uint32_t *reg_val) { SWSS_LOG_ENTER(); + if (MDIO_MISSING_DEV_ADDR == device_addr) + { + return SAI_STATUS_FAILURE; + } uint64_t key = device_addr; key <<= 32; key |= start_reg_addr; @@ -90,6 +96,10 @@ static sai_status_t MockMdioCl22Write( _In_ const uint32_t *reg_val) { SWSS_LOG_ENTER(); + if (MDIO_MISSING_DEV_ADDR == device_addr) + { + return SAI_STATUS_FAILURE; + } uint64_t key = device_addr; key <<= 32; key |= start_reg_addr; @@ -137,6 +147,9 @@ TEST(MdioIpcServer, mdioAccess) EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xC0DE); + rc = mdio_read(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* MDIO write */ mdioDevRegValMap.clear(); data = 0xBEEF; @@ -149,6 +162,9 @@ TEST(MdioIpcServer, mdioAccess) SWSS_LOG_NOTICE("reg = %x", mdioDevRegValMap[key]); EXPECT_EQ(mdioDevRegValMap[key], 0xBEEF); + rc = mdio_write(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* MDIO CL22 read */ mdioDevCl22RegValMap.clear(); data = 0x0; @@ -161,6 +177,9 @@ TEST(MdioIpcServer, mdioAccess) EXPECT_EQ(rc, SAI_STATUS_SUCCESS); EXPECT_EQ(data, 0xFEED); + rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* MDIO CL22 write */ mdioDevCl22RegValMap.clear(); data = 0xCAFE; @@ -173,5 +192,8 @@ TEST(MdioIpcServer, mdioAccess) SWSS_LOG_NOTICE("cl22 reg = %x", mdioDevCl22RegValMap[key]); EXPECT_EQ(mdioDevCl22RegValMap[key], 0xCAFE); + rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + mdio_server->stopMdioThread(); } From b3aa4a2bd19fa86968f3ccfa62c3c70f0d6e0de4 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 20:37:30 +0000 Subject: [PATCH 26/30] Add ipc client timeout test Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index c82634bafb..d7109a01f1 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -195,5 +195,10 @@ TEST(MdioIpcServer, mdioAccess) rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* Test ipc client timeout */ + sleep(MDIO_CLIENT_TIMEOUT+1); + rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); + EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + mdio_server->stopMdioThread(); } From 9d1f7fe11854d9a440894c5bce3f2f6bb2c51ad0 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 21:53:54 +0000 Subject: [PATCH 27/30] Add ipc connect fail test Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 41 ++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index d7109a01f1..7047d10a11 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -200,5 +200,46 @@ TEST(MdioIpcServer, mdioAccess) rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); EXPECT_EQ(rc, SAI_STATUS_SUCCESS); + mdio_server->stopMdioThread(); + sleep(1); +} + +TEST(MdioIpcServer, mdioConnect) +{ + SWSS_LOG_ENTER(); + std::shared_ptr mdio_sai(new MockableSaiInterface()); + mdio_sai->mock_switchMdioRead = MockMdioRead; + mdio_sai->mock_switchMdioWrite = MockMdioWrite; + mdio_sai->mock_switchMdioCl22Read = MockMdioCl22Read; + mdio_sai->mock_switchMdioCl22Write = MockMdioCl22Write; + char path[64]; + strcpy(path, SYNCD_IPC_SOCK_SYNCD); + if (open(path, O_DIRECTORY) < 0) + { + SWSS_LOG_NOTICE("Directory %s does not exist", SYNCD_IPC_SOCK_SYNCD); + if (mkdir(SYNCD_IPC_SOCK_SYNCD, 0755) < 0) + { + SWSS_LOG_WARN("Can not create directory %s", SYNCD_IPC_SOCK_SYNCD); + } + } + std::shared_ptr mdio_server(new MdioIpcServer(mdio_sai, 0)); + mdio_server->setSwitchId(0x21000000000000); + mdio_server->startMdioThread(); + sleep(1); + + uint32_t data; + uint64_t key; + sai_status_t rc; + + /* ipc connect fail without setIpcTestMode() on non broadcom platform */ + mdioDevRegValMap.clear(); + data = 0; + key = 0x4; + key <<= 32; + key |= 0x1A; + mdioDevRegValMap[key] = 0xC0DE; + rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 1, &data); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + mdio_server->stopMdioThread(); } From 7f7c771ca8b3c01d358df5082b17cbab0b088d33 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 22:39:31 +0000 Subject: [PATCH 28/30] Wait for client timeout before another test Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index 7047d10a11..cd3803096d 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -201,7 +201,7 @@ TEST(MdioIpcServer, mdioAccess) EXPECT_EQ(rc, SAI_STATUS_SUCCESS); mdio_server->stopMdioThread(); - sleep(1); + sleep(MDIO_CLIENT_TIMEOUT+1); } TEST(MdioIpcServer, mdioConnect) From e3a33a0e6fc2caf141e0e9724931b7716831495d Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 12 Apr 2023 23:29:17 +0000 Subject: [PATCH 29/30] Add negative tests of more than one register access Signed-off-by: Jiahua Wang --- unittest/syncd/TestMdioIpcServer.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/unittest/syncd/TestMdioIpcServer.cpp b/unittest/syncd/TestMdioIpcServer.cpp index cd3803096d..2d99472fb9 100644 --- a/unittest/syncd/TestMdioIpcServer.cpp +++ b/unittest/syncd/TestMdioIpcServer.cpp @@ -134,6 +134,7 @@ TEST(MdioIpcServer, mdioAccess) uint32_t data; uint64_t key; sai_status_t rc; + uint32_t data2[2]; /* MDIO read */ mdioDevRegValMap.clear(); @@ -150,6 +151,9 @@ TEST(MdioIpcServer, mdioAccess) rc = mdio_read(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); EXPECT_NE(rc, SAI_STATUS_SUCCESS); + rc = mdio_read(0xF0F0F0F0F0F0F0F0, 0x4, 0x1A, 2, data2); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* MDIO write */ mdioDevRegValMap.clear(); data = 0xBEEF; @@ -165,6 +169,9 @@ TEST(MdioIpcServer, mdioAccess) rc = mdio_write(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); EXPECT_NE(rc, SAI_STATUS_SUCCESS); + rc = mdio_write(0xF0F0F0F0F0F0F0F0, 0x3, 0x1B, 2, data2); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* MDIO CL22 read */ mdioDevCl22RegValMap.clear(); data = 0x0; @@ -180,6 +187,9 @@ TEST(MdioIpcServer, mdioAccess) rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); EXPECT_NE(rc, SAI_STATUS_SUCCESS); + rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x2, 0x1C, 2, data2); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* MDIO CL22 write */ mdioDevCl22RegValMap.clear(); data = 0xCAFE; @@ -195,6 +205,9 @@ TEST(MdioIpcServer, mdioAccess) rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, MDIO_MISSING_DEV_ADDR, MDIO_MISSING_REG_ADDR, 1, &data); EXPECT_NE(rc, SAI_STATUS_SUCCESS); + rc = mdio_write_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 2, data2); + EXPECT_NE(rc, SAI_STATUS_SUCCESS); + /* Test ipc client timeout */ sleep(MDIO_CLIENT_TIMEOUT+1); rc = mdio_read_cl22(0xF0F0F0F0F0F0F0F0, 0x1, 0x1D, 1, &data); From 0afececce863bdee98b113291bdb9ccc0f112452 Mon Sep 17 00:00:00 2001 From: Jiahua Wang Date: Wed, 19 Apr 2023 17:51:44 +0000 Subject: [PATCH 30/30] Remove extern "C" from implementation file Signed-off-by: Jiahua Wang --- syncd/MdioIpcClient.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/syncd/MdioIpcClient.cpp b/syncd/MdioIpcClient.cpp index 1b85279a15..015d6e4ae3 100644 --- a/syncd/MdioIpcClient.cpp +++ b/syncd/MdioIpcClient.cpp @@ -121,7 +121,7 @@ static int syncd_mdio_ipc_command(char *cmd, char *resp) /* Function to read data from MDIO interface */ -extern "C" sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, +sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, uint32_t *data) { // SWSS_LOG_ENTER(); // disabled @@ -154,7 +154,7 @@ extern "C" sai_status_t mdio_read(uint64_t platform_context, uint32_t mdio_addr, } /* Function to write data to MDIO interface */ -extern "C" sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, +sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, const uint32_t *data) { // SWSS_LOG_ENTER(); // disabled @@ -186,7 +186,7 @@ extern "C" sai_status_t mdio_write(uint64_t platform_context, uint32_t mdio_addr } /* Function to read data using clause 22 from MDIO interface */ -extern "C" sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, +sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, uint32_t *data) { // SWSS_LOG_ENTER(); // disabled @@ -219,7 +219,7 @@ extern "C" sai_status_t mdio_read_cl22(uint64_t platform_context, uint32_t mdio_ } /* Function to write data using clause 22 to MDIO interface */ -extern "C" sai_status_t mdio_write_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, +sai_status_t mdio_write_cl22(uint64_t platform_context, uint32_t mdio_addr, uint32_t reg_addr, uint32_t number_of_registers, const uint32_t *data) { // SWSS_LOG_ENTER(); // disabled