[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 |
neethajohn
left a comment
neethajohn
left a comment
There was a problem hiding this comment.
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.
Why is 3800 pool size excluded?
There was a problem hiding this comment.
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"}} | ||
| } |
| "q_lossy_profile": {"dynamic_th": "3", "pool": "[BUFFER_POOL|egress_lossy_pool]", "size": "0"}} | ||
| } | ||
| }, | ||
| "version_1_0_5": { |
| } | ||
| }, | ||
| "version_1_0_4": { | ||
| # version 1.0.4 is introduced for updating the buffer settings |
There was a problem hiding this comment.
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.
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.
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.
all versions do not seem to have "buffer_profiles" defined
There was a problem hiding this comment.
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>
4 participants
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)