sonic-yang-models updates for MPLS #7881

qbdwlr · 2021-06-15T18:55:18Z

What I did
SONiC YANG model support in buildimage for MPLS:

sonic-yang-model support for MPLS enable/disable
sonic-yang-model support for MPLS CRM thresholds

Why I did it
SONiC yang-models support for MPLS

How I verified it
Unit-tests in sonic-swss/tests/test_mpls.py and sonic-utilities/tests
System tests in sonic-mgmt

Details if related
Refer to MPLS HLD sonic-net/SONiC#706

kperumalbfn · 2021-06-24T17:13:28Z

src/sonic-yang-models/yang-models/sonic-interface.yang

@@ -68,6 +68,12 @@ module sonic-interface {
 					}
                                        default "0";
 				}


Can you add Yang model tests for mpls enable/disable on RIFs? Most of tests are in sonic-yang-models/tests/yang_model_tests/tests

@kperumalbfn I looked at the types of tests currently under sonic-yang-models. It looked like it was mostly parameter validation. Since this mpls knob has two fixed values, I wasn't sure what more needed to be checked for that. Do you have something specific in mind? Can you provide a pointer to an example?

I think it will be good to have a test for one type of interface. SAI default value for SAI_ROUTER_INTERFACE_ATTR_ADMIN_MPLS_STATE is disabled, we can use that. There are examples for NAT_ZONE in sonic-yang-models/tests/yang_model_tests/tests/interface.json

@kperumalbfn Type for mpls attribute has been changed from string to enum, which I believe should eliminate the need for valid/invalid attribute value tests. No explicit default should be defined in yang models due to lack of support for this atttribute in many existing platforms.

kperumalbfn · 2021-06-24T17:16:54Z

src/sonic-yang-models/yang-models/sonic-crm.yang

@@ -353,6 +353,46 @@ module sonic-crm {
                    type threshold;
                }

+                leaf mpls_inseg_threshold_type {


Please add tests for MPLS CRM.

@kperumalbfn I looked at the existing CRM tests in sonic-yang-models. It appears that there exist currently generic parameter checking that is applicable for all CRM types. I don't think there needs to be anything specific for MPLS, since it can already make use of the generic CRM tests.

@kperumalbfn I have added CRM tests for the 2 MPLS CRM types as requested.

kperumalbfn · 2021-06-24T17:27:02Z

src/sonic-yang-models/yang-models/sonic-interface.yang

+				leaf mpls {
+					description "Enable/disable MPLS routing for the interface";
+					type string {
+						pattern "enable|disable";


Default value for all the interface types?

@kperumalbfn I don't think we want to add a default value here, do we? What happens if the platform does not support this attribute? I think we do not want to explicitly set the attribute in that case.

venkatmahalingam · 2021-06-28T04:55:15Z

src/sonic-yang-models/yang-models/sonic-vlan.yang

+					description "Enable/disable MPLS routing for the vlan interface";
+					type enumeration {
+						enum enable;
+						enum disable;


As discussed in the meeting, please define typedef for enable/disable and use it here.

@venkatmahalingam There seems to be some inconsistency with regards to typedefs in sonic-yang-models. I see instances of enable/disable enum in sonic-device_metadata.yang and sonic-flex_counter.yang without a generic typedef being referenced. Also, I see that sonic-types.yang now requires a revision. I would suggest that a new revision of sonic-types.yang that incorporates a generic enable/disable typedef should be outside of the scope of this PR and done as a general cleanup of the yang_models to avoid churn on yang revisions.

why can't we add typedef for enable/disable in the sonic-types.yang and use it in this YANG? we can correct typedef in the other modules later?

@venkatmahalingam Presumably any addition to sonic-types.yang requires a new revision for that file. It makes more sense to bundle multiple changes to sonic-types.yang together to avoid revision churn. This should be done as general cleanup prior to new release and should not be included in the scope of this PR.

liat-grozovik · 2021-08-22T12:58:59Z

@qbdwlr can you pls share some update on the review comments handling? it is blocking dpb from being executed on master.

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

This PR is needed to solve the DPB break in the current master. DPB is nonfunctional for more than two months now.

zhangyanzhao · 2021-08-26T17:26:17Z

@qbdwlr would you please help to address this PR? #7881 (comment) @caizhenghui-juniper please help to drive the PR merge. Thanks.

qbdwlr · 2021-08-31T19:13:07Z

@zhangyanzhao @liat-grozovik There is nothing preventing this PR from being merged in its current state. It may be merged now to resolve DPB issue.

zhangyanzhao · 2021-09-02T17:31:55Z

We will create a new PR to track the "why can't we add typedef for enable/disable in the sonic-types.yang and use it in this YANG? we can correct typedef in the other modules later?" issue. @venkatmahalingam please approve this PR and Guohan will merge.

SONiC YANG model support in buildimage for MPLS: sonic-yang-model support for MPLS enable/disable sonic-yang-model support for MPLS CRM thresholds

qbdwlr requested a review from lguohan as a code owner June 15, 2021 18:55

This was referenced Jun 15, 2021

MPLS for SONiC infrastructure HLD sonic-net/SONiC#706

Merged

libnl3 updates for MPLS #7195

Merged

qbdwlr force-pushed the yangMpls branch 2 times, most recently from afcd489 to cefc1ca Compare June 21, 2021 00:09

qbdwlr mentioned this pull request Jun 24, 2021

[DPB] Port Breakout Fails with "sonic_yang(3):All Keys are not parsed in CRM" #7956

Closed

anshuv-mfst added the YANG label Jun 24, 2021

anshuv-mfst added this to In progress in yang via automation Jun 24, 2021

kperumalbfn reviewed Jun 24, 2021

View reviewed changes

anshuv-mfst moved this from In progress to Review in progress in yang Jun 24, 2021

qbdwlr force-pushed the yangMpls branch from 05c29bb to e41531b Compare June 24, 2021 19:23

venkatmahalingam reviewed Jun 28, 2021

View reviewed changes

qbdwlr force-pushed the yangMpls branch from e41531b to 2a430ac Compare July 1, 2021 17:13

judyjoseph added the Request for 202106 Branch label Aug 20, 2021

sonic-yang-models updates for MPLS

583d5ef

qbdwlr force-pushed the yangMpls branch from 2a430ac to 583d5ef Compare August 28, 2021 15:00

venkatmahalingam approved these changes Sep 2, 2021

View reviewed changes

lguohan merged commit 7c9be31 into sonic-net:master Sep 2, 2021

yang automation moved this from Review in progress to Done Sep 2, 2021

judyjoseph added the Included in 202106 Branch label Sep 2, 2021

judyjoseph pushed a commit that referenced this pull request Sep 2, 2021

[yang]: sonic-yang-models updates for MPLS (#7881)

e5e273d

SONiC YANG model support in buildimage for MPLS: sonic-yang-model support for MPLS enable/disable sonic-yang-model support for MPLS CRM thresholds

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sonic-yang-models updates for MPLS #7881

sonic-yang-models updates for MPLS #7881

qbdwlr commented Jun 15, 2021 •

edited

kperumalbfn Jun 24, 2021

qbdwlr Jun 24, 2021

kperumalbfn Jun 25, 2021

qbdwlr Aug 31, 2021

kperumalbfn Jun 24, 2021

qbdwlr Jun 24, 2021

qbdwlr Aug 31, 2021

kperumalbfn Jun 24, 2021

qbdwlr Jun 24, 2021

venkatmahalingam Jun 28, 2021 •

edited

qbdwlr Aug 31, 2021

venkatmahalingam Sep 2, 2021

qbdwlr Sep 2, 2021

liat-grozovik commented Aug 22, 2021

zhangyanzhao commented Aug 26, 2021

zhangyanzhao commented Aug 26, 2021 •

edited

qbdwlr commented Aug 31, 2021

zhangyanzhao commented Sep 2, 2021

sonic-yang-models updates for MPLS #7881

sonic-yang-models updates for MPLS #7881

Conversation

qbdwlr commented Jun 15, 2021 • edited

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

Choose a reason for hiding this comment

venkatmahalingam Jun 28, 2021 • edited

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

liat-grozovik commented Aug 22, 2021

zhangyanzhao commented Aug 26, 2021

zhangyanzhao commented Aug 26, 2021 • edited

qbdwlr commented Aug 31, 2021

zhangyanzhao commented Sep 2, 2021

qbdwlr commented Jun 15, 2021 •

edited

venkatmahalingam Jun 28, 2021 •

edited

zhangyanzhao commented Aug 26, 2021 •

edited