-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
"show bgp ipv4 detail json" consumes too much memory #16880
Open
pguibert6WIND
wants to merge
6
commits into
FRRouting:master
Choose a base branch
from
pguibert6WIND:bgp_detail_no_json_alloc_2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
"show bgp ipv4 detail json" consumes too much memory #16880
pguibert6WIND
wants to merge
6
commits into
FRRouting:master
from
pguibert6WIND:bgp_detail_no_json_alloc_2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pguibert6WIND
changed the title
Bgp detail no json alloc 2
"show bgp ipv4 detail json" consumes too much memory
Sep 20, 2024
pguibert6WIND
force-pushed
the
bgp_detail_no_json_alloc_2
branch
from
September 23, 2024 08:34
1a0e960
to
9bec399
Compare
pguibert6WIND
force-pushed
the
bgp_detail_no_json_alloc_2
branch
6 times, most recently
from
September 26, 2024 08:13
69093ca
to
6209576
Compare
When using the 'show bgp ipv4 json detail' against a full route, the BGP memory consumed increases a lot, and is never restored. Before the show: > root@dut-orwell-nianticvf:~# ps -aux | grep bgpd > root 12263 10.8 10.1 1213920 1033452 ? Ssl 10:43 1:42 /usr/bin/bgpd -A 127.0.0.1 -M snmp -M rpki -M bmp After the show: > root@dut-orwell-nianticvf:~# ps -aux | grep bgpd > root 11772 4.9 19.3 2150668 1970548 ? Ssl 08:59 2:09 /usr/bin/bgpd -A 127.0.0.1 -M snmp -M rpki -M bmp This is not a memory leak, since applying multiple times the command does not increase the memory. What happens is that some internal structures of BGP store a json structure: AsPath, Communities, Large Communities. This series of commits intends to do the following for the following objects: ASPaths, communities and large communities: - replace the usage of the internal json structure by a dynamically built json object. - remove the no more used json object referenced in the BGP internals. That first commit addresses the case with ASpaths: instead of relying on the internal json structure, the json aspath objects are allocated and freed at usage. The implementation relies on the json_object_set_serializer() utility. The function overrides the json buffer build at display time. Do fallback to the original code when this utility is not present in json-c library (aka < 0.13). Signed-off-by: Philippe Guibert <[email protected]>
Create the community_json_list() function, that stores the community list output in a passed buffer, and optionally in the json community list passed array. This commit does not have any functional impact. Signed-off-by: Philippe Guibert <[email protected]>
The community_get_json() API is used to return a json object. At display time, this object will use the community_list_json_to_string() function to handle the output. Signed-off-by: Philippe Guibert <[email protected]>
Create the lcommunity_json_list() function, that returns the large community list output in a returned buffer, and optionally in the json large community list passed array. This commit does not have any functional impact. Signed-off-by: Philippe Guibert <[email protected]>
The lcommunity_get_json() API is used to return a json object. At display time, this object will use the lcommunity_json_to_string() function to handle the output. Signed-off-by: Philippe Guibert <[email protected]>
This API proposes to use the sprintbuf() method to facilitate the forge of buffers in json. Note that this buffer is limited to 128 bytes, so it can not be used everywhere as it without any checks. Signed-off-by: Philippe Guibert <[email protected]>
pguibert6WIND
force-pushed
the
bgp_detail_no_json_alloc_2
branch
from
September 26, 2024 10:22
6209576
to
de17bbb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After having loaded a full route, the memory consumption still increases after executing the 'show bgp ipv4 json detail' command. The memory consumed is related to json objects stored for BGP as paths, standard and large communities.
The below pull request proposes to incrementally display each path detailed information, without relying on the json storage.
Memory consumed after BGP routes have been loaded:
Memory consumed after the 'show bgp ipv4 json detail' command is executed, without fix:
after fix:
I get a memory gain (roughly 500 MB spared, -35% memory).
I get a timing status (8 seconds, +32% speed)