From 9ca4407147535baf10193d7eb5bdd7214f524097 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 30 Apr 2024 13:38:54 +0300 Subject: [PATCH 1/2] bgpd: Print old/new states of graceful restart FSM To better debug what's going on before/after. Signed-off-by: Donatas Abraitis (cherry picked from commit 7b5595b61dfbd27ba05b8f5416882514a1e2f94f) --- bgpd/bgp_fsm.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 234dbb0715..502648697c 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -2895,19 +2895,22 @@ int bgp_neighbor_graceful_restart(struct peer *peer, peer_old_state = bgp_peer_gr_mode_get(peer); - if (peer_old_state == PEER_INVALID) { - zlog_debug("[BGP_GR] peer_old_state == Invalid state !"); + if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) + zlog_debug("%s [BGP_GR] peer_old_state: %d", __func__, + peer_old_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 (peer_new_state == PEER_INVALID) { - zlog_debug( - "[BGP_GR] Invalid bgp graceful restart command used !"); + 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, From 25986a7f5bf6b5b31c599aa48d2ff55bac439123 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 30 Apr 2024 13:40:08 +0300 Subject: [PATCH 2/2] bgpd: Apply NOOP when doing negative commands for GR operations E.g.: ``` % The Graceful Restart command used is not valid at this moment. zsh: exit 1 vtysh -c configure -c 'router bgp' -c 'no neighbor 127.0.0.1 graceful-restart 1 ``` This does not make sense frr-reload to fail. Instead, just ignore such requests if they are just NOOP. Signed-off-by: Donatas Abraitis (cherry picked from commit 9ebdb8e80d83a9087deaffee93df4bc90ab6e7ee) --- bgpd/bgpd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a0b6cfb500..f6854202db 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1446,11 +1446,11 @@ int bgp_peer_gr_init(struct peer *peer) { /* PEER_GLOBAL_INHERIT Mode */ /* Event-> */ /* PEER_GR_CMD */ /* NO_PEER_GR_CMD */ - { PEER_GR, bgp_peer_gr_action }, { PEER_INVALID, NULL }, + { PEER_GR, bgp_peer_gr_action }, { PEER_GLOBAL_INHERIT, NULL }, /* Event-> */ /* PEER_DISABLE_CMD */ /* NO_PEER_DISABLE_CMD */ - { PEER_DISABLE, bgp_peer_gr_action}, { PEER_INVALID, NULL }, + { PEER_DISABLE, bgp_peer_gr_action }, { PEER_GLOBAL_INHERIT, NULL }, /* Event-> */ /* PEER_HELPER_cmd */ /* NO_PEER_HELPER_CMD */ - { PEER_HELPER, bgp_peer_gr_action }, { PEER_INVALID, NULL } + { PEER_HELPER, bgp_peer_gr_action }, { PEER_GLOBAL_INHERIT, NULL } } }; memcpy(&peer->PEER_GR_FSM, local_Peer_GR_FSM,