From 1c4c40696f5526ef7e8684a268f414c962346007 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 28 Apr 2022 17:01:35 +0200 Subject: [PATCH 01/23] bgpd: fix prefix VRF leaking with 'no network import-check' Prefixes that are stated in the network command cannot be leaked to the other VRFs BGP table whether or not they are present in the origin VRF RIB. Always validate the nexthop of BGP static routes (i.e. defined with the network statement) if 'no network import-check' is defined on the source BGP session. Signed-off-by: Louis Scalbert Signed-off-by: Philippe Guibert --- bgpd/bgp_mplsvpn.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 0270695c2f..7a1bbe2efd 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1060,9 +1060,11 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, { struct bgp_path_info *bpi_ultimate; struct bgp *bgp_nexthop; + struct bgp_table *table; bool nh_valid; bpi_ultimate = bgp_get_imported_bpi_ultimate(source_bpi); + table = bgp_dest_table(bpi_ultimate->net); if (bpi->extra && bpi->extra->bgp_orig) bgp_nexthop = bpi->extra->bgp_orig; @@ -1070,13 +1072,21 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, bgp_nexthop = bgp_orig; /* - * No nexthop tracking for redistributed routes or for + * No nexthop tracking for redistributed routes, + * for static (i.e. coming from the bgp network statement or for * EVPN-imported routes that get leaked. */ if (bpi_ultimate->sub_type == BGP_ROUTE_REDISTRIBUTE || is_pi_family_evpn(bpi_ultimate)) nh_valid = 1; - else + else if (bpi_ultimate->type == ZEBRA_ROUTE_BGP && + bpi_ultimate->sub_type == BGP_ROUTE_STATIC && table && + (table->safi == SAFI_UNICAST || + table->safi == SAFI_LABELED_UNICAST)) { + /* Routes from network statement */ + if (!CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK)) + nh_valid = 1; + } else /* * TBD do we need to do anything about the * 'connected' parameter? From 0f001a82a862df4dfd105264f75b901927b7597a Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 28 Apr 2022 18:32:20 +0200 Subject: [PATCH 02/23] bgpd: fix prefix VRF leaking with 'network import-check' (1/5) If 'network import-check' is defined on the source BGP session, prefixes that are stated in 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. It fixes the issue when the 'rt import' statement is defined after the 'network' ones. Signed-off-by: Louis Scalbert --- bgpd/bgp_mplsvpn.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 7a1bbe2efd..eb0d9d2826 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1084,7 +1084,11 @@ static bool leak_update_nexthop_valid(struct bgp *to_bgp, struct bgp_dest *bn, (table->safi == SAFI_UNICAST || table->safi == SAFI_LABELED_UNICAST)) { /* Routes from network statement */ - if (!CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK)) + if (CHECK_FLAG(bgp_nexthop->flags, BGP_FLAG_IMPORT_CHECK)) + nh_valid = bgp_find_or_add_nexthop( + to_bgp, bgp_nexthop, afi, safi, bpi_ultimate, + NULL, 0, p); + else nh_valid = 1; } else /* From 1565c70984d286aceb2ed30de27a2d64359ca3c5 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 29 Apr 2022 14:25:21 +0200 Subject: [PATCH 03/23] bgpd: fix prefix VRF leaking with 'network import-check' (2/5) Move setting of some variables backwards to prepare next commits. Signed-off-by: Louis Scalbert --- bgpd/bgp_mplsvpn.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index eb0d9d2826..84fbfbe259 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1883,6 +1883,24 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ int debug = BGP_DEBUG(vpn, VPN_LEAK_TO_VRF); + /* + * For VRF-2-VRF route-leaking, + * the source will be the originating VRF. + * + * If ACCEPT_OWN mechanism is enabled, then we SHOULD(?) + * get the source VRF (BGP) by looking at the RD. + */ + struct bgp *src_bgp = bgp_lookup_by_rd(path_vpn, prd, afi); + + if (path_vpn->extra && path_vpn->extra->bgp_orig) + src_vrf = path_vpn->extra->bgp_orig; + else if (src_bgp) + src_vrf = src_bgp; + else + src_vrf = from_bgp; + + bn = bgp_afi_node_get(to_bgp->rib[afi][safi], afi, safi, p, NULL); + if (!vpn_leak_from_vpn_active(to_bgp, afi, &debugmsg)) { if (debug) zlog_debug("%s: skipping: %s", __func__, debugmsg); @@ -2065,22 +2083,6 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ zlog_debug("%s: pfx %pBD: num_labels %d", __func__, path_vpn->net, num_labels); - /* - * For VRF-2-VRF route-leaking, - * the source will be the originating VRF. - * - * If ACCEPT_OWN mechanism is enabled, then we SHOULD(?) - * get the source VRF (BGP) by looking at the RD. - */ - struct bgp *src_bgp = bgp_lookup_by_rd(path_vpn, prd, afi); - - if (path_vpn->extra && path_vpn->extra->bgp_orig) - src_vrf = path_vpn->extra->bgp_orig; - else if (src_bgp) - src_vrf = src_bgp; - else - src_vrf = from_bgp; - leak_update(to_bgp, bn, new_attr, afi, safi, path_vpn, pLabels, num_labels, src_vrf, &nexthop_orig, nexthop_self_flag, debug); From d0a55f87e9cd4d67e9514f31cfe589eeaec0ed4a Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 5 Jul 2022 15:22:12 +0200 Subject: [PATCH 04/23] bgpd: fix prefix VRF leaking with 'network import-check' (3/5) "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 --- bgpd/bgp_nht.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index cf8ff524e9..78482aeb40 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -858,24 +858,22 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id) tree = &bgp->nexthop_cache_table[afi]; bnc_nhc = bnc_find(tree, &match, nhr.srte_color, 0); - if (!bnc_nhc) { - if (BGP_DEBUG(nht, NHT)) - zlog_debug( - "parse nexthop update(%pFX(%u)(%s)): bnc info not found for nexthop cache", - &nhr.prefix, nhr.srte_color, bgp->name_pretty); - } else + if (bnc_nhc) bgp_process_nexthop_update(bnc_nhc, &nhr, false); + else if (BGP_DEBUG(nht, NHT)) + zlog_debug( + "parse nexthop update(%pFX(%u)(%s)): bnc info not found for nexthop cache", + &nhr.prefix, nhr.srte_color, bgp->name_pretty); tree = &bgp->import_check_table[afi]; bnc_import = bnc_find(tree, &match, nhr.srte_color, 0); - if (!bnc_import) { - if (BGP_DEBUG(nht, NHT)) - zlog_debug( - "parse nexthop update(%pFX(%u)(%s)): bnc info not found for import check", - &nhr.prefix, nhr.srte_color, bgp->name_pretty); - } else + if (bnc_import) bgp_process_nexthop_update(bnc_import, &nhr, true); + else if (BGP_DEBUG(nht, NHT)) + zlog_debug( + "parse nexthop update(%pFX(%u)(%s)): bnc info not found for import check", + &nhr.prefix, nhr.srte_color, bgp->name_pretty); /* * HACK: if any BGP route is dependant on an SR-policy that doesn't From 1e24860bf7042a96bc0f22df60f73e7aa04f31f6 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 29 Apr 2022 14:26:04 +0200 Subject: [PATCH 05/23] bgpd: fix prefix VRF leaking with 'network import-check' (4/5) If 'network import-check' is defined on the source BGP session, prefixes that are stated in 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 ' 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 --- bgpd/bgp_attr.c | 1 + bgpd/bgp_attr.h | 3 +++ bgpd/bgp_mplsvpn.c | 13 +++++++++++++ bgpd/bgp_nht.c | 22 ++++++++++++++++++++-- 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 337a82b945..47775e1412 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -866,6 +866,7 @@ bool attrhash_cmp(const void *p1, const void *p2) && attr1->df_pref == attr2->df_pref && attr1->df_alg == attr2->df_alg && attr1->nh_ifindex == attr2->nh_ifindex + && attr1->nh_flag == attr2->nh_flag && attr1->nh_lla_ifindex == attr2->nh_lla_ifindex && attr1->distance == attr2->distance && srv6_l3vpn_same(attr1->srv6_l3vpn, attr2->srv6_l3vpn) diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index a34da1a6de..575b10541e 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -170,6 +170,9 @@ struct attr { uint32_t med; uint32_t local_pref; ifindex_t nh_ifindex; + uint8_t nh_flag; + +#define BGP_ATTR_NH_VALID 0x01 /* Path origin attribute */ uint8_t origin; diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 84fbfbe259..42ae3562c2 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1878,6 +1878,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ uint32_t num_labels = 0; int nexthop_self_flag = 1; struct bgp_path_info *bpi_ultimate = NULL; + struct bgp_path_info *bpi; int origin_local = 0; struct bgp *src_vrf; @@ -1960,6 +1961,18 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ community_strip_accept_own(&static_attr); + for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) { + if (bpi->extra && bpi->extra->parent == path_vpn) + break; + } + + if (bpi && + leak_update_nexthop_valid(to_bgp, bn, &static_attr, afi, safi, + path_vpn, bpi, src_vrf, p, debug)) + SET_FLAG(static_attr.nh_flag, BGP_ATTR_NH_VALID); + else + UNSET_FLAG(static_attr.nh_flag, BGP_ATTR_NH_VALID); + /* * Nexthop: stash and clear * diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 78482aeb40..d7707a8c0c 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -46,6 +46,7 @@ #include "bgpd/bgp_flowspec_util.h" #include "bgpd/bgp_evpn.h" #include "bgpd/bgp_rd.h" +#include "bgpd/bgp_mplsvpn.h" extern struct zclient *zclient; @@ -834,10 +835,13 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id) { struct bgp_nexthop_cache_head *tree = NULL; struct bgp_nexthop_cache *bnc_nhc, *bnc_import; + struct bgp_path_info *pi; + struct bgp_dest *dest; struct bgp *bgp; struct prefix match; struct zapi_route nhr; afi_t afi; + safi_t safi; bgp = bgp_lookup_by_vrf_id(vrf_id); if (!bgp) { @@ -868,9 +872,23 @@ void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id) tree = &bgp->import_check_table[afi]; bnc_import = bnc_find(tree, &match, nhr.srte_color, 0); - if (bnc_import) + if (bnc_import) { bgp_process_nexthop_update(bnc_import, &nhr, true); - else if (BGP_DEBUG(nht, NHT)) + + safi = nhr.safi; + if (bgp->rib[afi][safi]) { + dest = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi, + &match, NULL); + + for (pi = bgp_dest_get_bgp_path_info(dest); pi; + pi = pi->next) + if (pi->peer == bgp->peer_self && + pi->type == ZEBRA_ROUTE_BGP && + pi->sub_type == BGP_ROUTE_STATIC) + vpn_leak_from_vrf_update( + bgp_get_default(), bgp, pi); + } + } else if (BGP_DEBUG(nht, NHT)) zlog_debug( "parse nexthop update(%pFX(%u)(%s)): bnc info not found for import check", &nhr.prefix, nhr.srte_color, bgp->name_pretty); From acf31ef73b4a73dad5723105cdde0d589f2a1d4a Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 5 May 2022 18:06:24 +0200 Subject: [PATCH 06/23] bgpd: fix prefix VRF leaking with 'network import-check' (5/5) 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 --- bgpd/bgp_nht.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index d7707a8c0c..a192b037ac 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -389,7 +389,7 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, if (pi && is_route_parent_evpn(pi)) bnc->is_evpn_gwip_nexthop = true; - if (is_bgp_static_route) { + if (is_bgp_static_route && !CHECK_FLAG(bnc->flags, BGP_STATIC_ROUTE)) { SET_FLAG(bnc->flags, BGP_STATIC_ROUTE); /* If we're toggling the type, re-register */ @@ -424,8 +424,8 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop, SET_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); - } else if (peer && !connected - && CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)) { + } else if (peer && !connected && + CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)) { UNSET_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_REGISTERED); UNSET_FLAG(bnc->flags, BGP_NEXTHOP_VALID); From e7192e9d24638fec3c482982df2ec0763c453ce0 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 22 Apr 2022 18:08:08 +0200 Subject: [PATCH 07/23] lib: add a function to get the VRF or loopback interface Add a function to find the VRF or the loopback interface: the loopback interface for the default VRF and the VRF master interface otherwise. Signed-off-by: Louis Scalbert --- lib/if.c | 14 ++++++++++++++ lib/if.h | 1 + 2 files changed, 15 insertions(+) diff --git a/lib/if.c b/lib/if.c index 70c0c18141..db73210036 100644 --- a/lib/if.c +++ b/lib/if.c @@ -564,6 +564,20 @@ size_t if_lookup_by_hwaddr(const uint8_t *hw_addr, size_t addrsz, return count; } +/* Get the VRF loopback interface, i.e. the loopback on the default VRF + * or the VRF interface. + */ +struct interface *if_get_vrf_loopback(vrf_id_t vrf_id) +{ + struct interface *ifp = NULL; + struct vrf *vrf = vrf_lookup_by_id(vrf_id); + + FOR_ALL_INTERFACES (vrf, ifp) + if (if_is_loopback(ifp)) + return ifp; + + return NULL; +} /* Get interface by name if given name interface doesn't exist create one. */ diff --git a/lib/if.h b/lib/if.h index 91dcd46247..a653246ccb 100644 --- a/lib/if.h +++ b/lib/if.h @@ -532,6 +532,7 @@ static inline bool if_address_is_local(const void *matchaddr, int family, struct vrf; extern struct interface *if_lookup_by_name_vrf(const char *name, struct vrf *vrf); extern struct interface *if_lookup_by_name(const char *ifname, vrf_id_t vrf_id); +extern struct interface *if_get_vrf_loopback(vrf_id_t vrf_id); extern struct interface *if_get_by_name(const char *ifname, vrf_id_t vrf_id, const char *vrf_name); From 09e370e5fff2001f47509b4eae34bb749c18086b Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 13 Jul 2022 14:52:16 +0200 Subject: [PATCH 08/23] lib: fix clang warning Fix a CLANG warning Signed-off-by: Louis Scalbert --- lib/if.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/if.c b/lib/if.c index db73210036..6766a04b37 100644 --- a/lib/if.c +++ b/lib/if.c @@ -580,7 +580,8 @@ struct interface *if_get_vrf_loopback(vrf_id_t vrf_id) } /* Get interface by name if given name interface doesn't exist create - one. */ + * one. + */ struct interface *if_get_by_name(const char *name, vrf_id_t vrf_id, const char *vrf_name) { From 14aabc01565a918c223b7811572fae0316810422 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Mon, 25 Apr 2022 15:14:49 +0200 Subject: [PATCH 09/23] bgpd: fix invalid nexthop interface on leaked routes There is two cases where the nexthop interface is incorrect: - Case 1: leaked routes from prefixes stated in 'network ' are inactive because they have no nexthop IP address or interface. - Case 2: leaked routes from 'redistribute connected' contains the original nexthop interface. ====== Case 1 ====== > router bgp 5227 vrf r1-cust1 > bgp router-id 192.168.1.1 > no bgp network import-check > ! > address-family ipv4 unicast > network 10.2.3.4/32 > network 192.168.1.0/24 > rd vpn export 10:1 > rt vpn import 52:100 > rt vpn export 52:101 > export vpn > import vpn > exit-address-family > exit > ! > router bgp 5227 vrf r1-cust4 > bgp router-id 192.168.1.1 > ! > address-family ipv4 unicast > network 29.0.0.0/24 > rd vpn export 10:1 > rt vpn import 52:101 > rt vpn export 52:100 > export vpn > import vpn > exit-address-family > exit Extract from the routing table: > VRF r1-cust1: > S>* 192.0.0.0/24 [1/0] via 192.168.1.2, r1-eth4, weight 1, 00:47:53 > C>* 192.168.1.0/24 is directly connected, r1-eth4, 00:44:15 > B>* 29.0.0.0/24 [20/0] is directly connected, unknown (vrf r1-cust4), inactive, weight 1, 00:00:02 > > VRF r1-cust4: > B 10.2.3.4/32 [20/0] is directly connected, unknown (vrf r1-cust1) inactive, weight 1, 00:00:02 > C>* 29.0.0.0/24 is directly connected, r1-cust5, 00:27:40 > B 192.0.0.0/24 [20/0] is directly connected, unknown (vrf r1-cust1) inactive, weight 1, 00:03:40 > B 192.168.1.0/24 [20/0] is directly connected, unknown (vrf r1-cust1) inactive, weight 1, 00:00:02 ====== Case 2 ====== The previous is modified with the following settings: > router bgp 5227 vrf r1-cust1 > address-family ipv4 unicast > no network 192.168.1.0/24 > redistribute connected > ! > vrf r1-cust1 > ip route 29.0.0.0/24 r1-cust5 nexthop-vrf r1-cust5 Extract from the routing table: > VRF r1-cust1: > S>* 192.0.0.0/24 [1/0] via 192.168.1.2, r1-eth4, weight 1, 00:47:53 > C>* 192.168.1.0/24 is directly connected, r1-eth4, 00:44:15 > S>* 29.0.0.0/24 [1/0] is directly connected, r1-cust5 (vrf r1-cust5), weight 1, 00:00:30 > > VRF r1-cust4: > B 10.2.3.4/32 [20/0] is directly connected, unknown (vrf r1-cust1) inactive, weight 1, 00:00:02 > C>* 29.0.0.0/24 is directly connected, r1-cust5, 00:27:40 > B 192.0.0.0/24 [20/0] is directly connected, unknown (vrf r1-cust1) inactive, weight 1, 00:03:40 > B>* 192.168.1.0/24 [20/0] is directly connected, r1-eth4 (vrf r1-cust1), weight 1, 00:00:02 The nexthop interface is r1-eth4. It causes issue to traffic leaving r1-cust4. The following ping to r1-eth4 local address 192.168.1.1 from r1-cust5 local add does not respond. > # tcpdump -lnni r1-cust1 'icmp' & > # ip vrf exec r1-cust4 ping -c1 192.168.1.1 -I 29.0.0.1 > PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data. PING 192.168.1.1 (192.168.1.1) from 29.0.0.1 : 56(84) bytes of data. 18:49:20.635638 IP 29.0.0.1 > 192.168.1.1: ICMP echo request, id 15897, seq 1, length 64 18:49:27.113827 IP 29.0.0.1 > 29.0.0.1: ICMP host 192.168.1.1 unreachable, length 92 Fix description: When leaking prefix from other VRFs, if the nexthop IP address is not set in the bgp path info attribures, reset nh_ifindex to the index of master interface of the incoming BGP instance. The result is for case 1 and 2: > VRF r1-cust1: > S>* 192.0.0.0/24 [1/0] via 192.168.1.2, r1-eth4, weight 1, 00:47:53 > C>* 192.168.1.0/24 is directly connected, r1-eth4, 00:44:15 > B>* 29.0.0.0/24 [20/0] is directly connected, r1-cust4 (vrf r1-cust4), weight 1, 00:00:08 > > VRF r1-cust4: > B>* 10.2.3.4/32 [20/0] is directly connected, r1-cust1 (vrf r1-cust1), weight 1, 00:00:08 > C>* 29.0.0.0/24 is directly connected, r1-cust5, 00:27:40 > B>* 192.0.0.0/24 [20/0] is directly connected, r1-cust1 (vrf r1-cust1), weight 1, 00:00:08 > B>* 192.168.1.0/24 [20/0] is directly connected, r1-cust1 (vrf r1-cust1), weight 1, 00:00:08 > # tcpdump -lnni r1-cust1 'icmp' & > # ping -c1 192.168.1.1 -I 29.0.0.1 > PING 192.168.1.1 (192.168.1.1) from 29.0.0.1 : 56(84) bytes of data. > 18:48:32.506281 IP 29.0.0.1 > 192.168.1.1: ICMP echo request, id 15870, seq 1, length 64 > 64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=0.050 ms > 18:48:32.506304 IP 192.168.1.1 > 29.0.0.1: ICMP echo reply, id 15870, seq 1, length 64 Signed-off-by: Louis Scalbert 1, 00:47:53 4:15 vrf r1-cust4), inactive, weight 1, 00:00:02 vrf r1-cust1) inactive, weight 1, 00:00:02 40 (vrf r1-cust1) inactive, weight 1, 00:03:40 n (vrf r1-cust1) inactive, weight 1, 00:00:02 dress is not the index of 1, 00:47:53 4:15 (vrf r1-cust4), weight 1, 00:00:08 (vrf r1-cust1), weight 1, 00:00:08 40 (vrf r1-cust1), weight 1, 00:00:08 t1 (vrf r1-cust1), weight 1, 00:00:08 --- bgpd/bgp_mplsvpn.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 42ae3562c2..bf6df88d2e 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1881,6 +1881,7 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ struct bgp_path_info *bpi; int origin_local = 0; struct bgp *src_vrf; + struct interface *ifp; int debug = BGP_DEBUG(vpn, VPN_LEAK_TO_VRF); @@ -2015,6 +2016,13 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ break; } + if (static_attr.nexthop.s_addr == INADDR_ANY && + IN6_IS_ADDR_UNSPECIFIED(&static_attr.mp_nexthop_global)) { + ifp = if_get_vrf_loopback(src_vrf->vrf_id); + if (ifp) + static_attr.nh_ifindex = ifp->ifindex; + } + /* * route map handling */ From 6b74c9fa660556f53e9592bdaa492d2466db3c3c Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 11 May 2022 17:41:36 +0200 Subject: [PATCH 10/23] topotests: update ospf_multi_vrf_bgp_route_leak Leaked connected routes have now the following nexthop interfaces: - lo for routes imported from the default VRF - or the VRF interface for routes imported from the other VRFs. Signed-off-by: Louis Scalbert --- .../ospf_multi_vrf_bgp_route_leak/r1/zebra-vrf-default.txt | 2 +- .../ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-default.txt | 2 +- .../ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-ray.txt | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/topotests/ospf_multi_vrf_bgp_route_leak/r1/zebra-vrf-default.txt b/tests/topotests/ospf_multi_vrf_bgp_route_leak/r1/zebra-vrf-default.txt index 86c089ab3b..6bafbbb556 100644 --- a/tests/topotests/ospf_multi_vrf_bgp_route_leak/r1/zebra-vrf-default.txt +++ b/tests/topotests/ospf_multi_vrf_bgp_route_leak/r1/zebra-vrf-default.txt @@ -5,5 +5,5 @@ B>* 10.0.3.0/24 [20/20] via 10.0.30.3, r1-eth2 (vrf neno), weight 1, XX:XX:XX O>* 10.0.4.0/24 [110/20] via 10.0.20.2, r1-eth1, weight 1, XX:XX:XX O 10.0.20.0/24 [110/10] is directly connected, r1-eth1, weight 1, XX:XX:XX C>* 10.0.20.0/24 is directly connected, r1-eth1, XX:XX:XX -B>* 10.0.30.0/24 [20/0] is directly connected, r1-eth2 (vrf neno), weight 1, XX:XX:XX +B>* 10.0.30.0/24 [20/0] is directly connected, neno (vrf neno), weight 1, XX:XX:XX O>* 10.0.40.0/24 [110/20] via 10.0.20.2, r1-eth1, weight 1, XX:XX:XX diff --git a/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-default.txt b/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-default.txt index 9681d8a04e..3ed6b1b3a1 100644 --- a/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-default.txt +++ b/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-default.txt @@ -7,4 +7,4 @@ B>* 10.0.4.0/24 [20/20] via 10.0.40.4, r2-eth2 (vrf ray), weight 1, XX:XX:XX O 10.0.20.0/24 [110/10] is directly connected, r2-eth1, weight 1, XX:XX:XX C>* 10.0.20.0/24 is directly connected, r2-eth1, XX:XX:XX O>* 10.0.30.0/24 [110/20] via 10.0.20.1, r2-eth1, weight 1, XX:XX:XX -B>* 10.0.40.0/24 [20/0] is directly connected, r2-eth2 (vrf ray), weight 1, XX:XX:XX +B>* 10.0.40.0/24 [20/0] is directly connected, ray (vrf ray), weight 1, XX:XX:XX diff --git a/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-ray.txt b/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-ray.txt index ce9903ae71..4ad8441d85 100644 --- a/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-ray.txt +++ b/tests/topotests/ospf_multi_vrf_bgp_route_leak/r2/zebra-vrf-ray.txt @@ -1,9 +1,9 @@ VRF ray: B 10.0.1.0/24 [20/20] via 10.0.20.1, r2-eth1 (vrf default) inactive, weight 1, XX:XX:XX -B 10.0.2.0/24 [20/0] is directly connected, r2-eth0 (vrf default) inactive, weight 1, XX:XX:XX +B 10.0.2.0/24 [20/0] is directly connected, lo (vrf default) inactive, weight 1, XX:XX:XX B>* 10.0.3.0/24 [20/20] via 10.0.20.1, r2-eth1 (vrf default), weight 1, XX:XX:XX O>* 10.0.4.0/24 [110/20] via 10.0.40.4, r2-eth2, weight 1, XX:XX:XX -B 10.0.20.0/24 [20/0] is directly connected, r2-eth1 (vrf default) inactive, weight 1, XX:XX:XX +B 10.0.20.0/24 [20/0] is directly connected, lo (vrf default) inactive, weight 1, XX:XX:XX B>* 10.0.30.0/24 [20/20] via 10.0.20.1, r2-eth1 (vrf default), weight 1, XX:XX:XX O 10.0.40.0/24 [110/10] is directly connected, r2-eth2, weight 1, XX:XX:XX C>* 10.0.40.0/24 is directly connected, r2-eth2, XX:XX:XX From 6030b8b40d1f09787040b418162bb8a15e25bf66 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 26 Apr 2022 16:57:45 +0200 Subject: [PATCH 11/23] bgpd: update route leaking when a VRF loopback is received At bgpd startup, VRF instances are sent from zebra before the interfaces. When importing a l3vpn prefix from another local VRF instance, the interfaces are not known yet. The prefix nexthop interface cannot be set to the loopback or the VRF interface, which causes setting invalid routes in zebra. Update route leaking when the loopback or a VRF interface is received from zebra. At a VRF interface deletion, zebra voluntarily sends a ZEBRA_INTERFACE_ADD message to move it to VRF_DEFAULT. Do not update if such a message is received. VRF destruction will destroy all the related routes without adding codes. Signed-off-by: Louis Scalbert --- bgpd/bgp_attr.c | 12 ++++++++++++ bgpd/bgp_attr.h | 1 + bgpd/bgp_mplsvpn.c | 11 ++++++++++- bgpd/bgp_route.c | 6 ++++++ bgpd/bgp_zebra.c | 30 ++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 47775e1412..9352ab4650 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2255,6 +2255,12 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, return BGP_ATTR_PARSE_WITHDRAW; } attr->nh_ifindex = peer->nexthop.ifp->ifindex; + if (if_is_operative(peer->nexthop.ifp)) + SET_FLAG(attr->nh_flag, + BGP_ATTR_NH_IF_OPERSTATE); + else + UNSET_FLAG(attr->nh_flag, + BGP_ATTR_NH_IF_OPERSTATE); } break; case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL: @@ -2272,6 +2278,12 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, return BGP_ATTR_PARSE_WITHDRAW; } attr->nh_ifindex = peer->nexthop.ifp->ifindex; + if (if_is_operative(peer->nexthop.ifp)) + SET_FLAG(attr->nh_flag, + BGP_ATTR_NH_IF_OPERSTATE); + else + UNSET_FLAG(attr->nh_flag, + BGP_ATTR_NH_IF_OPERSTATE); } if (attr->mp_nexthop_len == BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL) { diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 575b10541e..67677f999d 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -173,6 +173,7 @@ struct attr { uint8_t nh_flag; #define BGP_ATTR_NH_VALID 0x01 +#define BGP_ATTR_NH_IF_OPERSTATE 0x02 /* Path origin attribute */ uint8_t origin; diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index bf6df88d2e..41a00ab60b 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -2021,7 +2021,16 @@ static bool vpn_leak_to_vrf_update_onevrf(struct bgp *to_bgp, /* to */ ifp = if_get_vrf_loopback(src_vrf->vrf_id); if (ifp) static_attr.nh_ifindex = ifp->ifindex; - } + } else if (static_attr.nh_ifindex) + ifp = if_lookup_by_index(static_attr.nh_ifindex, + src_vrf->vrf_id); + else + ifp = NULL; + + if (ifp && if_is_operative(ifp)) + SET_FLAG(static_attr.nh_flag, BGP_ATTR_NH_IF_OPERSTATE); + else + UNSET_FLAG(static_attr.nh_flag, BGP_ATTR_NH_IF_OPERSTATE); /* * route map handling diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 95493c11f8..bfb74e9379 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -8679,6 +8679,7 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p, afi_t afi; route_map_result_t ret; struct bgp_redist *red; + struct interface *ifp; /* Make default attribute. */ bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_INCOMPLETE); @@ -8728,6 +8729,11 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p, } attr.nh_type = nhtype; attr.nh_ifindex = ifindex; + ifp = if_lookup_by_index(ifindex, bgp->vrf_id); + if (ifp && if_is_operative(ifp)) + SET_FLAG(attr.nh_flag, BGP_ATTR_NH_IF_OPERSTATE); + else + UNSET_FLAG(attr.nh_flag, BGP_ATTR_NH_IF_OPERSTATE); attr.med = metric; attr.distance = distance; diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 5ab1a6afa6..dd20763d8b 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -234,6 +234,7 @@ static int bgp_ifp_up(struct interface *ifp) struct connected *c; struct nbr_connected *nc; struct listnode *node, *nnode; + struct bgp *bgp_default = bgp_get_default(); struct bgp *bgp; bgp = ifp->vrf->info; @@ -256,6 +257,14 @@ static int bgp_ifp_up(struct interface *ifp) hook_call(bgp_vrf_status_changed, bgp, ifp); bgp_nht_ifp_up(ifp); + if (bgp_default && if_is_loopback(ifp)) { + vpn_leak_zebra_vrf_label_update(bgp, AFI_IP); + vpn_leak_zebra_vrf_label_update(bgp, AFI_IP6); + vpn_leak_zebra_vrf_sid_update(bgp, AFI_IP); + vpn_leak_zebra_vrf_sid_update(bgp, AFI_IP6); + vpn_leak_postchange_all(); + } + return 0; } @@ -264,6 +273,7 @@ static int bgp_ifp_down(struct interface *ifp) struct connected *c; struct nbr_connected *nc; struct listnode *node, *nnode; + struct bgp *bgp_default = bgp_get_default(); struct bgp *bgp; struct peer *peer; @@ -303,6 +313,14 @@ static int bgp_ifp_down(struct interface *ifp) hook_call(bgp_vrf_status_changed, bgp, ifp); bgp_nht_ifp_down(ifp); + if (bgp_default && if_is_loopback(ifp)) { + vpn_leak_zebra_vrf_label_update(bgp, AFI_IP); + vpn_leak_zebra_vrf_label_update(bgp, AFI_IP6); + vpn_leak_zebra_vrf_sid_update(bgp, AFI_IP); + vpn_leak_zebra_vrf_sid_update(bgp, AFI_IP6); + vpn_leak_postchange_all(); + } + return 0; } @@ -3188,6 +3206,7 @@ extern struct zebra_privs_t bgpd_privs; static int bgp_ifp_create(struct interface *ifp) { + struct bgp *bgp_default = bgp_get_default(); struct bgp *bgp; if (BGP_DEBUG(zebra, ZEBRA)) @@ -3202,6 +3221,17 @@ static int bgp_ifp_create(struct interface *ifp) bgp_update_interface_nbrs(bgp, ifp, ifp); hook_call(bgp_vrf_status_changed, bgp, ifp); + + if (bgp_default && + (if_is_loopback_exact(ifp) || + (if_is_vrf(ifp) && ifp->vrf->vrf_id != VRF_DEFAULT))) { + vpn_leak_zebra_vrf_label_update(bgp, AFI_IP); + vpn_leak_zebra_vrf_label_update(bgp, AFI_IP6); + vpn_leak_zebra_vrf_sid_update(bgp, AFI_IP); + vpn_leak_zebra_vrf_sid_update(bgp, AFI_IP6); + vpn_leak_postchange_all(); + } + return 0; } From 36c61d5c8b0e43558bc73fbaa85573ad58c82dca Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 11 May 2022 17:37:34 +0200 Subject: [PATCH 12/23] topotests: update bgp_vrf_route_leak_basic Update bgp_vrf_route_leak_basic to set up the VRF interfaces. Otherwise the routes to the VRF interface are inactives. Signed-off-by: Louis Scalbert --- tests/topotests/bgp_vrf_route_leak_basic/r1/zebra.conf | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/topotests/bgp_vrf_route_leak_basic/r1/zebra.conf b/tests/topotests/bgp_vrf_route_leak_basic/r1/zebra.conf index 35038557df..731a00829d 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/r1/zebra.conf +++ b/tests/topotests/bgp_vrf_route_leak_basic/r1/zebra.conf @@ -16,3 +16,9 @@ int dummy4 ip address 10.0.3.1/24 no shut ! +int EVA + no shut +! +int DONNA + no shut +! From 86a1c2963266fe5a0683278668c276c748f32552 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 26 Apr 2022 16:45:42 +0200 Subject: [PATCH 13/23] bgpd: fix route recursion on leaked routes Leaked recursive routes are not resolved. > VRF r1-cust1: > B> 5.1.0.0/24 [200/98] via 99.0.0.1 (recursive), weight 1, 00:00:08 > * via 192.168.1.2, r1-eth4, weight 1, 00:00:08 > B>* 99.0.0.1/32 [200/0] via 192.168.1.2, r1-eth4, weight 1, 00:00:08 > VRF r1-cust4: > B 5.1.0.0/24 [20/98] via 99.0.0.1 (vrf r1-cust1) inactive, weight 1, 00:00:08 > B>* 99.0.0.1/32 [20/0] via 192.168.1.2, r1-eth4 (vrf r1-cust1), weight 1, 00:00:08 When announcing the routes to zebra, use the peer of the ultimate bgp path info instead of the one of the first parent path info to determine whether the route is recursive. The result is: > VRF r1-cust4: > B> 5.1.0.0/24 [20/98] via 99.0.0.1 (vrf r1-cust1) (recursive), weight 1, 00:00:02 > * via 192.168.1.2, r1-eth4 (vrf r1-cust1), weight 1, 00:00:02 > B>* 99.0.0.1/32 [20/0] via 192.168.1.2, r1-eth4 (vrf r1-cust1), weight 1, 00:00:02 Signed-off-by: Louis Scalbert --- bgpd/bgp_zebra.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index dd20763d8b..4f1808829c 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1325,6 +1325,7 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p, uint8_t distance; struct peer *peer; struct bgp_path_info *mpinfo; + struct bgp_path_info *bpi_ultimate; struct bgp *bgp_orig; uint32_t metric; struct attr local_attr; @@ -1373,13 +1374,9 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p, peer = info->peer; - if (info->type == ZEBRA_ROUTE_BGP - && info->sub_type == BGP_ROUTE_IMPORTED) { - - /* Obtain peer from parent */ - if (info->extra && info->extra->parent) - peer = ((struct bgp_path_info *)(info->extra->parent)) - ->peer; + if (info->type == ZEBRA_ROUTE_BGP) { + bpi_ultimate = bgp_get_imported_bpi_ultimate(info); + peer = bpi_ultimate->peer; } tag = info->attr->tag; From 667a4e92da1d789b5aa3da15c3f80d3f4658a53e Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 29 Apr 2022 19:41:57 +0200 Subject: [PATCH 14/23] bgpd: move mp_nexthop_prefer_global boolean attribute to nh_flag Previous commits have introduced a new 8 bits nh_flag in the attr struct that has increased the memory footprint. Move the mp_nexthop_prefer_global boolean in the attr structure that takes 8 bits to the new nh_flag in order to go back to the previous memory utilization. Signed-off-by: Louis Scalbert --- bgpd/bgp_attr.h | 4 +--- bgpd/bgp_mpath.c | 14 ++++++++++---- bgpd/bgp_nht.c | 3 ++- bgpd/bgp_route.c | 22 +++++++++++++--------- bgpd/bgp_routemap.c | 4 ++-- bgpd/bgp_snmp_bgp4v2.c | 6 ++++-- bgpd/bgp_zebra.c | 3 ++- 7 files changed, 34 insertions(+), 22 deletions(-) diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 67677f999d..4eb742422d 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -174,6 +174,7 @@ struct attr { #define BGP_ATTR_NH_VALID 0x01 #define BGP_ATTR_NH_IF_OPERSTATE 0x02 +#define BGP_ATTR_NH_MP_PREFER_GLOBAL 0x04 /* MP Nexthop preference */ /* Path origin attribute */ uint8_t origin; @@ -224,9 +225,6 @@ struct attr { /* MP Nexthop length */ uint8_t mp_nexthop_len; - /* MP Nexthop preference */ - uint8_t mp_nexthop_prefer_global; - /* Static MAC for EVPN */ uint8_t sticky; diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c index 32a5e14b11..84c847d796 100644 --- a/bgpd/bgp_mpath.c +++ b/bgpd/bgp_mpath.c @@ -142,15 +142,21 @@ int bgp_path_info_nexthop_cmp(struct bgp_path_info *bpi1, &bpi2->attr->mp_nexthop_global); break; case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL: - addr1 = (bpi1->attr->mp_nexthop_prefer_global) + addr1 = (CHECK_FLAG( + bpi1->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) ? bpi1->attr->mp_nexthop_global : bpi1->attr->mp_nexthop_local; - addr2 = (bpi2->attr->mp_nexthop_prefer_global) + addr2 = (CHECK_FLAG( + bpi2->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) ? bpi2->attr->mp_nexthop_global : bpi2->attr->mp_nexthop_local; - if (!bpi1->attr->mp_nexthop_prefer_global - && !bpi2->attr->mp_nexthop_prefer_global) + if (!CHECK_FLAG(bpi1->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL) && + !CHECK_FLAG(bpi2->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) compare = !bgp_interface_same( bpi1->peer->ifp, bpi2->peer->ifp); diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index a192b037ac..b6b0c584d7 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -1005,7 +1005,8 @@ static int make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p) */ else if (pi->attr->mp_nexthop_len == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL) { - if (pi->attr->mp_nexthop_prefer_global) + if (CHECK_FLAG(pi->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) p->u.prefix6 = pi->attr->mp_nexthop_global; else diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index bfb74e9379..4370c30476 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -9416,9 +9416,10 @@ void route_vty_out(struct vty *vty, const struct prefix *p, "link-local"); if ((IPV6_ADDR_CMP(&attr->mp_nexthop_global, - &attr->mp_nexthop_local) - != 0) - && !attr->mp_nexthop_prefer_global) + &attr->mp_nexthop_local) != + 0) && + !CHECK_FLAG(attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) json_object_boolean_true_add( json_nexthop_ll, "used"); else @@ -9430,10 +9431,11 @@ void route_vty_out(struct vty *vty, const struct prefix *p, } else { /* Display LL if LL/Global both in table unless * prefer-global is set */ - if (((attr->mp_nexthop_len - == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL) - && !attr->mp_nexthop_prefer_global) - || (path->peer->conf_if)) { + if (((attr->mp_nexthop_len == + BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL) && + !CHECK_FLAG(attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) || + (path->peer->conf_if)) { if (path->peer->conf_if) { len = vty_out(vty, "%s", path->peer->conf_if); @@ -10695,7 +10697,8 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn, json_object_boolean_true_add(json_nexthop_ll, "accessible"); - if (!attr->mp_nexthop_prefer_global) + if (!CHECK_FLAG(attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) json_object_boolean_true_add(json_nexthop_ll, "used"); else @@ -10705,7 +10708,8 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn, vty_out(vty, " (%s) %s\n", inet_ntop(AF_INET6, &attr->mp_nexthop_local, buf, INET6_ADDRSTRLEN), - attr->mp_nexthop_prefer_global + CHECK_FLAG(attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL) ? "(prefer-global)" : "(used)"); } diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 1ce2eb4352..f779b34371 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -3531,11 +3531,11 @@ route_set_ipv6_nexthop_prefer_global(void *rule, const struct prefix *prefix, if (CHECK_FLAG(peer->rmap_type, PEER_RMAP_TYPE_IN) || CHECK_FLAG(peer->rmap_type, PEER_RMAP_TYPE_IMPORT)) { /* Set next hop preference to global */ - path->attr->mp_nexthop_prefer_global = true; + SET_FLAG(path->attr->nh_flag, BGP_ATTR_NH_MP_PREFER_GLOBAL); SET_FLAG(path->attr->rmap_change_flags, BATTR_RMAP_IPV6_PREFER_GLOBAL_CHANGED); } else { - path->attr->mp_nexthop_prefer_global = false; + UNSET_FLAG(path->attr->nh_flag, BGP_ATTR_NH_MP_PREFER_GLOBAL); SET_FLAG(path->attr->rmap_change_flags, BATTR_RMAP_IPV6_PREFER_GLOBAL_CHANGED); } diff --git a/bgpd/bgp_snmp_bgp4v2.c b/bgpd/bgp_snmp_bgp4v2.c index 2d70aa94d3..467a33694a 100644 --- a/bgpd/bgp_snmp_bgp4v2.c +++ b/bgpd/bgp_snmp_bgp4v2.c @@ -684,7 +684,8 @@ static uint8_t *bgp4v2PathAttrTable(struct variable *v, oid name[], case BGP_ATTR_NHLEN_IPV6_GLOBAL: return SNMP_INTEGER(2); case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL: - if (path->attr->mp_nexthop_prefer_global) + if (CHECK_FLAG(path->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) return SNMP_INTEGER(2); else return SNMP_INTEGER(4); @@ -698,7 +699,8 @@ static uint8_t *bgp4v2PathAttrTable(struct variable *v, oid name[], case BGP_ATTR_NHLEN_IPV6_GLOBAL: return SNMP_IP6ADDRESS(path->attr->mp_nexthop_global); case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL: - if (path->attr->mp_nexthop_prefer_global) + if (CHECK_FLAG(path->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) return SNMP_IP6ADDRESS( path->attr->mp_nexthop_global); else diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 4f1808829c..82fdce3b44 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1025,7 +1025,8 @@ bgp_path_info_to_ipv6_nexthop(struct bgp_path_info *path, ifindex_t *ifindex) || path->attr->mp_nexthop_len == BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL) { /* Check if route-map is set to prefer global over link-local */ - if (path->attr->mp_nexthop_prefer_global) { + if (CHECK_FLAG(path->attr->nh_flag, + BGP_ATTR_NH_MP_PREFER_GLOBAL)) { nexthop = &path->attr->mp_nexthop_global; if (IN6_IS_ADDR_LINKLOCAL(nexthop)) *ifindex = path->attr->nh_ifindex; From c6b38684bd040bdf6e2877f60eda7d8dc009ab87 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 6 Jul 2022 15:37:44 +0200 Subject: [PATCH 15/23] zebra: delete kernel routes using an interface with no more IPv4 address When the last IPv4 address of an interface is deleted, Linux removes all routes using this interface without any Netlink advertisement. Routes that have a IPv4 nexthop are correctly removed from the FRR RIB. However, routes that only have an interface with no more IPv4 addresses as a nexthop remains in the FRR RIB. In this situation, among the routes that this particular interface nexthop: - remove from the zebra kernel routes - reinstall the routes that have been added from FRR. It is useful when the nexthop is for example a VRF interface. Add related test cases in the zebra_netlink topotest. Signed-off-by: Louis Scalbert --- zebra/connected.c | 72 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 6 deletions(-) diff --git a/zebra/connected.c b/zebra/connected.c index c01be58e82..57c7f1925b 100644 --- a/zebra/connected.c +++ b/zebra/connected.c @@ -387,10 +387,14 @@ void connected_down(struct interface *ifp, struct connected *ifc) .ifindex = ifp->ifindex, .vrf_id = ifp->vrf->vrf_id, }; - struct zebra_vrf *zvrf; - uint32_t count = 0; + struct zebra_vrf *zvrf, *zvrf_iter; + uint32_t count_ipv4 = 0; struct listnode *cnode; struct connected *c; + struct route_table *table; + struct route_node *rn; + struct route_entry *re, *next; + struct vrf *vrf; zvrf = ifp->vrf->info; if (!zvrf) { @@ -456,12 +460,14 @@ void connected_down(struct interface *ifp, struct connected *ifc) prefix_copy(&cp, CONNECTED_PREFIX(c)); apply_mask(&cp); - if (prefix_same(&p, &cp) && - !CHECK_FLAG(c->conf, ZEBRA_IFC_DOWN)) - count++; + if (CHECK_FLAG(c->conf, ZEBRA_IFC_DOWN)) + continue; - if (count >= 1) + if (prefix_same(&p, &cp)) return; + + if (cp.family == AF_INET) + count_ipv4++; } /* @@ -474,6 +480,60 @@ void connected_down(struct interface *ifp, struct connected *ifc) rib_delete(afi, SAFI_MULTICAST, zvrf->vrf->vrf_id, ZEBRA_ROUTE_CONNECT, 0, 0, &p, NULL, &nh, 0, zvrf->table_id, 0, 0, false); + /* When the last IPv4 address of an interface is deleted, Linux removes + * all routes using this interface without any Netlink advertisement. + * The removed routes include those that only have this particular + * interface as a nexthop. Among those, remove the kernel one from the + * FRR RIB and reinstall the other that have been added from FRR. + */ + if (afi == AFI_IP && count_ipv4 == 0 && if_is_operative(ifp)) { + RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) { + zvrf_iter = vrf->info; + + if (!zvrf_iter) + continue; + + table = zvrf_iter->table[AFI_IP][SAFI_UNICAST]; + if (!table) + continue; + + for (rn = route_top(table); rn; + rn = srcdest_route_next(rn)) { + RNODE_FOREACH_RE_SAFE (rn, re, next) { + if (CHECK_FLAG(re->status, + ROUTE_ENTRY_REMOVED)) + continue; + if (re->nhe->ifp != ifp) + continue; + if (re->type == ZEBRA_ROUTE_KERNEL) + rib_delete( + afi, SAFI_UNICAST, + zvrf_iter->vrf->vrf_id, + re->type, 0, re->flags, + &rn->p, NULL, &nh, 0, + zvrf_iter->table_id, + re->metric, + re->distance, false); + else if (re->type != + ZEBRA_ROUTE_CONNECT) { + SET_FLAG(re->status, + ROUTE_ENTRY_CHANGED); + UNSET_FLAG( + re->status, + ROUTE_ENTRY_INSTALLED); + rib_add(afi, SAFI_UNICAST, + zvrf_iter->vrf->vrf_id, + re->type, 0, 0, &rn->p, + NULL, &nh, re->nhe_id, + zvrf_iter->table_id, + re->metric, 0, + re->distance, 0, false); + } + } + } + } + } + /* Schedule LSP forwarding entries for processing, if appropriate. */ if (zvrf->vrf->vrf_id == VRF_DEFAULT) { if (IS_ZEBRA_DEBUG_MPLS) From 5f6c0ba6d2aa63f41a37f595a1531527cde76340 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 6 May 2022 20:03:55 +0200 Subject: [PATCH 16/23] bgpd: resend routes deleted by kernel after interface addresses deletion When the last IPv4 address of an interface is deleted, Linux removes all routes includes BGP ones using this interface without any Netlink advertisement. bgpd keeps them in RIB as valid (e.g. installed in FIB). The previous patch invalidates the associated nexthop groups in zebra but bgpd is not notified of the event. > 2022/05/09 17:37:52.925 ZEBRA: [TQKA8-0276P] Not Notifying Owner: connected about prefix 29.0.0.0/24(40) 3 vrf: 7 Look for the bgp_path_info that are unsynchronized with the kernel and flag them for refresh in their attributes. A VPN route leaking update is calles and the refresh flag triggers a route refresh to zebra and then a kernel FIB installation. Signed-off-by: Louis Scalbert --- bgpd/bgp_attr.h | 1 + bgpd/bgp_mplsvpn.c | 1 + bgpd/bgp_zebra.c | 49 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 4eb742422d..f8beb5fba9 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -175,6 +175,7 @@ struct attr { #define BGP_ATTR_NH_VALID 0x01 #define BGP_ATTR_NH_IF_OPERSTATE 0x02 #define BGP_ATTR_NH_MP_PREFER_GLOBAL 0x04 /* MP Nexthop preference */ +#define BGP_ATTR_NH_REFRESH 0x08 /* Path origin attribute */ uint8_t origin; diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 41a00ab60b..0dce714fbd 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1280,6 +1280,7 @@ leak_update(struct bgp *to_bgp, struct bgp_dest *bn, if (debug) zlog_debug("%s: ->%s: %pBD Found route, changed attr", __func__, to_bgp->name_pretty, bn); + UNSET_FLAG(bpi->attr->nh_flag, BGP_ATTR_NH_REFRESH); return bpi; } diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 82fdce3b44..952b8fcc56 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -408,10 +408,16 @@ static int bgp_interface_address_add(ZAPI_CALLBACK_ARGS) static int bgp_interface_address_delete(ZAPI_CALLBACK_ARGS) { struct listnode *node, *nnode; + struct bgp_path_info *pi; + struct bgp_table *table; + struct bgp_dest *dest; struct connected *ifc; struct peer *peer; - struct bgp *bgp; + struct bgp *bgp, *from_bgp, *bgp_default; + struct listnode *next; struct prefix *addr; + afi_t afi; + safi_t safi; bgp = bgp_lookup_by_vrf_id(vrf_id); @@ -439,9 +445,6 @@ static int bgp_interface_address_delete(ZAPI_CALLBACK_ARGS) * we do not want the peering to bounce. */ for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { - afi_t afi; - safi_t safi; - if (addr->family == AF_INET) continue; @@ -457,6 +460,44 @@ static int bgp_interface_address_delete(ZAPI_CALLBACK_ARGS) } } + bgp_default = bgp_get_default(); + afi = family2afi(addr->family); + safi = SAFI_UNICAST; + + /* When the last IPv4 address was deleted, Linux removes all routes + * using the interface so that bgpd needs to re-send them. + */ + if (bgp_default && afi == AFI_IP) { + for (ALL_LIST_ELEMENTS_RO(bm->bgp, next, from_bgp)) { + table = from_bgp->rib[afi][safi]; + if (!table) + continue; + + for (dest = bgp_table_top(table); dest; + dest = bgp_route_next(dest)) { + for (pi = bgp_dest_get_bgp_path_info(dest); pi; + pi = pi->next) { + if (pi->type == ZEBRA_ROUTE_BGP && + pi->attr && + pi->attr->nh_ifindex == + ifc->ifp->ifindex) { + SET_FLAG(pi->attr->nh_flag, + BGP_ATTR_NH_REFRESH); + } + } + } + + if (from_bgp->inst_type != BGP_INSTANCE_TYPE_VRF) + continue; + + vpn_leak_postchange(BGP_VPN_POLICY_DIR_TOVPN, afi, + bgp_default, from_bgp); + + vpn_leak_postchange(BGP_VPN_POLICY_DIR_FROMVPN, afi, + bgp_default, from_bgp); + } + } + connected_free(&ifc); return 0; From a7e794215c379f975be34179a034d0c2765cbbbb Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 12 May 2022 14:54:32 +0200 Subject: [PATCH 17/23] topotests: add ability to check that a prefix is not in BGP RIB Add an "exist" key to check the existence of a prefix in the BGP RIB. Useful to check that a prefix has not leaked by error. Signed-off-by: Louis Scalbert --- tests/topotests/lib/bgprib.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/topotests/lib/bgprib.py b/tests/topotests/lib/bgprib.py index 35a57d0a99..ced6dd7de3 100644 --- a/tests/topotests/lib/bgprib.py +++ b/tests/topotests/lib/bgprib.py @@ -48,7 +48,15 @@ class BgpRib: for pfx in pfxtbl.keys(): if debug: self.log("trying pfx %s" % pfx) - if pfx != want["p"]: + if "exist" in want and want["exist"] == False: + if pfx == want["p"]: + if debug: + self.log("unexpected route: pfx=" + want["p"]) + return 0 + if debug: + self.log("unwant pfx=" + want["p"] + ", not " + pfx) + continue + elif pfx != want["p"]: if debug: self.log("want pfx=" + want["p"] + ", not " + pfx) continue @@ -75,6 +83,9 @@ class BgpRib: if debug: self.log("missing route: pfx=" + want["p"] + ", nh=" + want["n"]) return 0 + if "exist" in want and want["exist"] == False: + return 1 + return 0 def RequireVpnRoutes(self, target, title, wantroutes, debug=0): import json From 90bdefa0943f049878e28dd5f449e869dd3c4024 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 12 May 2022 15:37:36 +0200 Subject: [PATCH 18/23] topotests: add retry in BGP RIB check Add a retry option in the BGP RIB test. Signed-off-by: Louis Scalbert --- tests/topotests/lib/bgprib.py | 180 +++++++++++++++++++--------------- 1 file changed, 103 insertions(+), 77 deletions(-) diff --git a/tests/topotests/lib/bgprib.py b/tests/topotests/lib/bgprib.py index ced6dd7de3..01439373c5 100644 --- a/tests/topotests/lib/bgprib.py +++ b/tests/topotests/lib/bgprib.py @@ -37,6 +37,7 @@ from lib.lutil import luCommand, luResult, LUtil import json import re +import time # gpz: get rib in json form and compare against desired routes class BgpRib: @@ -87,52 +88,63 @@ class BgpRib: return 1 return 0 - def RequireVpnRoutes(self, target, title, wantroutes, debug=0): + def RequireVpnRoutes(self, target, title, wantroutes, retry=0, wait=1, debug=0): import json logstr = "RequireVpnRoutes " + str(wantroutes) - # non json form for humans - luCommand( - target, - 'vtysh -c "show bgp ipv4 vpn"', - ".", - "None", - "Get VPN RIB (non-json)", - ) - ret = luCommand( - target, - 'vtysh -c "show bgp ipv4 vpn json"', - ".*", - "None", - "Get VPN RIB (json)", - ) - if re.search(r"^\s*$", ret): - # degenerate case: empty json means no routes - if len(wantroutes) > 0: - luResult(target, False, title, logstr) - return - luResult(target, True, title, logstr) - rib = json.loads(ret) - rds = rib["routes"]["routeDistinguishers"] - for want in wantroutes: - found = 0 - if debug: - self.log("want rd %s" % want["rd"]) - for rd in rds.keys(): - if rd != want["rd"]: - continue + retry += 1 + while retry: + retry -= 1 + # non json form for humans + luCommand( + target, + 'vtysh -c "show bgp ipv4 vpn"', + ".", + "None", + "Get VPN RIB (non-json)", + ) + ret = luCommand( + target, + 'vtysh -c "show bgp ipv4 vpn json"', + ".*", + "None", + "Get VPN RIB (json)", + ) + if re.search(r"^\s*$", ret): + # degenerate case: empty json means no routes + if len(wantroutes) > 0: + luResult(target, False, title, logstr) + return + luResult(target, True, title, logstr) + rib = json.loads(ret) + rds = rib["routes"]["routeDistinguishers"] + for want in wantroutes: + found = 0 if debug: - self.log("found rd %s" % rd) - table = rds[rd] - if self.routes_include_wanted(table, want, debug): - found = 1 - break - if not found: - luResult(target, False, title, logstr) - return - luResult(target, True, title, logstr) + self.log("want rd %s" % want["rd"]) + for rd in rds.keys(): + if rd != want["rd"]: + continue + if debug: + self.log("found rd %s" % rd) + table = rds[rd] + if self.routes_include_wanted(table, want, debug): + found = 1 + break + if not found: + if retry: + break + luResult(target, False, title, logstr) + return + if not found and retry: + time.sleep(wait) + continue + luResult(target, True, title, logstr) + break - def RequireUnicastRoutes(self, target, afi, vrf, title, wantroutes, debug=0): + def RequireUnicastRoutes( + self, target, afi, vrf, title, wantroutes, retry=0, wait=1, debug=0 + ): logstr = "RequireUnicastRoutes %s" % str(wantroutes) vrfstr = "" if vrf != "": @@ -141,48 +153,62 @@ class BgpRib: if (afi != "ipv4") and (afi != "ipv6"): self.log("ERROR invalid afi") - cmdstr = "show bgp %s %s unicast" % (vrfstr, afi) - # non json form for humans - cmd = 'vtysh -c "%s"' % cmdstr - luCommand(target, cmd, ".", "None", "Get %s %s RIB (non-json)" % (vrfstr, afi)) - cmd = 'vtysh -c "%s json"' % cmdstr - ret = luCommand( - target, cmd, ".*", "None", "Get %s %s RIB (json)" % (vrfstr, afi) - ) - if re.search(r"^\s*$", ret): - # degenerate case: empty json means no routes - if len(wantroutes) > 0: - luResult(target, False, title, logstr) + retry += 1 + while retry: + retry -= 1 + cmdstr = "show bgp %s %s unicast" % (vrfstr, afi) + # non json form for humans + cmd = 'vtysh -c "%s"' % cmdstr + luCommand( + target, cmd, ".", "None", "Get %s %s RIB (non-json)" % (vrfstr, afi) + ) + cmd = 'vtysh -c "%s json"' % cmdstr + ret = luCommand( + target, cmd, ".*", "None", "Get %s %s RIB (json)" % (vrfstr, afi) + ) + if re.search(r"^\s*$", ret): + # degenerate case: empty json means no routes + if len(wantroutes) > 0: + luResult(target, False, title, logstr) + return + luResult(target, True, title, logstr) + rib = json.loads(ret) + try: + table = rib["routes"] + # KeyError: 'routes' probably means missing/bad VRF + except KeyError as err: + if vrf != "": + errstr = "-script ERROR: check if wrong vrf (%s)" % (vrf) + else: + errstr = "-script ERROR: check if vrf missing" + if retry: + time.sleep(wait) + continue + luResult(target, False, title + errstr, logstr) return + # if debug: + # self.log("table=%s" % table) + for want in wantroutes: + if debug: + self.log("want=%s" % want) + if not self.routes_include_wanted(table, want, debug): + if retry: + time.sleep(wait) + continue + luResult(target, False, title, logstr) + return luResult(target, True, title, logstr) - rib = json.loads(ret) - try: - table = rib["routes"] - # KeyError: 'routes' probably means missing/bad VRF - except KeyError as err: - if vrf != "": - errstr = "-script ERROR: check if wrong vrf (%s)" % (vrf) - else: - errstr = "-script ERROR: check if vrf missing" - luResult(target, False, title + errstr, logstr) - return - # if debug: - # self.log("table=%s" % table) - for want in wantroutes: - if debug: - self.log("want=%s" % want) - if not self.routes_include_wanted(table, want, debug): - luResult(target, False, title, logstr) - return - luResult(target, True, title, logstr) + break BgpRib = BgpRib() -def bgpribRequireVpnRoutes(target, title, wantroutes, debug=0): - BgpRib.RequireVpnRoutes(target, title, wantroutes, debug) +def bgpribRequireVpnRoutes(target, title, wantroutes, retry=0, wait=1, debug=0): + BgpRib.RequireVpnRoutes(target, title, wantroutes, retry, wait, debug) -def bgpribRequireUnicastRoutes(target, afi, vrf, title, wantroutes, debug=0): - BgpRib.RequireUnicastRoutes(target, afi, vrf, title, wantroutes, debug) +def bgpribRequireUnicastRoutes( + target, afi, vrf, title, wantroutes, retry=0, wait=1, debug=0 +): + BgpRib.RequireUnicastRoutes(target, afi, vrf, title, wantroutes, retry, wait, debug) From a1d9f6f2f2d2db5980f7b11e3ac2be9f68f4b063 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 12 May 2022 14:57:17 +0200 Subject: [PATCH 19/23] topotests: add VRF leak tests in bgp_l3vpn_to_bgp_vrf Check that route leaking between VRF within a router works properly. Signed-off-by: Louis Scalbert --- .../bgp_l3vpn_to_bgp_vrf/ce1/zebra.conf | 2 + .../bgp_l3vpn_to_bgp_vrf/ce2/zebra.conf | 2 + .../bgp_l3vpn_to_bgp_vrf/ce3/bgpd.conf | 1 + .../bgp_l3vpn_to_bgp_vrf/ce3/zebra.conf | 1 + .../bgp_l3vpn_to_bgp_vrf/ce4/bgpd.conf | 1 + .../bgp_l3vpn_to_bgp_vrf/ce4/zebra.conf | 1 + .../bgp_l3vpn_to_bgp_vrf/customize.py | 14 ++++ .../bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf | 48 +++++++++++++ .../bgp_l3vpn_to_bgp_vrf/r1/staticd.conf | 6 ++ .../bgp_l3vpn_to_bgp_vrf/r1/zebra.conf | 11 +++ .../scripts/check_linux_mpls.py | 21 ++++++ .../scripts/check_linux_vrf.py | 43 ++++++++++++ .../scripts/check_routes.py | 69 +++++++++++++++++-- 13 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/staticd.conf diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce1/zebra.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce1/zebra.conf index 46831bb711..375bbea9ff 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce1/zebra.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce1/zebra.conf @@ -4,6 +4,8 @@ hostname ce1 ! interface lo ip address 99.0.0.1/32 + ip address 5.1.0.1/24 + ip address 6.0.2.1/24 ! interface ce1-eth0 description to r1 diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce2/zebra.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce2/zebra.conf index fb4d8cc9c4..90dd3c55b4 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce2/zebra.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce2/zebra.conf @@ -4,6 +4,8 @@ hostname ce2 ! interface lo ip address 99.0.0.2/32 + ip address 5.1.0.1/24 + ip address 6.0.2.1/24 ! interface ce2-eth0 description to r3 diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/bgpd.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/bgpd.conf index e316de5690..cf7396eb12 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/bgpd.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/bgpd.conf @@ -19,6 +19,7 @@ router bgp 5227 network 5.1.3.0/24 route-map rm-nh network 6.0.1.0/24 route-map rm-nh network 6.0.2.0/24 route-map rm-nh-same + network 6.0.3.0/24 route-map rm-nh-same neighbor 192.168.1.1 activate exit-address-family ! diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/zebra.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/zebra.conf index 77a1163a4b..df6ac47b08 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/zebra.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce3/zebra.conf @@ -4,6 +4,7 @@ hostname ce3 ! interface lo ip address 99.0.0.3/32 + ip address 6.0.3.1/24 ! interface ce3-eth0 description to r4 diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/bgpd.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/bgpd.conf index 60d9e93108..9a6ca08a0b 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/bgpd.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/bgpd.conf @@ -19,6 +19,7 @@ router bgp 5228 vrf ce4-cust2 network 5.4.3.0/24 route-map rm-nh network 6.0.1.0/24 route-map rm-nh network 6.0.2.0/24 route-map rm-nh-same + network 6.0.3.0/24 route-map rm-nh-same neighbor 192.168.2.1 activate exit-address-family ! diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/zebra.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/zebra.conf index e55c9e779a..0e3a736292 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/zebra.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/ce4/zebra.conf @@ -4,6 +4,7 @@ hostname ce4 ! interface ce4-cust2 ip address 99.0.0.4/32 + ip address 6.0.3.1/24 ! interface ce4-eth0 description to r4 diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py index 5161d8471f..b2bf5f5f63 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/customize.py @@ -175,6 +175,20 @@ def ltemplatePreRouterStartHook(): "setup {0} vrf {0}-cust1, {0}-eth4. enabled mpls input.".format(rtr) ) # configure cust2 VRFs & MPLS + rtrs = ["r1"] + cmds = [ + "ip link add {0}-cust3 type vrf table 20", + "ip link set dev {0}-cust3 up", + "ip link add {0}-cust4 type vrf table 30", + "ip link set dev {0}-cust4 up", + "ip link add {0}-cust5 type vrf table 40", + "ip link set dev {0}-cust5 up", + ] + for rtr in rtrs: + for cmd in cmds: + cc.doCmd(tgen, rtr, cmd.format(rtr)) + logger.info("setup {0} vrf {0}-cust3 and{0}-cust4.".format(rtr)) + # configure cust2 VRFs & MPLS rtrs = ["r4"] cmds = [ "ip link add {0}-cust2 type vrf table 20", diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf index 8d42cfc0d8..24e9f95372 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/bgpd.conf @@ -11,6 +11,7 @@ log file bgpd.log debugging #debug bgp vpn leak-from-vrf #debug bgp vpn label #debug bgp updates out +#debug bgp nht router bgp 5226 bgp router-id 1.1.1.1 @@ -39,6 +40,11 @@ router bgp 5227 vrf r1-cust1 neighbor 192.168.1.2 timers 3 10 address-family ipv4 unicast + network 10.2.3.4/32 + network 192.0.0.0/24 + + redistribute connected + neighbor 192.168.1.2 activate neighbor 192.168.1.2 next-hop-self @@ -51,5 +57,47 @@ router bgp 5227 vrf r1-cust1 exit-address-family +router bgp 5228 vrf r1-cust3 + bgp router-id 192.168.1.1 + + address-family ipv4 unicast + rd vpn export 10:13 + rt vpn import 52:100 + + import vpn + export vpn + exit-address-family + + +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:14 + rt vpn export 52:100 + + import vpn + export vpn + exit-address-family + + +router bgp 5227 vrf r1-cust5 + bgp router-id 192.168.1.1 + + address-family ipv4 unicast + redistribute connected + + label vpn export 105 + rd vpn export 10:15 + rt vpn both 52:100 + + import vpn + export vpn + exit-address-family + ! end diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/staticd.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/staticd.conf new file mode 100644 index 0000000000..59430fdf99 --- /dev/null +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/staticd.conf @@ -0,0 +1,6 @@ +hostname r1 +log file staticd.log +! +vrf r1-cust1 + ip route 192.0.0.0/24 192.168.1.2 +exit-vrf diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/zebra.conf b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/zebra.conf index 221bc7a839..e81bc6b2ab 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/zebra.conf +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/r1/zebra.conf @@ -4,6 +4,9 @@ hostname r1 password zebra #debug zebra packet +#debug zebra rib detailed +#debug zebra dplane detailed +#debug zebra nexthop detail interface lo ip address 1.1.1.1/32 @@ -18,6 +21,14 @@ interface r1-eth4 ip address 192.168.1.1/24 no link-detect +interface r1-cust1 + ip address 10.4.5.6/24 + no link-detect + +interface r1-cust5 + ip address 29.0.0.1/32 + no link-detect + ip forwarding diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_mpls.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_mpls.py index 91a7adf997..89369241a8 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_mpls.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_mpls.py @@ -81,3 +81,24 @@ if ret != False and found != None: "wait", "CE3->CE4 (loopback) ping", ) + luCommand( + "r1", + "ip vrf exec r1-cust1 ping 6.0.3.1 -I 10.4.5.6 -c 1", + " 0. packet loss", + "wait", + "R1(r1-cust1)->CE3/4 (loopback) ping", + ) + luCommand( + "r1", + "ip vrf exec r1-cust1 ping 6.0.3.1 -I 10.4.5.6 -c 1", + " 0. packet loss", + "pass", + "R1(r1-cust1)->CE3/4 (loopback) ping", + ) + luCommand( + "r1", + "ip vrf exec r1-cust5 ping 6.0.3.1 -I 29.0.0.1 -c 1", + " 0. packet loss", + "pass", + "R1(r1-cust5)->CE3/4 ( (loopback) ping", + ) diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py index 75158b127e..762af94d3f 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py @@ -72,3 +72,46 @@ luCommand( "wait", "CE4->PE4 ping", ) +luCommand( + "r1", + "ip vrf exec r1-cust5 ping 192.168.1.1 -I 29.0.0.1 -c 1", + " 0. packet loss", + "pass", + "R1(r1-cust5)->R1(r1-cust1 - r1-eth4) ping", +) +luCommand( + "r1", + "ip vrf exec r1-cust5 ping 192.168.1.2 -I 29.0.0.1 -c 1", + " 0. packet loss", + "wait", + "R1(r1-cust5)->CE1 ping", +) +luCommand( + "r1", + "ip vrf exec r1-cust5 ping 192.168.1.2 -I 29.0.0.1 -c 1", + " 0. packet loss", + "pass", + "R1(r1-cust5)->CE1 ping", +) +luCommand( + "r1", + "ip vrf exec r1-cust5 ping 99.0.0.1 -I 29.0.0.1 -c 1", + " 0. packet loss", + "pass", + "R1(r1-cust5)->CE1 (loopback) ping", +) +luCommand( + "r1", + "ip vrf exec r1-cust5 ping 5.1.0.1 -I 29.0.0.1 -c 1", + " 0. packet loss", + "wait", + "R1(r1-cust5)->CE1 (loopback) ping", + time=30, +) +luCommand( + "r1", + "ip vrf exec r1-cust5 ping 5.1.0.1 -I 29.0.0.1 -c 1", + " 0. packet loss", + "pass", + "R1(r1-cust5)->CE1 (loopback) ping", +) diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py index 1e2758c1c9..f51fc6598e 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py @@ -54,15 +54,44 @@ bgpribRequireUnicastRoutes("ce4", "ipv4", "ce4-cust2", "Cust 4 routes in ce1", w # # r1 vtysh -c "show bgp vrf r1-cust1 ipv4" # -want_r1_cust1_routes = [ +want_r1_cust1_3_5_routes = [ {"p": "5.1.0.0/24", "n": "99.0.0.1"}, {"p": "5.1.1.0/24", "n": "99.0.0.1"}, {"p": "6.0.1.0/24", "n": "99.0.0.1"}, {"p": "6.0.2.0/24", "n": "99.0.0.1"}, + {"p": "10.2.3.4/32", "n": "0.0.0.0", "bp": False}, + {"p": "10.4.5.0/24", "n": "0.0.0.0", "bp": True}, + {"p": "28.0.0.0/24", "n": "0.0.0.0", "bp": True}, + {"p": "29.0.0.1/32", "n": "0.0.0.0", "bp": True}, {"p": "99.0.0.1/32", "n": "192.168.1.2"}, + {"p": "192.0.0.0/24", "n": "0.0.0.0", "bp": True}, + {"p": "192.168.1.0/24", "n": "0.0.0.0", "bp": True}, ] bgpribRequireUnicastRoutes( - "r1", "ipv4", "r1-cust1", "Customer 1 routes in r1 vrf", want_r1_cust1_routes + "r1", "ipv4", "r1-cust1", "Customer 1 routes in r1 vrf", want_r1_cust1_3_5_routes +) +bgpribRequireUnicastRoutes( + "r1", "ipv4", "r1-cust3", "Customer 3 routes in r1 vrf", want_r1_cust1_3_5_routes +) +bgpribRequireUnicastRoutes( + "r1", "ipv4", "r1-cust5", "Customer 5 routes in r1 vrf", want_r1_cust1_3_5_routes +) + +want_r1_cust4_routes = [ + {"p": "5.1.0.0/24", "n": "99.0.0.1", "exist": False}, + {"p": "5.1.1.0/24", "n": "99.0.0.1", "exist": False}, + {"p": "6.0.1.0/24", "n": "99.0.0.1", "exist": False}, + {"p": "6.0.2.0/24", "n": "99.0.0.1", "exist": False}, + {"p": "10.2.3.4/32", "n": "0.0.0.0", "exist": False}, + {"p": "10.4.5.0/24", "n": "0.0.0.0", "exist": False}, + {"p": "28.0.0.0/24", "n": "0.0.0.0", "bp": True}, + {"p": "29.0.0.1/32", "n": "0.0.0.0", "exist": False}, + {"p": "99.0.0.1/32", "n": "192.168.1.2", "exist": False}, + {"p": "192.0.0.0/24", "n": "0.0.0.0", "exist": False}, + {"p": "192.168.1.0/24", "n": "0.0.0.0", "exist": False}, +] +bgpribRequireUnicastRoutes( + "r1", "ipv4", "r1-cust4", "Customer 4 routes in r1 vrf", want_r1_cust4_routes ) want_r3_cust1_routes = [ @@ -70,10 +99,20 @@ want_r3_cust1_routes = [ {"p": "5.1.1.0/24", "n": "99.0.0.2"}, {"p": "6.0.1.0/24", "n": "99.0.0.2"}, {"p": "6.0.2.0/24", "n": "99.0.0.2"}, + {"p": "10.2.3.4/32", "n": "0.0.0.0", "exist": False}, + {"p": "28.0.0.0/24", "n": "1.1.1.1", "bp": True}, + {"p": "29.0.0.1/32", "n": "1.1.1.1", "bp": True}, {"p": "99.0.0.2/32", "n": "192.168.1.2"}, + {"p": "192.0.0.0/24", "n": "1.1.1.1", "bp": True}, + {"p": "192.168.1.0/24", "n": "1.1.1.1", "bp": True}, ] bgpribRequireUnicastRoutes( - "r3", "ipv4", "r3-cust1", "Customer 1 routes in r3 vrf", want_r3_cust1_routes + "r3", + "ipv4", + "r3-cust1", + "Customer 1 routes in r3 vrf", + want_r3_cust1_routes, + retry=30, ) want_r4_cust1_routes = [ @@ -81,10 +120,20 @@ want_r4_cust1_routes = [ {"p": "5.1.3.0/24", "n": "99.0.0.3"}, {"p": "6.0.1.0/24", "n": "99.0.0.3"}, {"p": "6.0.2.0/24", "n": "99.0.0.3"}, + {"p": "10.2.3.4/32", "n": "0.0.0.0", "exist": False}, + {"p": "28.0.0.0/24", "n": "1.1.1.1", "bp": True}, + {"p": "29.0.0.1/32", "n": "1.1.1.1", "bp": True}, {"p": "99.0.0.3/32", "n": "192.168.1.2"}, + {"p": "192.0.0.0/24", "n": "1.1.1.1", "bp": True}, + {"p": "192.168.1.0/24", "n": "1.1.1.1", "bp": True}, ] bgpribRequireUnicastRoutes( - "r4", "ipv4", "r4-cust1", "Customer 1 routes in r4 vrf", want_r4_cust1_routes + "r4", + "ipv4", + "r4-cust1", + "Customer 1 routes in r4 vrf", + want_r4_cust1_routes, + retry=30, ) want_r4_cust2_routes = [ @@ -92,10 +141,20 @@ want_r4_cust2_routes = [ {"p": "5.4.3.0/24", "n": "99.0.0.4"}, {"p": "6.0.1.0/24", "n": "99.0.0.4"}, {"p": "6.0.2.0/24", "n": "99.0.0.4"}, + {"p": "10.2.3.4/32", "n": "0.0.0.0", "exist": False}, + {"p": "28.0.0.0/24", "n": "1.1.1.1", "bp": True}, + {"p": "29.0.0.1/32", "n": "1.1.1.1", "bp": True}, {"p": "99.0.0.4/32", "n": "192.168.2.2"}, + {"p": "192.0.0.0/24", "n": "1.1.1.1", "bp": True}, + {"p": "192.168.1.0/24", "n": "1.1.1.1", "bp": True}, ] bgpribRequireUnicastRoutes( - "r4", "ipv4", "r4-cust2", "Customer 2 routes in r4 vrf", want_r4_cust2_routes + "r4", + "ipv4", + "r4-cust2", + "Customer 2 routes in r4 vrf", + want_r4_cust2_routes, + retry=30, ) ######################################################################## From 56748da55ffdbdb74242000625f728c8c4a3d59f Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 12 May 2022 16:12:34 +0200 Subject: [PATCH 20/23] topotests: add tests to bgp-vrf-route-leak-basic Signed-off-by: Louis Scalbert --- .../bgp_vrf_route_leak_basic/r1/bgpd.conf | 6 + .../test_bgp-vrf-route-leak-basic.py | 211 ++++++++++++++---- 2 files changed, 173 insertions(+), 44 deletions(-) diff --git a/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf b/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf index 03dfbf9322..0540a62096 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf +++ b/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf @@ -1,5 +1,11 @@ hostname r1 + +#debug bgp vpn leak-to-vrf +#debug bgp vpn leak-from-vrf +#debug bgp nht + + router bgp 99 vrf DONNA no bgp ebgp-requires-policy address-family ipv4 unicast diff --git a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py index 191a0b53ec..9c3a4bfdad 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py +++ b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py @@ -29,6 +29,7 @@ import os import sys from functools import partial import pytest +import time CWD = os.path.dirname(os.path.realpath(__file__)) sys.path.append(os.path.join(CWD, "../")) @@ -77,7 +78,103 @@ def teardown_module(mod): tgen.stop_topology() -def test_vrf_route_leak(): +def check_bgp_rib(router, vrf, in_fib): + if in_fib: + attr = [{"protocol": "bgp", "selected": True, "nexthops": [{"fib": True}]}] + else: + attr = [{"protocol": "bgp", "nexthops": []}] + + if vrf == "DONNA": + expect = { + "10.0.0.0/24": [ + { + "protocol": "connected", + } + ], + "10.0.1.0/24": attr, + "10.0.2.0/24": [{"protocol": "connected"}], + "10.0.3.0/24": attr, + } + else: + expect = { + "10.0.0.0/24": attr, + "10.0.1.0/24": [ + { + "protocol": "connected", + } + ], + "10.0.2.0/24": attr, + "10.0.3.0/24": [ + { + "protocol": "connected", + } + ], + } + + test_func = partial( + topotest.router_json_cmp, router, "show ip route vrf %s json" % vrf, expect + ) + return topotest.run_and_expect(test_func, None, count=10, wait=0.5) + + +def check_bgp_fib(router, vrf, in_rib): + # Check FIB + # DONNA + # 10.0.1.0/24 dev EVA proto bgp metric 20 + # 10.0.3.0/24 dev EVA proto bgp metric 20 + # EVA + # 10.0.0.0/24 dev DONNA proto bgp metric 20 + # 10.0.2.0/24 dev DONNA proto bgp metric 20 + + if vrf == "DONNA": + table = 1001 + nh_vrf = "EVA" + else: + table = 1002 + nh_vrf = "DONNA" + + negate = "" if in_rib else "! " + + cmd = "%sip route show table %s | grep %s" % (negate, table, nh_vrf) + result = False + retry = 5 + output = "" + while retry: + retry -= 1 + try: + output = router.cmd_raises(cmd) + result = True + break + except: + time.sleep(0.1) + + logger.info("VRF %s leaked FIB content %s: %s", vrf, cmd, output) + + return result, output + + +def check_bgp_ping(router, vrf): + if vrf == "DONNA": + cmd = "ip vrf exec DONNA ping -c1 10.0.1.1 -I 10.0.0.1" + else: + cmd = "ip vrf exec EVA ping -c1 10.0.0.1 -I 10.0.1.1" + + result = False + retry = 5 + output = "" + while retry: + retry -= 1 + try: + output = router.cmd_raises(cmd) + result = True + break + except: + time.sleep(0.1) + + return result, output + + +def test_vrf_route_leak_test1(): logger.info("Ensure that routes are leaked back and forth") tgen = get_topogen() # Don't run this test if we have any failure. @@ -86,53 +183,79 @@ def test_vrf_route_leak(): r1 = tgen.gears["r1"] - # Test DONNA VRF. - expect = { - "10.0.0.0/24": [ - { - "protocol": "connected", - } - ], - "10.0.1.0/24": [ - {"protocol": "bgp", "selected": True, "nexthops": [{"fib": True}]} - ], - "10.0.2.0/24": [{"protocol": "connected"}], - "10.0.3.0/24": [ - {"protocol": "bgp", "selected": True, "nexthops": [{"fib": True}]} - ], - } + for vrf in ["EVA", "DONNA"]: + result, diff = check_bgp_rib(r1, vrf, True) + assert result, "BGP RIB VRF {} check failed:\n{}".format(vrf, diff) + result, output = check_bgp_fib(r1, vrf, True) + assert result, "BGP FIB VRF {} check failed:\n{}".format(vrf, output) + result, output = check_bgp_ping(r1, vrf) + assert result, "Ping from VRF {} failed:\n{}".format(vrf, output) - test_func = partial( - topotest.router_json_cmp, r1, "show ip route vrf DONNA json", expect + +def test_vrf_route_leak_test2(): + logger.info( + "Ensure that leaked are still present after VRF iface IP address deletion" ) - result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5) - assert result, "BGP VRF DONNA check failed:\n{}".format(diff) + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) - # Test EVA VRF. - expect = { - "10.0.0.0/24": [ - {"protocol": "bgp", "selected": True, "nexthops": [{"fib": True}]} - ], - "10.0.1.0/24": [ - { - "protocol": "connected", - } - ], - "10.0.2.0/24": [ - {"protocol": "bgp", "selected": True, "nexthops": [{"fib": True}]} - ], - "10.0.3.0/24": [ - { - "protocol": "connected", - } - ], - } + r1 = tgen.gears["r1"] - test_func = partial( - topotest.router_json_cmp, r1, "show ip route vrf EVA json", expect - ) - result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5) - assert result, "BGP VRF EVA check failed:\n{}".format(diff) + logger.info("Adding and removing an IPv4 address to EVA and DONNA VRF ifaces") + r1.cmd("ip address add 1.1.1.1/32 dev EVA && ip address del 1.1.1.1/32 dev EVA") + r1.cmd("ip address add 2.2.2.2/32 dev DONNA && ip address del 2.2.2.2/32 dev DONNA") + + for vrf in ["EVA", "DONNA"]: + result, diff = check_bgp_rib(r1, vrf, True) + assert result, "BGP RIB VRF {} check failed:\n{}".format(vrf, diff) + result, output = check_bgp_fib(r1, vrf, True) + assert result, "BGP FIB VRF {} check failed:\n{}".format(vrf, output) + result, output = check_bgp_ping(r1, vrf) + assert result, "Ping from VRF {} failed:\n{}".format(vrf, output) + + +def test_vrf_route_leak_test3(): + logger.info("Ensure that setting down the VRF ifaces invalidates leaked routes") + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + logger.info("Setting down EVA and DONNA VRF ifaces") + r1.cmd("ip link set EVA down") + r1.cmd("ip link set DONNA down") + + for vrf in ["EVA", "DONNA"]: + result, diff = check_bgp_rib(r1, vrf, False) + assert result, "BGP RIB VRF {} check failed:\n{}".format(vrf, diff) + result, output = check_bgp_fib(r1, vrf, False) + assert result, "BGP FIB VRF {} check failed:\n{}".format(vrf, output) + + +def test_vrf_route_leak_test4(): + logger.info("Ensure that setting up the VRF ifaces validates leaked routes") + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + logger.info("Setting up EVA and DONNA VRF ifaces") + r1.cmd("ip link set EVA up") + r1.cmd("ip link set DONNA up") + + for vrf in ["EVA", "DONNA"]: + result, diff = check_bgp_rib(r1, vrf, True) + assert result, "BGP RIB VRF {} check failed:\n{}".format(vrf, diff) + result, output = check_bgp_fib(r1, vrf, True) + assert result, "BGP FIB VRF {} check failed:\n{}".format(vrf, output) + result, output = check_bgp_ping(r1, vrf) + assert result, "Ping from VRF {} failed:\n{}".format(vrf, output) def test_memory_leak(): From 767199c68368b04652824b64bdd580c4a44303c4 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 27 May 2022 13:59:34 +0200 Subject: [PATCH 21/23] topotests: raise an error if pinging from vrf is not possible Because of the issue described in the above link, pinging from vrf with the command "ip vrf exec ping -I " may fail. > root@topo:~# ip vrf exec vrf1 ping -c1 -I 192.168.2.1 192.168.1.1 > bind: Cannot assign requested address Raise an error if pinging its own IP from a VRF fails. This test should always work unless in the condition of this issue. Link: https://bugzilla.kernel.org/show_bug.cgi?id=203483 Signed-off-by: Louis Scalbert --- .../scripts/check_linux_vrf.py | 7 +++++++ .../test_bgp-vrf-route-leak-basic.py | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py index 762af94d3f..e9647898ab 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py @@ -72,6 +72,13 @@ luCommand( "wait", "CE4->PE4 ping", ) +ret = luCommand( + "r1", + "ip vrf exec r1-cust5 ping 29.0.0.1 -I 29.0.0.1 -c 1", + " 0. packet loss", + "pass", + "Ping its own IP. Check https://bugzilla.kernel.org/show_bug.cgi?id=203483 if it fails", +) luCommand( "r1", "ip vrf exec r1-cust5 ping 192.168.1.1 -I 29.0.0.1 -c 1", diff --git a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py index 9c3a4bfdad..be07c85997 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py +++ b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py @@ -174,6 +174,20 @@ def check_bgp_ping(router, vrf): return result, output +def check_bgp_ping_own_ip(router): + cmd = "ip vrf exec DONNA ping -c1 10.0.0.1 -I 10.0.0.1" + + output = "" + try: + output = router.cmd_raises(cmd) + result = True + except: + result = False + pass + + return result, output + + def test_vrf_route_leak_test1(): logger.info("Ensure that routes are leaked back and forth") tgen = get_topogen() @@ -183,6 +197,13 @@ def test_vrf_route_leak_test1(): r1 = tgen.gears["r1"] + result, output = check_bgp_ping_own_ip(r1) + assert ( + result + ), "Ping from VRF fails - check https://bugzilla.kernel.org/show_bug.cgi?id=203483\n:{}".format( + output + ) + for vrf in ["EVA", "DONNA"]: result, diff = check_bgp_rib(r1, vrf, True) assert result, "BGP RIB VRF {} check failed:\n{}".format(vrf, diff) From 4a62ec166966b5eee45b517136cb675263bc9466 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 8 Sep 2022 18:31:17 +0200 Subject: [PATCH 22/23] topotests: fix appropriate number of routes in bgp The number of routes in BGP ce devices was wrong. Change the expected values. Signed-off-by: Philippe Guibert --- .../bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py index f51fc6598e..3242e3bd3a 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_routes.py @@ -726,7 +726,7 @@ bgpribRequireUnicastRoutes( luCommand( "ce1", 'vtysh -c "show bgp ipv4 uni"', - "12 routes and 12", + "18 routes and 19", "wait", "Local and remote routes", 10, @@ -748,7 +748,7 @@ bgpribRequireUnicastRoutes( luCommand( "ce2", 'vtysh -c "show bgp ipv4 uni"', - "12 routes and 15", + "18 routes and 22", "wait", "Local and remote routes", 10, @@ -780,7 +780,7 @@ luCommand("r4", 'vtysh -c "show ip route vrf r4-cust2"') luCommand( "ce3", 'vtysh -c "show bgp ipv4 uni"', - "12 routes and 13", + "18 routes and 19", "wait", "Local and remote routes", 10, @@ -802,7 +802,7 @@ bgpribRequireUnicastRoutes( luCommand( "ce4", 'vtysh -c "show bgp vrf ce4-cust2 ipv4 uni"', - "12 routes and 14", + "18 routes and 21", "wait", "Local and remote routes", 10, From ce82e9026056e5aa7f60abdbf975d19a7fffc623 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 16 Dec 2022 15:09:36 +0100 Subject: [PATCH 23/23] bgpd: fix attrhash_cmp() clang-format Fix attrhash_cmp() clang-format Signed-off-by: Louis Scalbert --- bgpd/bgp_attr.c | 90 ++++++++++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 9352ab4650..17687e6252 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -825,56 +825,56 @@ bool attrhash_cmp(const void *p1, const void *p2) && attr1->med == attr2->med && attr1->local_pref == attr2->local_pref && attr1->rmap_change_flags == attr2->rmap_change_flags) { - if (attr1->aggregator_as == attr2->aggregator_as - && attr1->aggregator_addr.s_addr - == attr2->aggregator_addr.s_addr - && attr1->weight == attr2->weight - && attr1->tag == attr2->tag - && attr1->label_index == attr2->label_index - && attr1->mp_nexthop_len == attr2->mp_nexthop_len - && bgp_attr_get_ecommunity(attr1) - == bgp_attr_get_ecommunity(attr2) - && bgp_attr_get_ipv6_ecommunity(attr1) - == bgp_attr_get_ipv6_ecommunity(attr2) - && bgp_attr_get_lcommunity(attr1) - == bgp_attr_get_lcommunity(attr2) - && bgp_attr_get_cluster(attr1) - == bgp_attr_get_cluster(attr2) - && bgp_attr_get_transit(attr1) - == bgp_attr_get_transit(attr2) - && bgp_attr_get_aigp_metric(attr1) - == bgp_attr_get_aigp_metric(attr2) - && attr1->rmap_table_id == attr2->rmap_table_id - && (attr1->encap_tunneltype == attr2->encap_tunneltype) - && encap_same(attr1->encap_subtlvs, attr2->encap_subtlvs) + if (attr1->aggregator_as == attr2->aggregator_as && + attr1->aggregator_addr.s_addr == + attr2->aggregator_addr.s_addr && + attr1->weight == attr2->weight && + attr1->tag == attr2->tag && + attr1->label_index == attr2->label_index && + attr1->mp_nexthop_len == attr2->mp_nexthop_len && + bgp_attr_get_ecommunity(attr1) == + bgp_attr_get_ecommunity(attr2) && + bgp_attr_get_ipv6_ecommunity(attr1) == + bgp_attr_get_ipv6_ecommunity(attr2) && + bgp_attr_get_lcommunity(attr1) == + bgp_attr_get_lcommunity(attr2) && + bgp_attr_get_cluster(attr1) == + bgp_attr_get_cluster(attr2) && + bgp_attr_get_transit(attr1) == + bgp_attr_get_transit(attr2) && + bgp_attr_get_aigp_metric(attr1) == + bgp_attr_get_aigp_metric(attr2) && + attr1->rmap_table_id == attr2->rmap_table_id && + (attr1->encap_tunneltype == attr2->encap_tunneltype) && + encap_same(attr1->encap_subtlvs, attr2->encap_subtlvs) #ifdef ENABLE_BGP_VNC && encap_same(bgp_attr_get_vnc_subtlvs(attr1), bgp_attr_get_vnc_subtlvs(attr2)) #endif && IPV6_ADDR_SAME(&attr1->mp_nexthop_global, - &attr2->mp_nexthop_global) - && IPV6_ADDR_SAME(&attr1->mp_nexthop_local, - &attr2->mp_nexthop_local) - && IPV4_ADDR_SAME(&attr1->mp_nexthop_global_in, - &attr2->mp_nexthop_global_in) - && IPV4_ADDR_SAME(&attr1->originator_id, - &attr2->originator_id) - && overlay_index_same(attr1, attr2) - && !memcmp(&attr1->esi, &attr2->esi, sizeof(esi_t)) - && attr1->es_flags == attr2->es_flags - && attr1->mm_sync_seqnum == attr2->mm_sync_seqnum - && attr1->df_pref == attr2->df_pref - && attr1->df_alg == attr2->df_alg - && attr1->nh_ifindex == attr2->nh_ifindex - && attr1->nh_flag == attr2->nh_flag - && attr1->nh_lla_ifindex == attr2->nh_lla_ifindex - && attr1->distance == attr2->distance - && srv6_l3vpn_same(attr1->srv6_l3vpn, attr2->srv6_l3vpn) - && srv6_vpn_same(attr1->srv6_vpn, attr2->srv6_vpn) - && attr1->srte_color == attr2->srte_color - && attr1->nh_type == attr2->nh_type - && attr1->bh_type == attr2->bh_type - && attr1->otc == attr2->otc) + &attr2->mp_nexthop_global) && + IPV6_ADDR_SAME(&attr1->mp_nexthop_local, + &attr2->mp_nexthop_local) && + IPV4_ADDR_SAME(&attr1->mp_nexthop_global_in, + &attr2->mp_nexthop_global_in) && + IPV4_ADDR_SAME(&attr1->originator_id, + &attr2->originator_id) && + overlay_index_same(attr1, attr2) && + !memcmp(&attr1->esi, &attr2->esi, sizeof(esi_t)) && + attr1->es_flags == attr2->es_flags && + attr1->mm_sync_seqnum == attr2->mm_sync_seqnum && + attr1->df_pref == attr2->df_pref && + attr1->df_alg == attr2->df_alg && + attr1->nh_ifindex == attr2->nh_ifindex && + attr1->nh_flag == attr2->nh_flag && + attr1->nh_lla_ifindex == attr2->nh_lla_ifindex && + attr1->distance == attr2->distance && + srv6_l3vpn_same(attr1->srv6_l3vpn, attr2->srv6_l3vpn) && + srv6_vpn_same(attr1->srv6_vpn, attr2->srv6_vpn) && + attr1->srte_color == attr2->srte_color && + attr1->nh_type == attr2->nh_type && + attr1->bh_type == attr2->bh_type && + attr1->otc == attr2->otc) return true; }