diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 50c54a021d68..138d63f7a5f0 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3399,8 +3399,8 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, || new_select->sub_type == BGP_ROUTE_IMPORTED)) - bgp_zebra_announce(dest, old_select, - bgp); + bgp_zebra_route_install(dest, old_select, + bgp, true); } } @@ -3517,9 +3517,10 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, */ if (old_select && is_route_parent_evpn(old_select)) - bgp_zebra_withdraw(dest, old_select, bgp); + bgp_zebra_route_install(dest, old_select, bgp, + false); - bgp_zebra_announce(dest, new_select, bgp); + bgp_zebra_route_install(dest, new_select, bgp, true); } else { /* Withdraw the route from the kernel. */ if (old_select && old_select->type == ZEBRA_ROUTE_BGP @@ -3527,7 +3528,8 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, || old_select->sub_type == BGP_ROUTE_AGGREGATE || old_select->sub_type == BGP_ROUTE_IMPORTED)) - bgp_zebra_withdraw(dest, old_select, bgp); + bgp_zebra_route_install(dest, old_select, bgp, + false); } } @@ -4424,7 +4426,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, if (pi && pi->attr->rmap_table_id != new_attr.rmap_table_id) { if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) /* remove from RIB previous entry */ - bgp_zebra_withdraw(dest, pi, bgp); + bgp_zebra_route_install(dest, pi, bgp, false); } if (peer->sort == BGP_PEER_EBGP) { @@ -6050,7 +6052,7 @@ static void bgp_cleanup_table(struct bgp *bgp, struct bgp_table *table, || pi->sub_type == BGP_ROUTE_IMPORTED)) { if (bgp_fibupd_safi(safi)) - bgp_zebra_withdraw(dest, pi, bgp); + bgp_zebra_withdraw_actual(dest, pi, bgp); } dest = bgp_path_info_reap(dest, pi); diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h index 94bd816f55bf..3062407b6eeb 100644 --- a/bgpd/bgp_table.h +++ b/bgpd/bgp_table.h @@ -77,6 +77,8 @@ struct bgp_dest { STAILQ_ENTRY(bgp_dest) pq; struct zebra_announce_item zai; + struct bgp_path_info *za_bgp_pi; + struct bgp *za_bgp; uint64_t version; @@ -93,6 +95,8 @@ struct bgp_dest { #define BGP_NODE_LABEL_REQUESTED (1 << 7) #define BGP_NODE_SOFT_RECONFIG (1 << 8) #define BGP_NODE_PROCESS_CLEAR (1 << 9) +#define BGP_NODE_SCHEDULE_FOR_INSTALL (1 << 10) +#define BGP_NODE_SCHEDULE_FOR_DELETE (1 << 11) struct bgp_addpath_node_data tx_addpath; diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index c2216995bcae..d6ebdb3aa5ea 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1524,8 +1524,9 @@ static void bgp_debug_zebra_nh(struct zapi_route *api) } } -void bgp_zebra_announce(struct bgp_dest *dest, struct bgp_path_info *info, - struct bgp *bgp) +static enum zclient_send_status +bgp_zebra_announce_actual(struct bgp_dest *dest, struct bgp_path_info *info, + struct bgp *bgp) { struct bgp_path_info *bpi_ultimate; struct zapi_route api = { 0 }; @@ -1541,27 +1542,10 @@ void bgp_zebra_announce(struct bgp_dest *dest, struct bgp_path_info *info, struct bgp_table *bgp_tbl = bgp_dest_table(dest); const struct prefix *p = bgp_dest_get_prefix(dest); - /* - * BGP is installing this route and bgp has been configured - * to suppress announcements until the route has been installed - * let's set the fact that we expect this route to be installed - */ - if (BGP_SUPPRESS_FIB_ENABLED(bgp)) - SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING); - - /* 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(bgp)) - return; - - if (bgp->main_zebra_update_hold) - return; - if (bgp_tbl->safi == SAFI_FLOWSPEC) { bgp_pbr_update_entry(bgp, p, info, bgp_tbl->afi, bgp_tbl->safi, true); - return; + return ZCLIENT_SEND_SUCCESS; } /* Make Zebra API structure. */ @@ -1678,10 +1662,11 @@ void bgp_zebra_announce(struct bgp_dest *dest, struct bgp_path_info *info, zlog_debug("%s: %pFX: announcing to zebra (recursion %sset)", __func__, p, (recursion_flag ? "" : "NOT ")); } - zclient_route_send(is_add ? ZEBRA_ROUTE_ADD : ZEBRA_ROUTE_DELETE, - zclient, &api); + return zclient_route_send(is_add ? ZEBRA_ROUTE_ADD : ZEBRA_ROUTE_DELETE, + zclient, &api); } + /* Announce all routes of a table to zebra */ void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi) { @@ -1707,7 +1692,7 @@ void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi) && (pi->sub_type == BGP_ROUTE_NORMAL || pi->sub_type == BGP_ROUTE_IMPORTED))) - bgp_zebra_announce(dest, pi, bgp); + bgp_zebra_route_install(dest, pi, bgp, true); } /* Announce routes of any bgp subtype of a table to zebra */ @@ -1729,34 +1714,23 @@ void bgp_zebra_announce_table_all_subtypes(struct bgp *bgp, afi_t afi, for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED) && pi->type == ZEBRA_ROUTE_BGP) - bgp_zebra_announce(dest, pi, bgp); + bgp_zebra_route_install(dest, pi, bgp, true); } -void bgp_zebra_withdraw(struct bgp_dest *dest, struct bgp_path_info *info, - struct bgp *bgp) +enum zclient_send_status bgp_zebra_withdraw_actual(struct bgp_dest *dest, + struct bgp_path_info *info, + struct bgp *bgp) { struct zapi_route api; struct peer *peer; struct bgp_table *bgp_tbl = bgp_dest_table(dest); const struct prefix *p = bgp_dest_get_prefix(dest); - /* - * If we are withdrawing the route, we don't need to have this - * flag set. So unset it. - */ - UNSET_FLAG(info->net->flags, BGP_NODE_FIB_INSTALL_PENDING); - - /* 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(bgp)) - return; - if (bgp_tbl->safi == SAFI_FLOWSPEC) { peer = info->peer; bgp_pbr_update_entry(peer->bgp, p, info, bgp_tbl->afi, bgp_tbl->safi, false); - return; + return ZCLIENT_SEND_SUCCESS; } memset(&api, 0, sizeof(api)); @@ -1774,7 +1748,173 @@ void bgp_zebra_withdraw(struct bgp_dest *dest, struct bgp_path_info *info, zlog_debug("Tx route delete VRF %u %pFX", bgp->vrf_id, &api.prefix); - zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api); + return zclient_route_send(ZEBRA_ROUTE_DELETE, zclient, &api); +} + +/* + * Walk the new Fifo list one by one and invoke bgp_zebra_announce/withdraw + * to install/withdraw the routes to zebra. + * + * If status = ZCLIENT_SEND_SUCCESS (Buffer empt)y i.e. Zebra is free to + * receive more incoming data, then pick the next item on the list and + * continue processing. + * + * If status = ZCLIENT_SEND_BUFFERED (Buffer pending) i.e. Zebra is busy, + * break and bail out of the function because once at some point when zebra + * is free, a callback is triggered which inturn call this same function and + * continue processing items on list. + */ +#define ZEBRA_LOOP 1000 +static void bgp_handle_route_announcements_to_zebra(struct event *e) +{ + uint32_t count = 0; + struct bgp_dest *dest = NULL; + struct bgp_table *bgp_tbl = NULL; + enum zclient_send_status status = ZCLIENT_SEND_SUCCESS; + + /* + * In case users performs "no router bgp" or "SIGINT/SIGTERM", + * we can have a case where the write buffer callback when invoked after + * bgp_delete() can lead to accessing a freed table dest. Hence the below. + */ + if (bm->terminating || !bm->zebra_announce_head.sh.first) + return; + + while (count < ZEBRA_LOOP) { + dest = zebra_announce_pop(&bm->zebra_announce_head); + + if (!dest) + break; + + bgp_tbl = bgp_dest_table(dest); + + if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL)) { + status = bgp_zebra_announce_actual(dest, dest->za_bgp_pi, + bgp_tbl->bgp); + UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL); + } else if (CHECK_FLAG(dest->flags, + BGP_NODE_SCHEDULE_FOR_DELETE)) { + status = bgp_zebra_withdraw_actual(dest, dest->za_bgp_pi, + bgp_tbl->bgp); + UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE); + } + + bgp_path_info_unlock(dest->za_bgp_pi); + dest->za_bgp_pi = NULL; + dest->za_bgp = NULL; + bgp_dest_unlock_node(dest); + + if (status == ZCLIENT_SEND_BUFFERED) + break; + + count++; + } + + if (status != ZCLIENT_SEND_BUFFERED && + zebra_announce_count(&bm->zebra_announce_head)) + event_add_event(bm->master, + bgp_handle_route_announcements_to_zebra, NULL, + 0, &bm->t_bgp_zebra_route); +} + +/* + * Callback function invoked when zclient_flush_data() receives a BUFFER_EMPTY + * i.e. zebra is free to receive more incoming data. + */ +static void bgp_zebra_buffer_write_ready(void) +{ + bgp_handle_route_announcements_to_zebra(NULL); +} + +/* + * BGP is now keeping a list of dests with the dest having a pointer + * to the bgp_path_info that it will be working on. + * Here is the sequence of events that should happen: + * + * Current State New State Action + * ------------- --------- ------ + * ---- Install Place dest on list, save pi, mark + * as going to be installed + * ---- Withdrawal Place dest on list, save pi, mark + * as going to be deleted + * + * Install Install Leave dest on list, release old pi, + * save new pi, mark as going to be + * Installed + * Install Withdrawal Leave dest on list, release old pi, + * save new pi, mark as going to be + * withdrawan, remove install flag + * + * Withdrawal Install Special case, send withdrawal immediately + * Leave dest on list, release old pi, + * save new pi, mark as going to be + * installed. + * Withdrawal Withdrawal Leave dest on list, release old pi, + * save new pi, mark as going to be + * withdrawn. + */ +void bgp_zebra_route_install(struct bgp_dest *dest, struct bgp_path_info *info, + struct bgp *bgp, bool install) +{ + /* + * BGP is installing this route and bgp has been configured + * to suppress announcements until the route has been installed + * let's set the fact that we expect this route to be installed + */ + if (install) { + if (BGP_SUPPRESS_FIB_ENABLED(bgp)) + SET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING); + + if (bgp->main_zebra_update_hold) + return; + } else { + UNSET_FLAG(dest->flags, BGP_NODE_FIB_INSTALL_PENDING); + } + + /* + * 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(bgp)) + return; + + if (!CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL) && + !CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE)) { + zebra_announce_add_tail(&bm->zebra_announce_head, dest); + /* + * If neither flag is set and za_bgp_pi is not set then it is a bug + */ + assert(!dest->za_bgp_pi); + dest->za_bgp = bgp; + dest->za_bgp_pi = info; + bgp_path_info_lock(info); + bgp_dest_lock_node(dest); + } else if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL)) { + assert(dest->za_bgp_pi); + bgp_path_info_unlock(dest->za_bgp_pi); + bgp_path_info_lock(info); + dest->za_bgp_pi = info; + } else if (CHECK_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE)) { + assert(dest->za_bgp_pi); + if (install) + bgp_zebra_withdraw_actual(dest, dest->za_bgp_pi, bgp); + + bgp_path_info_unlock(dest->za_bgp_pi); + bgp_path_info_lock(info); + dest->za_bgp_pi = info; + } + + if (install) { + UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE); + SET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL); + } else { + UNSET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_INSTALL); + SET_FLAG(dest->flags, BGP_NODE_SCHEDULE_FOR_DELETE); + } + + event_add_event(bm->master, bgp_handle_route_announcements_to_zebra, + NULL, 0, &bm->t_bgp_zebra_route); } /* Withdraw all entries in a BGP instances RIB table from Zebra */ @@ -1795,7 +1935,7 @@ void bgp_zebra_withdraw_table_all_subtypes(struct bgp *bgp, afi_t afi, safi_t sa for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) { if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED) && (pi->type == ZEBRA_ROUTE_BGP)) - bgp_zebra_withdraw(dest, pi, bgp); + bgp_zebra_route_install(dest, pi, bgp, false); } } } @@ -3470,6 +3610,7 @@ void bgp_zebra_init(struct event_loop *master, unsigned short instance) zclient = zclient_new(master, &zclient_options_default, bgp_handlers, array_size(bgp_handlers)); zclient_init(zclient, ZEBRA_ROUTE_BGP, 0, &bgpd_privs); + zclient->zebra_buffer_write_ready = bgp_zebra_buffer_write_ready; zclient->zebra_connected = bgp_zebra_connected; zclient->zebra_capabilities = bgp_zebra_capabilities; zclient->nexthop_update = bgp_nexthop_update; diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h index 3a7fe6501851..9c033197f7db 100644 --- a/bgpd/bgp_zebra.h +++ b/bgpd/bgp_zebra.h @@ -28,11 +28,10 @@ extern void bgp_zebra_destroy(void); extern int bgp_zebra_get_table_range(struct zclient *zc, uint32_t chunk_size, uint32_t *start, uint32_t *end); extern int bgp_if_update_all(void); -extern void bgp_zebra_announce(struct bgp_dest *dest, - struct bgp_path_info *path, struct bgp *bgp); +extern void bgp_zebra_route_install(struct bgp_dest *dest, + struct bgp_path_info *path, struct bgp *bgp, + bool install); extern void bgp_zebra_announce_table(struct bgp *bgp, afi_t afi, safi_t safi); -extern void bgp_zebra_withdraw(struct bgp_dest *dest, - struct bgp_path_info *path, struct bgp *bgp); /* Announce routes of any bgp subtype of a table to zebra */ extern void bgp_zebra_announce_table_all_subtypes(struct bgp *bgp, afi_t afi, @@ -125,4 +124,7 @@ extern void bgp_zebra_send_nexthop_label(int cmd, mpls_label_t label, extern bool bgp_zebra_request_label_range(uint32_t base, uint32_t chunk_size, bool label_auto); extern void bgp_zebra_release_label_range(uint32_t start, uint32_t end); +extern enum zclient_send_status +bgp_zebra_withdraw_actual(struct bgp_dest *dest, struct bgp_path_info *info, + struct bgp *bgp); #endif /* _QUAGGA_BGP_ZEBRA_H */ diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 650148859eb3..7061860d3fd2 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3847,10 +3847,26 @@ int bgp_delete(struct bgp *bgp) afi_t afi; safi_t safi; int i; + struct bgp_dest *dest = NULL; struct graceful_restart_info *gr_info; assert(bgp); + /* + * In case of a no router bgp on a particular instance, we dont have to + * the entire zebra_announce list. Only when bm->bgp is NULL (No more + * BGP instances), we flush the entire zebra_announce list. + */ + while (zebra_announce_count(&bm->zebra_announce_head)) { + dest = zebra_announce_pop(&bm->zebra_announce_head); + if (dest->za_bgp == bgp) { + bgp_path_info_unlock(dest->za_bgp_pi); + bgp_dest_unlock_node(dest); + } else + zebra_announce_add_tail(&bm->zebra_announce_head, dest); + } + zebra_announce_fini(&bm->zebra_announce_head); + bgp_soft_reconfig_table_task_cancel(bgp, NULL, NULL); /* make sure we withdraw any exported routes */ @@ -4011,6 +4027,9 @@ int bgp_delete(struct bgp *bgp) /* Free interfaces in this instance. */ bgp_if_finish(bgp); + if (listcount(bm->bgp) == 0) + zebra_announce_fini(&bm->zebra_announce_head); + vrf = bgp_vrf_lookup_by_instance_type(bgp); bgp_handle_socket(bgp, vrf, VRF_UNKNOWN, false); if (vrf) @@ -8309,6 +8328,7 @@ void bgp_master_init(struct event_loop *master, const int buffer_size, bm->outq_limit = BM_DEFAULT_Q_LIMIT; bm->t_bgp_sync_label_manager = NULL; bm->t_bgp_start_label_manager = NULL; + bm->t_bgp_zebra_route = NULL; bgp_mac_init(); /* init the rd id space. @@ -8559,6 +8579,7 @@ void bgp_terminate(void) EVENT_OFF(bm->t_rmap_update); EVENT_OFF(bm->t_bgp_sync_label_manager); EVENT_OFF(bm->t_bgp_start_label_manager); + EVENT_OFF(bm->t_bgp_zebra_route); bgp_mac_finish(); } diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index a1c7791c35e1..53289665c057 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -175,6 +175,8 @@ struct bgp_master { struct event *t_bgp_sync_label_manager; struct event *t_bgp_start_label_manager; + struct event *t_bgp_zebra_route; + bool v6_with_v4_nexthops; /* To preserve ordering of installations into zebra across all Vrfs */ diff --git a/lib/zclient.c b/lib/zclient.c index 51ebb5627557..4cbd04c11693 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -282,6 +282,7 @@ static void zclient_flush_data(struct event *thread) zclient->sock, &zclient->t_write); break; case BUFFER_EMPTY: + /* Currently only Sharpd and Bgpd has callbacks defined */ if (zclient->zebra_buffer_write_ready) (*zclient->zebra_buffer_write_ready)(); break;