-
Notifications
You must be signed in to change notification settings - Fork 539
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
[Reclaim buffer][202106] Reclaim unused buffer for dynamic buffer model #1986
Closed
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
stephenxs
force-pushed
the
reclaim-buffer-202106
branch
3 times, most recently
from
November 19, 2021 02:03
a2ef5d8
to
0d21a95
Compare
stephenxs
changed the title
[Reclaim buffer][202106] Reclaim unused buffer for both traditional and dynamic buffer model
[Reclaim buffer][202106] Reclaim unused buffer for dynamic buffer model
Nov 19, 2021
stephenxs
force-pushed
the
reclaim-buffer-202106
branch
from
November 24, 2021 08:54
124a6c8
to
e963183
Compare
This is to backport community PR 1910 to 202106 branch. **What I did** Reclaim reserved buffer of unused ports for both dynamic and traditional models. This is done by - Removing lossless priority groups on unused ports. - Applying zero buffer profiles on the buffer objects of unused ports. - In the dynamic buffer model, the zero profiles are loaded from a JSON file and applied to `APPL_DB` if there are admin down ports. The default buffer configuration will be configured on all ports. Buffer manager will apply zero profiles on admin down ports. - In the static buffer model, the zero profiles are loaded by the buffer template. Signed-off-by: Stephen Sun <[email protected]> **Why I did it** **How I verified it** Regression test and vs test. **Details if related** ***Static buffer model*** Remove the lossless buffer priority group if the port is admin-down and the buffer profile aligns with the speed and cable length of the port. ***Dynamic buffer model*** ****Handle zero buffer pools and profiles**** 1. buffermgrd: add a CLI option to load the JSON file for zero profiles. 2. Load them from JSON file into the internal buffer manager's data structure 3. Apply them to APPL_DB once there is at least one admin-down port - Record zero profiles' names in the pool object it references. By doing so, the zero profile lists can be constructed according to the normal profile list. There should be one profile for each pool on the ingress/egress side. - And then apply the zero profiles to the buffer objects of the port. - Unload them from APPL_DB once all ports are admin-up since the zero pools and profiles are no longer referenced. Remove buffer pool counter id when the zero pool is removed. 4. Now that it's possible that a pool will be removed from the system, the watermark counter of the pool is removed ahead of the pool itself being removed. ****Handle port admin status change**** 1. Currently, there is a logic of removing buffer priority groups of admin down ports. This logic will be reused and extended for all buffer objects, including `BUFFER_QUEUE`, `BUFFER_PORT_INGRESS_PROFILE_LIST`, and `BUFFER_PORT_EGRESS_PROFILE_LIST`. - When the port is admin down, - The normal profiles are removed from the buffer objects of the port - The zero profiles, if provided, are applied to the port - When the port is admin up, - The zero profiles, if applied, are removed from the port - The normal profiles are applied to the port. 2. Ports orchagent exposes the number of queues and priority groups to STATE_DB. Buffer manager can take advantage of these values to apply zero profiles on all the priority groups and queues of the admin-down ports. In case it is not necessary to apply zero profiles on all priority groups or queues on a certain platform, `ids_to_reclaim` can be customized in the JSON file. 3. Handle all buffer tables, including `BUFFER_PG`, `BUFFER_QUEUE`, `BUFFER_PORT_INGRESS_PROFILE_LIST` and `BUFFER_PORT_EGRESS_PROFILE_LIST` - Originally, only the `BUFFER_PG` table was cached in the dynamic buffer manager. - Now, all tables are cached in order to apply zero profiles when a port is admin down and apply normal profiles when it's up. - The index of such tables can include a single port or a list of ports, like `BUFFER_PG|Ethernet0|3-4` or `BUFFER_PG|Ethernet0,Ethernet4,Ethernet8|3-4`. Originally, there is a logic to handle such indexes for the `BUFFER_PG` table. Now it is reused and extended to handle all the tables. 4. [Mellanox] Plugin to calculate buffer pool size: - Originally, buffer for the queue, buffer profile list, etc. were not reclaimed for admin-down ports so they are reserved for all ports. - Now, they are reserved for admin-up ports only. ****Accelerate the progress of applying buffer tables to APPL_DB**** This is an optimization on top of reclaiming buffer. 1. Don't apply buffer profiles, buffer objects to `APPL_DB` before buffer pools are applied when the system is starting. This is to apply the items in an order from referenced items to referencing items and try to avoid buffer orchagent retrying due to referenced table items. However, it is still possible that the referencing items are handled before referenced items. In that case, there should not be any error message. 2. [Mellanox] Plugin to calculate buffer pool size: Return the buffer pool sizes value currently in APPL_DB if the pool sizes are not able to be calculated due to lacking some information. This typically happens at the system start. This is to accelerate the progress of pushing tables to APPL_DB.
stephenxs
force-pushed
the
reclaim-buffer-202106
branch
from
December 3, 2021 14:08
124a6c8
to
2dfe25b
Compare
neethajohn
approved these changes
Dec 6, 2021
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
- Improve admin down test cases - Restore cable length to 0m after test in order to prevent traditional buffer manager from creating lossless profiles Signed-off-by: Stephen Sun <[email protected]>
Signed-off-by: stephens <[email protected]>
Depends on #2038 |
neethajohn
approved these changes
Dec 8, 2021
/Azp run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run LGTM |
No pipelines are associated with this pull request. |
LGTM failed due to dependency on #2038 |
Currently, the PR depends on #2118 for the vstest failure. |
EdenGri
pushed a commit
to EdenGri/sonic-swss
that referenced
this pull request
Feb 28, 2022
- What I did Two bugs were found related to this feature. This PR included fixes for this. 1. non-default since argument is not being applied. Happening because subprocess_exec(["date", "--date='{}'".format(since_cfg)]) is failing. Replacing this with subprocess_exec(["date", "--date={}".format(since_cfg)]) solved the problem. 2. core_cleanup is not working because of the unnecessary recent_file_creation check. - How I did it Remove '' from date - How to verify it Run manual test flow which found the issue Signed-off-by: Vivek Reddy Karri <[email protected]>
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Azure.sonic-swss |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
This is to backport community PR #1910 to 202106 branch.
Depends on #2038 which backports #1996 to 202106 branch.
What I did
Reclaim reserved buffer of unused ports for both dynamic and traditional models.
This is done by
APPL_DB
if there are admin down ports.The default buffer configuration will be configured on all ports. Buffer manager will apply zero profiles on admin down ports.
Signed-off-by: Stephen Sun [email protected]
Why I did it
How I verified it
Regression test and vs test.
Details if related
Static buffer model
Remove the lossless buffer priority group if the port is admin-down and the buffer profile aligns with the speed and cable length of the port.
Dynamic buffer model
Handle zero buffer pools and profiles
By doing so, the zero profile lists can be constructed according to the normal profile list. There should be one profile for each pool on the ingress/egress side.
Remove buffer pool counter id when the zero pool is removed.
Handle port admin status change
BUFFER_QUEUE
,BUFFER_PORT_INGRESS_PROFILE_LIST
, andBUFFER_PORT_EGRESS_PROFILE_LIST
.Buffer manager can take advantage of these values to apply zero profiles on all the priority groups and queues of the admin-down ports.
In case it is not necessary to apply zero profiles on all priority groups or queues on a certain platform,
ids_to_reclaim
can be customized in the JSON file.BUFFER_PG
,BUFFER_QUEUE
,BUFFER_PORT_INGRESS_PROFILE_LIST
andBUFFER_PORT_EGRESS_PROFILE_LIST
BUFFER_PG
table was cached in the dynamic buffer manager.BUFFER_PG|Ethernet0|3-4
orBUFFER_PG|Ethernet0,Ethernet4,Ethernet8|3-4
. Originally, there is a logic to handle such indexes for theBUFFER_PG
table. Now it is reused and extended to handle all the tables.Accelerate the progress of applying buffer tables to APPL_DB
This is an optimization on top of reclaiming buffer.
APPL_DB
before buffer pools are applied when the system is starting.This is to apply the items in an order from referenced items to referencing items and try to avoid buffer orchagent retrying due to referenced table items.
However, it is still possible that the referencing items are handled before referenced items. In that case, there should not be any error message.
Return the buffer pool sizes value currently in APPL_DB if the pool sizes are not able to be calculated due to lacking some information. This typically happens at the system start.
This is to accelerate the progress of pushing tables to APPL_DB.