SONiC QOS Scheduler, WRED, Queue Yangs by AshokDaparthi · Pull Request #7281 · sonic-net/sonic-buildimage

AshokDaparthi · 2021-04-09T21:32:33Z

Why I did it

Created SONiC Yang model for QOS
Tables: SCHEDULER, WRED_PROFILE, QUEUE table.
How I did it

Defined Yang models for QOS based on Guideline doc:
https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md
and
https://github.com/Azure/sonic-utilities/blob/master/doc/Command-Reference.md
How to verify it

sonic_yang_models package build

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

anshuv-mfst · 2021-04-15T22:53:12Z

Hi @neethajohn - could you please take a look, thanks.

nazariig · 2021-04-28T16:41:58Z

src/sonic-yang-models/yang-models/sonic-queue.yang

@AshokDaparthi how do you expect to handle key reference notation from swss-schema.md?

It seems that current YANG model won't be able to handle existing config_db.json:
Example:

"QUEUE": { "Ethernet4|0": { "scheduler": "[SCHEDULER|scheduler.0]" }, "Ethernet4|1": { "scheduler": "[SCHEDULER|scheduler.0]" }, "Ethernet4|2": { "scheduler": "[SCHEDULER|scheduler.0]" }, "Ethernet4|3": { "wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]", "scheduler": "[SCHEDULER|scheduler.1]" } } "SCHEDULER": { "scheduler.0": { "type": "DWRR", "weight": "14" }, "scheduler.1": { "type": "DWRR", "weight": "15" } } "WRED_PROFILE": { "AZURE_LOSSLESS": { "red_max_threshold": "2097152", "wred_green_enable": "true", "ecn": "ecn_all", "green_min_threshold": "1048576", "red_min_threshold": "1048576", "wred_yellow_enable": "true", "yellow_min_threshold": "1048576", "green_max_threshold": "2097152", "green_drop_probability": "5", "yellow_max_threshold": "2097152", "wred_red_enable": "true", "yellow_drop_probability": "5", "red_drop_probability": "5" } }

root@sonic:/home/admin# redis-cli -n 4 HGETALL 'QUEUE|Ethernet4|3' 1) "scheduler" 2) "[SCHEDULER|scheduler.1]" 3) "wred_profile" 4) "[WRED_PROFILE|AZURE_LOSSLESS]"

Which means that for scheduler/wred we expect to match:

scheduler -> scheduler.0

wred -> AZURE_LOSSLESS

and not:

scheduler -> SCHEDULER|scheduler.0

wred -> WRED_PROFILE|AZURE_LOSSLESS

@praveen-li can you please elaborate what should be the appropriate YANG model for SONiC ABNF key reference?

How do we expect to handle this one?

"BUFFER_PORT_EGRESS_PROFILE_LIST": { "Ethernet8": { "profile_list": "[BUFFER_PROFILE|egress_lossless_profile],[BUFFER_PROFILE|egress_lossy_profile]" },

root@sonic:/home/admin# redis-cli -n 4 HGETALL 'BUFFER_PORT_EGRESS_PROFILE_LIST|Ethernet8' 1) "profile_list" 2) "[BUFFER_PROFILE|egress_lossless_profile],[BUFFER_PROFILE|egress_lossy_profile]"

@nazariig, @praveen-li - We can discuss this over review call, i am not sure pyang will take care leafref validation even if we give ABNF format. Else we should convert to leafref to pattern. But we will miss in this case leafref check's in YANG. As of now i am not sure is there anyway to resent in YANG.

@nazariig - Back end changes to remove table ABNF format (example "[BUFFER_PROFILE|" )from field value is merged in latest master.

src/sonic-yang-models/yang-models/sonic-scheduler.yang

lguohan · 2021-05-06T17:12:14Z

src/sonic-yang-models/yang-models/sonic-types.yang

this is being discussed in another pr

removed operation

lguohan · 2021-05-06T17:12:49Z

src/sonic-yang-models/yang-models/sonic-wred-profile.yang

is the operation check needed?

removed operation based checks

src/sonic-yang-models/yang-models/sonic-wred-profile.yang

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

zhangyanzhao · 2021-09-02T17:24:29Z

Depends on sonic-net/sonic-utilities#1626

zhangyanzhao · 2021-09-02T21:50:41Z

Dependency sonic-net/sonic-utilities#1626 is merged

src/sonic-yang-models/yang-models/sonic-scheduler.yang

src/sonic-yang-models/yang-models/sonic-queue.yang

smaheshm · 2021-09-09T00:24:02Z

@AshokDaparthi Can you update the PR with suggestions. I see couple of open items, do they still need to get resolved? Are you waiting for inputs from the community?

AshokDaparthi · 2021-09-09T16:32:25Z

@smaheshm - Main issues blocking this PR is comments in sonic-queue.yang, Which needs DB format change. Below are PR raised sonic-net/sonic-utilities#1626
sonic-net/sonic-swss#1754
#7752
sonic-net/sonic-mgmt#3824 There is problems to pipeline testing because of all are interlinked changes and as per last call sonic-mgmt test cases needs both format in same code base based on release. It needs rework and find way to pass different config_db.json from yaml files with both new and old formats.

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

smaheshm · 2021-09-28T20:51:01Z

/azp run

azure-pipelines · 2021-09-28T20:51:20Z

Azure Pipelines successfully started running 1 pipeline(s).

neethajohn · 2021-09-30T16:59:51Z

@smaheshm - Main issues blocking this PR is comments in sonic-queue.yang, Which needs DB format change. Below are PR raised Azure/sonic-utilities#1626 Azure/sonic-swss#1754 #7752 Azure/sonic-mgmt#3824 There is problems to pipeline testing because of all are interlinked changes and as per last call sonic-mgmt test cases needs both format in same code base based on release. It needs rework and find way to pass different config_db.json from yaml files with both new and old formats.

all the dependent PRs (1626/1754/7752/3824) are merged

We still have sonic-net/sonic-swss#1937 pending and submodule update to ensure that the db reference changes are good. Once they are in, we can proceed with this PR

AshokDaparthi · 2021-09-30T19:27:24Z

/azpw run

mssonicbld · 2021-09-30T19:27:25Z

/AzurePipelines run

azure-pipelines · 2021-09-30T19:27:44Z

Azure Pipelines successfully started running 1 pipeline(s).

AshokDaparthi · 2021-10-04T17:46:49Z

@smaheshm - Could you please help to see why it failing, i ran sonic-yang-models, and sonic-yang-mgmt python wheels. it success for me.

smaheshm · 2021-10-04T18:21:32Z

@smaheshm - Could you please help to see why it failing, i ran sonic-yang-models, and sonic-yang-mgmt python wheels. it success for me.

looks like some process crashed, seen in other tests as well. I don't know if anyone's working on it. For I'd suggest update your branch and re-trigger the pipeline.

zhangyanzhao · 2021-10-07T17:12:37Z

Build failure need some help from people who can check the logs, looks like impacted by warmreboot test. Rajesh will re-push today

smaheshm · 2021-10-14T22:26:52Z

/azp run

azure-pipelines · 2021-10-14T22:27:12Z

Azure Pipelines successfully started running 1 pipeline(s).

AshokDaparthi · 2021-10-15T15:31:43Z

@smaheshm - How man times i restart i see same error kvmtest fails. Could you please help to see why it fails.

AshokDaparthi · 2021-10-15T23:19:05Z

@smaheshm, @lguohan - checks are passed finally, could you please review or merge it. since it already approved once.

…t#7281)" This reverts commit a99d78d.

…onic-net#7281)"" This reverts commit f0b2122.

…ernal branch Related work items: sonic-net#525, sonic-net#534, sonic-net#909, sonic-net#927, sonic-net#930, sonic-net#934, sonic-net#935, sonic-net#937, sonic-net#938, sonic-net#939, sonic-net#940, sonic-net#943, sonic-net#944, sonic-net#945, sonic-net#946, sonic-net#948, sonic-net#1702, sonic-net#1923, sonic-net#1936, sonic-net#1942, sonic-net#2315, sonic-net#7281, sonic-net#7375, sonic-net#8659, sonic-net#8799, sonic-net#8868, sonic-net#8875, sonic-net#8935, sonic-net#8942, sonic-net#8956, sonic-net#8961, sonic-net#8966, sonic-net#8969, sonic-net#8971, sonic-net#8978, sonic-net#8991, sonic-net#8996, sonic-net#9001, sonic-net#9008, #3146150

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

AshokDaparthi requested a review from lguohan as a code owner April 9, 2021 21:32

lguohan added the YANG YANG model related changes label Apr 10, 2021

anshuv-mfst requested a review from neethajohn April 15, 2021 22:52

AshokDaparthi force-pushed the sonic-qos-yangs branch from 78d3f58 to 2e8e561 Compare April 20, 2021 19:10

arlakshm requested a review from smaheshm April 23, 2021 17:50

nazariig reviewed Apr 28, 2021

View reviewed changes

rathnasabapathyv reviewed May 6, 2021

View reviewed changes

src/sonic-yang-models/yang-models/sonic-scheduler.yang Outdated Show resolved Hide resolved

lguohan reviewed May 6, 2021

View reviewed changes

rathnasabapathyv reviewed May 6, 2021

View reviewed changes

src/sonic-yang-models/yang-models/sonic-wred-profile.yang Outdated Show resolved Hide resolved

AshokDaparthi requested a review from jleveque as a code owner May 20, 2021 01:56

AshokDaparthi force-pushed the sonic-qos-yangs branch from 16c5886 to b41fb20 Compare May 23, 2021 23:48

This was referenced May 29, 2021

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

Merged

QOS fieldvalue reference ABNF format to string changes sonic-net/sonic-swss#1754

Merged

QOS fieldvalue reference ABNF format to string sonic-net/sonic-utilities#1626

Merged

smaheshm reviewed Sep 8, 2021

View reviewed changes

src/sonic-yang-models/yang-models/sonic-scheduler.yang Outdated Show resolved Hide resolved

smaheshm reviewed Sep 8, 2021

View reviewed changes

src/sonic-yang-models/yang-models/sonic-scheduler.yang Outdated Show resolved Hide resolved

smaheshm reviewed Sep 9, 2021

View reviewed changes

src/sonic-yang-models/yang-models/sonic-queue.yang Outdated Show resolved Hide resolved

AshokDaparthi force-pushed the sonic-qos-yangs branch from b41fb20 to dca5b23 Compare September 27, 2021 21:15

AshokDaparthi force-pushed the sonic-qos-yangs branch 2 times, most recently from 134a4e3 to c183a05 Compare September 29, 2021 19:24

lguohan previously approved these changes Sep 30, 2021

View reviewed changes

smaheshm closed this Oct 4, 2021

smaheshm reopened this Oct 4, 2021

AshokDaparthi dismissed lguohan’s stale review via 6b07b16 October 7, 2021 17:25

AshokDaparthi force-pushed the sonic-qos-yangs branch 2 times, most recently from 6b07b16 to ef1ff94 Compare October 12, 2021 16:29

SONiC QOS Scheduler, WRED, Queue Yangs

a9f69e4

AshokDaparthi force-pushed the sonic-qos-yangs branch from ef1ff94 to a9f69e4 Compare October 15, 2021 15:37

smaheshm approved these changes Oct 16, 2021

View reviewed changes

lguohan approved these changes Oct 18, 2021

View reviewed changes

smaheshm merged commit a99d78d into sonic-net:master Oct 18, 2021

dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Oct 29, 2021

Revert "[Yang model] SONiC QOS Scheduler, WRED, Queue Yangs (sonic-ne…

f0b2122

…t#7281)" This reverts commit a99d78d.

dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Oct 29, 2021

Revert "Revert "[Yang model] SONiC QOS Scheduler, WRED, Queue Yangs (s…

ae5f941

…onic-net#7281)"" This reverts commit f0b2122.

Conversation

AshokDaparthi commented Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anshuv-mfst commented Apr 15, 2021

Uh oh!

nazariig Apr 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nazariig Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

AshokDaparthi Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

AshokDaparthi Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lguohan May 6, 2021

Choose a reason for hiding this comment

Uh oh!

AshokDaparthi Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

lguohan May 6, 2021

Choose a reason for hiding this comment

Uh oh!

AshokDaparthi Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zhangyanzhao commented Sep 2, 2021

Uh oh!

zhangyanzhao commented Sep 2, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smaheshm commented Sep 9, 2021

Uh oh!

AshokDaparthi commented Sep 9, 2021

Uh oh!

smaheshm commented Sep 28, 2021

Uh oh!

azure-pipelines bot commented Sep 28, 2021

Uh oh!

neethajohn commented Sep 30, 2021

Uh oh!

AshokDaparthi commented Sep 30, 2021

Uh oh!

mssonicbld commented Sep 30, 2021

Uh oh!

azure-pipelines bot commented Sep 30, 2021

Uh oh!

AshokDaparthi commented Oct 4, 2021

Uh oh!

smaheshm commented Oct 4, 2021

Uh oh!

zhangyanzhao commented Oct 7, 2021

Uh oh!

smaheshm commented Oct 14, 2021

Uh oh!

azure-pipelines bot commented Oct 14, 2021

Uh oh!

AshokDaparthi commented Oct 15, 2021

Uh oh!

AshokDaparthi commented Oct 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

AshokDaparthi commented Apr 9, 2021 •

edited

Loading

nazariig Apr 28, 2021 •

edited

Loading