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 <donatas@opensourcerouting.org>

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 7b9400118b762e060926bf0656c7dae703a94845..64e3295a898dc88b5446348620820cbe96f90b44 100644 (file)
--- 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 9526b50fb9f257d54cb8dbe4904f49fbd8a06a99..019789dff84a8e86a54efae7c3f487d6c99dece6 100644 (file)
--- 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                                        \