Skip to content

Conversation

@lsang6WIND
Copy link
Contributor

Do not send bmp end of rib if the policy is not configured.

@lsang6WIND
Copy link
Contributor Author

ci:rerun

@riw777
Copy link
Member

riw777 commented Jun 24, 2025

Can you say why we shouldn't send EoR by default? Is there a use case, etc.? I don't think this will break any current deployments, but it might be good to say something about that as well (?).

@riw777 riw777 self-requested a review June 24, 2025 13:38
@lsang6WIND
Copy link
Contributor Author

lsang6WIND commented Jun 24, 2025

Currently, BMP sends an EoR for each supported policy type (Adj-RIB-In pre-policy, Adj-RIB-In post-policy, and Loc-RIB), regardless of the policy configuration. This may be confusing when a user only wants to monitor Adj-RIB-In pre-policy routes but still receives an EoR for Adj-RIB-In post-policy.

I don't think this will break any current deployments, but it might be good to say something about that as well (?).

I do not think, this will help in scenarios where the user wants to receive EoR messages only for the configured policies, making monitoring more precise.

Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

looks good

@ton31337
Copy link
Member

Could you put this explanation into the commit message directly before we merge?

Currently, BMP sends an EoR for each supported policy type (Adj-RIB-In
pre-policy, Adj-RIB-In post-policy, and Loc-RIB), regardless of the
policy configuration. This may be confusing when a user only wants to
monitor Adj-RIB-In pre-policy routes but still receives an EoR for
Adj-RIB-In post-policy.

Do not send bmp EoR if the policy was not configured.

Signed-off-by: Loïc Sang <[email protected]>
@riw777 riw777 merged commit d327af6 into FRRouting:master Jul 1, 2025
14 checks passed
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