From 1a8725b11fd84f875577f2066b4d6cccf4191933 Mon Sep 17 00:00:00 2001 From: anlan_cs Date: Fri, 12 Apr 2024 21:57:30 +0800 Subject: [PATCH 1/2] bpgd: adjust return value for the same command The same command should be accepted, it is an empty operation. Take `neighbor graceful-restart-helper` as an example: Before: ``` anlan(config-router)# neighbor 3.3.3.3 graceful-restart-helper Graceful restart configuration changed, reset this peer to take effect anlan(config-router)# neighbor 3.3.3.3 graceful-restart-helper Graceful restart configuration changed, reset this peer to take effect % The Graceful Restart command used is not valid at this moment. anlan(config-router)# ``` After: ``` anlan(config-router)# neighbor 3.3.3.3 graceful-restart-helper Graceful restart configuration changed, reset this peer to take effect anlan(config-router)# neighbor 3.3.3.3 graceful-restart-helper Graceful restart configuration changed, reset this peer to take effect anlan(config-router)# ``` Signed-off-by: anlan_cs --- bgpd/bgpd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index d98df754ef81..33da70fb9bbc 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1377,7 +1377,7 @@ int bgp_global_gr_init(struct bgp *bgp) /*GLOBAL_GR_cmd */ /*no_Global_GR_cmd*/ GLOBAL_GR, GLOBAL_INVALID, /*GLOBAL_DISABLE_cmd*//*no_Global_Disable_cmd*/ - GLOBAL_INVALID, GLOBAL_HELPER + GLOBAL_DISABLE, GLOBAL_HELPER }, /* GLOBAL_INVALID Mode */ { @@ -1411,13 +1411,13 @@ int bgp_peer_gr_init(struct peer *peer) /* Event-> */ /* PEER_DISABLE_CMD */ /* NO_PEER_DISABLE_CMD */ {PEER_DISABLE, bgp_peer_gr_action }, {PEER_INVALID, NULL }, /* Event-> */ /* PEER_HELPER_cmd */ /* NO_PEER_HELPER_CMD */ - { PEER_INVALID, NULL }, {PEER_GLOBAL_INHERIT, + { PEER_HELPER, NULL }, {PEER_GLOBAL_INHERIT, bgp_peer_gr_action } }, { /* PEER_GR Mode */ /* Event-> */ /* PEER_GR_CMD */ /* NO_PEER_GR_CMD */ - { PEER_INVALID, NULL }, { PEER_GLOBAL_INHERIT, + { PEER_GR, NULL }, { PEER_GLOBAL_INHERIT, bgp_peer_gr_action }, /* Event-> */ /* PEER_DISABLE_CMD */ /* NO_PEER_DISABLE_CMD */ {PEER_DISABLE, bgp_peer_gr_action }, { PEER_INVALID, NULL }, @@ -1429,7 +1429,7 @@ int bgp_peer_gr_init(struct peer *peer) /* Event-> */ /* PEER_GR_CMD */ /* NO_PEER_GR_CMD */ { PEER_GR, bgp_peer_gr_action }, { PEER_INVALID, NULL }, /* Event-> */ /* PEER_DISABLE_CMD */ /* NO_PEER_DISABLE_CMD */ - { PEER_INVALID, NULL }, { PEER_GLOBAL_INHERIT, + { PEER_DISABLE, NULL }, { PEER_GLOBAL_INHERIT, bgp_peer_gr_action }, /* Event-> */ /* PEER_HELPER_cmd */ /* NO_PEER_HELPER_CMD */ { PEER_HELPER, bgp_peer_gr_action }, { PEER_INVALID, NULL } From 70c4dea8a973ae023d15b6dcf52aa019bc21f414 Mon Sep 17 00:00:00 2001 From: anlan_cs Date: Fri, 12 Apr 2024 22:56:41 +0800 Subject: [PATCH 2/2] bgpd: prompt should be given only for the real change Prompt nothing for an empty (and failed) operation. Take `bgp graceful-restart` as an example: Before: ``` anlan(config-router)# bgp graceful-restart Graceful restart configuration changed, reset all peers to take effect anlan(config-router)# bgp graceful-restart Graceful restart configuration changed, reset all peers to take effect anlan(config-router)# ``` After: ``` anlan(config-router)# bgp graceful-restart Graceful restart configuration changed, reset all peers to take effect anlan(config-router)# bgp graceful-restart anlan(config-router)# ``` Signed-off-by: anlan_cs --- bgpd/bgp_vty.c | 138 +++++++++++++++++++++++++++---------------------- 1 file changed, 76 insertions(+), 62 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index bcd029dcad5b..2a11cd7b5564 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -3035,14 +3035,17 @@ DEFUN (bgp_graceful_restart, VTY_DECLVAR_CONTEXT(bgp, bgp); ret = bgp_gr_update_all(bgp, GLOBAL_GR_CMD); - - VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, bgp->peer, - ret); + if (ret == BGP_GR_SUCCESS) { + VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, + bgp->peer, + ret); + vty_out(vty, + "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 "); - vty_out(vty, - "Graceful restart configuration changed, reset all peers to take effect\n"); + return bgp_vty_return(vty, ret); } @@ -3062,14 +3065,16 @@ DEFUN (no_bgp_graceful_restart, int ret = BGP_GR_FAILURE; ret = bgp_gr_update_all(bgp, NO_GLOBAL_GR_CMD); - - VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, bgp->peer, - ret); + if (ret == BGP_GR_SUCCESS) { + VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, + bgp->peer, + ret); + vty_out(vty, + "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 "); - vty_out(vty, - "Graceful restart configuration changed, reset all peers to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3277,24 +3282,25 @@ DEFUN (bgp_graceful_restart_disable, VTY_DECLVAR_CONTEXT(bgp, bgp); ret = bgp_gr_update_all(bgp, GLOBAL_DISABLE_CMD); + if (ret == BGP_GR_SUCCESS) { + VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, + bgp->peer, + ret); + vty_out(vty, + "Graceful restart configuration changed, reset all peers to take effect\n"); - VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, - bgp->peer, ret); + for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { + bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, + CAPABILITY_CODE_RESTART, + CAPABILITY_ACTION_UNSET); + bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, + CAPABILITY_CODE_LLGR, + CAPABILITY_ACTION_UNSET); + } + } if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) - zlog_debug( - "[BGP_GR] bgp_graceful_restart_disable_cmd : END "); - vty_out(vty, - "Graceful restart configuration changed, reset all peers to take effect\n"); - - for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { - bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, - CAPABILITY_CODE_RESTART, - CAPABILITY_ACTION_UNSET); - bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, - CAPABILITY_CODE_LLGR, - CAPABILITY_ACTION_UNSET); - } + zlog_debug("[BGP_GR] bgp_graceful_restart_disable_cmd : END "); return bgp_vty_return(vty, ret); } @@ -3316,15 +3322,17 @@ DEFUN (no_bgp_graceful_restart_disable, int ret = BGP_GR_FAILURE; ret = bgp_gr_update_all(bgp, NO_GLOBAL_DISABLE_CMD); - - VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, bgp->peer, - ret); + if (ret == BGP_GR_SUCCESS) { + VTY_BGP_GR_ROUTER_DETECT_AND_SEND_CAPABILITY_TO_ZEBRA(bgp, + bgp->peer, + ret); + vty_out(vty, + "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 "); - vty_out(vty, - "Graceful restart configuration changed, reset all peers to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3352,15 +3360,16 @@ DEFUN (bgp_neighbor_graceful_restart_set, return CMD_WARNING_CONFIG_FAILED; ret = bgp_neighbor_graceful_restart(peer, PEER_GR_CMD); - - VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); - VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); + 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] bgp_neighbor_graceful_restart_set_cmd : END "); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3389,15 +3398,16 @@ DEFUN (no_bgp_neighbor_graceful_restart, "[BGP_GR] no_bgp_neighbor_graceful_restart_set_cmd : START "); ret = bgp_neighbor_graceful_restart(peer, NO_PEER_GR_CMD); - - VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); - VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); + 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_set_cmd : END "); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3427,15 +3437,16 @@ DEFUN (bgp_neighbor_graceful_restart_helper_set, ret = bgp_neighbor_graceful_restart(peer, PEER_HELPER_CMD); - - VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); - VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); + 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] bgp_neighbor_graceful_restart_helper_set_cmd : END "); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3464,15 +3475,16 @@ DEFUN (no_bgp_neighbor_graceful_restart_helper, "[BGP_GR] no_bgp_neighbor_graceful_restart_helper_set_cmd : START "); ret = bgp_neighbor_graceful_restart(peer, NO_PEER_HELPER_CMD); - - VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); - VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); + 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_helper_set_cmd : END "); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3500,18 +3512,19 @@ DEFUN (bgp_neighbor_graceful_restart_disable_set, return CMD_WARNING_CONFIG_FAILED; ret = bgp_neighbor_graceful_restart(peer, PEER_DISABLE_CMD); + if (ret == BGP_GR_SUCCESS) { + if (peer->bgp->t_startup) + bgp_peer_gr_flags_update(peer); - if (peer->bgp->t_startup) - bgp_peer_gr_flags_update(peer); - - VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); - VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); + 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 "); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); return bgp_vty_return(vty, ret); } @@ -3540,15 +3553,16 @@ DEFUN (no_bgp_neighbor_graceful_restart_disable, "[BGP_GR] no_bgp_neighbor_graceful_restart_disable_set_cmd : START "); ret = bgp_neighbor_graceful_restart(peer, NO_PEER_DISABLE_CMD); - - VTY_BGP_GR_ROUTER_DETECT(bgp, peer, peer->bgp->peer); - VTY_SEND_BGP_GR_CAPABILITY_TO_ZEBRA(peer->bgp, ret); + 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 "); - vty_out(vty, - "Graceful restart configuration changed, reset this peer to take effect\n"); return bgp_vty_return(vty, ret); }