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

bgpd: Enable combined logging for UPDATEs #14809

Conversation

ton31337
Copy link
Member

Before this patch if we enable two commands:

  • debug bgp updates in 10.0.0.1
  • debug bgp updates prefix 192.168.0.0/24

We never check both conditions if a prefix and neighbor match. If we see that we enabled logging for a specific neighbor, then we log all the messages related to that neighbor and ignore what prefix we expect.

With this patch, we have a combined check and look at prefix + neighbor.

Before this patch if we enable two commands:
* `debug bgp updates in 10.0.0.1`
* `debug bgp updates prefix 192.168.0.0/24`

We never check both conditions if a prefix and neighbor match. If we see that
we enabled logging for a specific neighbor, then we log all the messages related
to that neighbor and ignore what prefix we expect.

With this patch, we have a combined check and look at prefix + neighbor.

Signed-off-by: Donatas Abraitis <[email protected]>
@taspelund
Copy link

I completely disagree with how this change modifies the semantics of debugs.

Two conditions in a single statement should be treated as an AND. Two conditions in separate statements should be treated as an OR.

This is how pretty much everything else in FRR works, e.g. route-maps, and changing these semantics would remove the ability for an operator to debug updates EITHER for a prefix from any peer OR any prefix to/from a peer.

I would be on board with a change that allowed an operator to AND the two debug conditions, but that should fall within a single debug statement.
e g.
debug bgp updates in 10.0.0.1 prefix 192.168.0.0/24

@ton31337
Copy link
Member Author

I agree and disagree here, but having a command as you suggested makes more sense. Will add such a feature to an existing command as an extension to avoid breaking stuff.

@ton31337 ton31337 closed this Nov 16, 2023
@ton31337 ton31337 deleted the feature/bgp_debug_updates_combined_prefix_neighbor branch November 16, 2023 16:48
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