From: Donatas Abraitis Date: Wed, 9 Mar 2022 13:28:38 +0000 (+0200) Subject: bgpd: Add BGP configuration start/end markers X-Git-Tag: pim6-testing-20220430~129^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=refs%2Fpull%2F10838%2Fhead;p=mirror%2Ffrr.git bgpd: Add BGP configuration start/end markers Delay BGP configuration until we receive end-configuration hook to make sure we don't send partial updates to peer which leads to broken Graceful-Restart. Signed-off-by: David Lamparter Signed-off-by: Donatas Abraitis --- diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index dea1433f6d..3600e2f0ec 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -17599,11 +17599,67 @@ static const struct cmd_variable_handler bgp_var_peergroup[] = { {.tokenname = "PGNAME", .completions = bgp_ac_peergroup}, {.completions = NULL} }; +DEFINE_HOOK(bgp_config_end, (struct bgp *bgp), (bgp)); + +static struct thread *t_bgp_cfg; + +bool bgp_config_inprocess(void) +{ + return thread_is_scheduled(t_bgp_cfg); +} + +static void bgp_config_finish(struct thread *t) +{ + struct listnode *node; + struct bgp *bgp; + + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp)) + hook_call(bgp_config_end, bgp); +} + +static void bgp_config_start(void) +{ +#define BGP_PRE_CONFIG_MAX_WAIT_SECONDS 600 + THREAD_OFF(t_bgp_cfg); + thread_add_timer(bm->master, bgp_config_finish, NULL, + BGP_PRE_CONFIG_MAX_WAIT_SECONDS, &t_bgp_cfg); +} + +/* When we receive a hook the configuration is read, + * we start a timer to make sure we postpone sending + * EoR before route-maps are processed. + * This is especially valid if using `bgp route-map delay-timer`. + */ +static void bgp_config_end(void) +{ +#define BGP_POST_CONFIG_DELAY_SECONDS 1 + uint32_t bgp_post_config_delay = + thread_is_scheduled(bm->t_rmap_update) + ? thread_timer_remain_second(bm->t_rmap_update) + : BGP_POST_CONFIG_DELAY_SECONDS; + + /* If BGP config processing thread isn't running, then + * we can return and rely it's properly handled. + */ + if (!bgp_config_inprocess()) + return; + + THREAD_OFF(t_bgp_cfg); + + /* Start a new timer to make sure we don't send EoR + * before route-maps are processed. + */ + thread_add_timer(bm->master, bgp_config_finish, NULL, + bgp_post_config_delay, &t_bgp_cfg); +} + void bgp_vty_init(void) { cmd_variable_handler_register(bgp_var_neighbor); cmd_variable_handler_register(bgp_var_peergroup); + cmd_init_config_callbacks(bgp_config_start, bgp_config_end); + /* Install bgp top node. */ install_node(&bgp_node); install_node(&bgp_ipv4_unicast_node); diff --git a/bgpd/bgp_vty.h b/bgpd/bgp_vty.h index 93026c663a..4b393275d6 100644 --- a/bgpd/bgp_vty.h +++ b/bgpd/bgp_vty.h @@ -164,6 +164,7 @@ extern void bgp_config_write_rpkt_quanta(struct vty *vty, struct bgp *bgp); extern void bgp_config_write_listen(struct vty *vty, struct bgp *bgp); extern void bgp_config_write_coalesce_time(struct vty *vty, struct bgp *bgp); extern int bgp_vty_return(struct vty *vty, int ret); +extern bool bgp_config_inprocess(void); extern struct peer *peer_and_group_lookup_vty(struct vty *vty, const char *peer_str); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 38a106359e..b0be2d70fe 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1731,6 +1731,8 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, peer->v_routeadv = (peer_sort(peer) == BGP_PEER_IBGP) ? BGP_DEFAULT_IBGP_ROUTEADV : BGP_DEFAULT_EBGP_ROUTEADV; + if (bgp_config_inprocess()) + peer->shut_during_cfg = true; peer = peer_lock(peer); /* bgp peer list reference */ peer->group = group; @@ -7938,8 +7940,33 @@ void bgp_pthreads_finish(void) frr_pthread_stop_all(); } +static int peer_unshut_after_cfg(struct bgp *bgp) +{ + struct listnode *node; + struct peer *peer; + + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { + if (!peer->shut_during_cfg) + continue; + + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%s: released from config-pending hold", + peer->host); + + peer->shut_during_cfg = false; + if (peer_active(peer) && peer->status != Established) { + if (peer->status != Idle) + BGP_EVENT_ADD(peer, BGP_Stop); + BGP_EVENT_ADD(peer, BGP_Start); + } + } + + return 0; +} + void bgp_init(unsigned short instance) { + hook_register(bgp_config_end, peer_unshut_after_cfg); /* allocates some vital data structures used by peer commands in * vty_init */ diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index a9475f39a7..6eba48924c 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -771,6 +771,7 @@ DECLARE_HOOK(bgp_inst_delete, (struct bgp *bgp), (bgp)); DECLARE_HOOK(bgp_inst_config_write, (struct bgp *bgp, struct vty *vty), (bgp, vty)); +DECLARE_HOOK(bgp_config_end, (struct bgp *bgp), (bgp)); /* Thread callback information */ struct afi_safi_info { @@ -1676,6 +1677,8 @@ struct peer { /* Long-lived Graceful Restart */ struct llgr_info llgr[AFI_MAX][SAFI_MAX]; + bool shut_during_cfg; + QOBJ_FIELDS; }; DECLARE_QOBJ_TYPE(peer); @@ -1703,9 +1706,10 @@ DECLARE_QOBJ_TYPE(peer); /* Check if suppress start/restart of sessions to peer. */ #define BGP_PEER_START_SUPPRESSED(P) \ - (CHECK_FLAG((P)->flags, PEER_FLAG_SHUTDOWN) \ - || CHECK_FLAG((P)->sflags, PEER_STATUS_PREFIX_OVERFLOW) \ - || CHECK_FLAG((P)->bgp->flags, BGP_FLAG_SHUTDOWN)) + (CHECK_FLAG((P)->flags, PEER_FLAG_SHUTDOWN) || \ + CHECK_FLAG((P)->sflags, PEER_STATUS_PREFIX_OVERFLOW) || \ + CHECK_FLAG((P)->bgp->flags, BGP_FLAG_SHUTDOWN) || \ + (P)->shut_during_cfg) #define PEER_ROUTE_ADV_DELAY(peer) \ (CHECK_FLAG(peer->thread_flags, PEER_THREAD_SUBGRP_ADV_DELAY))