From 403e64f834b685e7a5bccccb8d8b808c32491bee Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sun, 11 Dec 2022 21:00:19 +0200 Subject: [PATCH] bgpd: Fix graceful-restart JSON outputs and the crash Without this patch: ``` donatas-pc# show bgp neighbors graceful-restart json vtysh: error reading from bgpd: Resource temporarily unavailable (11)Warning: closing connection to bgpd because of an I/O error! donatas-pc# ``` And, invalid JSON generated when multiple neighbors exist due to json_neighbor being freed in a loop. After the patch: ``` donatas-pc# show bgp ipv4 neighbors 192.168.10.124 graceful-restart json { "192.168.10.124":{ "neighborAddr":"192.168.10.124", "localGrMode":"Helper*", "remoteGrMode":"Restart", "rBit":false, "nBit":true, "timers":{ "configuredRestartTimer":120, "receivedRestartTimer":120 }, "ipv4Unicast":{ "fBit":true, "endOfRibStatus":{ "endOfRibSend":true, "endOfRibSentAfterUpdate":false, "endOfRibRecv":true }, "timers":{ "stalePathTimer":360 } } } } donatas-pc# show bgp neighbors graceful-restart json { "192.168.10.124":{ "neighborAddr":"192.168.10.124", "localGrMode":"Helper*", "remoteGrMode":"Restart", "rBit":false, "nBit":true, "timers":{ "configuredRestartTimer":120, "receivedRestartTimer":120 }, "ipv4Unicast":{ "fBit":true, "endOfRibStatus":{ "endOfRibSend":true, "endOfRibSentAfterUpdate":false, "endOfRibRecv":true }, "timers":{ "stalePathTimer":360 } } }, "2a02:4780:abc::2":{ "neighborAddr":"2a02:4780:abc::2", "localGrMode":"Helper*", "remoteGrMode":"Restart", "rBit":false, "nBit":true, "timers":{ "configuredRestartTimer":120, "receivedRestartTimer":120 }, "ipv4Unicast":{ "fBit":true, "endOfRibStatus":{ "endOfRibSend":true, "endOfRibSentAfterUpdate":false, "endOfRibRecv":true }, "timers":{ "stalePathTimer":360 } }, "ipv6Unicast":{ "fBit":true, "endOfRibStatus":{ "endOfRibSend":true, "endOfRibSentAfterUpdate":true, "endOfRibRecv":true }, "timers":{ "stalePathTimer":360 } } } } donatas-pc# ``` Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 132 ++++++++++++++++++++++--------------------------- bgpd/bgp_vty.h | 16 +++--- 2 files changed, 65 insertions(+), 83 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 7b9400118b..64e3295a89 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -158,9 +158,7 @@ static struct peer_group *listen_range_exists(struct bgp *bgp, struct prefix *range, int exact); static void bgp_show_global_graceful_restart_mode_vty(struct vty *vty, - struct bgp *bgp, - bool use_json, - json_object *json); + struct bgp *bgp); static int bgp_show_neighbor_graceful_restart_afi_all(struct vty *vty, enum show_type type, @@ -11856,7 +11854,6 @@ static void bgp_show_peer_afi_orf_cap(struct vty *vty, struct peer *p, static void bgp_show_neighnor_graceful_restart_flags(struct vty *vty, struct peer *p, - bool use_json, json_object *json) { bool rbit = false; @@ -11869,7 +11866,7 @@ static void bgp_show_neighnor_graceful_restart_flags(struct vty *vty, nbit = CHECK_FLAG(p->cap, PEER_CAP_GRACEFUL_RESTART_N_BIT_RCV); } - if (use_json) { + if (json) { json_object_boolean_add(json, "rBit", rbit); json_object_boolean_add(json, "nBit", nbit); } else { @@ -11880,12 +11877,11 @@ static void bgp_show_neighnor_graceful_restart_flags(struct vty *vty, static void bgp_show_neighbor_graceful_restart_remote_mode(struct vty *vty, struct peer *peer, - bool use_json, json_object *json) { const char *mode = "NotApplicable"; - if (!use_json) + if (!json) vty_out(vty, "\n Remote GR Mode: "); if (CHECK_FLAG(peer->cap, PEER_CAP_RESTART_ADV) @@ -11908,20 +11904,19 @@ static void bgp_show_neighbor_graceful_restart_remote_mode(struct vty *vty, } } - if (use_json) { + if (json) json_object_string_add(json, "remoteGrMode", mode); - } else + else vty_out(vty, mode, "\n"); } static void bgp_show_neighbor_graceful_restart_local_mode(struct vty *vty, struct peer *p, - bool use_json, json_object *json) { const char *mode = "Invalid"; - if (!use_json) + if (!json) vty_out(vty, " Local GR Mode: "); if (bgp_peer_gr_mode_get(p) == PEER_HELPER) @@ -11941,15 +11936,14 @@ static void bgp_show_neighbor_graceful_restart_local_mode(struct vty *vty, mode = "Invalid*"; } - if (use_json) { + if (json) json_object_string_add(json, "localGrMode", mode); - } else { + else vty_out(vty, mode, "\n"); - } } static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( - struct vty *vty, struct peer *peer, bool use_json, json_object *json) + struct vty *vty, struct peer *peer, json_object *json) { afi_t afi; safi_t safi; @@ -11966,7 +11960,7 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( !CHECK_FLAG(peer->cap, PEER_CAP_RESTART_RCV)) continue; - if (use_json) { + if (json) { json_afi_safi = json_object_new_object(); json_endofrib_status = json_object_new_object(); json_timer = json_object_new_object(); @@ -11977,7 +11971,7 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( else eor_flag = false; - if (!use_json) { + if (!json) { vty_out(vty, " %s:\n", get_afi_safi_str(afi, safi, false)); @@ -11988,25 +11982,25 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( CHECK_FLAG(peer->af_cap[afi][safi], PEER_CAP_RESTART_AF_PRESERVE_RCV)) { - if (use_json) { + if (json) { json_object_boolean_true_add(json_afi_safi, "fBit"); } else vty_out(vty, "True\n"); } else { - if (use_json) + if (json) json_object_boolean_false_add(json_afi_safi, "fBit"); else vty_out(vty, "False\n"); } - if (!use_json) + if (!json) vty_out(vty, " End-of-RIB sent: "); if (CHECK_FLAG(peer->af_sflags[afi][safi], PEER_STATUS_EOR_SEND)) { - if (use_json) { + if (json) { json_object_boolean_true_add( json_endofrib_status, "endOfRibSend"); @@ -12019,7 +12013,7 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( PRINT_EOR(eor_flag); } } else { - if (use_json) { + if (json) { json_object_boolean_false_add( json_endofrib_status, "endOfRibSend"); json_object_boolean_false_add( @@ -12033,25 +12027,25 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( } } - if (!use_json) + if (!json) vty_out(vty, " End-of-RIB received: "); if (CHECK_FLAG(peer->af_sflags[afi][safi], PEER_STATUS_EOR_RECEIVED)) { - if (use_json) + if (json) json_object_boolean_true_add( json_endofrib_status, "endOfRibRecv"); else vty_out(vty, "Yes\n"); } else { - if (use_json) + if (json) json_object_boolean_false_add( json_endofrib_status, "endOfRibRecv"); else vty_out(vty, "No\n"); } - if (use_json) { + if (json) { json_object_int_add(json_timer, "stalePathTimer", peer->bgp->stalepath_time); @@ -12111,7 +12105,7 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( peer->bgp->gr_info[afi][safi] .t_select_deferral)); } - if (use_json) { + if (json) { json_object_object_add(json_afi_safi, "endOfRibStatus", json_endofrib_status); json_object_object_add(json_afi_safi, "timers", @@ -12125,10 +12119,9 @@ static void bgp_show_neighbor_graceful_restart_capability_per_afi_safi( static void bgp_show_neighbor_graceful_restart_time(struct vty *vty, struct peer *p, - bool use_json, json_object *json) { - if (use_json) { + if (json) { json_object *json_timer = NULL; json_timer = json_object_new_object(); @@ -12164,7 +12157,7 @@ static void bgp_show_neighbor_graceful_restart_time(struct vty *vty, } static void bgp_show_peer_gr_status(struct vty *vty, struct peer *p, - bool use_json, json_object *json) + json_object *json) { char dn_flag[2] = {0}; /* '*' + v6 address of neighbor */ @@ -12174,7 +12167,7 @@ static void bgp_show_peer_gr_status(struct vty *vty, struct peer *p, dn_flag[0] = '*'; if (p->conf_if) { - if (use_json) + if (json) json_object_string_addf(json, "neighborAddr", "%pSU", &p->su); else @@ -12184,7 +12177,7 @@ static void bgp_show_peer_gr_status(struct vty *vty, struct peer *p, snprintf(neighborAddr, sizeof(neighborAddr), "%s%s", dn_flag, p->host); - if (use_json) + if (json) json_object_string_add(json, "neighborAddr", neighborAddr); else @@ -12192,7 +12185,7 @@ static void bgp_show_peer_gr_status(struct vty *vty, struct peer *p, } /* more gr info in new format */ - BGP_SHOW_PEER_GR_CAPABILITY(vty, p, use_json, json); + BGP_SHOW_PEER_GR_CAPABILITY(vty, p, json); } static void bgp_show_peer_afi(struct vty *vty, struct peer *p, afi_t afi, @@ -14174,7 +14167,7 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, thread_timer_remain_second(p->t_gr_stale) * 1000); /* more gr info in new format */ - BGP_SHOW_PEER_GR_CAPABILITY(vty, p, use_json, json_grace); + BGP_SHOW_PEER_GR_CAPABILITY(vty, p, json_grace); json_object_object_add(json_neigh, "gracefulRestartInfo", json_grace); } else { @@ -14220,7 +14213,7 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json, thread_timer_remain_second(p->t_gr_stale)); /* more gr info in new format */ - BGP_SHOW_PEER_GR_CAPABILITY(vty, p, use_json, NULL); + BGP_SHOW_PEER_GR_CAPABILITY(vty, p, NULL); } if (use_json) { @@ -14682,20 +14675,14 @@ static int bgp_show_neighbor_graceful_restart(struct vty *vty, struct bgp *bgp, enum show_type type, union sockunion *su, const char *conf_if, afi_t afi, - bool use_json) + json_object *json) { struct listnode *node, *nnode; struct peer *peer; int find = 0; safi_t safi = SAFI_UNICAST; - json_object *json = NULL; json_object *json_neighbor = NULL; - if (use_json) { - json = json_object_new_object(); - json_neighbor = json_object_new_object(); - } - for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) @@ -14704,15 +14691,15 @@ static int bgp_show_neighbor_graceful_restart(struct vty *vty, struct bgp *bgp, if ((peer->afc[afi][safi]) == 0) continue; + if (json) + json_neighbor = json_object_new_object(); + if (type == show_all) { - bgp_show_peer_gr_status(vty, peer, use_json, - json_neighbor); + bgp_show_peer_gr_status(vty, peer, json_neighbor); - if (use_json) { + if (json) json_object_object_add(json, peer->host, json_neighbor); - json_neighbor = NULL; - } } else if (type == show_peer) { if (conf_if) { @@ -14722,41 +14709,33 @@ static int bgp_show_neighbor_graceful_restart(struct vty *vty, struct bgp *bgp, && !strcmp(peer->hostname, conf_if))) { find = 1; bgp_show_peer_gr_status(vty, peer, - use_json, json_neighbor); } } else { if (sockunion_same(&peer->su, su)) { find = 1; bgp_show_peer_gr_status(vty, peer, - use_json, json_neighbor); } } - if (use_json && find) + if (json && find) json_object_object_add(json, peer->host, json_neighbor); } - if (find) { - json_neighbor = NULL; + if (find) break; - } } if (type == show_peer && !find) { - if (use_json) + if (json) json_object_boolean_true_add(json, "bgpNoSuchNeighbor"); else vty_out(vty, "%% No such neighbor\n"); } - if (use_json) { - if (json_neighbor) - json_object_free(json_neighbor); - vty_json(vty, json); - } else { + + if (!json) vty_out(vty, "\n"); - } return CMD_SUCCESS; } @@ -14869,7 +14848,7 @@ static int bgp_show_neighbor(struct vty *vty, struct bgp *bgp, static void bgp_show_neighbor_graceful_restart_vty(struct vty *vty, enum show_type type, const char *ip_str, - afi_t afi, bool use_json) + afi_t afi, json_object *json) { int ret; @@ -14881,21 +14860,20 @@ static void bgp_show_neighbor_graceful_restart_vty(struct vty *vty, if (!bgp) return; - if (!use_json) - bgp_show_global_graceful_restart_mode_vty(vty, bgp, use_json, - NULL); + if (!json) + bgp_show_global_graceful_restart_mode_vty(vty, bgp); if (ip_str) { ret = str2sockunion(ip_str, &su); if (ret < 0) - bgp_show_neighbor_graceful_restart( - vty, bgp, type, NULL, ip_str, afi, use_json); + bgp_show_neighbor_graceful_restart(vty, bgp, type, NULL, + ip_str, afi, json); else bgp_show_neighbor_graceful_restart(vty, bgp, type, &su, - NULL, afi, use_json); + NULL, afi, json); } else bgp_show_neighbor_graceful_restart(vty, bgp, type, NULL, NULL, - afi, use_json); + afi, json); } static void bgp_show_all_instances_neighbors_vty(struct vty *vty, @@ -15215,9 +15193,7 @@ DEFUN (show_ip_bgp_lcommunity_info, /* Graceful Restart */ static void bgp_show_global_graceful_restart_mode_vty(struct vty *vty, - struct bgp *bgp, - bool use_json, - json_object *json) + struct bgp *bgp) { @@ -15252,22 +15228,32 @@ static int bgp_show_neighbor_graceful_restart_afi_all(struct vty *vty, const char *ip_str, afi_t afi, bool use_json) { + json_object *json = NULL; + + if (use_json) + json = json_object_new_object(); + if ((afi == AFI_MAX) && (ip_str == NULL)) { afi = AFI_IP; while ((afi != AFI_L2VPN) && (afi < AFI_MAX)) { bgp_show_neighbor_graceful_restart_vty( - vty, type, ip_str, afi, use_json); + vty, type, ip_str, afi, json); afi++; } } else if (afi != AFI_MAX) { bgp_show_neighbor_graceful_restart_vty(vty, type, ip_str, afi, - use_json); + json); } else { + if (json) + json_object_free(json); return CMD_ERR_INCOMPLETE; } + if (json) + vty_json(vty, json); + return CMD_SUCCESS; } /* Graceful Restart */ diff --git a/bgpd/bgp_vty.h b/bgpd/bgp_vty.h index 9526b50fb9..019789dff8 100644 --- a/bgpd/bgp_vty.h +++ b/bgpd/bgp_vty.h @@ -56,18 +56,14 @@ struct bgp; "V AS LocalAS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd PfxSnt Desc\n" #define BGP_SHOW_SUMMARY_HEADER_FAILED "EstdCnt DropCnt ResetTime Reason\n" -#define BGP_SHOW_PEER_GR_CAPABILITY(vty, p, use_json, json) \ +#define BGP_SHOW_PEER_GR_CAPABILITY(vty, p, json) \ do { \ - bgp_show_neighbor_graceful_restart_local_mode(vty, p, \ - use_json, json); \ - bgp_show_neighbor_graceful_restart_remote_mode( \ - vty, p, use_json, json); \ - bgp_show_neighnor_graceful_restart_flags(vty, p, use_json, \ - json); \ - bgp_show_neighbor_graceful_restart_time(vty, p, use_json, \ - json); \ + bgp_show_neighbor_graceful_restart_local_mode(vty, p, json); \ + bgp_show_neighbor_graceful_restart_remote_mode(vty, p, json); \ + bgp_show_neighnor_graceful_restart_flags(vty, p, json); \ + bgp_show_neighbor_graceful_restart_time(vty, p, json); \ bgp_show_neighbor_graceful_restart_capability_per_afi_safi( \ - vty, p, use_json, json); \ + vty, p, json); \ } while (0) #define VTY_BGP_GR_DEFINE_LOOP_VARIABLE \ -- 2.39.5