-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Introduce a SONiC-specific communication channel between FRR and SONiC #18715
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
Merged
kperumalbfn
merged 1 commit into
sonic-net:master
from
cscarpitta:dplane_fpm_sonic_master
Sep 30, 2024
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is better if we do not change it by default, it is better to first introduce this as option and then wait till it becomes stable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lguohan
Many thanks for the review.
I added a compile-time option
ENABLE_DPLANE_FPM_SONIC.When this option is set, SONiC uses the new FPM module.
When this option is not set, SONiC uses the old FPM module and behaves exactly as it does today.
By default, this option is not set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am fine that we build them into image by default, but do not enable it by default. if you can do it at runtime, it will be easier for you, and i am fine with that.
does this dplane_fpm_sonic support all features as dplane_fpm_nl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lguohan
Yes, the
dplane_fpm_sonicsupports all features asdplane_fpm_nl. It is a copy of the olddplane_fpm_nl.Indeed, it is easier to compile this by default and keep the old
dplane_fpm_nlrunning by default.I reverted the compile option and changed the supervisord config file to keep
dplane_fpm_nlas default.