From 9ecf931b13cfb9a5aa35253acbdfc66bd2346607 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 10 Jun 2019 17:14:42 -0700 Subject: [PATCH 1/3] 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; From 48381346d7d8daddfacdcdf1980e35a216957d1e Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 10 Jun 2019 17:19:26 -0700 Subject: [PATCH 2/3] bgpd: router bgp export leaked vpn routes two bgp vrf instance has vrf route leak configured, when a source vrf x is deleted, its leaked routes are cleaned up from the destination and vpn table. With this change when a source bgp instance is reconfigured, export its routes back to destination vrfs where it is configured as leak. Ticket:CM-20534 CM-24484 Reviewed By: Testing Done: configure vrf leak between two vrf intances, delete and readd source vrf and checked its routes exported to vpn table and leaked vrfs table. Signed-off-by: Chirag Shah --- bgpd/bgp_mplsvpn.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++ bgpd/bgp_mplsvpn.h | 1 + bgpd/bgp_vty.c | 2 ++ 3 files changed, 75 insertions(+) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index e540815928..1156810510 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -2598,3 +2598,75 @@ int bgp_vpn_leak_unimport(struct bgp *from_bgp, struct vty *vty) } return 0; } + +/* When a router bgp is configured, there could be a bgp vrf + * instance importing routes from this newly configured + * bgp vrf instance. Export routes from configured + * bgp vrf to VPN. + * VRF Y has import from bgp vrf x, + * when a bgp vrf x instance is created, export its routes + * to VRF Y instance. + */ +void bgp_vpn_leak_export(struct bgp *from_bgp) +{ + afi_t afi; + const char *export_name; + char *vname; + struct listnode *node, *next; + struct ecommunity *ecom; + vpn_policy_direction_t idir, edir; + safi_t safi = SAFI_UNICAST; + struct bgp *to_bgp; + int debug; + + debug = (BGP_DEBUG(vpn, VPN_LEAK_TO_VRF) | + BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF)); + + idir = BGP_VPN_POLICY_DIR_FROMVPN; + edir = BGP_VPN_POLICY_DIR_TOVPN; + + export_name = (from_bgp->name ? XSTRDUP(MTYPE_TMP, from_bgp->name) + : XSTRDUP(MTYPE_TMP, 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; + + /* bgp instance has import list, check to see if newly + * configured bgp instance is the list. + */ + struct vpn_policy *to_vpolicy; + + to_vpolicy = &(to_bgp->vpn_policy[afi]); + for (ALL_LIST_ELEMENTS_RO(to_vpolicy->import_vrf, + node, vname)) { + if (strcmp(vname, export_name) != 0) + continue; + + if (debug) + zlog_debug("%s: found from_bgp %s in to_bgp %s import list, import routes.", + __func__, + export_name, to_bgp->name_pretty); + + ecom = from_bgp->vpn_policy[afi].rtlist[edir]; + /* remove import rt, it will be readded + * as part of import from vrf. + */ + if (ecom) + ecommunity_del_val( + to_vpolicy->rtlist[idir], + (struct ecommunity_val *) + ecom->val); + vrf_import_from_vrf(to_bgp, from_bgp, + afi, safi); + break; + + } + } + } +} diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index 154eb2be06..3234f7fc9d 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -267,5 +267,6 @@ 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); +extern void bgp_vpn_leak_export(struct bgp *from_bgp); #endif /* _QUAGGA_BGP_MPLSVPN_H */ diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 2e836fb40f..bde4b52af5 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1003,6 +1003,8 @@ DEFUN_NOSH (router_bgp, if (is_new_bgp && inst_type == BGP_INSTANCE_TYPE_DEFAULT) vpn_leak_postchange_all(); + if (inst_type == BGP_INSTANCE_TYPE_VRF) + bgp_vpn_leak_export(bgp); /* Pending: handle when user tries to change a view to vrf n vv. */ } From b20875ea0efe62d89033282bc6c9eaecfe3318d3 Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 29 Apr 2019 11:45:49 -0700 Subject: [PATCH 3/3] bgpd: vrf leak show cmds to check deleted instance When a source bgp vrf instance is deleted, ensure the referencing of it in vrf route leak show commands. Ticket:CM-20534 CM-24484 Signed-off-by: Chirag Shah --- bgpd/bgp_vty.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index bde4b52af5..ef7cf89450 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -11329,15 +11329,19 @@ static int bgp_show_route_leak_vty(struct vty *vty, const char *name, json_object_array_add(json_import_vrfs, json_object_new_string(vname)); + json_object_object_add(json, "importFromVrfs", + json_import_vrfs); dir = BGP_VPN_POLICY_DIR_FROMVPN; - ecom_str = ecommunity_ecom2str( + if (bgp->vpn_policy[afi].rtlist[dir]) { + ecom_str = ecommunity_ecom2str( bgp->vpn_policy[afi].rtlist[dir], ECOMMUNITY_FORMAT_ROUTE_MAP, 0); - json_object_object_add(json, "importFromVrfs", - json_import_vrfs); - json_object_string_add(json, "importRts", ecom_str); - - XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + json_object_string_add(json, "importRts", + ecom_str); + XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + } else + json_object_string_add(json, "importRts", + "none"); } if (!CHECK_FLAG(bgp->af_flags[afi][safi], @@ -11361,12 +11365,16 @@ static int bgp_show_route_leak_vty(struct vty *vty, const char *name, buf1, RD_ADDRSTRLEN)); dir = BGP_VPN_POLICY_DIR_TOVPN; - ecom_str = ecommunity_ecom2str( + if (bgp->vpn_policy[afi].rtlist[dir]) { + ecom_str = ecommunity_ecom2str( bgp->vpn_policy[afi].rtlist[dir], ECOMMUNITY_FORMAT_ROUTE_MAP, 0); - json_object_string_add(json, "exportRts", ecom_str); - - XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + json_object_string_add(json, "exportRts", + ecom_str); + XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + } else + json_object_string_add(json, "exportRts", + "none"); } if (use_json) { @@ -11399,12 +11407,16 @@ static int bgp_show_route_leak_vty(struct vty *vty, const char *name, vty_out(vty, " %s\n", vname); dir = BGP_VPN_POLICY_DIR_FROMVPN; - ecom_str = ecommunity_ecom2str( + ecom_str = NULL; + if (bgp->vpn_policy[afi].rtlist[dir]) { + ecom_str = ecommunity_ecom2str( bgp->vpn_policy[afi].rtlist[dir], ECOMMUNITY_FORMAT_ROUTE_MAP, 0); - vty_out(vty, "Import RT(s): %s\n", ecom_str); + vty_out(vty, "Import RT(s): %s\n", ecom_str); - XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + } else + vty_out(vty, "Import RT(s):\n"); } if (!CHECK_FLAG(bgp->af_flags[afi][safi], @@ -11427,11 +11439,14 @@ static int bgp_show_route_leak_vty(struct vty *vty, const char *name, buf1, RD_ADDRSTRLEN)); dir = BGP_VPN_POLICY_DIR_TOVPN; - ecom_str = ecommunity_ecom2str( + if (bgp->vpn_policy[afi].rtlist[dir]) { + ecom_str = ecommunity_ecom2str( bgp->vpn_policy[afi].rtlist[dir], ECOMMUNITY_FORMAT_ROUTE_MAP, 0); - vty_out(vty, "Export RT: %s\n", ecom_str); - XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + vty_out(vty, "Export RT: %s\n", ecom_str); + XFREE(MTYPE_ECOMMUNITY_STR, ecom_str); + } else + vty_out(vty, "Import RT(s):\n"); } }