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.

Sign up for GitHub

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

Jump to bottom

bgpd: Validate both nexthop information (NEXTHOP and NLRI) #17435

Merged
merged 2 commits into from
Nov 17, 2024
Merged

bgpd: Validate both nexthop information (NEXTHOP and NLRI) #17435

merged 2 commits into from
Nov 17, 2024
+198 −28

Conversation

ton31337
Copy link
Member

Closes #17308

@frrbot frrbot bot added bgp tests Topotests, make check, etc labels Nov 15, 2024
@ton31337 ton31337 force-pushed the fix/bgp_allow_0.0.0.0_if_multiple_nlris_exist branch 2 times, most recently from a89db02 to 0f5a020 Compare November 15, 2024 09:52
@ton31337 ton31337 marked this pull request as ready for review November 15, 2024 10:52
@ton31337 ton31337 mentioned this pull request Nov 15, 2024
Closed
2 tasks
mjstapp
mjstapp reviewed Nov 15, 2024
View reviewed changes
Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

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

the bgp change seemed good, but I had a question about the rename in lib/

lib/prefix.h Outdated Show resolved Hide resolved
@ton31337
tests: Check if IPv6 prefix has a valid nexthop if multiple NLRIs exist
229466e 
Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337
bgpd: Validate both nexthop information (NEXTHOP and NLRI)
a0d2734 
If we receive an IPv6 prefix e.g.: 2001:db8:100::/64 with nextop: 0.0.0.0, and
mp_nexthop: fc00::2, we should not treat this with an invalid nexthop because
of 0.0.0.0. We MUST check for MP_REACH attribute also and decide later if we
have at least one a valid nexthop.

Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337 ton31337 force-pushed the fix/bgp_allow_0.0.0.0_if_multiple_nlris_exist branch from 0f5a020 to a0d2734 Compare November 15, 2024 14:42
mjstapp
mjstapp approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

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

Thanks, looks good

@ton31337
Copy link
Member Author

@Mergifyio backport stable/10.2 stable/10.1 stable/10.0 stable/9.1 stable/9.0

@mergify Mergify
Copy link

mergify bot commented Nov 16, 2024 •
edited
Loading

backport stable/10.2 stable/10.1 stable/10.0 stable/9.1 stable/9.0

✅ Backports have been created

@Jafaral Jafaral merged commit 7c46eb6 into FRRouting:master Nov 17, 2024
11 checks passed
This was referenced Nov 17, 2024
Open
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjstapp mjstapp mjstapp approved these changes

@Jafaral Jafaral Jafaral approved these changes

Assignees
No one assigned
Labels
backport bgp master size/L tests Topotests, make check, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BGP] Nexthop 0.0.0.0 is treated as invalid nexthop in FRR 10.0.1, whereas in FRR8.5.4, it was a valid nexthop
3 participants
@ton31337 @Jafaral @mjstapp