From 15403f521a12b668e87ef8961c78e0ed97c6ff92 Mon Sep 17 00:00:00 2001 From: vivek Date: Sun, 25 Oct 2020 11:31:42 -0700 Subject: [PATCH] bgpd: Streamline GR config, act on change immediately Streamline the BGP graceful-restart configuration at the global and peer level some more. Similar to many other neighbor capability parameters like MP and ENHE, reset the session immediately upon a change to the configuration. This will be more aligned with the transactional UI model also and will not require a separate 'clear' command to be executed. Note: Peer-group graceful-restart configuration is not yet supported. Signed-off-by: Vivek Venkatraman --- bgpd/bgp_fsm.c | 379 +++++++++++++++++-------------------------------- bgpd/bgp_fsm.h | 5 +- bgpd/bgp_vty.c | 124 ++++------------ bgpd/bgp_vty.h | 47 +++--- bgpd/bgpd.c | 4 +- 5 files changed, 187 insertions(+), 372 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index d41ef8abba..8e1462f24b 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -2695,53 +2695,56 @@ int bgp_event_update(struct peer_connection *connection, } /* BGP GR Code */ -int bgp_gr_lookup_n_update_all_peer(struct bgp *bgp, - enum global_mode global_new_state, - enum global_mode global_old_state) +static inline void +bgp_peer_inherit_global_gr_mode(struct peer *peer, + enum global_mode global_gr_mode) +{ + switch (global_gr_mode) { + case GLOBAL_HELPER: + BGP_PEER_GR_HELPER_ENABLE(peer); + case GLOBAL_GR: + BGP_PEER_GR_ENABLE(peer); + case GLOBAL_DISABLE: + BGP_PEER_GR_DISABLE(peer); + default: + zlog_err("Unexpected Global GR mode %d", global_gr_mode); + } +} + +static void bgp_gr_update_mode_of_all_peers(struct bgp *bgp, + enum global_mode global_new_state) { struct peer *peer = {0}; struct listnode *node = {0}; struct listnode *nnode = {0}; enum peer_mode peer_old_state = PEER_INVALID; - for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%s [BGP_GR] Peer: (%s) :", __func__, - peer->host); + /* TODO: Need to handle peer-groups. */ + for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { peer_old_state = bgp_peer_gr_mode_get(peer); + if (peer_old_state != PEER_GLOBAL_INHERIT) + continue; - if (peer_old_state == PEER_GLOBAL_INHERIT) { + bgp_peer_inherit_global_gr_mode(peer, global_new_state); + bgp_peer_gr_flags_update(peer); - /* - *Reset only these peers and send a - *new open message with the change capabilities. - *Considering the mode to be "global_new_state" and - *do all operation accordingly - */ + if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) + zlog_debug("%pBP: Inherited Global GR mode, GR flags 0x%x peer flags 0x%" PRIx64 + "...resetting session", + peer, peer->peer_gr_new_status_flag, + peer->flags); - switch (global_new_state) { - case GLOBAL_HELPER: - BGP_PEER_GR_HELPER_ENABLE(peer); - break; - case GLOBAL_GR: - BGP_PEER_GR_ENABLE(peer); - break; - case GLOBAL_DISABLE: - BGP_PEER_GR_DISABLE(peer); - break; - case GLOBAL_INVALID: - zlog_debug("%s [BGP_GR] GLOBAL_INVALID", - __func__); - return BGP_ERR_GR_OPERATION_FAILED; - } - } + /* Reset session to match with behavior for other peer + * configs that require the session to be re-setup. + */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) { + peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + } else + bgp_session_reset(peer); } - - bgp->global_gr_present_state = global_new_state; - - return BGP_GR_SUCCESS; } int bgp_gr_update_all(struct bgp *bgp, enum global_gr_command global_gr_cmd) @@ -2749,46 +2752,27 @@ int bgp_gr_update_all(struct bgp *bgp, enum global_gr_command global_gr_cmd) enum global_mode global_new_state = GLOBAL_INVALID; enum global_mode global_old_state = GLOBAL_INVALID; - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%s [BGP_GR]START: global_gr_cmd :%s:", __func__, - print_global_gr_cmd(global_gr_cmd)); - global_old_state = bgp_global_gr_mode_get(bgp); + global_new_state = bgp->GLOBAL_GR_FSM[global_old_state][global_gr_cmd]; if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] global_old_gr_state :%s:", - print_global_gr_mode(global_old_state)); + zlog_debug("%s: Handle GR command %s, current GR state %s, new GR state %s", + bgp->name_pretty, print_global_gr_cmd(global_gr_cmd), + print_global_gr_mode(global_old_state), + print_global_gr_mode(global_new_state)); - if (global_old_state != GLOBAL_INVALID) { - global_new_state = - bgp->GLOBAL_GR_FSM[global_old_state][global_gr_cmd]; - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] global_new_gr_state :%s:", - print_global_gr_mode(global_new_state)); - } else { - zlog_err("%s [BGP_GR] global_old_state == GLOBAL_INVALID", - __func__); + if (global_old_state == GLOBAL_INVALID) return BGP_ERR_GR_OPERATION_FAILED; - } - - if (global_new_state == GLOBAL_INVALID) { - zlog_err("%s [BGP_GR] global_new_state == GLOBAL_INVALID", - __func__); + if (global_new_state == GLOBAL_INVALID) return BGP_ERR_GR_INVALID_CMD; - } - if (global_new_state == global_old_state) { - /* Trace msg */ - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "%s [BGP_GR] global_new_state == global_old_state :%s", - __func__, - print_global_gr_mode(global_new_state)); + if (global_new_state == global_old_state) return BGP_GR_NO_OPERATION; - } - return bgp_gr_lookup_n_update_all_peer(bgp, global_new_state, - global_old_state); + /* Update global GR mode and process all peers in instance. */ + bgp->global_gr_present_state = global_new_state; + bgp_gr_update_mode_of_all_peers(bgp, global_new_state); + + return BGP_GR_SUCCESS; } const char *print_peer_gr_mode(enum peer_mode pr_mode) @@ -2903,179 +2887,101 @@ int bgp_neighbor_graceful_restart(struct peer *peer, { enum peer_mode peer_new_state = PEER_INVALID; enum peer_mode peer_old_state = PEER_INVALID; - struct bgp_peer_gr peer_state; + struct bgp_peer_gr gr_fsm; int result = BGP_GR_FAILURE; - /* - * fetch peer_old_state from peer structure also - * fetch global_old_state from bgp structure, - * peer had a back pointer to bgpo struct ; - */ - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%s [BGP_GR] START:Peer: (%s) : peer_gr_cmd :%s:", - __func__, peer->host, - print_peer_gr_cmd(peer_gr_cmd)); - peer_old_state = bgp_peer_gr_mode_get(peer); + gr_fsm = peer->PEER_GR_FSM[peer_old_state][peer_gr_cmd]; + peer_new_state = gr_fsm.next_state; if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%s [BGP_GR] peer_old_state: %d", __func__, - peer_old_state); + zlog_debug("%pBP: Handle GR command %s, current GR state %s, new GR state %s", + peer, print_peer_gr_cmd(peer_gr_cmd), + print_peer_gr_mode(peer_old_state), + print_peer_gr_mode(peer_new_state)); if (peer_old_state == PEER_INVALID) return BGP_ERR_GR_OPERATION_FAILED; - peer_state = peer->PEER_GR_FSM[peer_old_state][peer_gr_cmd]; - peer_new_state = peer_state.next_state; - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%s [BGP_GR] peer_new_state: %d", __func__, - peer_new_state); - if (peer_new_state == PEER_INVALID) return BGP_ERR_GR_INVALID_CMD; - if (peer_new_state != peer_old_state) { - result = peer_state.action_fun(peer, peer_old_state, - peer_new_state); - } else { - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] peer_old_state == peer_new_state !"); + if (peer_new_state == peer_old_state) return BGP_GR_NO_OPERATION; - } - if (result == BGP_GR_SUCCESS) { - - /* Update the mode i.e peer_new_state into the peer structure */ - peer->peer_gr_present_state = peer_new_state; - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] Successfully change the state of the peer to : %s : !", - print_peer_gr_mode(peer_new_state)); - - return BGP_GR_SUCCESS; - } + result = gr_fsm.action_fun(peer, peer_old_state, peer_new_state); return result; } -unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_peer_state, - enum peer_mode new_peer_state) +static inline bool gr_mode_matches(enum peer_mode peer_gr_mode, + enum global_mode global_gr_mode) { - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "%s [BGP_GR] Move peer from old_peer_state :%s: to new_peer_state :%s: !!!!", - __func__, print_peer_gr_mode(old_peer_state), - print_peer_gr_mode(new_peer_state)); + if ((peer_gr_mode == PEER_HELPER && global_gr_mode == GLOBAL_HELPER) || + (peer_gr_mode == PEER_GR && global_gr_mode == GLOBAL_GR) || + (peer_gr_mode == PEER_DISABLE && global_gr_mode == GLOBAL_DISABLE)) + return true; + return false; +} - enum global_mode bgp_gr_global_mode = GLOBAL_INVALID; - unsigned int ret = BGP_GR_FAILURE; +unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_state, + enum peer_mode new_state) +{ + enum global_mode global_gr_mode = bgp_global_gr_mode_get(peer->bgp); + bool session_reset = true; - if (old_peer_state == new_peer_state) { - /* Nothing to do over here as the present and old state is the - * same */ + if (old_state == new_state) return BGP_GR_NO_OPERATION; - } - if ((old_peer_state == PEER_INVALID) - || (new_peer_state == PEER_INVALID)) { - /* something bad happend , print error message */ + if ((old_state == PEER_INVALID) || (new_state == PEER_INVALID)) return BGP_ERR_GR_INVALID_CMD; - } - - bgp_gr_global_mode = bgp_global_gr_mode_get(peer->bgp); - - if ((old_peer_state == PEER_GLOBAL_INHERIT) - && (new_peer_state != PEER_GLOBAL_INHERIT)) { - /* fetch the Mode running in the Global state machine - *from the bgp structure into a variable called - *bgp_gr_global_mode - */ - - /* Here we are checking if the - *1. peer_new_state == global_mode == helper_mode - *2. peer_new_state == global_mode == GR_mode - *3. peer_new_state == global_mode == disabled_mode - */ + global_gr_mode = bgp_global_gr_mode_get(peer->bgp); + if ((old_state == PEER_GLOBAL_INHERIT) && + (new_state != PEER_GLOBAL_INHERIT)) { BGP_PEER_GR_GLOBAL_INHERIT_UNSET(peer); - if ((int)new_peer_state == (int)bgp_gr_global_mode) { - /* This is incremental updates i.e no tear down - * of the existing session - * as the peer is already working in the same mode. + if (gr_mode_matches(new_state, global_gr_mode)) + /* Peer was inheriting the global state and + * its new state still is the same, so a + * session reset is not needed. */ - ret = BGP_GR_SUCCESS; - } else { - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] Peer state changed from :%s ", - print_peer_gr_mode(old_peer_state)); - - bgp_peer_move_to_gr_mode(peer, new_peer_state); - - ret = BGP_GR_SUCCESS; - } - } - /* In the case below peer is going into Global inherit mode i.e. - * the peer would work as the mode configured at the global level - */ - else if ((new_peer_state == PEER_GLOBAL_INHERIT) - && (old_peer_state != PEER_GLOBAL_INHERIT)) { - /* Here in this case it would be destructive - * in all the cases except one case when, - * Global GR is configured Disabled - * and present_peer_state is not disable - */ - + session_reset = false; + } else if ((new_state == PEER_GLOBAL_INHERIT) && + (old_state != PEER_GLOBAL_INHERIT)) { BGP_PEER_GR_GLOBAL_INHERIT_SET(peer); - if ((int)old_peer_state == (int)bgp_gr_global_mode) { - /* This is incremental updates - *i.e no tear down of the existing session - *as the peer is already working in the same mode. - */ - ret = BGP_GR_SUCCESS; - } else { - /* Destructive always */ - /* Tear down the old session - * and send the new capability - * as per the bgp_gr_global_mode + if (gr_mode_matches(old_state, global_gr_mode)) + /* Peer is inheriting the global state and + * its old state was also the same, so a + * session reset is not needed. */ + session_reset = false; + } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] Peer state changed from :%s", - print_peer_gr_mode(old_peer_state)); - - bgp_peer_move_to_gr_mode(peer, bgp_gr_global_mode); + /* Ensure we move to the new state and update flags */ + bgp_peer_move_to_gr_mode(peer, new_state); - ret = BGP_GR_SUCCESS; - } - } else { - /* - *This else case, it include all the cases except --> - *(new_peer_state != Peer_Global) && - *( old_peer_state != Peer_Global ) + if (session_reset) { + /* Reset session to match with behavior for other peer + * configs that require the session to be re-setup. */ - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] Peer state changed from :%s", - print_peer_gr_mode(old_peer_state)); - - bgp_peer_move_to_gr_mode(peer, new_peer_state); - - ret = BGP_GR_SUCCESS; + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->connection->status)) { + peer->last_reset = PEER_DOWN_CAPABILITY_CHANGE; + bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + } else + bgp_session_reset(peer); } - return ret; + return BGP_GR_SUCCESS; } -inline void bgp_peer_move_to_gr_mode(struct peer *peer, int new_state) +void bgp_peer_move_to_gr_mode(struct peer *peer, enum peer_mode new_state) { - int bgp_global_gr_mode = bgp_global_gr_mode_get(peer->bgp); + enum global_mode global_gr_mode = bgp_global_gr_mode_get(peer->bgp); + enum peer_mode old_state = bgp_peer_gr_mode_get(peer); switch (new_state) { case PEER_HELPER: @@ -3089,57 +2995,38 @@ inline void bgp_peer_move_to_gr_mode(struct peer *peer, int new_state) break; case PEER_GLOBAL_INHERIT: BGP_PEER_GR_GLOBAL_INHERIT_SET(peer); - - if (bgp_global_gr_mode == GLOBAL_HELPER) { - BGP_PEER_GR_HELPER_ENABLE(peer); - } else if (bgp_global_gr_mode == GLOBAL_GR) { - BGP_PEER_GR_ENABLE(peer); - } else if (bgp_global_gr_mode == GLOBAL_DISABLE) { - BGP_PEER_GR_DISABLE(peer); - } else { - zlog_err( - "[BGP_GR] Default switch inherit mode ::: SOMETHING IS WRONG !!!"); - } + bgp_peer_inherit_global_gr_mode(peer, global_gr_mode); break; + case PEER_INVALID: default: zlog_err( "[BGP_GR] Default switch mode ::: SOMETHING IS WRONG !!!"); break; } + bgp_peer_gr_flags_update(peer); + peer->peer_gr_present_state = new_state; + if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] Peer state changed --to--> : %d : !", - new_state); + zlog_debug("%pBP: Peer GR mode changed from %s to %s, GR flags 0x%x peer flags 0x%" PRIx64, + peer, print_peer_gr_mode(old_state), + print_peer_gr_mode(new_state), + peer->peer_gr_new_status_flag, peer->flags); } void bgp_peer_gr_flags_update(struct peer *peer) { - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("%s [BGP_GR] called !", __func__); if (CHECK_FLAG(peer->peer_gr_new_status_flag, PEER_GRACEFUL_RESTART_NEW_STATE_HELPER)) SET_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER); else UNSET_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER); - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] Peer %s Flag PEER_FLAG_GRACEFUL_RESTART_HELPER : %s : !", - peer->host, - (CHECK_FLAG(peer->flags, - PEER_FLAG_GRACEFUL_RESTART_HELPER) - ? "Set" - : "UnSet")); + if (CHECK_FLAG(peer->peer_gr_new_status_flag, PEER_GRACEFUL_RESTART_NEW_STATE_RESTART)) SET_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART); else UNSET_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART); - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] Peer %s Flag PEER_FLAG_GRACEFUL_RESTART : %s : !", - peer->host, - (CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART) - ? "Set" - : "UnSet")); + if (CHECK_FLAG(peer->peer_gr_new_status_flag, PEER_GRACEFUL_RESTART_NEW_STATE_INHERIT)) SET_FLAG(peer->flags, @@ -3147,28 +3034,28 @@ void bgp_peer_gr_flags_update(struct peer *peer) else UNSET_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_GLOBAL_INHERIT); + if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] Peer %s Flag PEER_FLAG_GRACEFUL_RESTART_GLOBAL_INHERIT : %s : !", - peer->host, - (CHECK_FLAG(peer->flags, - PEER_FLAG_GRACEFUL_RESTART_GLOBAL_INHERIT) - ? "Set" - : "UnSet")); - - if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART) - && !CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER)) { - zlog_debug("[BGP_GR] Peer %s UNSET PEER_STATUS_NSF_MODE!", - peer->host); + zlog_debug("%pBP: Peer flags updated to 0x%" PRIx64 + ", GR flags 0x%x, GR mode %s", + peer, peer->flags, peer->peer_gr_new_status_flag, + print_peer_gr_mode(bgp_peer_gr_mode_get(peer))); + /* + * If GR has been completely disabled for the peer and we were + * acting as the Helper for the peer (i.e., keeping stale routes + * and running the restart timer or stalepath timer), clear those + * states. + */ + if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART) && + !CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER)) { UNSET_FLAG(peer->sflags, PEER_STATUS_NSF_MODE); if (CHECK_FLAG(peer->sflags, PEER_STATUS_NSF_WAIT)) { - + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%pBP: GR disabled, stopping NSF and clearing stale routes", + peer); peer_nsf_stop(peer); - zlog_debug( - "[BGP_GR] Peer %s UNSET PEER_STATUS_NSF_WAIT!", - peer->host); } } } diff --git a/bgpd/bgp_fsm.h b/bgpd/bgp_fsm.h index bcdd49193f..85c488962f 100644 --- a/bgpd/bgp_fsm.h +++ b/bgpd/bgp_fsm.h @@ -151,7 +151,7 @@ int bgp_neighbor_graceful_restart(struct peer *peer, enum peer_gr_command peer_gr_cmd); unsigned int bgp_peer_gr_action(struct peer *peer, enum peer_mode old_peer_state, enum peer_mode new_peer_state); -void bgp_peer_move_to_gr_mode(struct peer *peer, int new_state); +void bgp_peer_move_to_gr_mode(struct peer *peer, enum peer_mode new_state); unsigned int bgp_peer_gr_helper_enable(struct peer *peer); unsigned int bgp_peer_gr_enable(struct peer *peer); unsigned int bgp_peer_gr_global_inherit(struct peer *peer); @@ -160,9 +160,6 @@ enum peer_mode bgp_peer_gr_mode_get(struct peer *peer); enum global_mode bgp_global_gr_mode_get(struct bgp *bgp); enum peer_mode bgp_get_peer_gr_mode_from_flags(struct peer *peer); unsigned int bgp_peer_gr_global_inherit_unset(struct peer *peer); -int bgp_gr_lookup_n_update_all_peer(struct bgp *bgp, - enum global_mode global_new_state, - enum global_mode global_old_state); void bgp_peer_gr_flags_update(struct peer *peer); const char *print_peer_gr_mode(enum peer_mode pr_mode); const char *print_peer_gr_cmd(enum peer_gr_command pr_gr_cmd); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 3dec4dd265..f5aa2e03bf 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -3123,10 +3123,6 @@ DEFUN (bgp_graceful_restart, return bgp_global_gr_config_vty(vty, true, false); int ret = BGP_GR_FAILURE; - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] bgp_graceful_restart_cmd : START "); - VTY_DECLVAR_CONTEXT(bgp, bgp); ret = bgp_inst_gr_config_vty(vty, bgp, true, false); @@ -3135,9 +3131,6 @@ DEFUN (bgp_graceful_restart, "Graceful restart configuration changed, reset all peers to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] bgp_graceful_restart_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3153,10 +3146,6 @@ DEFUN (no_bgp_graceful_restart, return bgp_global_gr_config_vty(vty, false, false); VTY_DECLVAR_CONTEXT(bgp, bgp); - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] no_bgp_graceful_restart_cmd : START "); - int ret = BGP_GR_FAILURE; ret = bgp_inst_gr_config_vty(vty, bgp, false, false); @@ -3168,9 +3157,6 @@ DEFUN (no_bgp_graceful_restart, "Graceful restart configuration changed, reset all peers to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] no_bgp_graceful_restart_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3460,10 +3446,6 @@ DEFUN (bgp_graceful_restart_disable, struct listnode *node, *nnode; struct peer *peer; - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_graceful_restart_disable_cmd : START "); - VTY_DECLVAR_CONTEXT(bgp, bgp); ret = bgp_inst_gr_config_vty(vty, bgp, true, true); @@ -3481,9 +3463,6 @@ DEFUN (bgp_graceful_restart_disable, } } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug("[BGP_GR] bgp_graceful_restart_disable_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3499,11 +3478,6 @@ DEFUN (no_bgp_graceful_restart_disable, return bgp_global_gr_config_vty(vty, false, true); VTY_DECLVAR_CONTEXT(bgp, bgp); - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_graceful_restart_disable_cmd : START "); - int ret = BGP_GR_FAILURE; ret = bgp_inst_gr_config_vty(vty, bgp, false, true); @@ -3512,10 +3486,6 @@ DEFUN (no_bgp_graceful_restart_disable, "Graceful restart configuration changed, reset all peers to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_graceful_restart_disable_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3533,13 +3503,14 @@ DEFUN (bgp_neighbor_graceful_restart_set, VTY_BGP_GR_DEFINE_LOOP_VARIABLE; - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_neighbor_graceful_restart_set_cmd : START "); - peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + vty_out(vty, + "Per peer-group graceful-restart configuration is not yet supported\n"); + return CMD_WARNING_CONFIG_FAILED; + } result = bgp_neighbor_graceful_restart(peer, PEER_GR_CMD); if (result == BGP_GR_SUCCESS) { @@ -3549,15 +3520,7 @@ DEFUN (bgp_neighbor_graceful_restart_set, "Graceful restart configuration changed, reset this peer to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_neighbor_graceful_restart_set_cmd : END "); - - if (ret != BGP_GR_SUCCESS) - vty_out(vty, - "As part of configuring graceful-restart, capability send to zebra failed\n"); - - return bgp_vty_return(vty, result); + return bgp_vty_return(vty, ret); } DEFUN (no_bgp_neighbor_graceful_restart, @@ -3578,10 +3541,11 @@ DEFUN (no_bgp_neighbor_graceful_restart, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_neighbor_graceful_restart_set_cmd : START "); + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + vty_out(vty, + "Per peer-group graceful-restart configuration is not yet supported\n"); + return CMD_WARNING_CONFIG_FAILED; + } result = bgp_neighbor_graceful_restart(peer, NO_PEER_GR_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3591,14 +3555,6 @@ DEFUN (no_bgp_neighbor_graceful_restart, "Graceful restart configuration changed, reset this peer to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_neighbor_graceful_restart_set_cmd : END "); - - if (ret != BGP_GR_SUCCESS) - vty_out(vty, - "As part of configuring graceful-restart, capability send to zebra failed\n"); - return bgp_vty_return(vty, result); } @@ -3616,15 +3572,14 @@ DEFUN (bgp_neighbor_graceful_restart_helper_set, VTY_BGP_GR_DEFINE_LOOP_VARIABLE; - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_neighbor_graceful_restart_helper_set_cmd : START "); - peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); - if (!peer) return CMD_WARNING_CONFIG_FAILED; - + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + vty_out(vty, + "Per peer-group graceful-restart configuration is not yet supported\n"); + return CMD_WARNING_CONFIG_FAILED; + } ret = bgp_neighbor_graceful_restart(peer, PEER_HELPER_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3634,10 +3589,6 @@ DEFUN (bgp_neighbor_graceful_restart_helper_set, "Graceful restart configuration changed, reset this peer to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_neighbor_graceful_restart_helper_set_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3659,10 +3610,11 @@ DEFUN (no_bgp_neighbor_graceful_restart_helper, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_neighbor_graceful_restart_helper_set_cmd : START "); + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + vty_out(vty, + "Per peer-group graceful-restart configuration is not yet supported\n"); + return CMD_WARNING_CONFIG_FAILED; + } ret = bgp_neighbor_graceful_restart(peer, NO_PEER_HELPER_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3672,10 +3624,6 @@ DEFUN (no_bgp_neighbor_graceful_restart_helper, "Graceful restart configuration changed, reset this peer to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_neighbor_graceful_restart_helper_set_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3693,13 +3641,14 @@ DEFUN (bgp_neighbor_graceful_restart_disable_set, VTY_BGP_GR_DEFINE_LOOP_VARIABLE; - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_neighbor_graceful_restart_disable_set_cmd : START "); - peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + vty_out(vty, + "Per peer-group graceful-restart configuration is not yet supported\n"); + return CMD_WARNING_CONFIG_FAILED; + } ret = bgp_neighbor_graceful_restart(peer, PEER_DISABLE_CMD); if (ret == BGP_GR_SUCCESS) { @@ -3708,14 +3657,8 @@ DEFUN (bgp_neighbor_graceful_restart_disable_set, VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR]bgp_neighbor_graceful_restart_disable_set_cmd : END "); - return bgp_vty_return(vty, ret); } @@ -3737,23 +3680,18 @@ DEFUN (no_bgp_neighbor_graceful_restart_disable, peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg); if (!peer) return CMD_WARNING_CONFIG_FAILED; - - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_neighbor_graceful_restart_disable_set_cmd : START "); + if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + vty_out(vty, + "Per peer-group graceful-restart configuration is not yet supported\n"); + return CMD_WARNING_CONFIG_FAILED; + } ret = bgp_neighbor_graceful_restart(peer, NO_PEER_DISABLE_CMD); if (ret == BGP_GR_SUCCESS) { VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); } - if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] no_bgp_neighbor_graceful_restart_disable_set_cmd : END "); - return bgp_vty_return(vty, ret); } diff --git a/bgpd/bgp_vty.h b/bgpd/bgp_vty.h index addd71757d..6d86f6ba08 100644 --- a/bgpd/bgp_vty.h +++ b/bgpd/bgp_vty.h @@ -60,8 +60,6 @@ struct bgp; #define VTY_BGP_GR_ROUTER_DETECT(_bgp, _peer, _peer_list) \ do { \ - if (_peer->bgp->t_startup) \ - bgp_peer_gr_flags_update(_peer); \ for (ALL_LIST_ELEMENTS(_peer_list, node, nnode, peer_loop)) { \ if (CHECK_FLAG(peer_loop->flags, \ PEER_FLAG_GRACEFUL_RESTART)) \ @@ -84,30 +82,27 @@ struct bgp; } \ } while (0) -#define VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA( \ - _bgp, _peer_list, _ret) \ - do { \ - struct peer *peer_loop; \ - bool gr_router_detected = false; \ - struct listnode *node = {0}; \ - struct listnode *nnode = {0}; \ - for (ALL_LIST_ELEMENTS(_peer_list, node, nnode, peer_loop)) { \ - if (peer_loop->bgp->t_startup) \ - bgp_peer_gr_flags_update(peer_loop); \ - if (CHECK_FLAG(peer_loop->flags, \ - PEER_FLAG_GRACEFUL_RESTART)) \ - gr_router_detected = true; \ - } \ - if (gr_router_detected \ - && _bgp->present_zebra_gr_state == ZEBRA_GR_DISABLE) { \ - if (bgp_zebra_send_capabilities(_bgp, false)) \ - _ret = BGP_ERR_INVALID_VALUE; \ - } else if (!gr_router_detected \ - && _bgp->present_zebra_gr_state \ - == ZEBRA_GR_ENABLE) { \ - if (bgp_zebra_send_capabilities(_bgp, true)) \ - _ret = BGP_ERR_INVALID_VALUE; \ - } \ +#define VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(_bgp, \ + _peer_list, _ret) \ + do { \ + struct peer *peer_loop; \ + bool gr_router_detected = false; \ + struct listnode *node = { 0 }; \ + struct listnode *nnode = { 0 }; \ + for (ALL_LIST_ELEMENTS(_peer_list, node, nnode, peer_loop)) { \ + if (CHECK_FLAG(peer_loop->flags, \ + PEER_FLAG_GRACEFUL_RESTART)) \ + gr_router_detected = true; \ + } \ + if (gr_router_detected && \ + _bgp->present_zebra_gr_state == ZEBRA_GR_DISABLE) { \ + if (bgp_zebra_send_capabilities(_bgp, false)) \ + _ret = BGP_ERR_INVALID_VALUE; \ + } else if (!gr_router_detected && \ + _bgp->present_zebra_gr_state == ZEBRA_GR_ENABLE) { \ + if (bgp_zebra_send_capabilities(_bgp, true)) \ + _ret = BGP_ERR_INVALID_VALUE; \ + } \ } while (0) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index c9abeb35da..17d94bd575 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1475,9 +1475,7 @@ int bgp_peer_gr_init(struct peer *peer) { PEER_HELPER, bgp_peer_gr_action }, { PEER_GLOBAL_INHERIT, NULL } } }; - memcpy(&peer->PEER_GR_FSM, local_Peer_GR_FSM, - sizeof(local_Peer_GR_FSM)); - peer->peer_gr_present_state = PEER_GLOBAL_INHERIT; + memcpy(&peer->PEER_GR_FSM, local_Peer_GR_FSM, sizeof(local_Peer_GR_FSM)); bgp_peer_move_to_gr_mode(peer, PEER_GLOBAL_INHERIT); return BGP_GR_SUCCESS; -- 2.39.5