From 99acebcdc9c7c850953106f488004196db1b4da5 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 20 Mar 2025 23:15:30 +0100 Subject: [PATCH] bgpd: fix check validity of a VPN SRv6 route with modified nexthop When exporting a VPN SRv6 route, the path may not be considered valid if the nexthop is not valid. This is the case when the 'nexthop vpn export' command is used. The below example illustrates that the VPN path to 2001:1::/64 is not selected, as the expected nexthop to find in vrf10 is the one configured: > # show running-config > router bgp 1 vrf vrf10 > address-family ipv6 unicast > nexthop vpn export 2001::1 > # show bgp ipv6 vpn > [..] > Route Distinguisher: 1:10 > 2001:1::/64 2001::1@4 0 0 65001 i > UN=2001::1 EC{99:99} label=16 sid=2001:db8:1:1:: sid_structure=[40,24,16,0] type=bgp, subtype=5 The analysis indicates that the 2001::1 nexthop is considered. > 2025/03/20 21:47:53.751853 BGP: [RD1WY-YE9EC] leak_update: entry: leak-to=VRF default, p=2001:1::/64, type=10, sub_type=0 > 2025/03/20 21:47:53.751855 BGP: [VWNP2-DNMFV] Found existing bnc 2001::1/128(0)(VRF vrf10) flags 0x82 ifindex 0 #paths 2 peer 0x0, resolved prefix UNK prefix > 2025/03/20 21:47:53.751856 BGP: [VWC2R-4REXZ] leak_update_nexthop_valid: 2001:1::/64 nexthop is not valid (in VRF vrf10) > 2025/03/20 21:47:53.751857 BGP: [HX87B-ZXWX9] leak_update: ->VRF default: 2001:1::/64: Found route, no change Actually, to check the nexthop validity, only the source path in the VRF has the correct nexthop. Fix this by reusing the source path information instead of the current one. > 2025/03/20 22:43:51.703521 BGP: [RD1WY-YE9EC] leak_update: entry: leak-to=VRF default, p=2001:1::/64, type=10, sub_type=0 > 2025/03/20 22:43:51.703523 BGP: [VWNP2-DNMFV] Found existing bnc fe80::b812:37ff:fe13:d441/128(0)(VRF vrf10) flags 0x87 ifindex 0 #paths 2 peer 0x0, resolved prefix fe80::/64 > 2025/03/20 22:43:51.703525 BGP: [VWC2R-4REXZ] leak_update_nexthop_valid: 2001:1::/64 nexthop is valid (in VRF vrf10) > 2025/03/20 22:43:51.703526 BGP: [HX87B-ZXWX9] leak_update: ->VRF default: 2001:1::/64: Found route, no change Signed-off-by: Philippe Guibert --- bgpd/bgp_evpn.c | 3 +-- bgpd/bgp_fsm.c | 6 ++---- bgpd/bgp_mplsvpn.c | 18 ++++++++++-------- bgpd/bgp_nht.c | 42 +++++++++++++++++++++++++----------------- bgpd/bgp_nht.h | 9 ++++----- bgpd/bgp_route.c | 12 ++++++++---- 6 files changed, 50 insertions(+), 40 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 488f635b81..3962539797 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -3184,8 +3184,7 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf, /* Gateway IP nexthop should be resolved */ if (bre && bre->type == OVERLAY_INDEX_GATEWAY_IP) { - if (bgp_find_or_add_nexthop(bgp_vrf, bgp_vrf, afi, safi, pi, - NULL, 0, NULL)) + if (bgp_find_or_add_nexthop(bgp_vrf, bgp_vrf, afi, safi, pi, NULL, 0, NULL, NULL)) bgp_path_info_set_flag(dest, pi, BGP_PATH_VALID); else { if (BGP_DEBUG(nht, NHT)) { diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 607e34e8d3..18a15ae3a5 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -94,10 +94,8 @@ int bgp_peer_reg_with_nht(struct peer *peer) connected = 1; return bgp_find_or_add_nexthop(peer->bgp, peer->bgp, - family2afi( - peer->connection->su.sa.sa_family), - SAFI_UNICAST, NULL, peer, connected, - NULL); + family2afi(peer->connection->su.sa.sa_family), SAFI_UNICAST, + NULL, peer, connected, NULL, NULL); } static void peer_xfer_stats(struct peer *peer_dst, struct peer *peer_src) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index ff64f8361d..6f29286030 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1088,10 +1088,8 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, /* the route is defined with the "network " command */ if (CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK)) - nh_valid = bgp_find_or_add_nexthop(to_bgp, bgp_nexthop, - afi, SAFI_UNICAST, - bpi_ultimate, NULL, - 0, p); + nh_valid = bgp_find_or_add_nexthop(to_bgp, bgp_nexthop, afi, SAFI_UNICAST, + bpi_ultimate, NULL, 0, p, bpi_ultimate); else /* if "no bgp network import-check" is set, * then mark the nexthop as valid. @@ -1105,8 +1103,12 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, * TBD do we need to do anything about the * 'connected' parameter? */ - nh_valid = bgp_find_or_add_nexthop(to_bgp, bgp_nexthop, afi, - safi, bpi, NULL, 0, p); + /* VPN paths: the new bpi may be altered like + * with 'nexthop vpn export' command. Use the bpi_ultimate + * to find the original nexthop + */ + nh_valid = bgp_find_or_add_nexthop(to_bgp, bgp_nexthop, afi, safi, bpi, NULL, 0, p, + bpi_ultimate); /* * If you are using SRv6 VPN instead of MPLS, it need to check @@ -1594,8 +1596,8 @@ vpn_leak_from_vrf_get_per_nexthop_label(afi_t afi, struct bgp_path_info *pi, bgp_nexthop = from_bgp; nh_afi = BGP_ATTR_NH_AFI(afi, pi->attr); - nh_valid = bgp_find_or_add_nexthop(from_bgp, bgp_nexthop, nh_afi, - SAFI_UNICAST, pi, NULL, 0, NULL); + nh_valid = bgp_find_or_add_nexthop(from_bgp, bgp_nexthop, nh_afi, SAFI_UNICAST, pi, NULL, 0, + NULL, NULL); if (!nh_valid && is_bgp_static_route && !CHECK_FLAG(from_bgp->flags, BGP_FLAG_IMPORT_CHECK)) { diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 76ac6a5e96..16938ed44d 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -39,7 +39,7 @@ extern struct zclient *zclient; static void register_zebra_rnh(struct bgp_nexthop_cache *bnc); static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc); static bool make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p, - struct bgp *bgp_nexthop); + struct bgp *bgp_nexthop, struct bgp_path_info *pi_source); static void bgp_nht_ifp_initial(struct event *thread); DEFINE_HOOK(bgp_nht_path_update, (struct bgp *bgp, struct bgp_path_info *pi, bool valid), @@ -298,10 +298,9 @@ void bgp_unlink_nexthop_by_peer(struct peer *peer) * A route and its nexthop might belong to different VRFs. Therefore, * we need both the bgp_route and bgp_nexthop pointers. */ -int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, - afi_t afi, safi_t safi, struct bgp_path_info *pi, - struct peer *peer, int connected, - const struct prefix *orig_prefix) +int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, afi_t afi, safi_t safi, + struct bgp_path_info *pi, struct peer *peer, int connected, + const struct prefix *orig_prefix, struct bgp_path_info *source_pi) { struct bgp_nexthop_cache_head *tree = NULL; struct bgp_nexthop_cache *bnc; @@ -331,7 +330,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, /* This will return true if the global IPv6 NH is a link local * addr */ - if (!make_prefix(afi, pi, &p, bgp_nexthop)) + if (!make_prefix(afi, pi, &p, bgp_nexthop, source_pi)) return 1; /* @@ -989,7 +988,8 @@ void bgp_cleanup_nexthops(struct bgp *bgp) * make_prefix - make a prefix structure from the path (essentially * path's node. */ -static bool make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p, struct bgp *bgp_nexthop) +static bool make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p, + struct bgp *bgp_nexthop, struct bgp_path_info *source_pi) { int is_bgp_static = ((pi->type == ZEBRA_ROUTE_BGP) @@ -999,12 +999,20 @@ static bool make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p, str struct bgp_dest *net = pi->net; const struct prefix *p_orig = bgp_dest_get_prefix(net); struct in_addr ipv4; - struct peer *peer = pi->peer; - struct attr *attr = pi->attr; + struct peer *peer; + struct attr *attr; bool local_sid = false; struct bgp *bgp = bgp_get_default(); struct prefix_ipv6 tmp_prefix; + if (source_pi) { + attr = source_pi->attr; + peer = source_pi->peer; + } else { + peer = pi->peer; + attr = pi->attr; + } + if (p_orig->family == AF_FLOWSPEC) { if (!peer) return false; @@ -1033,10 +1041,10 @@ static bool make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p, str break; case AFI_IP6: p->family = AF_INET6; - if (bgp && bgp->srv6_locator && bgp->srv6_enabled && attr->srv6_l3vpn) { + if (bgp && bgp->srv6_locator && bgp->srv6_enabled && pi->attr->srv6_l3vpn) { tmp_prefix.family = AF_INET6; tmp_prefix.prefixlen = IPV6_MAX_BITLEN; - tmp_prefix.prefix = attr->srv6_l3vpn->sid; + tmp_prefix.prefix = pi->attr->srv6_l3vpn->sid; if (bgp_nexthop->vpn_policy[afi].tovpn_sid_locator && bgp_nexthop->vpn_policy[afi].tovpn_sid) local_sid = prefix_match(&bgp_nexthop->vpn_policy[afi] @@ -1046,17 +1054,17 @@ static bool make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p, str local_sid = prefix_match(&bgp_nexthop->tovpn_sid_locator->prefix, &tmp_prefix); } - if (local_sid == false && attr->srv6_l3vpn) { + if (local_sid == false && pi->attr->srv6_l3vpn) { p->prefixlen = IPV6_MAX_BITLEN; - if (attr->srv6_l3vpn->transposition_len != 0 && + if (pi->attr->srv6_l3vpn->transposition_len != 0 && BGP_PATH_INFO_NUM_LABELS(pi)) { - IPV6_ADDR_COPY(&p->u.prefix6, &attr->srv6_l3vpn->sid); + IPV6_ADDR_COPY(&p->u.prefix6, &pi->attr->srv6_l3vpn->sid); transpose_sid(&p->u.prefix6, decode_label(&pi->extra->labels->label[0]), - attr->srv6_l3vpn->transposition_offset, - attr->srv6_l3vpn->transposition_len); + pi->attr->srv6_l3vpn->transposition_offset, + pi->attr->srv6_l3vpn->transposition_len); } else - IPV6_ADDR_COPY(&(p->u.prefix6), &(attr->srv6_l3vpn->sid)); + IPV6_ADDR_COPY(&(p->u.prefix6), &(pi->attr->srv6_l3vpn->sid)); } else if (is_bgp_static) { p->u.prefix6 = p_orig->u.prefix6; p->prefixlen = p_orig->prefixlen; diff --git a/bgpd/bgp_nht.h b/bgpd/bgp_nht.h index 345089ac5a..76536d6ebf 100644 --- a/bgpd/bgp_nht.h +++ b/bgpd/bgp_nht.h @@ -25,11 +25,10 @@ extern void bgp_nexthop_update(struct vrf *vrf, struct prefix *match, * peer - The BGP peer associated with this NHT * connected - True if NH MUST be a connected route */ -extern int bgp_find_or_add_nexthop(struct bgp *bgp_route, - struct bgp *bgp_nexthop, afi_t a, - safi_t safi, struct bgp_path_info *p, - struct peer *peer, int connected, - const struct prefix *orig_prefix); +extern int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, afi_t a, + safi_t safi, struct bgp_path_info *p, struct peer *peer, + int connected, const struct prefix *orig_prefix, + struct bgp_path_info *source_pi); /** * bgp_unlink_nexthop() - Unlink the nexthop object from the path structure. diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 569bec66e0..a5f171fcbf 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -4905,6 +4905,7 @@ bgp_update_nexthop_reachability_check(struct bgp *bgp, struct peer *peer, struct { bool connected; afi_t nh_afi; + struct bgp_path_info *bpi_ultimate = NULL; if (((afi == AFI_IP || afi == AFI_IP6) && (safi == SAFI_UNICAST || safi == SAFI_LABELED_UNICAST || @@ -4920,13 +4921,16 @@ bgp_update_nexthop_reachability_check(struct bgp *bgp, struct peer *peer, struct struct bgp *bgp_nexthop = bgp; - if (pi->extra && pi->extra->vrfleak && pi->extra->vrfleak->bgp_orig) + if (pi->extra && pi->extra->vrfleak && pi->extra->vrfleak->bgp_orig) { bgp_nexthop = pi->extra->vrfleak->bgp_orig; + if (pi->sub_type == BGP_ROUTE_IMPORTED) + bpi_ultimate = bgp_get_imported_bpi_ultimate(pi); + } nh_afi = BGP_ATTR_NH_AFI(afi, pi->attr); if (bgp_find_or_add_nexthop(bgp, bgp_nexthop, nh_afi, safi, pi, NULL, connected, - bgp_nht_param_prefix) || + bgp_nht_param_prefix, bpi_ultimate) || CHECK_FLAG(peer->flags, PEER_FLAG_IS_RFAPI_HD)) { if (accept_own) bgp_path_info_set_flag(dest, pi, BGP_PATH_ACCEPT_OWN); @@ -6941,8 +6945,8 @@ static void bgp_nexthop_reachability_check(afi_t afi, safi_t safi, /* Nexthop reachability check. */ if (safi == SAFI_UNICAST || safi == SAFI_LABELED_UNICAST) { if (CHECK_FLAG(bgp->flags, BGP_FLAG_IMPORT_CHECK)) { - if (bgp_find_or_add_nexthop(bgp, bgp_nexthop, afi, safi, - bpi, NULL, 0, p)) + if (bgp_find_or_add_nexthop(bgp, bgp_nexthop, afi, safi, bpi, NULL, 0, p, + NULL)) bgp_path_info_set_flag(dest, bpi, BGP_PATH_VALID); else { -- 2.39.5