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

Jump to bottom

[Reclaim buffer] [Mellanox] Db migrator support reclaiming reserved buffer for unused ports #1822

Merged
merged 16 commits into from
Nov 29, 2021
Merged

[Reclaim buffer] [Mellanox] Db migrator support reclaiming reserved buffer for unused ports #1822

merged 16 commits into from
Nov 29, 2021

Conversation

stephenxs
Copy link
Collaborator

What I did

Db migrator support reclaiming reserved buffer for unused ports

Signed-off-by: Stephen Sun stephens@nvidia.com

How I did it

For admin down ports, if the buffer objects configuration aligns with default configuration, set the buffer objects configuration as:

  • Dynamic model: all normal buffer objects are configured on admin down ports. Buffer manager will apply zero profiles on admin down ports.
  • Static model: zero buffer objects are configured on admin down ports.

How to verify it

  1. Unit test.
  2. Manually test.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@stephenxs
Reclaim unused buffer
0d81cf1 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Adjust buffer testcases to avoid noise when verifying reclaiming buffer
901e69b 
- Set admin_status to up for all ports so that the reclaiming buffer migrator won't interfere it

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Adjust test case for reclaiming buffer
144550a 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Don't generate a unified zero buffer items on admin down ports for qu…
46c9f71 
…eues

Generate a zero buffer item for 0-2, 3-4, 5-6 separatedly.
The previous approach can be problematic in case switching between lossless and lossy isn't supported on the platform

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Adjust comments
d66689e 
- Add more comments
- Adjust code

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs stephenxs mentioned this pull request Sep 15, 2021
Merged
@lgtm-com
Copy link

lgtm-com bot commented Sep 26, 2021

This pull request introduces 1 alert when merging 499bbeb into 0d538d3 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

stephens and others added 6 commits September 26, 2021 15:08
@stephenxs
Fix issue: default lossless PG should be left in CONFIG_DB
b93c565 
By doing so, the buffer manager is able to remove it from APPL_DB in warm reboot

Signed-off-by: stephens <stephens@contoso.com>

Conflicts:
	scripts/mellanox_buffer_migrator.py
@stephenxs
Fix LGTM alerts
d87ab15 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Fix the target db version of portchannel-key test
106e805 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Fix issue: some redundant table entries were added to APPL_DB
3e18418 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Add testcases for reclaiming buffer warm reboot test
bbf0797 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Adjust db migrator according to qos table reference change
e45fde7 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Collaborator Author

stephenxs commented Oct 19, 2021 •
edited
Loading

Detailed changes
scripts/db_migrator.py: add version 2_0_4. mellanox buffer migrator will be called for reclaiming buffer if db version is upgraded from 2_0_3 to 2_0_4

scripts/mellanox_buffer_migrator.py: do the real work of reclaiming buffer

  • if the buffer configuration was default in the old version, meaning

    • no items in BUFFER_QUEUE, BUFFER_PORT_INGRESS_PROFILE_LIST, and BUFFER_PORT_EGRESS_PROFILE_LIST
    • no items in BUFFER_PG or only PG 3-4 is configured according to speed and cable length in the traditional model or NULL in the dynamic model

    the zero profiles will be applied on the inactive port in the traditional model and normal profiles will be configured in the dynamic model (buffer manager will apply zero profiles to APPL_DB)

  • warm-reboot: remove all the dynamic profiles that are no longer referenced any more

other files: test cases.

@neethajohn
Copy link
Contributor

Detailed changes scripts/db_migrator.py: add version 2_0_4. mellanox buffer migrator will be called for reclaiming buffer if db version is upgraded from 2_0_3 to 2_0_4

scripts/mellanox_buffer_migrator.py: do the real work of reclaiming buffer

  • if the buffer configuration was default in the old version, meaning

    • no items in BUFFER_QUEUE, BUFFER_PORT_INGRESS_PROFILE_LIST, and BUFFER_PORT_EGRESS_PROFILE_LIST
    • no items in BUFFER_PG or only PG 3-4 is configured according to speed and cable length in the traditional model or NULL in the dynamic model
      the zero profiles will be applied on the inactive port in the traditional model and normal profiles will be configured in the dynamic model (buffer manager will apply zero profiles to APPL_DB)

  • warm-reboot: remove all the dynamic profiles that are no longer referenced any more

other files: test cases.

Can you recheck the warm-boot case? Will there be issues when some objects present in older version are no longer present after warm-boot?

@stephenxs
Copy link
Collaborator Author

  • warm-reboot: remove all the dynamic profiles that are no longer referenced any more

other files: test cases.

Can you recheck the warm-boot case? Will there be issues when some objects present in older version are no longer present after warm-boot?

Thanks for bringing this up. I don’t see a risk after reflecting it because only the profiles that are not be referenced will be removed.

@stephenxs
Fix typo in log message
0732976 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Enhance the logic to determine pool model (single pool/double pools)
ae6dc24 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
stephenxs
stephenxs commented Oct 22, 2021
View reviewed changes
scripts/db_migrator.py Outdated Show resolved Hide resolved
scripts/mellanox_buffer_migrator.py Outdated Show resolved Hide resolved
This was referenced Oct 26, 2021
Merged
Merged
@stephenxs stephenxs marked this pull request as ready for review November 1, 2021 03:19
@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Add a missed file
a6b83c3 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs stephenxs requested a review from neethajohn November 23, 2021 01:16
@stephenxs stephenxs requested a review from neethajohn November 23, 2021 15:39
@neethajohn neethajohn merged commit 67a267b into sonic-net:master Nov 29, 2021
@stephenxs stephenxs deleted the reclaim-buffer branch November 29, 2021 21:43
@aravindmani-1 aravindmani-1 mentioned this pull request Dec 8, 2021
Closed
5 tasks
@DavidZagury DavidZagury mentioned this pull request Dec 8, 2021
Closed
5 tasks
abdosi pushed a commit that referenced this pull request Dec 8, 2021
@stephenxs @abdosi
[Reclaim buffer] [Mellanox] Db migrator support reclaiming reserved b…
71cf3ee 
…uffer for unused ports (#1822)

Signed-off-by: Stephen Sun stephens@nvidia.com

What I did
Db migrator support reclaiming reserved buffer for unused ports

How I did it
For admin down ports, if the buffer objects configuration aligns with default configuration, set the buffer objects configuration as:

Dynamic model: all normal buffer objects are configured on admin down ports. Buffer manager will apply zero profiles on admin down ports.
Static model: zero buffer objects are configured on admin down ports.

How to verify it
Unit test.
Manually test.
liat-grozovik pushed a commit that referenced this pull request Dec 11, 2021
@stephenxs
[Reclaiming buffer][202012] Database migrator for reclaiming buffer (#…
32d18d2 
…1898)

This is to cherry-pick community PR #1822 to 202012.

- What I did
Db migrator support reclaiming reserved buffer for unused ports

As there is no empty slot for database version between 202012 and master (202111), this migration will be done regardless of whether database version is changed. The DB migrator should be idempotent.

- How I did it
For admin down ports, if the buffer objects configuration aligns with default configuration, set the buffer objects configuration as:
1. Dynamic model: all normal buffer objects are configured on admin down ports. Buffer manager will apply zero profiles on admin down ports.
2. Static model: zero buffer objects are configured on admin down ports.

- How to verify it
Unit test and manually 

Signed-off-by: Stephen Sun <stephens@nvidia.com>
liat-grozovik pushed a commit that referenced this pull request Dec 20, 2021
@stephenxs
[Reclaiming buffer][202106] Database migrator for reclaiming buffer (#…
642f508 
…1897)

This is to cherry-pick community PR #1822 to 202106.

- What I did
Db migrator support reclaiming reserved buffer for unused ports
As there is no empty slot for database version between 202106 and master (202111), this migration will be done regardless of whether database version is changed. The DB migrator should be idempotent.

- How I did it
For admin down ports, if the buffer objects configuration aligns with default configuration, set the buffer objects configuration as:
1. Dynamic model: all normal buffer objects are configured on admin down ports. Buffer manager will apply zero profiles on admin down ports.
2. Static model: zero buffer objects are configured on admin down ports.

- How to verify it
1. Unit test.
2. Manually test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neethajohn neethajohn neethajohn approved these changes

@keboliu keboliu keboliu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stephenxs @mssonicbld @neethajohn @keboliu