From 3103e8d22f772f101a5c0d85f5423bf1550cdbf7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 20 Mar 2018 09:18:01 -0400 Subject: [PATCH 1/2] bgpd: Fix peer withdrawal and route leaking for vpn's and vrf's When a peer is removed the routes are withdrawn via bgp_process_main_one As such we need to put a bit of code in to handle this situation for the vpn/vrf route leaking code. I think this code path is also called for when a vrf's route is changed and I believe we will end up putting a bit more code here to handle the nexthop changes. I've also started trying to document the bgp_process_main_one function a bit better. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 35732d1bf5..5e122efe8f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2095,6 +2095,25 @@ struct bgp_process_queue { unsigned int queued; }; +/* + * old_select = The old best path + * new_select = the new best path + * + * if (!old_select && new_select) + * We are sending new information on. + * + * if (old_select && new_select) { + * if (new_select != old_select) + * We have a new best path send a change + * else + * We've received a update with new attributes that needs + * to be passed on. + * } + * + * if (old_select && !new_select) + * We have no eligible route that we can announce or the rn + * is being removed. + */ static void bgp_process_main_one(struct bgp *bgp, struct bgp_node *rn, afi_t afi, safi_t safi) { @@ -3664,10 +3683,12 @@ static wq_item_status bgp_clear_route_node(struct work_queue *wq, void *data) struct bgp_node *rn = cnq->rn; struct peer *peer = wq->spec.data; struct bgp_info *ri; + struct bgp *bgp; afi_t afi = bgp_node_table(rn)->afi; safi_t safi = bgp_node_table(rn)->safi; assert(rn && peer); + bgp = peer->bgp; /* It is possible that we have multiple paths for a prefix from a peer * if that peer is using AddPath. @@ -3686,8 +3707,18 @@ static wq_item_status bgp_clear_route_node(struct work_queue *wq, void *data) /* If this is an EVPN route, process for * un-import. */ if (safi == SAFI_EVPN) - bgp_evpn_unimport_route(peer->bgp, afi, safi, + bgp_evpn_unimport_route(bgp, afi, safi, &rn->p, ri); + /* Handle withdraw for VRF route-leaking and L3VPN */ + if (SAFI_UNICAST == safi + && (bgp->inst_type == BGP_INSTANCE_TYPE_VRF || + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT)) + vpn_leak_from_vrf_withdraw(bgp_get_default(), + bgp, ri); + if (SAFI_MPLS_VPN == safi && + bgp->inst_type == BGP_INSTANCE_TYPE_DEFAULT) + vpn_leak_to_vrf_withdraw(bgp, ri); + bgp_rib_remove(rn, ri, peer, afi, safi); } } From 568e10ca581e0519eb613f0fcd08dff794e27cd3 Mon Sep 17 00:00:00 2001 From: vivek Date: Wed, 1 Nov 2017 13:36:46 -0700 Subject: [PATCH 2/2] bgpd: Use BGP instance to derive the VRF for route uninstall When uninstalling routes from zebra, ensure that the BGP instance for which processing is being done is used to derive the VRF. It is incorrect to derive the VRF from the peer when dealing with scenarios like VRF route leaking, EVPN symmetric/external routing etc., where the peer which sourced the route could belong to a different VRF. Signed-off-by: Vivek Venkatraman Reviewed-by: Donald Sharp Reviewed-by: Mitesh Kanjariya Ticket: CM-18413 Reviewed By: CCR-6778 Testing Done: Manual testing of BGP route withdraw/delete, bgp-min --- bgpd/bgp_route.c | 18 +++++++++++------- bgpd/bgp_zebra.c | 11 ++++++----- bgpd/bgp_zebra.h | 3 ++- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5e122efe8f..41dff0a7e2 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2313,7 +2313,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_node *rn, || old_select->sub_type == BGP_ROUTE_AGGREGATE || old_select->sub_type == BGP_ROUTE_IMPORTED)) - bgp_zebra_withdraw(p, old_select, safi); + bgp_zebra_withdraw(p, old_select, bgp, safi); } } @@ -3984,7 +3984,8 @@ void bgp_clear_stale_route(struct peer *peer, afi_t afi, safi_t safi) } } -static void bgp_cleanup_table(struct bgp_table *table, safi_t safi) +static void bgp_cleanup_table(struct bgp *bgp, struct bgp_table *table, + safi_t safi) { struct bgp_node *rn; struct bgp_info *ri; @@ -4000,7 +4001,8 @@ static void bgp_cleanup_table(struct bgp_table *table, safi_t safi) || ri->sub_type == BGP_ROUTE_IMPORTED)) { if (bgp_fibupd_safi(safi)) - bgp_zebra_withdraw(&rn->p, ri, safi); + bgp_zebra_withdraw(&rn->p, ri, + bgp, safi); bgp_info_reap(rn, ri); } } @@ -4015,7 +4017,8 @@ void bgp_cleanup_routes(struct bgp *bgp) for (afi = AFI_IP; afi < AFI_MAX; ++afi) { if (afi == AFI_L2VPN) continue; - bgp_cleanup_table(bgp->rib[afi][SAFI_UNICAST], SAFI_UNICAST); + bgp_cleanup_table(bgp, bgp->rib[afi][SAFI_UNICAST], + SAFI_UNICAST); /* * VPN and ENCAP and EVPN tables are two-level (RD is top level) */ @@ -4025,7 +4028,7 @@ void bgp_cleanup_routes(struct bgp *bgp) for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; rn = bgp_route_next(rn)) { if (rn->info) { - bgp_cleanup_table( + bgp_cleanup_table(bgp, (struct bgp_table *)(rn->info), safi); bgp_table_finish((struct bgp_table **)&( @@ -4038,7 +4041,7 @@ void bgp_cleanup_routes(struct bgp *bgp) for (rn = bgp_table_top(bgp->rib[afi][safi]); rn; rn = bgp_route_next(rn)) { if (rn->info) { - bgp_cleanup_table( + bgp_cleanup_table(bgp, (struct bgp_table *)(rn->info), safi); bgp_table_finish((struct bgp_table **)&( @@ -4052,7 +4055,8 @@ void bgp_cleanup_routes(struct bgp *bgp) for (rn = bgp_table_top(bgp->rib[AFI_L2VPN][SAFI_EVPN]); rn; rn = bgp_route_next(rn)) { if (rn->info) { - bgp_cleanup_table((struct bgp_table *)(rn->info), + bgp_cleanup_table(bgp, + (struct bgp_table *)(rn->info), SAFI_EVPN); bgp_table_finish((struct bgp_table **)&(rn->info)); rn->info = NULL; diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 486ea3937a..bc6ee73da8 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1303,7 +1303,8 @@ void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi) safi); } -void bgp_zebra_withdraw(struct prefix *p, struct bgp_info *info, safi_t safi) +void bgp_zebra_withdraw(struct prefix *p, struct bgp_info *info, + struct bgp *bgp, safi_t safi) { struct zapi_route api; struct peer *peer; @@ -1329,12 +1330,12 @@ void bgp_zebra_withdraw(struct prefix *p, struct bgp_info *info, safi_t safi) /* Don't try to install if we're not connected to Zebra or Zebra doesn't * know of this instance. */ - if (!bgp_install_info_to_zebra(peer->bgp)) + if (!bgp_install_info_to_zebra(bgp)) return; memset(&api, 0, sizeof(api)); memcpy(&api.rmac, &(info->attr->rmac), sizeof(struct ethaddr)); - api.vrf_id = peer->bgp->vrf_id; + api.vrf_id = bgp->vrf_id; api.type = ZEBRA_ROUTE_BGP; api.safi = safi; api.prefix = *p; @@ -1353,14 +1354,14 @@ void bgp_zebra_withdraw(struct prefix *p, struct bgp_info *info, safi_t safi) if ((peer->sort == BGP_PEER_EBGP && peer->ttl != 1) || CHECK_FLAG(peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK) - || bgp_flag_check(peer->bgp, BGP_FLAG_DISABLE_NH_CONNECTED_CHK)) + || bgp_flag_check(bgp, BGP_FLAG_DISABLE_NH_CONNECTED_CHK)) SET_FLAG(api.flags, ZEBRA_FLAG_ALLOW_RECURSION); if (bgp_debug_zebra(p)) { char buf[PREFIX_STRLEN]; prefix2str(&api.prefix, buf, sizeof(buf)); - zlog_debug("Tx route delete VRF %u %s", peer->bgp->vrf_id, buf); + zlog_debug("Tx route delete VRF %u %s", bgp->vrf_id, buf); } zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api); diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h index da5160bc16..c30f63039b 100644 --- a/bgpd/bgp_zebra.h +++ b/bgpd/bgp_zebra.h @@ -33,7 +33,8 @@ extern void bgp_config_write_redistribute(struct vty *, struct bgp *, afi_t, extern void bgp_zebra_announce(struct bgp_node *, struct prefix *, struct bgp_info *, struct bgp *, afi_t, safi_t); extern void bgp_zebra_announce_table(struct bgp *, afi_t, safi_t); -extern void bgp_zebra_withdraw(struct prefix *, struct bgp_info *, safi_t); +extern void bgp_zebra_withdraw(struct prefix *, struct bgp_info *, + struct bgp *, safi_t); extern void bgp_zebra_initiate_radv(struct bgp *bgp, struct peer *peer); extern void bgp_zebra_terminate_radv(struct bgp *bgp, struct peer *peer);