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

protobuf file generation for SAI headers #1980

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

raghunandan403
Copy link

The changes does the following:

  • The above changes, when given "make rpc" from meta folder, will generate a sai.proto file.

  • have made changes related to protobuf in:

    • rpc/SAI/Struct.pm
    • rpc/SAI/Function.pm
    • rpc/SAI/Typedef.pm
    • rpc/SAI/Variable.pm
    • special handling for protobufs added into: rpc/SAI/RPC/ProtoBufName/Type.pm & meta/rpc/SAI/RPC/ProtoBufName/Variable.pm.
  • A new Template toolkit added for generating proto that consist of

    • messages, rpc_service, Enums/attributes for all features supported in saiheaders.
  • The Enums are generated. But, not all enums have a value initialised. So, rt now, I commented Enums generation. I will fix this in coming weeks

  • A new gensaiprotorpc.pl is added in meta directory. this will have script to generate the .proto.

  • A new target is added in meta/Makefile "protorpc". When the make is given, it runs the above pearl file. This will build a .proto and also generate .c and .h file.

As a part of the make of the SAI, both thrift and proto will be built. To check if the .proto is generated properly, the script will try to generate the .c and .h file based on the .proto file. If the .proto file has some issue, the make will fail and bail out.

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 7, 2024

please fix build errors and sign your commit to pass DCO

Signed-off-by: raghunandan403 <raghunandan.403@gmail.com>
Signed-off-by: raghunandan403 <raghunandan.403@gmail.com>
@raghunandan403
Copy link
Author

raghunandan403 commented Mar 7, 2024

@kcudnik ,

  • I see the build failing in the Azure pipeline. But, the build passes in my local workspace.
  • I have created protobuf_name in the new package in the diff. I doubt if this new files created is being taken by the Azure pipeline. Can you please help here.
  • One other thing we need to make change in the azure pipeline is: "install proto compiler" along with "install thrift compiler"
  • I see the below error.

Building SAI meta XML...
make[1]: Entering directory '/__w/1/s/meta'
make[1]: 'xml' is up to date.
make[1]: Leaving directory '/__w/1/s/meta'
Can't locate object method "protobuf_name" via package "SAI::Type" at
/__w/1/s/meta/rpc/SAI/Function.pm line 164 (#1)
(F) You called a method correctly, and it correctly indicated a package
functioning as a class, but that package doesn't define that particular
method, nor does any of its base classes. See perlobj.

Uncaught exception from user code:
Can't locate object method "protobuf_name" via package "SAI::Type" at /__w/1/s/meta/rpc/SAI/Function.pm line 164.
SAI::Function::_find_count_arg(SAI::Function=HASH(0x55d5df4303e0), SAI::Function::Argument=HASH(0x55d5df4304a0)) called at /__w/1/s/meta/rpc/SAI/Function.pm line 201
SAI::Function::resolve_arg_dependencies(SAI::Function=HASH(0x55d5df4303e0)) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/Class/MOP/Method/Wrapped.pm line 56
SAI::Function::_wrapped_resolve_arg_dependencies(SAI::Function=HASH(0x55d5df4303e0)) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/Class/MOP/Method/Wrapped.pm line 95
SAI::Function::resolve_arg_dependencies(SAI::Function=HASH(0x55d5df4303e0)) called at /__w/1/s/meta/rpc/SAI/Function.pm line 225
SAI::Function::BUILD(SAI::Function=HASH(0x55d5df4303e0), HASH(0x55d5df4303c8)) called at constructor SAI::Function::new (defined at /__w/1/s/meta/rpc/SAI/Function.pm line 273) line 69
SAI::Function::new("SAI::Function", "xml_typedef", HASH(0x55d5df2fa668)) called at gensairpc.pl line 623
main::get_function(HASH(0x55d5df454340), HASH(0x55d5deb7ba00), HASH(0x55d5df2fa668), "acl") called at gensairpc.pl line 412
main::get_definitions() called at gensairpc.pl line 156
Parsing...
make: *** [Makefile:138: sai.thrift] Error 255

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 7, 2024

hmm, not sure why this errors shows on pipeline, maybe perl in pipeline needs some extra include directory for search in path

@raghunandan403
Copy link
Author

hmm, not sure why this errors shows on pipeline, maybe perl in pipeline needs some extra include directory for search in path

How do i take it forward?
Can i get access to the pipeline code and how they have written code there? I will check and try to fix. Also, I need to fix "install proto compiler".

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 7, 2024

That's the problem there is no access to pipeline everything is in yml file in SAI repo you can see. Configuration there

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 7, 2024

use warnings;
use diagnostics;

use 5.020;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why explicit perl version ?

Copy link
Author

@raghunandan403 raghunandan403 Mar 11, 2024

Choose a reason for hiding this comment

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

Agree Kamil. Hardcoding of pearl isnt mandatory.
But, i picked up from the gensairpc.pl file.

@file gensairpc.pl

@brief This module generates RPC interface of SAI for PTF

use strict;
use warnings;
use diagnostics;

use 5.020;

Copy link
Collaborator

Choose a reason for hiding this comment

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

i would like to remove that, unless you need very specific feature that is present in that specific version, then please add comments why this version is necessary

@bhagatgit
Copy link

@raghunandan403
Thank you for implementing protobuf file generation. We have been working on something similar. Is there a HLD associated with this code PR ? Are there any applications planned with this ?

@raghunandan403
Copy link
Author

@raghunandan403 Thank you for implementing protobuf file generation. We have been working on something similar. Is there a HLD associated with this code PR ? Are there any applications planned with this ?

The code is at :
https://github.com/raghunandan403/proto-sai

I had some hurdles in committing the code. The sanity tests were failing for unknown reason.. I havent spent the time to get it committed. But, we are using this internally for DPU based implementation.

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

Successfully merging this pull request may close these issues.

3 participants