sonic-net / sonic-sairedis

SAI object interface to Redis database, as used in the SONiC project
Other
56 stars 259 forks source link

Clean up and refactor duplicated code and auto generate sai stub for libraries #1384

Closed kcudnik closed 2 months ago

kcudnik commented 2 months ago

Many refactoring changes include:

kcudnik commented 2 months ago

later on entire libsaistub can be auto generated from saimetadata, it's just as stub for API call

aviramd commented 1 month ago

Hi Kamil,

I kindly need some clarification regarding this modification. To be more specific, I will reference the example of the sai_query_attribute_capability API. I have traced the flow of sai_query_attribute_capability calls from switchorch.cpp. Before this change, there was a module named sai_redis_interfacequery.cpp, but It was removed in this commit . Additionally, I cannot locate any implementation of sai_query_attribute_capability in swss. Could you please clarify if this is an auto-generated file and if it gets deleted after the build? Thanks Aviram

kcudnik commented 1 month ago

Hey, all the apis you are referring are now auto generated and implementation is in lib and vslib Sai.cpp, take a look at stub.pl file and generated files in lib and vslib die after make sai_redis.cpp and sai_vs.cpp

aviramd commented 1 month ago

Hey, I understand that all the APIs are now auto-generated . However, since the auto-generated files are deleted, it makes it difficult to debug and review the code.

Would it be possible to consider deleting the auto-generated files just before regenerating them? This way, we can have access to the generated code for debugging and review purposes until we need to regenerate them.

kcudnik commented 1 month ago

Not sure where is the problem? Files are auto generated if sai changes, if no changes are to sai directory then files stay the same, there is no need for debug generated code, only the one that is I. Sai.cpp. Class

aviramd commented 1 month ago

The auto-generated files are deleted after the build process. As a result, anyone unfamiliar with the format of these files will encounter gaps when trying to trace the code flow. This issue also affects debugging with gdb, as parts of the code will be missing. However, now that I understand the format and flow, I no longer need these files.

kcudnik commented 1 month ago

CPP auto.generared files are not deleted unless you make clean

aviramd commented 1 month ago

I clearly see that sai_redis.cpp is deleted after build (I am not on latest master but I didn't see and changes related on latest commits)

please check build syncd_1.0.0_arm64.deb.log

syncd_1.0.0_arm64.deb.log

aviramd commented 1 month ago

Can you please explain or provide a reference that explains the concepts behind ClientSai, ServerSai, ClientServerSai, RedisRemoteSaiInterface, and proxylib?

kcudnik commented 3 weeks ago

Currently there is no document on that, you would need to go to git blame to see corresponding commit descriptions, but basically client server is a way to talk to libsaitedis from multiple clients, this was idea to make tests and later on some live action on syncd when orhagent is already running, bit this idea is not utilized, for lib proxy is new idea for dash project, to be able to also communicate to some sai apis while syncd is already running but from other modules, client server is running above syncd, and lib proxy under syncd, redisremoteinterfsce is interface that communicates from libsaitedys to syncd and is utilized by orhagent, all those interfaces uses serialization and serialization code, which In multiple places is duplicated and this needs to be merged to some serialize class and reused, but currently I'm no vacationand will refactor this when needed, probably when bulk apis will need to be implemented since not all serialization a support bulk