-
Notifications
You must be signed in to change notification settings - Fork 650
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
[db_migrator][201911] Support shared headroom in db_migrator on Mellanox platform #1261
[db_migrator][201911] Support shared headroom in db_migrator on Mellanox platform #1261
Conversation
…oom pool Signed-off-by: Stephen Sun <stephens@nvidia.com>
1. rewording: partial => condensed extract => extend 2. remove mapping method "platform" which is not used yet Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
This pull request introduces 1 alert when merging dba77b9 into c0df635 - view on LGTM.com new alerts:
|
Signed-off-by: Stephen Sun <stephens@nvidia.com>
retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mellanox db migrator logic is getting complicated as we are adding more versions and skus. We will need a test to actually ensure that the db migrator is working as expected when trying to move across/between branches.
"spc1_t0_pool": {"doublepool": { "size": "5029836" }, "egress_lossless_pool": { "size": "14024599"}}, | ||
"spc1_t1_pool": {"doublepool": { "size": "2097100" }, "egress_lossless_pool": { "size": "14024599"}}, | ||
"spc2_t0_pool": {"doublepool": { "size": "14983147" }, "egress_lossless_pool": { "size": "34340822"}}, | ||
"spc2_t1_pool": {"doublepool": { "size": "9158635" }, "egress_lossless_pool": { "size": "34340822"}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is 3800 pool size excluded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, 3800 isn't deployed into the production environment. So we don't need to support migrate configuration between different versions of 201911.
We will support migrating configuration from the latest 201911 to master for 3800.
"pg_lossless_50000_300m_profile": {"size": "121856", "xon": "18432"}, | ||
"pg_lossless_100000_300m_profile": {"size": "206848", "xon": "18432"}, | ||
"pg_lossless_200000_300m_profile": {"size": "376832", "xon": "18432"}} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3800 headroom?
"q_lossy_profile": {"dynamic_th": "3", "pool": "[BUFFER_POOL|egress_lossy_pool]", "size": "0"}} | ||
} | ||
}, | ||
"version_1_0_5": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update comment below
}, | ||
"version_1_0_4": { | ||
# version 1.0.4 is introduced for updating the buffer settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible to add comments on which db migrator version maps to which SONiC branch? It will be easier to cross check all the buffer settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. will add comments for that.
scripts/mellanox_buffer_migrator.py
Outdated
new_config_name = new_config_map | ||
return new_config_name | ||
|
||
def mlnx_migrate_extend_condensed_pool(self, pool_config, config_name = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the spaces between assignment, config_name=None
default_buffer_profiles_old = self.mlnx_default_buffer_parameters(old_version, "buffer_profiles") | ||
default_buffer_profiles_new = self.mlnx_default_buffer_parameters(new_version, "buffer_profiles") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all versions do not seem to have "buffer_profiles" defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the version not having it, it will get None and the this part will be skipped
Ask for some unit test in this PR. |
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@neethajohn please check if comments handled and you can approve. |
Signed-off-by: Stephen Sun <stephens@nvidia.com>
This pull request introduces 7 alerts when merging bba5ce7 into a9ef1c5 - view on LGTM.com new alerts:
|
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs , please fix the lgtm alerts. the new testcases are failing as well. Can you check? |
Checking... |
This pull request introduces 3 alerts when merging 71aa4da into a9ef1c5 - view on LGTM.com new alerts:
|
Signed-off-by: stephens <stephens@contoso.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
…ffer migrator Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs can you please confirm all the files on this PR indeed required? there are 83 files changed |
Yes, all of them are required. Most of the are the mock config_dbs for the db_migrator unit test for each version and SKUs. |
Do not set PG to Buffer porfile mapping again if already exist. (sonic-net#1261) [sub intf] Use m_lag_id to be the parent port object id when parent port is LAG (sonic-net#1235)
[Submodule update] sonic-utilities - [db_migrator][201911] Support shared headroom in db_migrator on Mellanox platform (sonic-net#1261) - Multi-ASIC support show ip/v6 route additional parameters (sonic-net#1333) Signed-off-by: Stephen Sun <stephens@nvidia.com>
Added support to upgrade a switch from old version to one with shared headroom pool support
- What I did
- How I did it
- How to verify it
Verify upgrade from 201811 to 201911 with shared headroom functionality
Verify upgrade from early 201911 to 201911 with shared headroom functionality
- 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)