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

[sai-gen] Refactor the templates of libsai implementation #648

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

jimmyzhai
Copy link
Collaborator

@jimmyzhai jimmyzhai commented Dec 5, 2024

Base on #647, refactor the templates of libsai implementation
to make it more readable and maintainable. It replaces one single template saiapi.cpp.j2 with several
small templates under directory SAI/templates/impls.

The changes will not change any generated cpp code for sai api implementation:

junhuazhai@junhuazhai-dev-vm:~/workspace/DASH/dash-pipeline$ diff -Nur -x '*.[od]' ~/DASH/dash-pipeline/SAI/lib/ SAI/lib
junhuazhai@junhuazhai-dev-vm:~/workspace/DASH/dash-pipeline$

@jimmyzhai jimmyzhai requested a review from r12f December 5, 2024 08:14
@KrisNey-MSFT
Copy link
Collaborator

@r12f for viz

Copy link
Collaborator

@r12f r12f left a comment

Choose a reason for hiding this comment

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

Reviewed and sync'ed offline. This PR is refactor only and trying to target no changes in the libsai, hence many issues are not addressed, such as per action parameters, duplicated code for setting P4 values and bulk entry creation and removals. These issues are intended to be addressed in later changes, so we can avoid building too large PR to review.

@KrisNey-MSFT
Copy link
Collaborator

@r12f would you like for me to merge it today?

@KrisNey-MSFT
Copy link
Collaborator

breaks libsai templates into smaller ones; want to be mindful of compatibility; will generate separate PRs for forward movement

@KrisNey-MSFT KrisNey-MSFT merged commit e16ceb4 into sonic-net:main Dec 11, 2024
4 checks passed
r12f pushed a commit that referenced this pull request Dec 23, 2024
Following #648, continue to
refactor libsai implementation for easier read and maintenance:

1. Avoid using compilated j2 template as best
2. Use c++ code directly to express the logic of API implementation,
based on p4 table metadata info (p4meta.[h, cpp])
3. Implement APIs set/get attribute by using common `DashSai::set` and
`DashSai::get`, which fix the issue
#650
4. Update libsai test (vnet_out.cpp) to cover sanity test of APIs
set/get attribute

Note: To refactor APIs create/remove with current approach in next PR. 

The generated sample code of APIs set/get attribute is as below:

- SAI object objectID

![image](https://github.com/user-attachments/assets/2df7e786-6d7b-4503-a26f-6f2463748867)
p4meta `outbound_routing_group_meta_table` definition

![image](https://github.com/user-attachments/assets/ddf3e5ee-6fab-4ede-b589-48600b885088)

- SAI object entry

![image](https://github.com/user-attachments/assets/c4d47442-c4bf-4c88-9e8f-14b54f03668e)
p4meta `outbound_routing_entry_meta_table`  definition

![image](https://github.com/user-attachments/assets/deb64ef6-e90c-4d2d-9aad-9aa6db133521)
@jimmyzhai jimmyzhai deleted the libsai branch January 5, 2025 14:07
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