-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add bfd session params to dest register msg #14839
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? |
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
@@ -352,6 +352,32 @@ static int _ptm_msg_read(struct stream *msg, int command, vrf_id_t vrf_id, | |||
* - 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
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. |
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.