QOS fieldvalue reference ABNF format to string changes #1754

AshokDaparthi · 2021-05-20T16:46:03Z

Depends on sonic-net/sonic-utilities#1626
Depends on sonic-net/sonic-buildimage#7752

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]"
},

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 and made it as string.
Example:
"Ethernet92|3": {
"scheduler": "scheduler.1",
"wred_profile": "AZURE_LOSSLESS"
},

Dependent pul; requests:
sonic-net/sonic-buildimage#7752 - To modify platfrom files
sonic-net/sonic-buildimage#7281 - Yang model
sonic-net/sonic-utilities#1626 - DB migration

stephenxs

Hi @AshokDaparthi
I haven't got time to go over the entire PR. Some comments for now:

Need to update the vs test cases as well, like test_buffer_dynamic.py, test_speed.py, and test_qos_map.py
I believe the current vs test is failing without these changes.
Need to update the db schema description.
Can you fill the fields in the PR description?
By doing so reviewers can have a better understanding of this PR.

Thanks.

AshokDaparthi · 2021-05-21T19:38:07Z

@stephenxs - Thanks for information, i will dig to test cases. i compiled only for sonic-broadcom.bin , i do not see any failure at build time.

stephenxs · 2021-05-24T00:41:57Z

@stephenxs - Thanks for information, i will dig to test cases. i compiled only for sonic-broadcom.bin , i do not see any failure at build time.

Yes. The vs test won't run when the sonic image (like sonic-broadcom.bin) is being built. It runs as part of the checks of the PR (Azure.sonic-swss (Test vstest)). To set up a local environment for vs test, you can refer this guide

AshokDaparthi · 2021-06-20T18:57:14Z

/azpw run

AshokDaparthi · 2021-06-22T03:01:18Z

/azpw run

AshokDaparthi · 2021-06-22T16:36:55Z

/azpw run

stephenxs · 2021-07-09T10:25:30Z

cfgmgr/buffermgr.cpp

@@ -284,14 +277,6 @@ void BufferMgr::doBufferTableTask(Consumer &consumer, ProducerStateTable &applTa

            for (auto i : kfvFieldsValues(t))
            {
-                SWSS_LOG_INFO("Inserting field %s value %s", fvField(i).c_str(), fvValue(i).c_str());


transformSeperator which is called in L269 can be removed as well. There is no separator now.
The function transformSeperator and transformReference can be removed as they are no longer called

transformSeperator is used to convert from config_db to app_db

stephenxs · 2021-07-09T10:54:32Z

cfgmgr/buffermgrdyn.cpp

@@ -1951,8 +1939,7 @@ task_process_status BufferMgrDynamic::handleOneBufferPgEntry(const string &key,
            {
                // Headroom override
                pureDynamic = false;
-                transformReference(value);


transformReference is no longer called and can be removed.

stephenxs · 2021-07-09T11:20:53Z

orchagent/orch.cpp

        // Deem it as a valid format
        // clear both type_name and object_name
        // as an indication to the caller that
        // such a case has been encountered
-        type_name.clear();


Can an empty string be passed by redis-db? If no and this is a valid flow, we need to find another way to indicate clearing reference. Probably we can use "NULL".

stephenxs · 2021-07-09T11:24:47Z

orchagent/orch.cpp

@@ -301,7 +301,7 @@ bool Orch::bake()
 }

 /*
- Validates reference has proper format which is [table_name:object_name]
+- Validates reference has proper format which is not ABNF [table_name:object_name]


Emphasizing not ABNF looks strange. I think Validates reference has proper format which is object_name should be good.

stephenxs · 2021-07-09T11:26:58Z

orchagent/orch.cpp

-    tokens = tokenize(ref_content, delimiter);
-    if (tokens.size() != 2)
+
+    if ((ref_in[0] == ref_start) || (ref_in[ref_in.size()-1] == ref_end))


Not sure whether we need to check whether it's surrounded by [] here.

lguohan · 2021-08-26T17:44:28Z

/azp run

azure-pipelines · 2021-08-26T17:44:40Z

Azure Pipelines successfully started running 1 pipeline(s).

zhangyanzhao · 2021-08-26T17:51:39Z

@AshokDaparthi please help to monitor the build progress and check the result.

rathnasabapathyv · 2021-08-26T17:57:07Z

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 (#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 : sonic-net/sonic-buildimage#7752
sonic-swss PR : #1754

AshokDaparthi · 2021-09-23T02:21:25Z

/azpw run

mssonicbld · 2021-09-23T02:21:26Z

/AzurePipelines run

azure-pipelines · 2021-09-23T02:21:36Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2021-09-23T04:20:59Z

/azp run

azure-pipelines · 2021-09-23T04:21:09Z

Azure Pipelines successfully started running 1 pipeline(s).

AshokDaparthi · 2021-09-23T04:26:11Z

@stephenxs, @neethajohn - Please relook, i did modification to pass swss test cases until sonic-buildimage check in happens. Plan is to push ASAP and update sub modules.

AshokDaparthi · 2021-09-23T15:12:54Z

/azpw run

mssonicbld · 2021-09-23T15:12:56Z

/AzurePipelines run

azure-pipelines · 2021-09-23T15:13:08Z

Azure Pipelines successfully started running 1 pipeline(s).

…ests Skip test cases until sonic-buildimage changes in Skip test cases until sonic-buildimage changes in

AshokDaparthi · 2021-09-24T18:42:08Z

@neethajohn @stephenxs - Could you please help to review and merge, so that i can push other PR's

prsunny · 2021-09-24T23:25:48Z

@stephenxs , could you please sign-off?

stephenxs

LGTM.

stephenxs · 2021-09-24T23:29:49Z

@stephenxs , could you please sign-off?

@prsunny approved.

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

stephenxs · 2021-09-28T22:10:05Z

@stephenxs, @neethajohn - Please relook, i did modification to pass swss test cases until sonic-buildimage check in happens. Plan is to push ASAP and update sub modules.

Hi @AshokDaparthi
Now the PR sonic-net/sonic-buildimage#7752 has been merged. Can you open a PR to remove the code added for passing the CI?
Thanks

AshokDaparthi · 2021-09-28T22:51:51Z

@stephenxs - Yes, i can do. I am waiting for build to be ready with today sonic-buildimage changes. i did not see this yet in artifacts. only docker-sonic-vs.gz comes out of today image only checks will pass. So i will rise it tomorrow.

What I did Reverted skipped test_buffer_dynamic as part of #1754 Why I did it How I verified it sudo pytest --dvsname=vs --forcedvs -sv --keeptb test_buffer_dynamic.py ======================================================= test session starts ======================================================== platform linux -- Python 3.6.9, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3 cachedir: .pytest_cache rootdir: /home/ashokd/swss-vs/ashok-swss/sonic-swss/tests plugins: flaky-3.7.0 collected 9 items test_buffer_dynamic.py::TestBufferMgrDyn::test_changeSpeed remove extra link dummy PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_changeCableLen PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_MultipleLosslessPg PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_headroomOverride PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_mtuUpdate PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_nonDefaultAlpha PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_sharedHeadroomPool PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_shutdownPort PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_autoNegPort PASSED

* QOS field reference ABNF format to string changes

What I did Reverted skipped test_buffer_dynamic as part of sonic-net#1754 Why I did it How I verified it sudo pytest --dvsname=vs --forcedvs -sv --keeptb test_buffer_dynamic.py ======================================================= test session starts ======================================================== platform linux -- Python 3.6.9, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3 cachedir: .pytest_cache rootdir: /home/ashokd/swss-vs/ashok-swss/sonic-swss/tests plugins: flaky-3.7.0 collected 9 items test_buffer_dynamic.py::TestBufferMgrDyn::test_changeSpeed remove extra link dummy PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_changeCableLen PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_MultipleLosslessPg PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_headroomOverride PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_mtuUpdate PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_nonDefaultAlpha PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_sharedHeadroomPool PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_shutdownPort PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_autoNegPort PASSED

…nic-net#1754) What I did Allow system with no ports in config db run without errors. This is needed for modular system which should boot properly without line cards. How I did it Do not raise warning if ports dictionary is empty. How to verify it Run show interfaces status

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

stephenxs suggested changes May 21, 2021

View reviewed changes

stephenxs requested review from liat-grozovik, kcudnik and neethajohn May 21, 2021 01:18

AshokDaparthi force-pushed the sonic-qos-yangs branch 2 times, most recently from 42b15cb to f0896e7 Compare May 29, 2021 00:10

AshokDaparthi force-pushed the sonic-qos-yangs branch from f0896e7 to b3e507e Compare June 20, 2021 19:09

AshokDaparthi force-pushed the sonic-qos-yangs branch from b3e507e to 4273441 Compare June 28, 2021 17:21

AshokDaparthi requested a review from prsunny as a code owner June 28, 2021 17:21

AshokDaparthi force-pushed the sonic-qos-yangs branch from 4273441 to c0e0862 Compare June 28, 2021 20:25

AshokDaparthi force-pushed the sonic-qos-yangs branch from c0e0862 to 868f939 Compare July 8, 2021 02:36

venkatmahalingam mentioned this pull request Jul 8, 2021

Adding mmu buffer yang files sonic-net/sonic-buildimage#7838

Merged

stephenxs reviewed Jul 9, 2021

View reviewed changes

AshokDaparthi force-pushed the sonic-qos-yangs branch from 868f939 to b42784f Compare July 13, 2021 21:22

AshokDaparthi mentioned this pull request Jul 20, 2021

Modified test cases by QOS DB reference format remove sonic-net/sonic-mgmt#3824

Merged

stephenxs previously approved these changes Jul 30, 2021

View reviewed changes

AshokDaparthi dismissed stephenxs’s stale review via 3ab4d4b August 31, 2021 19:18

AshokDaparthi force-pushed the sonic-qos-yangs branch from b42784f to 3ab4d4b Compare August 31, 2021 19:18

AshokDaparthi and others added 2 commits September 23, 2021 13:33

QOS field reference ABNF format to string changes

03f2860

Allow old db format until sonic-buildimage code merges to pass swss t…

b0cc853

…ests Skip test cases until sonic-buildimage changes in Skip test cases until sonic-buildimage changes in

AshokDaparthi force-pushed the sonic-qos-yangs branch from 6c20b35 to b0cc853 Compare September 23, 2021 20:33

neethajohn approved these changes Sep 24, 2021

View reviewed changes

stephenxs approved these changes Sep 24, 2021

View reviewed changes

prsunny merged commit a3fdaf4 into sonic-net:master Sep 24, 2021

AshokDaparthi mentioned this pull request Sep 25, 2021

Submodule update for swss sonic-net/sonic-buildimage#8839

Merged

AshokDaparthi mentioned this pull request Sep 29, 2021

Reverted skipped test_buffer_dynamic test cases #1937

Merged

raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021

QOS fieldvalue reference ABNF format to string changes (sonic-net#1754)

72a4051

* QOS field reference ABNF format to string changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QOS fieldvalue reference ABNF format to string changes #1754

QOS fieldvalue reference ABNF format to string changes #1754

AshokDaparthi commented May 20, 2021 •

edited

Loading

stephenxs left a comment

AshokDaparthi commented May 21, 2021

stephenxs commented May 24, 2021

AshokDaparthi commented Jun 20, 2021

AshokDaparthi commented Jun 22, 2021

AshokDaparthi commented Jun 22, 2021

stephenxs Jul 9, 2021

AshokDaparthi Jul 13, 2021

stephenxs Jul 9, 2021

AshokDaparthi Jul 13, 2021

stephenxs Jul 9, 2021

stephenxs Jul 9, 2021

AshokDaparthi Jul 13, 2021

stephenxs Jul 9, 2021

lguohan commented Aug 26, 2021

azure-pipelines bot commented Aug 26, 2021

zhangyanzhao commented Aug 26, 2021

rathnasabapathyv commented Aug 26, 2021 •

edited

Loading

AshokDaparthi commented Sep 23, 2021

mssonicbld commented Sep 23, 2021

azure-pipelines bot commented Sep 23, 2021

prsunny commented Sep 23, 2021

azure-pipelines bot commented Sep 23, 2021

AshokDaparthi commented Sep 23, 2021

AshokDaparthi commented Sep 23, 2021

mssonicbld commented Sep 23, 2021

azure-pipelines bot commented Sep 23, 2021

AshokDaparthi commented Sep 24, 2021

prsunny commented Sep 24, 2021

stephenxs left a comment

stephenxs commented Sep 24, 2021

stephenxs commented Sep 28, 2021

AshokDaparthi commented Sep 28, 2021

QOS fieldvalue reference ABNF format to string changes #1754

QOS fieldvalue reference ABNF format to string changes #1754

Conversation

AshokDaparthi commented May 20, 2021 • edited Loading

stephenxs left a comment

Choose a reason for hiding this comment

AshokDaparthi commented May 21, 2021

stephenxs commented May 24, 2021

AshokDaparthi commented Jun 20, 2021

AshokDaparthi commented Jun 22, 2021

AshokDaparthi commented Jun 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Aug 26, 2021

azure-pipelines bot commented Aug 26, 2021

zhangyanzhao commented Aug 26, 2021

rathnasabapathyv commented Aug 26, 2021 • edited Loading

AshokDaparthi commented Sep 23, 2021

mssonicbld commented Sep 23, 2021

azure-pipelines bot commented Sep 23, 2021

prsunny commented Sep 23, 2021

azure-pipelines bot commented Sep 23, 2021

AshokDaparthi commented Sep 23, 2021

AshokDaparthi commented Sep 23, 2021

mssonicbld commented Sep 23, 2021

azure-pipelines bot commented Sep 23, 2021

AshokDaparthi commented Sep 24, 2021

prsunny commented Sep 24, 2021

stephenxs left a comment

Choose a reason for hiding this comment

stephenxs commented Sep 24, 2021

stephenxs commented Sep 28, 2021

AshokDaparthi commented Sep 28, 2021

AshokDaparthi commented May 20, 2021 •

edited

Loading

rathnasabapathyv commented Aug 26, 2021 •

edited

Loading