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

Static Port channel commit #79

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

Conversation

prasadsriramps
Copy link
Contributor

@prasadsriramps prasadsriramps commented Jul 10, 2023

Description:

This change enhances the port channel feature by enabling users to configure PortChannel as static (load balancing) or dynamic (LACP). Modifications were added based on the previous SONiC community HLD - sonic-net/SONiC#1039.

Test logs - Static portchannel log.pdf

@ishidawataru
Copy link
Contributor

@prasadsriramps Thanks for your work. Are you testing this with an actual device?

@prasadsriramps
Copy link
Contributor Author

Apologies. forgot to attach test logs. updated the PR description to add the same.

@ishidawataru
Copy link
Contributor

ishidawataru commented Jul 12, 2023

Thanks. The test logs look really good.

Can you update here to include mode inside the state container?

Goldstone YANG models follow the config / state structure of OpenConfig models.

When querying the sysrepo operational datastore, the config containers in the model are populated automatically by copying the contents of the sysrepo running datastore. This indicates the configuration intention of the operator. On the other hand, the state containers need to be populated manually in the oper_cb to share the actual status of the system.

@prasadsriramps
Copy link
Contributor Author

Hi Wataru,

The requested changes have been made.

I also bring to your attention another PR in the usonic repository (oopt-goldstone/usonic#6) that addresses the teamd related changes for static portchannel. Kindly review them whenever possible.

regards,
Prasad Sriram

@ishidawataru
Copy link
Contributor

@prasadsriramps Thanks. I left several review comments. Please take a look. I'll check the usonic PR.

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

Successfully merging this pull request may close these issues.

3 participants