-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add bfd session params to dest register msg #14839
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
base: master
Are you sure you want to change the base?
Conversation
|
Hi, I am actually trying to check if a change like this for frr-bfdd is worth for the upstream. Please advise. If you need more information on the change then please do let me know. Kind regards, |
b5709dc to
453252f
Compare
453252f to
95b0074
Compare
|
how do you plan to ensure that people using the old way of communicating are not suddenly broken with this new addition to the read? |
rzalamena
left a comment
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.
Changes looks good, but it seems no protocols are using this. Do you have any examples of usage? Do you have a follow on PR to add the usage and maybe tests?
bfdd/ptm_adapter.c
Outdated
| * - c: profile name length. | ||
| * - X bytes: profile name. | ||
| * | ||
| * New format (with session id and status): |
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.
No need to replicate the New format, just update the previous section with the new fields.
Allow protocols to generate bfd session id and set the bfd session enable status for bfd sessions via the ZEBRA_BFD_DEST_REGISTER message. This is useful in case of "Stacking" when the Active(session enabled) and Standby(session disabled) switches run the same BFD session and if the Active fails and the Standby becomes Active but the session remains UP without affecting the routing protocols with the DOWN event. Signed-off-by: Lokesh Dhoundiyal <[email protected]>
Could be done via keeping the new fields inside STREAM_READABLE check. |
We are planning on making changes to support running BGP in a stacked (HA) environment, but they are still a very early work in progress. Thanks. |
95b0074 to
c1be110
Compare
riw777
left a comment
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.
this looks like good functionality to add ... code looks okay, but it would be good to get other eyes on this
|
How long do y'all think it will be before the code that relies on these changes will be ready to commit? Would it be better to put this in as a work in progress and wait for that code to be ready so this can all be reviewed together? |
|
is anyone still working on this? |
|
Hi, Unfortunately, The related work has been put on hold at the moment. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
This PR is stale because it has been open 180 days with no activity. Comment or remove the |
Allow protocols to create bfd session ids and control the bfd session status and configure it in frr-bfd via the ZEBRA_BFD_DEST_REGISTER message. This is useful in case of "Stacking" when the Active(session enabled) and Standby(session disabled) switches run the same BFD session and if the Active fails and the Standby becomes Active but the session remains UP without affecting the routing protocols with the DOWN event.