Skip to content

SONIC QOS YANG - Remove qos tables field value reference format#7752

Merged
neethajohn merged 1 commit intosonic-net:masterfrom
AshokDaparthi:sonic-yang-qos-fv-db-ref-remove
Sep 28, 2021
Merged

SONIC QOS YANG - Remove qos tables field value reference format#7752
neethajohn merged 1 commit intosonic-net:masterfrom
AshokDaparthi:sonic-yang-qos-fv-db-ref-remove

Conversation

@AshokDaparthi
Copy link
Copy Markdown
Contributor

@AshokDaparthi AshokDaparthi commented May 29, 2021

Depends on sonic-net/sonic-utilities#1626
Depends on sonic-net/sonic-swss#1754

QOS tables in config db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue to other qos tables.

Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},

This format is not consistent with other DB schema followed in sonic.
And also this reference in DB is not required, This is taken care by YANG "leafref".

Removed this format from all platform files to consistent with other sonic db schema.
Example:
"Ethernet92|3": {
"scheduler": "scheduler.1",
"wred_profile": "AZURE_LOSSLESS"
},

Dependent pull requests:
#7752 - To modify platfrom files
#7281 - Yang model
sonic-net/sonic-utilities#1626 - DB migration
sonic-net/sonic-swss#1754 - swss change to remove ABNF format

@anshuv-mfst
Copy link
Copy Markdown

Hi @neethajohn , @stephenxs- could you please review this PR. This is related to sonic-net/sonic-utilities#1626 and sonic-net/sonic-swss#1754, thanks.

@stephenxs
Copy link
Copy Markdown
Collaborator

Hi @neethajohn , @stephenxs- could you please review this PR. This is related to Azure/sonic-utilities#1626 and Azure/sonic-swss#1754, thanks.

Sure. Will review it.
Btw, is there a PR for the regression test? I think we need to update regression test as well

@neethajohn
Copy link
Copy Markdown
Contributor

Hi @neethajohn , @stephenxs- could you please review this PR. This is related to Azure/sonic-utilities#1626 and Azure/sonic-swss#1754, thanks.

Sure. Will review it.
Btw, is there a PR for the regression test? I think we need to update regression test as well

Are you referring to the sonic-mgmt test? I don't think there is a PR yet. @AshokDaparthi, please update that as well

@AshokDaparthi
Copy link
Copy Markdown
Contributor Author

Hi @neethajohn , @stephenxs- could you please review this PR. This is related to Azure/sonic-utilities#1626 and Azure/sonic-swss#1754, thanks.

Sure. Will review it.
Btw, is there a PR for the regression test? I think we need to update regression test as well

Are you referring to the sonic-mgmt test? I don't think there is a PR yet. @AshokDaparthi, please update that as well

@neethajohn - I will explore what to update, i am not aware of sonic-mgmt tests.

@AshokDaparthi
Copy link
Copy Markdown
Contributor Author

@stephenxs @neethajohn - Regarding sonic-mgmt test cases, i am not doing any new functionality, Do you want me to change existing test cases to remove the ABNF format reference?

@neethajohn
Copy link
Copy Markdown
Contributor

@stephenxs @neethajohn - Regarding sonic-mgmt test cases, i am not doing any new functionality, Do you want me to change existing test cases to remove the ABNF format reference?

Yes

stephenxs
stephenxs previously approved these changes Jul 12, 2021
@AshokDaparthi
Copy link
Copy Markdown
Contributor Author

@stephenxs @neethajohn - Updated the sonic-mgmt test cases sonic-net/sonic-mgmt#3824

@stephenxs
Copy link
Copy Markdown
Collaborator

@AshokDaparthi can you fix the conflicts

@rathnasabapathyv
Copy link
Copy Markdown
Collaborator

As per the discussion in yang-subgroup-meeting (8/26) with @lguohan , we have decided to merge sonic-utilities PR (sonic-net/sonic-utilities#1626) 1st followed by sub-module update in sonic-buildimage. Then this sonic-swss PR (sonic-net/sonic-swss#1754) should be merged.

Merge can happen in this order:
sonic-utilities PR for DB-migration logic : sonic-net/sonic-utilities#1626
sonic-buildimage PR for new config-DB changes : #7752
sonic-swss PR : sonic-net/sonic-swss#1754

@AshokDaparthi AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from d94b14e to 569722a Compare August 31, 2021 19:34
lguohan pushed a commit to sonic-net/sonic-utilities that referenced this pull request Sep 2, 2021
Qos tables in config db and app db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue other qos tables.

Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},

AppDB:
"BUFFER_QUEUE_TABLE:Ethernet88:3-4": {
"profile": "[BUFFER_PROFILE_TABLE:egress_lossless_profile]"
},

1#This format is not consistent with other DB schema followed in sonic.
2# Added db_migrator.py case to  change from old format in config_db and appl_db  to new format. 
3#Modified the test case 

Dependent pull requests: 
sonic-net/sonic-buildimage#7752  - To modify platfrom files 
sonic-net/sonic-buildimage#7281 - Yang model 
sonic-net/sonic-swss#1754    - swss change to remove ABNF format
@neethajohn
Copy link
Copy Markdown
Contributor

@AshokDaparthi, please resolve the merge conflicts

@AshokDaparthi AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from 569722a to 011a918 Compare September 3, 2021 21:59
@neethajohn
Copy link
Copy Markdown
Contributor

@AshokDaparthi, how are these changes verified? Also you have only mentioned #7281 as a dependency in this PR. That contains only the SCHEDULER, WRED_PROFILE and QUEUE related changes. What about the other MMU tables (BUFFER PROFILE, BUFFER_POOL etc)? since this PR contains changes for all MMU/QOS attributes

@AshokDaparthi
Copy link
Copy Markdown
Contributor Author

@neethajohn - #7281 this PR is YNAG PR. For ALL QOS YANG PR's are blocked by this DB format. These YNAG PR's for buffers will are opened by broadcom else once this PR's are merged i will push sonic YNAG PR's.
Regarding testing, i get sonic-utilities , sonic-swss. sonic-buildiimage changes and created image and tested manually broadcom platform.

@neethajohn neethajohn changed the title SONIC QOS YANG - Remove qos tables field value refernce format SONIC QOS YANG - Remove qos tables field value reference format Sep 14, 2021
@AshokDaparthi AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from 011a918 to 8311b3d Compare September 23, 2021 15:59
@neethajohn
Copy link
Copy Markdown
Contributor

@AshokDaparthi , can you please mention the merge order that you had discussed with Prince in this PR

@AshokDaparthi
Copy link
Copy Markdown
Contributor Author

@neethajohn - Merged order is below
1#sonic-net/sonic-swss#1754
2# sonic-net/sonic-mgmt#3824
3# submodule update of swss
4# Merge this pull request
Can you help with merge first 2.

@AshokDaparthi AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from 8311b3d to a8963a5 Compare September 26, 2021 22:18
@AshokDaparthi AshokDaparthi force-pushed the sonic-yang-qos-fv-db-ref-remove branch from a8963a5 to 3e4acdf Compare September 28, 2021 03:12
@neethajohn neethajohn merged commit 6cbdf11 into sonic-net:master Sep 28, 2021
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
Qos tables in config db and app db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue other qos tables.

Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},

AppDB:
"BUFFER_QUEUE_TABLE:Ethernet88:3-4": {
"profile": "[BUFFER_PROFILE_TABLE:egress_lossless_profile]"
},

1#This format is not consistent with other DB schema followed in sonic.
2# Added db_migrator.py case to  change from old format in config_db and appl_db  to new format. 
3#Modified the test case 

Dependent pull requests: 
sonic-net/sonic-buildimage#7752  - To modify platfrom files 
sonic-net/sonic-buildimage#7281 - Yang model 
sonic-net/sonic-swss#1754    - swss change to remove ABNF format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants