Skip to content

Conversation

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

Copy link
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
Member Author

ci:rerun

@ton31337
Copy link
Member

Do we need to fix the styling (frrbot)?

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