]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: fix check validity of a VPN SRv6 route with modified nexthop
authorPhilippe Guibert <philippe.guibert@6wind.com>
Thu, 20 Mar 2025 22:15:30 +0000 (23:15 +0100)
committerPhilippe Guibert <philippe.guibert@6wind.com>
Mon, 24 Mar 2025 08:17:01 +0000 (09:17 +0100)
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 <philippe.guibert@6wind.com>
bgpd/bgp_evpn.c
bgpd/bgp_fsm.c
bgpd/bgp_mplsvpn.c
bgpd/bgp_nht.c
bgpd/bgp_nht.h
bgpd/bgp_route.c

index 488f635b81e5a957ef1dd844bb3d4f1993fa5c3e..39625397979dc7f5e7ab84ac809bb008953b045b 100644 (file)
@@ -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)) {
index 607e34e8d3f9b2860a3eee821d2c8e9087d8b26a..18a15ae3a524e0109abb0592cd9a6352f79e73ae 100644 (file)
@@ -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)
index ff64f8361d5279e3958e4f1577fd1d07bf3bbfdf..6f29286030c55142ca6676d0ba6ca32eddba14f1 100644 (file)
@@ -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 <prefix>" 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)) {
index 76ac6a5e9683b1023ff456c764cb0904bcc2fd26..16938ed44d1745bfdc7492689553b046d5327982 100644 (file)
@@ -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;
index 345089ac5acf7aa0378fd6b27d0d5af372c583ad..76536d6ebf33bfe84c0ca2aaa285cecde59812d5 100644 (file)
@@ -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.
index 569bec66e0bc19ec835b43375bd471ea7503b28a..a5f171fcbf346802b852121706f3c3c5a74264eb 100644 (file)
@@ -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 {