Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: show json output changes to optimize various show commands #17431

Merged
merged 6 commits into from
Jan 7, 2025
Merged

bgpd: show json output changes to optimize various show commands #17431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7decdc5
bgpd: [EVPN GR] Added JSON support for "show bgp router"
Pdoijode Nov 14, 2023
9c55301
bgpd: show bgp l2vpn evpn route detail json stringify changes
ashred-lnx Nov 2, 2023
4838bac
bgpd: neighbors received-routes/advertised-routes stringify changes
Nov 27, 2023
c8290e9
bgpd: Nexthop Stringify changes
ashred-lnx Nov 7, 2023
355d5e2
bgpd: add topotest for show bgp router json command
krishna-samy Nov 15, 2024
4d8207c
bgpd: add documentation for show bgp router json command
krishna-samy Nov 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 42 additions & 27 deletions bgpd/bgp_evpn_vty.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3115,6 +3115,7 @@ static void evpn_show_all_routes(struct vty *vty, struct bgp *bgp, int type,
afi_t afi;
safi_t safi;
uint32_t prefix_cnt, path_cnt;
int first = true;

afi = AFI_L2VPN;
safi = SAFI_EVPN;
Expand All @@ -3139,8 +3140,15 @@ static void evpn_show_all_routes(struct vty *vty, struct bgp *bgp, int type,
prefix_rd2str((struct prefix_rd *)rd_destp, rd_str,
sizeof(rd_str), bgp->asnotation);

if (json)
if (json) {
if (first) {
vty_out(vty, "\"%s\":", rd_str);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by these literal json outputs: why do we want to hard-code these kinds of components in the output? if we're not managing the json object hierarchy correctly, I'd rather we fix that than stick some hard-coded tokens in the output like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the parameters were identified to stringify them in the output so that the overall memory consumption is reduced when the json command was executed in a high scale setup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we talked about this issue at the dev meeting last week, and I thought we had agreement that we wouldn't hard-code json-ish strings in the output stream - but these are still present?

first = false;
} else {
vty_out(vty, ",\"%s\":", rd_str);
}
json_rd = json_object_new_object();
}

rd_header = 1;

Expand Down Expand Up @@ -3255,18 +3263,18 @@ static void evpn_show_all_routes(struct vty *vty, struct bgp *bgp, int type,
}

if (json) {
if (add_rd_to_json)
json_object_object_add(json, rd_str, json_rd);
else {
if (add_rd_to_json) {
vty_json_no_pretty(vty, json_rd);
} else {
vty_out(vty, "{}");
json_object_free(json_rd);
json_rd = NULL;
}
}
}

if (json) {
json_object_int_add(json, "numPrefix", prefix_cnt);
json_object_int_add(json, "numPaths", path_cnt);
vty_out(vty, ",\"numPrefix\":%u", prefix_cnt);
vty_out(vty, ",\"numPaths\":%u", path_cnt);
} else {
if (prefix_cnt == 0) {
vty_out(vty, "No EVPN prefixes %sexist\n",
Expand All @@ -3284,20 +3292,18 @@ int bgp_evpn_show_all_routes(struct vty *vty, struct bgp *bgp, int type,
{
json_object *json = NULL;

if (use_json)
if (use_json) {
json = json_object_new_object();
vty_out(vty, "{\n");
}

evpn_show_all_routes(vty, bgp, type, json, detail, false);

if (use_json)
/*
* We are using no_pretty here because under extremely high
* settings (lots of routes with many different paths) this can
* save several minutes of output when FRR is run on older cpu's
* or more underperforming routers out there. So for route
* scale, we need to use no_pretty json.
*/
vty_json_no_pretty(vty, json);
if (use_json) {
vty_out(vty, "}\n");
json_object_free(json);
}

return CMD_SUCCESS;
}

Expand Down Expand Up @@ -4946,8 +4952,10 @@ DEFUN(show_bgp_l2vpn_evpn_route,
if (!bgp)
return CMD_WARNING;

if (uj)
if (uj) {
json = json_object_new_object();
vty_out(vty, "{\n");
}

if (bgp_evpn_cli_parse_type(&type, argv, argc) < 0)
return CMD_WARNING;
Expand All @@ -4960,13 +4968,10 @@ DEFUN(show_bgp_l2vpn_evpn_route,

evpn_show_all_routes(vty, bgp, type, json, detail, self_orig);

/*
* This is an extremely expensive operation at scale
* and as such we need to save as much time as is
* possible.
*/
if (uj)
vty_json_no_pretty(vty, json);
if (uj) {
vty_out(vty, "}\n");
json_object_free(json);
}

return CMD_SUCCESS;
}
Expand Down Expand Up @@ -5023,10 +5028,20 @@ DEFUN(show_bgp_l2vpn_evpn_route_rd,
if (bgp_evpn_cli_parse_type(&type, argv, argc) < 0)
return CMD_WARNING;

if (rd_all)
if (rd_all) {
if (uj)
vty_out(vty, "{\n");

evpn_show_all_routes(vty, bgp, type, json, 1, false);
else

if (uj) {
vty_out(vty, "}\n");
json_object_free(json);
return CMD_SUCCESS;
}
} else {
evpn_show_route_rd(vty, bgp, &prd, type, json);
}

if (uj)
vty_json(vty, json);
Expand Down
Loading
Loading