Skip to content

bgpd: Allow BGP NHT resolved nodes to go early#19008

Merged
Jafaral merged 1 commit intoFRRouting:masterfrom
donaldsharp:bgp_nht_faster_bgp
Jun 16, 2025
Merged

bgpd: Allow BGP NHT resolved nodes to go early#19008
Jafaral merged 1 commit intoFRRouting:masterfrom
donaldsharp:bgp_nht_faster_bgp

Conversation

@donaldsharp
Copy link
Copy Markdown
Member

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.

@donaldsharp
Copy link
Copy Markdown
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.

Comment thread bgpd/bgp_nht.c
Copy link
Copy Markdown
Member

@ton31337 ton31337 left a 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]>
@donaldsharp
Copy link
Copy Markdown
Member Author

ci:rerun

@ton31337
Copy link
Copy Markdown
Member

Do we need to fix the styling (frrbot)?

@donaldsharp
Copy link
Copy Markdown
Member Author

no the styling is ok imo

@Jafaral Jafaral merged commit c612b15 into FRRouting:master Jun 16, 2025
16 of 17 checks passed
@donaldsharp donaldsharp deleted the bgp_nht_faster_bgp branch July 30, 2025 17:06
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)
jaredmauch pushed a commit to jaredmauch/frr that referenced this pull request Apr 14, 2026
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]>
jaredmauch pushed a commit to jaredmauch/frr that referenced this pull request Apr 14, 2026
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]>
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