-
Notifications
You must be signed in to change notification settings - Fork 1.4k
bgpd: Allow BGP NHT resolved nodes to go early #19008
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
I did not add any topotests for this because there are a non-trivial number of existing topotests that automatically take advantage of this change. |
ton31337
reviewed
Jun 11, 2025
548265b to
27c4691
Compare
ton31337
approved these changes
Jun 12, 2025
Member
ton31337
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When BGP is using itself as a underlay, let's attempt to notice this and in this situation process those useful routes as early as possible. Imagine that BGP is resolving routes through itself. In this situation BGP is going to also have NHT for those routes. When this happens, have bgp notice that there are dependent routes that resolve through itself, and when we receive those routes move them to the front of the processing queue to allow the underlay to work it's way through the system as early as is possible. Signed-off-by: Donald Sharp <[email protected]>
27c4691 to
1ff56cc
Compare
Member
Author
|
ci:rerun |
Member
|
Do we need to fix the styling (frrbot)? |
Member
Author
|
no the styling is ok imo |
mike-dubrovsky
added a commit
to mike-dubrovsky/frr-bgp-show-rib
that referenced
this pull request
Dec 7, 2025
When bgp_afi_node_get() is called, it locks the node via
route_lock_node() before returning it to the caller. The caller is
responsible for unlocking the node when done.
Call chain:
bgp_afi_node_get()
bgp_node_get()
route_node_get()
route_lock_node()
In bgp_bnc_mark_nht_important() and bgp_nexthop_update(), the route
nodes obtained from bgp_afi_node_get() were not being unlocked after
use, causing a reference count leak.
These issues were introduced by:
- Recent commit FRRouting#19008
(bgp_bnc_mark_nht_important function)
- VRF leaking commit FRRouting#15238
(bgp_nexthop_update function)
mike-dubrovsky
added a commit
to mike-dubrovsky/frr-bgp-show-rib
that referenced
this pull request
Dec 8, 2025
When bgp_afi_node_get() is called, it locks the node via
route_lock_node() before returning it to the caller. The caller is
responsible for unlocking the node when done.
Call chain:
bgp_afi_node_get()
bgp_node_get()
route_node_get()
route_lock_node()
In bgp_bnc_mark_nht_important() and bgp_nexthop_update(), the route
nodes obtained from bgp_afi_node_get() were not being unlocked after
use, causing a reference count leak.
These issues were introduced by:
- Recent commit FRRouting#19008
(bgp_bnc_mark_nht_important function)
- VRF leaking commit FRRouting#15238
(bgp_nexthop_update function)
Signed-off-by: Mike Dubrovsky <[email protected]>
mergify bot
pushed a commit
that referenced
this pull request
Dec 17, 2025
When bgp_afi_node_get() is called, it locks the node via
route_lock_node() before returning it to the caller. The caller is
responsible for unlocking the node when done.
Call chain:
bgp_afi_node_get()
bgp_node_get()
route_node_get()
route_lock_node()
In bgp_bnc_mark_nht_important() and bgp_nexthop_update(), the route
nodes obtained from bgp_afi_node_get() were not being unlocked after
use, causing a reference count leak.
These issues were introduced by:
- Recent commit #19008
(bgp_bnc_mark_nht_important function)
- VRF leaking commit #15238
(bgp_nexthop_update function)
Signed-off-by: Mike Dubrovsky <[email protected]>
(cherry picked from commit f36be98)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When BGP is using itself as a underlay, let's attempt to notice this and in this situation process those useful routes as early as possible.
Imagine that BGP is resolving routes through itself. In this situation BGP is going to also have NHT for those routes. When this happens, have bgp notice
that there are dependent routes that resolve through itself, and when we receive those routes move them to the front of the processing queue to allow the
underlay to work it's way through the system as early as is possible.