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

operational-state (datastore) change notifications #17796

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

Conversation

choppsv1
Copy link
Contributor

@choppsv1 choppsv1 commented Jan 8, 2025

Add framework for notifications sent to front-end clients for changes to operational state (datastore). This is to support various YANG push RFCs such as:

  • RFC7923: Requirements for Subscription to YANG Datastores
  • RFC8639: Subscription to YANG Notifications
  • RFC8640: Dynamic Subscription to YANG Events and Datastores over NETCONF
  • RFC8641: Subscription to YANG Notifications for Datastore Updates
  • RFC8650: Dynamic Subscription to YANG Events and Datastores over RESTCONF

@frrbot frrbot bot added libfrr mgmt FRR Management Infra tests Topotests, make check, etc labels Jan 8, 2025
@choppsv1 choppsv1 force-pushed the chopps/datastore-notifications branch 7 times, most recently from d9ed722 to dc66226 Compare January 8, 2025 17:43
@choppsv1 choppsv1 marked this pull request as draft January 8, 2025 20:30
@choppsv1 choppsv1 force-pushed the chopps/datastore-notifications branch 2 times, most recently from d8c15b9 to 94fe28d Compare January 10, 2025 05:30
@choppsv1 choppsv1 marked this pull request as ready for review January 10, 2025 05:30
@choppsv1 choppsv1 force-pushed the chopps/datastore-notifications branch from 94fe28d to 66e4ed0 Compare January 10, 2025 06:59
Signed-off-by: Christian Hopps <[email protected]>
Signed-off-by: Christian Hopps <[email protected]>
- Additionally push the selectors down to the backends

Signed-off-by: Christian Hopps <[email protected]>
@choppsv1 choppsv1 force-pushed the chopps/datastore-notifications branch from 66e4ed0 to 275b1e0 Compare January 10, 2025 12:03
@frrbot frrbot bot added the bugfix label Jan 10, 2025

nb_notif_add(abs_path);

if (abs_path != path)
Copy link
Member

Choose a reason for hiding this comment

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

What is the chance that abs_path and path are equal and path was passed in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

abs_path starts out assigned as NULL, it is then either assigned the value path or it is a new allocation from lyd_path(). path is not currently allowed to be NULL (notice we use path[0] without checking for path == NULL). So path can only ever equal abs_path when we set it that way (which is what our equality check is deciding, did we set it or newly allocate it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That said, I don't see a reason we couldn't also support a NULL path for updating the path/value of the tree node itself, so I'll modify the code to also handle path == NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually nm, this gets tricky deeper in the implementation, i'll leave extended functionality for later if we need it.

@choppsv1 choppsv1 force-pushed the chopps/datastore-notifications branch from 275b1e0 to 806c76a Compare January 10, 2025 19:09
@choppsv1 choppsv1 force-pushed the chopps/datastore-notifications branch from 806c76a to 26f0f0a Compare January 10, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix libfrr master mgmt FRR Management Infra size/XXL tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants