SONiC CBF MAPs Yang #9116

Cosmin-Jinga-MS · 2021-10-29T08:54:29Z

Why I did it

Created SONiC Yang model for the following CBF MAPs:
DSCP_TO_FC_MAP
EXP_TO_FC_MAP

How I did it

Defined Yang models for CBF MAPs 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

Description for the changelog

Added CBF yangs for DSCP_TO_FC_MAP and EXP_TO_FC_MAP resolves #9108

Signed-off-by: [email protected]

smaheshm · 2021-10-29T22:21:01Z

@AshokDaparthi Since you've done similar work for QoS can you review this PR. Thanks!

Why I did it Created SONiC Yang model for the following CBF MAPs: DSCP_TO_FC_MAP EXP_TO_FC How I did it Defined Yang models for CBF MAPs 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 Signed-off-by: [email protected]

Cosmin-Jinga-MS · 2021-11-04T09:11:10Z

/azpw run

mssonicbld · 2021-11-04T09:11:12Z

/AzurePipelines run

azure-pipelines · 2021-11-04T09:11:31Z

Azure Pipelines successfully started running 1 pipeline(s).

TACappleman · 2021-11-10T11:33:12Z

@smaheshm would you be able to review this, as we've not got any response otherwise?

Cosmin-Jinga-MS · 2021-11-11T10:28:00Z

@smaheshm @lguohan can this PR be approved and merged?
I see Ashok has approved it.

smaheshm · 2021-11-12T21:58:32Z

@praveen-li Can you also review this.. Thanks!

Cosmin-Jinga-MS · 2021-11-18T06:04:55Z

Hi @praveen-li, can I have an estimate for when you can have this PR reviewed?

praveen-li · 2021-11-18T07:35:00Z

Hi @praveen-li, can I have an estimate for when you can have this PR reviewed?

I will try to finish this Friday Evening.

TACappleman · 2021-11-23T10:50:57Z

@praveen-li How's the review going? We'd like to get this into the 202111 review with the rest of the CBF feature if possible

Cosmin-Jinga-MS · 2021-11-29T05:36:08Z

@praveen-li any chance to have this reviewed today?

praveen-li · 2021-11-30T05:08:46Z

src/sonic-yang-mgmt/sonic_yang_ext.py

@@ -15,6 +15,9 @@
            'MAP_PFC_PRIORITY_TO_QUEUE_LIST',
            'PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_LIST']

+cbf_maps_model = ['DSCP_TO_FC_MAP_LIST',


This for translation for a specific type, so let`s mark it as Type_1_list_maps_model. Add all lists in it.

praveen-li · 2021-11-30T05:09:42Z

src/sonic-yang-mgmt/sonic_yang_ext.py

@@ -413,7 +416,7 @@ def _yangConvert(val):
        return vValue

    """
-    Xlate a Qos Maps list
+    Xlate a Maps list


Xlate a Type 1 Lists, Similar to Qos Maps. Example Below:

praveen-li · 2021-11-30T05:10:25Z

src/sonic-yang-mgmt/sonic_yang_ext.py

@@ -465,7 +468,7 @@ def _yangConvert(val):
            }
    }
    """
-    def _xlateQosMapList(self, model, yang, config, table, exceptionList):
+    def _xlateMapList(self, model, yang, config, table, exceptionList):


_xlateType1MapList

praveen-li · 2021-11-30T05:11:00Z

src/sonic-yang-mgmt/sonic_yang_ext.py

-        if model['@name'] in qos_maps_model:
-            self.sysLog(msg="_xlateQosMapList: {}".format(model['@name']))
-            self._xlateQosMapList(model, yang,config, table, exceptionList)
+        if model['@name'] in qos_maps_model or model['@name'] in cbf_maps_model:


same list maps.

praveen-li · 2021-11-30T05:11:28Z

src/sonic-yang-mgmt/sonic_yang_ext.py

@@ -788,14 +791,14 @@ def _revYangConvert(val):
    }
    """

-    def _revQosMapXlateList(self, model, yang, config, table):
+    def _revMapXlateList(self, model, yang, config, table):


_revXlateType1MapList

praveen-li · 2021-11-30T05:13:55Z

src/sonic-yang-models/yang-models/sonic-dscp-fc-map.yang

+            "Initial revision.";
+    }
+
+    container sonic-dscp-fc-map {


I am not reviewing YANG models, since I am not SME for this. Thx

I will reviews, thanks for reviewing the rest.

@smaheshm since Praveen has approved the other changes is this PR ready to be merged in?
I've made a small update after his approval to correct some code comments only.

smaheshm · 2021-12-01T16:23:24Z

@Cosmin-Jinga-MS Once the comments are addressed this PR is good to go.

Cosmin-Jinga-MS · 2021-12-02T08:32:46Z

@praveen-li please have a look over the changes and let me know if this is what you expected.

praveen-li

LGTM except few Comments section changes.

praveen-li · 2021-12-02T18:09:38Z

src/sonic-yang-mgmt/sonic_yang_ext.py

@@ -744,7 +746,7 @@ def _revYangConvert(val):

    """
    Rev xlate from <TABLE>_LIST to table in config DB
-    QOS MAP Yang has inner list, each inner list key:val should
+    Type 1 MAPs Yang has inner list, each inner list key:val should


Type 1 Maps Lists or Type 1 Lists

Will update, thanks

praveen-li · 2021-12-02T18:10:05Z

src/sonic-yang-mgmt/sonic_yang_ext.py

    This function will xlate from a dict in config DB to a Yang JSON list
    using yang model. Output will be go in self.xlateJson

    Note: Exceptions from this function are collected in exceptionList and
    are displayed only when an entry is not xlated properly from ConfigDB
    to sonic_yang.json.

-    QOS MAPS Yang has inner list, which is diffrent from config DB.
+    Type 1 map Yang has inner list, which is diffrent from config DB.


Type 1 Lists has inner list, which is different from generic lists in config DB.

Will update, thanks

praveen-li · 2021-12-02T18:13:26Z

src/sonic-yang-models/tests/yang_model_tests/tests/cbfmaps.json

+  },
+  "DSCP_TO_FC_MAP_CREATE_INVALID_DSCP": {
+    "desc": "Configure a DSCP to Forwarding class map with invalid key.",
+    "eStr": "Invalid DSCP"


eStr should match to the error from Libyang. This may not pass. Do you see successful build of PKG.

list DSCP_TO_FC_MAP { //this is list inside list for storing mapping between two fields key "dscp"; leaf dscp { type string { pattern "6[0-3]|[1-5][0-9]?|[0-9]?" { error-message "Invalid DSCP"; error-app-tag dscp-invalid; } } }

Unless I misunderstood what you meant I think the error strings do match in this case.
Can you please expand on what I need to run to build PKG in order to check?(I'd expect it to be in order since the pipeline checks have passed)

Cosmin-Jinga-MS requested a review from lguohan as a code owner October 29, 2021 08:54

smaheshm self-requested a review October 29, 2021 22:21

Cosmin-Jinga-MS force-pushed the cbf_yangs branch from 03cce0b to b9cc150 Compare November 2, 2021 12:10

Updated sample config

121ceff

smaheshm mentioned this pull request Nov 4, 2021

[CBF][Yang] Create yang model for the following: "DSCP_TO_FC_MAP" "EXP_TO_FC_MAP" #9108

Closed

smaheshm added the YANG YANG model related changes label Nov 4, 2021

Fix sonic-yang-mgmt

c27d392

AshokDaparthi previously approved these changes Nov 10, 2021

View reviewed changes

praveen-li reviewed Nov 30, 2021

View reviewed changes

Changed Xlate functions to handle general Type 1 lists

bb0d9c8

Cosmin-Jinga-MS dismissed AshokDaparthi’s stale review via bb0d9c8 December 2, 2021 07:41

Cosmin-Jinga-MS requested a review from qiluo-msft as a code owner December 2, 2021 07:41

praveen-li previously approved these changes Dec 2, 2021

View reviewed changes

praveen-li reviewed Dec 2, 2021

View reviewed changes

Small update correcting code comments

6f192dc

Cosmin-Jinga-MS dismissed praveen-li’s stale review via 6f192dc December 3, 2021 06:10

smaheshm approved these changes Dec 6, 2021

View reviewed changes

smaheshm merged commit c5d0df1 into sonic-net:master Dec 6, 2021

ghooo mentioned this pull request Jan 12, 2022

[generic_config_updater] VRF modification failed when loopback interface is assigned with two or more IP addresses sonic-net/sonic-utilities#2000

Closed

ghooo mentioned this pull request Jan 24, 2022

[GCU] Update path to xpath conversion logic to most recent sonic YANG updates Supporting `Type_1_list_maps_model sonic-net/sonic-utilities#2034

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SONiC CBF MAPs Yang #9116

SONiC CBF MAPs Yang #9116

Cosmin-Jinga-MS commented Oct 29, 2021

smaheshm commented Oct 29, 2021

Cosmin-Jinga-MS commented Nov 4, 2021

mssonicbld commented Nov 4, 2021

azure-pipelines bot commented Nov 4, 2021

TACappleman commented Nov 10, 2021

Cosmin-Jinga-MS commented Nov 11, 2021

smaheshm commented Nov 12, 2021

Cosmin-Jinga-MS commented Nov 18, 2021

praveen-li commented Nov 18, 2021

TACappleman commented Nov 23, 2021

Cosmin-Jinga-MS commented Nov 29, 2021

praveen-li Nov 30, 2021

praveen-li Nov 30, 2021

praveen-li Nov 30, 2021

praveen-li Nov 30, 2021

praveen-li Nov 30, 2021

praveen-li Nov 30, 2021

smaheshm Dec 1, 2021

Cosmin-Jinga-MS Dec 3, 2021

smaheshm commented Dec 1, 2021

Cosmin-Jinga-MS commented Dec 2, 2021

praveen-li left a comment

praveen-li Dec 2, 2021

Cosmin-Jinga-MS Dec 3, 2021

praveen-li Dec 2, 2021

Cosmin-Jinga-MS Dec 3, 2021

praveen-li Dec 2, 2021

Cosmin-Jinga-MS Dec 3, 2021 •

edited

Loading

SONiC CBF MAPs Yang #9116

SONiC CBF MAPs Yang #9116

Conversation

Cosmin-Jinga-MS commented Oct 29, 2021

Why I did it

How I did it

How to verify it

Description for the changelog

smaheshm commented Oct 29, 2021

Cosmin-Jinga-MS commented Nov 4, 2021

mssonicbld commented Nov 4, 2021

azure-pipelines bot commented Nov 4, 2021

TACappleman commented Nov 10, 2021

Cosmin-Jinga-MS commented Nov 11, 2021

smaheshm commented Nov 12, 2021

Cosmin-Jinga-MS commented Nov 18, 2021

praveen-li commented Nov 18, 2021

TACappleman commented Nov 23, 2021

Cosmin-Jinga-MS commented Nov 29, 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

smaheshm commented Dec 1, 2021

Cosmin-Jinga-MS commented Dec 2, 2021

praveen-li left a 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

Cosmin-Jinga-MS Dec 3, 2021 • edited Loading

Choose a reason for hiding this comment

Cosmin-Jinga-MS Dec 3, 2021 •

edited

Loading