Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Google Proto Buff for SAI-RPC available? #1830

Open
raghunandan403 opened this issue Jul 31, 2023 · 10 comments
Open

Question: Google Proto Buff for SAI-RPC available? #1830

raghunandan403 opened this issue Jul 31, 2023 · 10 comments

Comments

@raghunandan403
Copy link

raghunandan403 commented Jul 31, 2023

Hi guys,
Im looking for SAI RPC implemented with GPB (rather than Thrift). This is more a question rather than a issue.
We have GPB based compilation and infra available in our NOS. So, looking for a GPB based implementation for SAI-RPC.

Our thought is:
Our NOS [SAI RPC Client(GPB)] <-> [SAI RPC Server (GPB) <-> SAI <-> Adapter layer] running on our NOS

@kcudnik
Copy link
Collaborator

kcudnik commented Jul 31, 2023

Not sure, but probably nobody done that yet

@raghunandan403
Copy link
Author

Kamil,
If we want to do this GPB and give it back to community, whats the process to do it?
Any recommendations/suggestions you give, to start this off?

@kcudnik
Copy link
Collaborator

kcudnik commented Jul 31, 2023

I think you could first provide some high level design how you want to do it, also whether you want to implement this in SAI repo, since SAI repo defines only interface headers, so since you want to implement libsai.so which underline uses GPB, it will require code to do it, you can ether make it here or separate repo for that, i think this could be good topic for discussion in SAI community meeting.

PS. do you have GPB documentation somewhere, is it opensource project ? any links or references to it?

@raghunandan403
Copy link
Author

Kamil,
https://github.com/protocolbuffers/protobuf is the repo for GPB.

  • GPB (.proto) file will provide a serialisation format for packets of typed, structured data for all SAI supported features to make RPC from south-bound of NOS to server (running on other OS which includes SAI).

Right now, we don't have any GPB notes/HLD for SAI. We will try to comeup with a draft in few weeks if community is OK to go-ahead with this layer on top of SAI.
At high level, the thought process is to use the same xml generated (while compiling for thrift rpc) and use that xml to generate a .proto, server and client libraries for all supported features in SAI.

We are thinking about:

  • when we "make sai",
    • .proto files will be generated along with gpb server and client APIs.
    • libsai.so will be built containing the server-code, sai-adapter.
    • clinet lib.so will also be build.

@kcudnik
Copy link
Collaborator

kcudnik commented Jul 31, 2023

Thanks, i just made a quick look, i sorry but i dont have time to do a deep dive into that lib.

Your steps sounds good and i think it could be beneficial to eveyrone to have such rpc.

I think it should be easy to generate proto files from SAI headers, considering that we have all headers validated and in consistent form. You just need a small parser for SAI headers.

@raghunandan403
Copy link
Author

sai-grpc-writedown.docx

I have written down a document on what I plan to do to generate a .proto file.
Please review it. I have the code ready too.

Please let me know on your comments.

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 4, 2024

as entry document looks good

@raghunandan403
Copy link
Author

raghunandan403 commented Mar 7, 2024

Trying to push my diff to a branch "protobuf". I get the following error:

======
git push origin protobuf
ERROR: Permission to opencomputeproject/SAI.git denied to raghunandan403.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights

========

Kamil,
Can you help here ?

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 7, 2024

you are trying to push to origin, you need to create fork for your repo, push to your repo branch and then create new PR here

@raghunandan403
Copy link
Author

Created the same. Pull request is at:
#1980

Please review the diffs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants