Add mdio register read/write via rpc #1084
Conversation
| typedef enum _sai_redis_switch_register_fn_t | ||
| { | ||
| SAI_REDIS_SWITCH_REGISTER_FN_NULL = 0, | ||
| SAI_REDIS_SWITCH_REGISTER_FN_MDIO_CL22_READ, | ||
| SAI_REDIS_SWITCH_REGISTER_FN_MDIO_CL22_WRITE, | ||
| SAI_REDIS_SWITCH_REGISTER_FN_MDIO_CL45_READ, | ||
| SAI_REDIS_SWITCH_REGISTER_FN_MDIO_CL45_WRITE, | ||
| } sai_redis_switch_register_fn_t; |
There was a problem hiding this comment.
why this is not the peart of SAI ?
| static constexpr uint32_t IPC_TIMEOUT_SEC = 2; // in seconds | ||
| static constexpr char IPC_SERVER[] = "/var/run/redis/syncd.sock"; | ||
|
|
||
| class Ipc: public swss::Selectable |
There was a problem hiding this comment.
should this be ipcserver ?
also move public swss::Selectable to next line
| if (m_sock >= 0) | ||
| { | ||
| close(m_sock); | ||
| } |
There was a problem hiding this comment.
move implemntation to cpp file
| std::shared_ptr<RpcStub> rpcAt(int rpc) const; | ||
| void registerRpc(const std::shared_ptr<RpcStub> &stub); | ||
|
|
||
| int sendto(const IpcMsg *msg, const struct sockaddr *dest_addr, socklen_t addrlen); | ||
| int recvfrom(IpcMsg *msg, int size, struct sockaddr *src_addr, socklen_t *addrlen); | ||
| int send(const IpcMsg *msg); | ||
| int recv(IpcMsg *msg, int size); |
There was a problem hiding this comment.
all pamameters inlcuding constructor should be 1 per line and have prefix specifier In or Out etc, take a look at ohter files .h
| std::unordered_map<int, std::shared_ptr<RpcStub>> m_rpcMap; | ||
| }; | ||
|
|
||
| class IpcClient: public Ipc |
There was a problem hiding this comment.
move to separate file IpcClient.cpp
| uint32_t reg_data[0]; | ||
| }; | ||
|
|
||
| class RpcMdio: public RpcStub { |
There was a problem hiding this comment.
file hsould be named as class name
| if (m_ipcServer && m_switches.size() == 1) | ||
| { | ||
| m_ipcServer->setSwitchId(objectRid); | ||
| m_ipcServer->setSwitchApi(dynamic_cast<VendorSai*>(m_vendorSai.get())->get_switch_api()); |
There was a problem hiding this comment.
dont use get_swutch_api, provide VendorSai instead and expose mdio api functions
| * If IPC server is enabled, it needs to remember switch object rid and | ||
| * switch api, used by the calling of switch_mdio_read/write. | ||
| */ | ||
| if (m_ipcServer && m_switches.size() == 1) |
There was a problem hiding this comment.
this will onlyu work for first switch and possible if many will be created then this will not be executed
| @@ -1270,3 +1270,10 @@ sai_status_t VendorSai::logSet( | |||
|
|
|||
| return sai_log_set(api, log_level); | |||
| } | |||
|
|
|||
| const sai_switch_api_t* VendorSai::get_switch_api(void) const | |||
There was a problem hiding this comment.
please remove and expose proper apis for mdio
| @@ -30,6 +30,8 @@ namespace syncd | |||
|
|
|||
| sai_status_t uninitialize(void) override; | |||
|
|
|||
| const sai_switch_api_t* get_switch_api(void) const; | |||
No description provided.