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: fix peer up message for loc-rib not sent #17545

Merged

Conversation

pguibert6WIND
Copy link
Member

At startup, there is no peer up message for loc-rib instance peer. Instead, a global peer up message with address 0.0.0.0 is sent.

Such message is wrong, violates the RFC and should be dropped by a strict collector. Actually, the peer type message sent is wrong, and should be set to LOC-RIB peer type.

Fix this by changing the peer type of peer up message to either loc-rib or global instance peer type.

Fixes: 035304c ("bgpd: bmp loc-rib peer up/down for vrfs")

@pguibert6WIND pguibert6WIND force-pushed the peerup_loc_rib_wrong_format branch from 4d9fcf5 to 601576f Compare December 2, 2024 10:34
@pguibert6WIND pguibert6WIND changed the title bgpd: fix peer up message with wrong peer type bgpd: fix peer up message for loc-rib not sent Dec 2, 2024
@pguibert6WIND pguibert6WIND marked this pull request as draft December 2, 2024 10:44
@pguibert6WIND pguibert6WIND force-pushed the peerup_loc_rib_wrong_format branch from 601576f to 829d9a9 Compare December 2, 2024 10:50
@pguibert6WIND pguibert6WIND marked this pull request as ready for review December 2, 2024 10:51
@mxyns
Copy link
Contributor

mxyns commented Dec 2, 2024

hi @pguibert6WIND,
i think this is already fixed in #14847

https://github.com/FRRouting/frr/blob/f49f7cfe217ab163ace2c64b3e0374ee6c5da516/bgpd/bgp_bmp.c#L692C1-L694C1

please let me know if i'm wrong

@pguibert6WIND
Copy link
Member Author

ci:rerun

@pguibert6WIND
Copy link
Member Author

hi @pguibert6WIND, i think this is already fixed in #14847

https://github.com/FRRouting/frr/blob/f49f7cfe217ab163ace2c64b3e0374ee6c5da516/bgpd/bgp_bmp.c#L692C1-L694C1

please let me know if i'm wrong

thanks to confirm this is an issue.
I could check that outgoing wireshark packets are now compliant.

@pguibert6WIND pguibert6WIND force-pushed the peerup_loc_rib_wrong_format branch from 829d9a9 to 5c1e11c Compare December 3, 2024 14:52
@github-actions github-actions bot added size/M and removed size/S labels Dec 3, 2024
@pguibert6WIND
Copy link
Member Author

added test to ensure everything is correct.

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

@pguibert6WIND
Copy link
Member Author

ci:rerun one more time

@mxyns
Copy link
Contributor

mxyns commented Dec 4, 2024

looks good thanks

@pguibert6WIND pguibert6WIND force-pushed the peerup_loc_rib_wrong_format branch from 5c1e11c to 1c5fb9e Compare December 4, 2024 15:07
At startup, there is no peer up message for loc-rib instance peer.
Instead, a global peer up message with address 0.0.0.0 is sent.

Such message is wrong, violates the RFC and should be dropped by
a strict collector. Actually, the peer type message sent is wrong,
and should be set to LOC-RIB peer type.

Fix this by changing the peer type of peer up message to either
loc-rib or global instance peer type.

Fixes: 035304c ("bgpd: bmp loc-rib peer up/down for vrfs")
Signed-off-by: Philippe Guibert <[email protected]>
Add a test at startup to ensure that peer up message for loc-rib is
correctly set.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND pguibert6WIND force-pushed the peerup_loc_rib_wrong_format branch from 1c5fb9e to f921a8d Compare December 5, 2024 14:44
@pguibert6WIND
Copy link
Member Author

ci:rerun unrelated

@ton31337 ton31337 merged commit 03ea25a into FRRouting:master Dec 6, 2024
11 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.

4 participants