Skip to content

Conversation

@louis-6wind
Copy link
Contributor

If 'bgp network import-check' is defined on the source BGP session,
prefixes that are defined with the network command cannot be leaked to
the other VRFs BGP table even if they are present in the origin VRF RIB.

Always validate the nexthop of BGP static routes (i.e. defined with the
network statement) if 'network import-check' is defined on the source
BGP session and the prefix is present in source RIB.

If 'bgp network import-check' is defined on the source BGP session,
prefixes that are defined with the network command cannot be leaked to
the other VRFs BGP table even if they are present in the origin VRF RIB.

Always validate the nexthop of BGP static routes (i.e. defined with the
network statement) if 'network import-check' is defined on the source
BGP session and the prefix is present in source RIB.

Signed-off-by: Louis Scalbert <[email protected]>
"if not XX else" statements are confusing.

Replace two "if not XX else" statements by "if XX else" to prepare next
commits. The patch is only cosmetic.

Signed-off-by: Louis Scalbert <[email protected]>
If 'bgp network import-check' is defined on the source BGP session,
prefixes that are defined with the network command cannot be leaked to
the other VRFs BGP table even if they are present in the origin VRF RIB
if the 'rt import' statement is defined after the 'network <prefix>'
ones.

When a prefix nexthop is updated, update the prefix route leaking. The
current state of nexthop validation is now stored in the attributes of
the bgp path info. Attributes are compared with the previous ones at
route leaking update so that a nexthop validation change now triggers
the update of destination VRF BGP table.

Signed-off-by: Louis Scalbert <[email protected]>
The following configuration creates an infinite routing leaking loop
because 'rt vpn both' parameters are the same in both VRFs.

> router bgp 5227 vrf r1-cust4
>    no bgp network import-check
>    bgp router-id 192.168.1.1
>    address-family ipv4 unicast
>      network 28.0.0.0/24
>      rd vpn export 10:12
>      rt vpn both 52:100
>      import vpn
>      export vpn
>    exit-address-family
> !
> router bgp 5227 vrf r1-cust5
>    no bgp network import-check
>    bgp router id 192.168.1.1
>    address-family ipv4 unicast
>      network 29.0.0.0/24
>      rd vpn export 10:13
>      rt vpn both 52:100
>      import vpn
>      export vpn
>    exit-address-family

The previous commit has added a routing leak update when a nexthop
update is received from zebra. It indirectly calls
bgp_find_or_add_nexthop() in which a static route triggers a nexthop
cache entry registration that triggers a nexthop update from zebra.

Do not register again the nexthop cache entry if the BGP_STATIC_ROUTE is
already set.

Signed-off-by: Louis Scalbert <[email protected]>
Test vpnv4 route leaking with import-check

Signed-off-by: Louis Scalbert <[email protected]>
@ton31337 ton31337 merged commit cd869eb into FRRouting:master Jan 30, 2024
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.

2 participants