From 9ecf931b13cfb9a5aa35253acbdfc66bd2346607 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 10 Jun 2019 17:14:42 -0700 Subject: [PATCH] bgpd: no router bgp cleanup vrf leaked vpn routes A VRF leak is configured between two vrfs, bgp VRF X and VRF Y. When a bgp VRF X is removed, unimport bgp VRF X routes from VPN and VRF Y. If VRF X is also importing from bgp VRF Y, remove X from export list of Y and do required route cleanup. Ticket:CM-20534 CM-24484 Reviewed By: Testing Done: Before deleteing vrf1002: nl1# show ip route vrf vrf1003 9.9.2.4/32 Routing entry for 9.9.2.4/32 Known via "bgp", distance 200, metric 0, vrf vrf1003, best Last update 00:04:51 ago * 200.2.8.2, via swp1.2(vrf vrf1002) * 200.2.9.2, via swp2.2(vrf vrf1002) * 200.2.10.2, via swp3.2(vrf vrf1002) Instance vrf1003: This VRF is importing IPv4 Unicast routes from the following VRFs: vrf1002 Import RT(s): 6.0.2.9:2 This VRF is exporting IPv4 Unicast routes to the following VRFs: vrf1002 RD: 6.0.3.9:3 Export RT: 6.0.3.9:3 After deleting vrf1002: nl1(config)# no router bgp 64902 vrf vrf1002 nl1# show ip route vrf vrf1003 9.9.2.4/32 Routing entry for 9.9.2.4/32 Known via "bgp", distance 20, metric 0, vrf vrf1003, best Last update 00:00:32 ago * 200.3.8.2, via swp1.3 * 200.3.9.2, via swp2.3 * 200.3.10.2, via swp3.3 Instance vrf1003: This VRF is importing IPv4 Unicast routes from the following VRFs: vrf1002 Import RT(s): This VRF is not exporting IPv4 Unicast routes to any other VRF nl1# show bgp ipv4 vpn No BGP prefixes displayed, 0 exist Readd vrf1002: points back to source vrf nl1# show ip route vrf vrf1003 9.9.2.4/32 Routing entry for 9.9.2.4/32 Known via "bgp", distance 200, metric 0, vrf vrf1003, best Last update 00:00:21 ago * 200.2.8.2, via swp1.2(vrf vrf1002) * 200.2.9.2, via swp2.2(vrf vrf1002) * 200.2.10.2, via swp3.2(vrf vrf1002) Signed-off-by: Chirag Shah --- bgpd/bgp_mplsvpn.c | 149 +++++++++++++++++++++++++++++++++++++++++---- bgpd/bgp_mplsvpn.h | 1 + bgpd/bgp_vty.c | 3 + 3 files changed, 142 insertions(+), 11 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 355bc93320..e540815928 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -1614,11 +1614,13 @@ void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, { const char *export_name; vpn_policy_direction_t idir, edir; - char *vname; - char buf[1000]; + char *vname, *tmp_name; + char buf[RD_ADDRSTRLEN]; struct ecommunity *ecom; bool first_export = false; int debug; + struct listnode *node; + bool is_inst_match = false; export_name = to_bgp->name ? to_bgp->name : VRF_DEFAULT_NAME; idir = BGP_VPN_POLICY_DIR_FROMVPN; @@ -1634,13 +1636,41 @@ void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, vname = (from_bgp->name ? XSTRDUP(MTYPE_TMP, from_bgp->name) : XSTRDUP(MTYPE_TMP, VRF_DEFAULT_NAME)); - listnode_add(to_bgp->vpn_policy[afi].import_vrf, vname); + /* Check the import_vrf list of destination vrf for the source vrf name, + * insert otherwise. + */ + for (ALL_LIST_ELEMENTS_RO(to_bgp->vpn_policy[afi].import_vrf, + node, tmp_name)) { + if (strcmp(vname, tmp_name) == 0) { + is_inst_match = true; + break; + } + } + if (!is_inst_match) + listnode_add(to_bgp->vpn_policy[afi].import_vrf, + vname); - if (!listcount(from_bgp->vpn_policy[afi].export_vrf)) - first_export = true; + /* Check if the source vrf already exports to any vrf, + * first time export requires to setup auto derived RD/RT values. + * Add the destination vrf name to export vrf list if it is + * not present. + */ + is_inst_match = false; vname = XSTRDUP(MTYPE_TMP, export_name); - listnode_add(from_bgp->vpn_policy[afi].export_vrf, vname); - + if (!listcount(from_bgp->vpn_policy[afi].export_vrf)) { + first_export = true; + } else { + for (ALL_LIST_ELEMENTS_RO(from_bgp->vpn_policy[afi].export_vrf, + node, tmp_name)) { + if (strcmp(vname, tmp_name) == 0) { + is_inst_match = true; + break; + } + } + } + if (!is_inst_match) + listnode_add(from_bgp->vpn_policy[afi].export_vrf, + vname); /* Update import RT for current VRF using export RT of the VRF we're * importing from. First though, make sure "import_vrf" has that * set. @@ -1702,7 +1732,7 @@ void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, const char *export_name, *tmp_name; vpn_policy_direction_t idir, edir; char *vname; - struct ecommunity *ecom; + struct ecommunity *ecom = NULL; struct listnode *node; int debug; @@ -1747,10 +1777,12 @@ void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, if (to_bgp->vpn_policy[afi].import_vrf->count == 0) { UNSET_FLAG(to_bgp->af_flags[afi][safi], BGP_CONFIG_VRF_TO_VRF_IMPORT); - ecommunity_free(&to_bgp->vpn_policy[afi].rtlist[idir]); + if (to_bgp->vpn_policy[afi].rtlist[idir]) + ecommunity_free(&to_bgp->vpn_policy[afi].rtlist[idir]); } else { ecom = from_bgp->vpn_policy[afi].rtlist[edir]; - ecommunity_del_val(to_bgp->vpn_policy[afi].rtlist[idir], + if (ecom) + ecommunity_del_val(to_bgp->vpn_policy[afi].rtlist[idir], (struct ecommunity_val *)ecom->val); vpn_leak_postchange(idir, afi, bgp_get_default(), to_bgp); } @@ -1783,8 +1815,11 @@ void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, * * import_vrf and export_vrf must match in having * the in/out names as appropriate. + * export_vrf list could have been cleaned up + * as part of no router bgp source instnace. */ - assert(vname); + if (!vname) + return; listnode_delete(from_bgp->vpn_policy[afi].export_vrf, vname); XFREE(MTYPE_TMP, vname); @@ -2471,3 +2506,95 @@ void vpn_leak_postchange_all(void) bgp); } } + +/* When a bgp vrf instance is unconfigured, remove its routes + * from the VPN table and this vrf could be importing routes from other + * bgp vrf instnaces, unimport them. + * VRF X and VRF Y are exporting routes to each other. + * When VRF X is deleted, unimport its routes from all target vrfs, + * also VRF Y should unimport its routes from VRF X table. + * This will ensure VPN table is cleaned up appropriately. + */ +int bgp_vpn_leak_unimport(struct bgp *from_bgp, struct vty *vty) +{ + struct bgp *to_bgp; + const char *tmp_name; + char *vname; + struct listnode *node, *next; + safi_t safi = SAFI_UNICAST; + afi_t afi; + bool is_vrf_leak_bind; + int debug; + + if (from_bgp->inst_type != BGP_INSTANCE_TYPE_VRF) + return 0; + + debug = (BGP_DEBUG(vpn, VPN_LEAK_TO_VRF) | + BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF)); + + tmp_name = from_bgp->name ? from_bgp->name : VRF_DEFAULT_NAME; + + for (afi = 0; afi < AFI_MAX; ++afi) { + /* vrf leak is for IPv4 and IPv6 Unicast only */ + if (afi != AFI_IP && afi != AFI_IP6) + continue; + + for (ALL_LIST_ELEMENTS_RO(bm->bgp, next, to_bgp)) { + if (from_bgp == to_bgp) + continue; + + /* Unimport and remove source vrf from the + * other vrfs import list. + */ + struct vpn_policy *to_vpolicy; + + is_vrf_leak_bind = false; + to_vpolicy = &(to_bgp->vpn_policy[afi]); + for (ALL_LIST_ELEMENTS_RO(to_vpolicy->import_vrf, node, + vname)) { + if (strcmp(vname, tmp_name) == 0) { + is_vrf_leak_bind = true; + break; + } + } + /* skip this bgp instance as there is no leak to this + * vrf instance. + */ + if (!is_vrf_leak_bind) + continue; + + if (debug) + zlog_debug("%s: unimport routes from %s to_bgp %s afi %s import vrfs count %u", + __func__, from_bgp->name_pretty, + to_bgp->name_pretty, afi2str(afi), + to_vpolicy->import_vrf->count); + + vrf_unimport_from_vrf(to_bgp, from_bgp, afi, safi); + + /* readd vrf name as unimport removes import vrf name + * from the destination vrf's import list where the + * `import vrf` configuration still exist. + */ + vname = XSTRDUP(MTYPE_TMP, tmp_name); + listnode_add(to_bgp->vpn_policy[afi].import_vrf, + vname); + SET_FLAG(to_bgp->af_flags[afi][safi], + BGP_CONFIG_VRF_TO_VRF_IMPORT); + + /* If to_bgp exports its routes to the bgp vrf + * which is being deleted, un-import the + * to_bgp routes from VPN. + */ + for (ALL_LIST_ELEMENTS_RO(to_bgp->vpn_policy[afi] + .export_vrf, node, + vname)) { + if (strcmp(vname, tmp_name) == 0) { + vrf_unimport_from_vrf(from_bgp, to_bgp, + afi, safi); + break; + } + } + } + } + return 0; +} diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index 2a6c0e1708..154eb2be06 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -266,5 +266,6 @@ extern vrf_id_t get_first_vrf_for_redirect_with_rt(struct ecommunity *eckey); extern void vpn_leak_postchange_all(void); extern void vpn_handle_router_id_update(struct bgp *bgp, bool withdraw, bool is_config); +extern int bgp_vpn_leak_unimport(struct bgp *from_bgp, struct vty *vty); #endif /* _QUAGGA_BGP_MPLSVPN_H */ diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index a18a3bb952..2e836fb40f 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1085,6 +1085,9 @@ DEFUN (no_router_bgp, } } + if (bgp_vpn_leak_unimport(bgp, vty)) + return CMD_WARNING_CONFIG_FAILED; + bgp_delete(bgp); return CMD_SUCCESS; -- 2.39.5