Merge pull request #16093 from louis-6wind/fix-show-route-memory

zebra: fix Out Of Memory issue when displaying large route tables in JSON

4 files changed, 70 insertions, 61 deletions

diff --git a/lib/vty.c b/lib/vty.c
index 628c694e9..0dcd118a9 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -390,6 +390,21 @@ int vty_json_no_pretty(struct vty *vty, struct json_object *json)
 	return vty_json_helper(vty, json, JSON_C_TO_STRING_NOSLASHESCAPE);
 }
 
+
+void vty_json_key(struct vty *vty, const char *key, bool *first_key)
+{
+	vty_out(vty, "%s\"%s\":", *first_key ? "{" : ",", key);
+	*first_key = false;
+}
+
+void vty_json_close(struct vty *vty, bool first_key)
+{
+	if (first_key)
+		/* JSON was not opened */
+		vty_out(vty, "{");
+	vty_out(vty, "}\n");
+}
+
 void vty_json_empty(struct vty *vty, struct json_object *json)
 {
 	json_object *jsonobj = json;
diff --git a/lib/vty.h b/lib/vty.h
index a9570ef04..c336a816c 100644
--- a/lib/vty.h
+++ b/lib/vty.h
@@ -378,6 +378,8 @@ extern bool vty_set_include(struct vty *vty, const char *regexp);
  */
 extern int vty_json(struct vty *vty, struct json_object *json);
 extern int vty_json_no_pretty(struct vty *vty, struct json_object *json);
+void vty_json_key(struct vty *vty, const char *key, bool *first_key);
+void vty_json_close(struct vty *vty, bool first_key);
 extern void vty_json_empty(struct vty *vty, struct json_object *json);
 /* post fd to be passed to the vtysh client
  * fd is owned by the VTY code after this and will be closed when done
diff --git a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py
index 3938a966e..6d4b436bc 100644
--- a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py
+++ b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py
@@ -338,6 +338,21 @@ interface EVA
     result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
     assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
 
+    """
+    Check that "show ip route vrf DONNA json" and the JSON at key "DONNA" of
+    "show ip route vrf all json" gives the same result.
+    """
+
+    def check_vrf_table(router, vrf, expect):
+        output = router.vtysh_cmd("show ip route vrf all json", isjson=True)
+        vrf_table = output.get(vrf, {})
+
+        return topotest.json_cmp(vrf_table, expect)
+
+    test_func = partial(check_vrf_table, r1, "DONNA", expect)
+    result, diff = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
+    assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
+
 
 def test_vrf_route_leak_donna_after_eva_up():
     logger.info("Ensure that route states change after EVA interface goes up")
diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c
index 44720754b..c31218a7c 100644
--- a/zebra/zebra_vty.c
+++ b/zebra/zebra_vty.c
@@ -60,8 +60,8 @@ struct route_show_ctx {
 };
 
 static int do_show_ip_route(struct vty *vty, const char *vrf_name, afi_t afi,
-			    safi_t safi, bool use_fib, json_object *vrf_json,
-			    bool use_json, route_tag_t tag,
+			    safi_t safi, bool use_fib, bool use_json,
+			    route_tag_t tag,
 			    const struct prefix *longer_prefix_p,
 			    bool supernets_only, int type,
 			    unsigned short ospf_instance_id, uint32_t tableid,
@@ -153,8 +153,8 @@ DEFPY (show_ip_rpf,
 	};
 
 	return do_show_ip_route(vty, VRF_DEFAULT_NAME, ip ? AFI_IP : AFI_IP6,
-				SAFI_MULTICAST, false, NULL, uj, 0, NULL, false,
-				0, 0, 0, false, &ctx);
+				SAFI_MULTICAST, false, uj, 0, NULL, false, 0, 0,
+				0, false, &ctx);
 }
 
 DEFPY (show_ip_rpf_addr,
@@ -858,19 +858,20 @@ static void vty_show_ip_route_detail_json(struct vty *vty,
 	vty_json(vty, json);
 }
 
-static void
-do_show_route_helper(struct vty *vty, struct zebra_vrf *zvrf,
-		     struct route_table *table, afi_t afi, bool use_fib,
-		     json_object *vrf_json, route_tag_t tag,
-		     const struct prefix *longer_prefix_p, bool supernets_only,
-		     int type, unsigned short ospf_instance_id, bool use_json,
-		     uint32_t tableid, bool show_ng, struct route_show_ctx *ctx)
+static void do_show_route_helper(struct vty *vty, struct zebra_vrf *zvrf,
+				 struct route_table *table, afi_t afi,
+				 bool use_fib, route_tag_t tag,
+				 const struct prefix *longer_prefix_p,
+				 bool supernets_only, int type,
+				 unsigned short ospf_instance_id, bool use_json,
+				 uint32_t tableid, bool show_ng,
+				 struct route_show_ctx *ctx)
 {
 	struct route_node *rn;
 	struct route_entry *re;
+	bool first_json = true;
 	int first = 1;
 	rib_dest_t *dest;
-	json_object *json = NULL;
 	json_object *json_prefix = NULL;
 	uint32_t addr;
 	char buf[BUFSIZ];
@@ -886,9 +887,6 @@ do_show_route_helper(struct vty *vty, struct zebra_vrf *zvrf,
 	 *   => display the VRF and table if specific
 	 */
 
-	if (use_json && !vrf_json)
-		json = json_object_new_object();
-
 	/* Show all routes. */
 	for (rn = route_top(table); rn; rn = srcdest_route_next(rn)) {
 		dest = rib_dest_from_rnode(rn);
@@ -961,28 +959,20 @@ do_show_route_helper(struct vty *vty, struct zebra_vrf *zvrf,
 
 		if (json_prefix) {
 			prefix2str(&rn->p, buf, sizeof(buf));
-			if (!vrf_json)
-				json_object_object_add(json, buf, json_prefix);
-			else
-				json_object_object_add(vrf_json, buf,
-						       json_prefix);
+			vty_json_key(vty, buf, &first_json);
+			vty_json_no_pretty(vty, json_prefix);
+
 			json_prefix = NULL;
 		}
 	}
 
-	/*
-	 * This is an extremely expensive operation at scale
-	 * and non-pretty reduces memory footprint significantly.
-	 */
-	if (use_json && !vrf_json) {
-		vty_json_no_pretty(vty, json);
-		json = NULL;
-	}
+	if (use_json)
+		vty_json_close(vty, first_json);
 }
 
 static void do_show_ip_route_all(struct vty *vty, struct zebra_vrf *zvrf,
-				 afi_t afi, bool use_fib, json_object *vrf_json,
-				 bool use_json, route_tag_t tag,
+				 afi_t afi, bool use_fib, bool use_json,
+				 route_tag_t tag,
 				 const struct prefix *longer_prefix_p,
 				 bool supernets_only, int type,
 				 unsigned short ospf_instance_id, bool show_ng,
@@ -1002,15 +992,15 @@ static void do_show_ip_route_all(struct vty *vty, struct zebra_vrf *zvrf,
 			continue;
 
 		do_show_ip_route(vty, zvrf_name(zvrf), afi, SAFI_UNICAST,
-				 use_fib, vrf_json, use_json, tag,
-				 longer_prefix_p, supernets_only, type,
-				 ospf_instance_id, zrt->tableid, show_ng, ctx);
+				 use_fib, use_json, tag, longer_prefix_p,
+				 supernets_only, type, ospf_instance_id,
+				 zrt->tableid, show_ng, ctx);
 	}
 }
 
 static int do_show_ip_route(struct vty *vty, const char *vrf_name, afi_t afi,
-			    safi_t safi, bool use_fib, json_object *vrf_json,
-			    bool use_json, route_tag_t tag,
+			    safi_t safi, bool use_fib, bool use_json,
+			    route_tag_t tag,
 			    const struct prefix *longer_prefix_p,
 			    bool supernets_only, int type,
 			    unsigned short ospf_instance_id, uint32_t tableid,
@@ -1045,7 +1035,7 @@ static int do_show_ip_route(struct vty *vty, const char *vrf_name, afi_t afi,
 		return CMD_SUCCESS;
 	}
 
-	do_show_route_helper(vty, zvrf, table, afi, use_fib, vrf_json, tag,
+	do_show_route_helper(vty, zvrf, table, afi, use_fib, tag,
 			     longer_prefix_p, supernets_only, type,
 			     ospf_instance_id, use_json, tableid, show_ng, ctx);
 
@@ -1744,13 +1734,13 @@ DEFPY (show_route,
        "Nexthop Group Information\n")
 {
 	afi_t afi = ipv4 ? AFI_IP : AFI_IP6;
+	bool first_vrf_json = true;
 	struct vrf *vrf;
 	int type = 0;
 	struct zebra_vrf *zvrf;
 	struct route_show_ctx ctx = {
 		.multi = vrf_all || table_all,
 	};
-	json_object *root_json = NULL;
 
 	if (!vrf_is_backend_netns()) {
 		if ((vrf_all || vrf_name) && (table || table_all)) {
@@ -1772,43 +1762,30 @@ DEFPY (show_route,
 	}
 
 	if (vrf_all) {
-		if (!!json)
-			root_json = json_object_new_object();
 		RB_FOREACH (vrf, vrf_name_head, &vrfs_by_name) {
-			json_object *vrf_json = NULL;
-
 			if ((zvrf = vrf->info) == NULL
 			    || (zvrf->table[afi][SAFI_UNICAST] == NULL))
 				continue;
-
-			if (!!json)
-				vrf_json = json_object_new_object();
-
+			if (json)
+				vty_json_key(vty, zvrf_name(zvrf),
+					     &first_vrf_json);
 			if (table_all)
 				do_show_ip_route_all(vty, zvrf, afi, !!fib,
-						     vrf_json, !!json, tag,
+						     !!json, tag,
 						     prefix_str ? prefix : NULL,
 						     !!supernets_only, type,
 						     ospf_instance_id, !!ng,
 						     &ctx);
 			else
 				do_show_ip_route(vty, zvrf_name(zvrf), afi,
-						 SAFI_UNICAST, !!fib, vrf_json,
-						 !!json, tag,
-						 prefix_str ? prefix : NULL,
+						 SAFI_UNICAST, !!fib, !!json,
+						 tag, prefix_str ? prefix : NULL,
 						 !!supernets_only, type,
 						 ospf_instance_id, table, !!ng,
 						 &ctx);
-
-			if (!!json)
-				json_object_object_add(root_json,
-						       zvrf_name(zvrf),
-						       vrf_json);
-		}
-		if (!!json) {
-			vty_json_no_pretty(vty, root_json);
-			root_json = NULL;
 		}
+		if (json)
+			vty_json_close(vty, first_vrf_json);
 	} else {
 		vrf_id_t vrf_id = VRF_DEFAULT;
 
@@ -1823,13 +1800,13 @@ DEFPY (show_route,
 			return CMD_SUCCESS;
 
 		if (table_all)
-			do_show_ip_route_all(vty, zvrf, afi, !!fib, NULL, !!json,
-					     tag, prefix_str ? prefix : NULL,
+			do_show_ip_route_all(vty, zvrf, afi, !!fib, !!json, tag,
+					     prefix_str ? prefix : NULL,
 					     !!supernets_only, type,
 					     ospf_instance_id, !!ng, &ctx);
 		else
 			do_show_ip_route(vty, vrf->name, afi, SAFI_UNICAST,
-					 !!fib, NULL, !!json, tag,
+					 !!fib, !!json, tag,
 					 prefix_str ? prefix : NULL,
 					 !!supernets_only, type,
 					 ospf_instance_id, table, !!ng, &ctx);