Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1fd8a92
Add mdio IPC client library
jiahua-wang Sep 28, 2022
1db84cf
Add -Wno-format-truncation to CPPFLAGS
jiahua-wang Sep 29, 2022
67715d2
Merge branch 'sonic-net:master' into ipc-client
jiahua-wang Oct 11, 2022
69f9d23
Add MdioIpcCommon.h and syncd.links
jiahua-wang Oct 14, 2022
d95c2bb
Add SWSS_LOG_ENTER() but disabled to pass swsslogentercheck.sh
jiahua-wang Oct 21, 2022
70462c7
Add SWSS_LOG_ENTER() but disabled to pass swsslogentercheck.sh
jiahua-wang Oct 23, 2022
9da4562
Merge branch 'sonic-net:master' into ipc-client
jiahua-wang Oct 24, 2022
c789f84
Merge branch 'sonic-net:master' into ipc-client
jiahua-wang Oct 28, 2022
39f8b59
Merge branch 'master' into ipc-client
jiahua-wang Mar 9, 2023
fcde33b
Add unit test for MdioIpcServer class and MdioIpcClient code
jiahua-wang Mar 16, 2023
1937037
Merge branch 'master' into ipc-client
jiahua-wang Mar 16, 2023
042b5d6
Add log message to check existence of directory /var/run/sswsyncd
jiahua-wang Mar 16, 2023
5f4e7bc
Change from TEST() to TEST_F()
jiahua-wang Mar 17, 2023
8665f8c
Merge branch 'master' into ipc-client
jiahua-wang Mar 23, 2023
e9f6483
Create directory /var/run/sswsyncd with mode 777
jiahua-wang Mar 23, 2023
cc08e19
Add CAP_DAC_OVERRIDE capability for system directory creation in sync…
jiahua-wang Mar 23, 2023
519cb6b
Not to create directory /var/run/sswsyncd
jiahua-wang Mar 23, 2023
3f820d7
Create directory /var/run/sswsyncd with mode 755
jiahua-wang Mar 27, 2023
11d9882
Add tests_LDFLAGS, add IPC capabilities
jiahua-wang Mar 29, 2023
f0d52f9
Give enough time for the server thread to finish setting up socket
jiahua-wang Mar 30, 2023
fce22a9
Call setSwitchId()
jiahua-wang Mar 30, 2023
4766672
Use TEST()
jiahua-wang Mar 30, 2023
12c2718
Add conditional flag SONIC_ASIC_PLATFORM_VS
jiahua-wang Apr 6, 2023
fe907f7
Add conditional flag SONIC_ASIC_PLATFORM_GENERIC and SONIC_ASIC_PLATF…
jiahua-wang Apr 10, 2023
4c5a6e4
Remove SONIC_ASIC_PLATFORM_GENERIC and SONIC_ASIC_PLATFORM_UNDEFINED,…
jiahua-wang Apr 10, 2023
c175d8a
Merge branch 'master' into ipc-client
jiahua-wang Apr 11, 2023
118e10d
Remove -DMDIO_ACCESS_USE_NPU in azure_pipeline, add MdioIpcServer::se…
jiahua-wang Apr 12, 2023
d34f7ff
Add NPU to dictionary
jiahua-wang Apr 12, 2023
abfd217
Change sleep 3 to sleep 1
jiahua-wang Apr 12, 2023
84b8dbd
only use TEST_F
jiahua-wang Apr 12, 2023
4e18bb3
Combine 4 TEST() into 1
jiahua-wang Apr 12, 2023
51065d7
Add negtive test
jiahua-wang Apr 12, 2023
b3aa4a2
Add ipc client timeout test
jiahua-wang Apr 12, 2023
9d1f7fe
Add ipc connect fail test
jiahua-wang Apr 12, 2023
7f7c771
Wait for client timeout before another test
jiahua-wang Apr 12, 2023
e3a33a0
Add negative tests of more than one register access
jiahua-wang Apr 12, 2023
0afecec
Remove extern "C" from implementation file
jiahua-wang Apr 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions debian/syncd.dirs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
usr/bin
usr/lib
1 change: 1 addition & 0 deletions debian/syncd.install
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ usr/bin/saidiscovery
usr/bin/saiasiccmp
usr/bin/syncd*
syncd/scripts/* usr/bin
usr/lib/*/libMdioIpcClient.so.*
2 changes: 2 additions & 0 deletions debian/syncd.links
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#! /usr/bin/dh-exec
/usr/lib/${DEB_HOST_MULTIARCH}/libMdioIpcClient.so.0 /usr/lib/${DEB_HOST_MULTIARCH}/libMdioIpcClient.so
48 changes: 48 additions & 0 deletions meta/DummySaiInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions meta/DummySaiInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
15 changes: 14 additions & 1 deletion syncd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ endif

bin_PROGRAMS = syncd syncd_request_shutdown syncd_tests

noinst_LIBRARIES = libSyncd.a libSyncdRequestShutdown.a
lib_LTLIBRARIES = libMdioIpcClient.la

noinst_LIBRARIES = libSyncd.a libSyncdRequestShutdown.a libMdioIpcClient.a

libSyncd_a_SOURCES = \
AsicOperation.cpp \
Expand Down Expand Up @@ -91,6 +93,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) -Wno-format-truncation
libMdioIpcClient_a_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS)

libMdioIpcClient_la_CPPFLAGS = $(CODE_COVERAGE_CPPFLAGS) -Wno-format-truncation
libMdioIpcClient_la_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) $(CODE_COVERAGE_CXXFLAGS)
libMdioIpcClient_la_LIBADD = -lswsscommon $(CODE_COVERAGE_LIBS)

syncd_tests_SOURCES = tests.cpp
syncd_tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON)
syncd_tests_LDFLAGS = -Wl,-rpath,$(top_srcdir)/lib/.libs -Wl,-rpath,$(top_srcdir)/meta/.libs
Expand Down
251 changes: 251 additions & 0 deletions syncd/MdioIpcClient.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
/* Includes */
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#include <time.h>
#include <pthread.h>
#include <mutex>

#include "MdioIpcClient.h"
#include "MdioIpcCommon.h"

#include "swss/logger.h"

static std::mutex ipcMutex;

/* Global variables */

static int syncd_mdio_ipc_command(char *cmd, char *resp)
{
// SWSS_LOG_ENTER(); // disabled

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, SYNCD_IPC_SOCK_SYNCD);
fd = open(path, O_DIRECTORY);
if (fd < 0)
{
SWSS_LOG_INFO("Program is not run on host\n");
strcpy(path, SYNCD_IPC_SOCK_HOST);
fd = open(path, O_DIRECTORY);
if (fd < 0)
{
SWSS_LOG_ERROR("Unable to open the directory for IPC\n");
return errno;
}
}
}

if (sock <= 0)
{
sock = socket(AF_UNIX, SOCK_STREAM, 0);
if (sock < 0)
{
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, SYNCD_IPC_SOCK_FILE, getpid());
unlink(caddr.sun_path);
if (bind(sock, (struct sockaddr *)&caddr, sizeof(caddr)) < 0)
{
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, SYNCD_IPC_SOCK_FILE);
if (connect(sock, (struct sockaddr *) &saddr, sizeof(saddr)) < 0)
{
SWSS_LOG_ERROR("connect() :%s", strerror(errno));
close(sock);
sock = 0;
unlink(caddr.sun_path);
return errno;
}
}

len = strlen(cmd);
ret = send(sock, cmd, len, 0);
if (ret < (ssize_t)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, SYNCD_IPC_BUFF_SIZE - 1, 0);
if (ret <= 0)
{
SWSS_LOG_ERROR("recv failed, ret=%ld\n", ret);
close(sock);
sock = 0;
unlink(caddr.sun_path);
return -EIO;
}

timeout = time(NULL) + MDIO_CLIENT_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)
{
// SWSS_LOG_ENTER(); // disabled

int rc = SAI_STATUS_FAILURE;
char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE];

if (number_of_registers > 1)
{
SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers);
return SAI_STATUS_FAILURE;
}

ipcMutex.lock();

sprintf(cmd, "mdio 0x%x 0x%x\n", mdio_addr, reg_addr);
rc = syncd_mdio_ipc_command(cmd, resp);
if (rc == 0)
{
*data = (uint32_t)strtoul(strchrnul(resp, ' ') + 1, NULL, 0);
rc = SAI_STATUS_SUCCESS;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If syncd_mdio_ipc_command(cmd, resp) < 0, for debugging, should at least add log for error code. Better to translate the error code to SAI status code on rc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add error log as the else condition statement.

else
{
SWSS_LOG_ERROR("syncd_mdio_ipc_command returns : %d\n", rc);
}

ipcMutex.unlock();
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)
{
// SWSS_LOG_ENTER(); // disabled

int rc = SAI_STATUS_FAILURE;
char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE];

if (number_of_registers > 1)
{
SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers);
return SAI_STATUS_FAILURE;
}

ipcMutex.lock();

sprintf(cmd, "mdio 0x%x 0x%x 0x%x\n", mdio_addr, reg_addr, *data);
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);
}

ipcMutex.unlock();
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,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all those extern C functions should be declared in extern C in header file, and not explicitly here

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which they are, so this extern should be redundant here

Copy link
Copy Markdown
Contributor Author

@jiahua-wang jiahua-wang Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to remove the extern C in .c file. I'm not convinced it's good practice to remove the extern "C" from the implementation file. I think it's better to be explicit in the implementation file.

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];

if (number_of_registers > 1)
{
SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers);
return SAI_STATUS_FAILURE;
}

ipcMutex.lock();

sprintf(cmd, "mdio-cl22 0x%x 0x%x\n", mdio_addr, reg_addr);
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);
}

ipcMutex.unlock();
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)
{
// SWSS_LOG_ENTER(); // disabled

int rc = SAI_STATUS_FAILURE;
char cmd[SYNCD_IPC_BUFF_SIZE], resp[SYNCD_IPC_BUFF_SIZE];

if (number_of_registers > 1)
{
SWSS_LOG_ERROR("Multiple register reads are not supported, num_of_registers: %d\n", number_of_registers);
return SAI_STATUS_FAILURE;
}

ipcMutex.lock();

sprintf(cmd, "mdio-cl22 0x%x 0x%x 0x%x\n", mdio_addr, reg_addr, *data);
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);
}

ipcMutex.unlock();
return rc;
}
Loading