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] Add support for dash configuration from sai interface #419

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

kcudnik
Copy link
Collaborator

@kcudnik kcudnik commented Aug 2, 2023

Adding this support will allow syncd to easy pass configuration parameters to dash libsai using service method table

Copy link
Collaborator

@chrispsommers chrispsommers left a comment

Choose a reason for hiding this comment

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

Code LGTM but I am not very familiar with the usage of these APIs. Can you give an example how they might be useful for DASH?

@kcudnik
Copy link
Collaborator Author

kcudnik commented Aug 2, 2023

for dash it should be transparent, main use case is for syncd, for example we load profile.ini file which configure many different aspects of libsai, in this case i allow to configure some dash api, like every variable in config.h and saidash.h keys, you can take a look of use case in syncd here: https://github.com/sonic-net/sonic-sairedis/blob/master/syncd/Syncd.cpp#L3165 when we load profile from file, if this config will be supproted in dash then we can modify easy those parameters from syncd pointing to right configuration files

most important line is here: https://github.com/sonic-net/sonic-sairedis/blob/master/syncd/Syncd.cpp#L3218

btw. all those SAI/template files i will move to proper classes, but it will take some time, and i dont want to do it in one big PR, i will take steps to it, so maybe at this point it could make confusion, but in my head and couple PRs in the future this works, trust me, im Jack Sparrow XD

@kcudnik
Copy link
Collaborator Author

kcudnik commented Aug 7, 2023

can we have this merged ?

@chrispsommers chrispsommers merged commit 1af97f1 into sonic-net:main Aug 7, 2023
3 checks passed
@kcudnik
Copy link
Collaborator Author

kcudnik commented Aug 7, 2023

thanks!

@kcudnik kcudnik deleted the config branch August 7, 2023 15:46
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.

2 participants