Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sonic-yang-models updates for MPLS #7881

Merged
merged 1 commit into from
Sep 2, 2021
Merged

sonic-yang-models updates for MPLS #7881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions src/sonic-yang-models/tests/yang_model_tests/tests/crm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,43 @@
"desc": "IPMC_WITH_WRONG_THRESHOLD_TYPE pattern failure.",
"eStrKey": "Pattern",
"eStr": ["wrong" ]
},
"MPLS_INSEG_WITH_WRONG_THRESHOLD_TYPE": {
"desc": "MPLS_INSEG_WITH_WRONG_THRESHOLD_TYPE pattern failure.",
"eStrKey": "Pattern",
"eStr": ["wrong" ]
},
"MPLS_INSEG_WITH_WRONG_PERCENTAGE": {
"desc": "MPLS_INSEG_WITH_WRONG_PERCENTAGE must condition failure.",
"eStrKey": "Must"
},
"MPLS_INSEG_WITH_HIGH_THRESHOLD_ERR": {
"desc": "MPLS_INSEG_WITH_HIGH_THRESHOLD_ERR must condition failure about high threshold being lower than low threshold.",
"eStr": ["high_threshold should be more than low_threshold"]
},
"MPLS_INSEG_WITH_CORRECT_USED_VALUE": {
"desc": "MPLS_INSEG_WITH_CORRECT_USED_VALUE no failure."
},
"MPLS_INSEG_WITH_CORRECT_FREE_VALUE": {
"desc": "MPLS_INSEG_WITH_CORRECT_FREE_VALUE no failure."
},
"MPLS_NH_WITH_WRONG_THRESHOLD_TYPE": {
"desc": "MPLS_NH_WITH_WRONG_THRESHOLD_TYPE pattern failure.",
"eStrKey": "Pattern",
"eStr": ["wrong" ]
},
"MPLS_NH_WITH_WRONG_PERCENTAGE": {
"desc": "MPLS_NH_WITH_WRONG_PERCENTAGE must condition failure.",
"eStrKey": "Must"
},
"MPLS_NH_WITH_HIGH_THRESHOLD_ERR": {
"desc": "MPLS_NH_WITH_HIGH_THRESHOLD_ERR must condition failure about high threshold being lower than low threshold.",
"eStr": ["high_threshold should be more than low_threshold"]
},
"MPLS_NH_WITH_CORRECT_USED_VALUE": {
"desc": "MPLS_NH_WITH_CORRECT_USED_VALUE no failure."
},
"MPLS_NH_WITH_CORRECT_USED_VALUE": {
"desc": "MPLS_NH_WITH_CORRECT_USED_VALUE no failure."
}
}
112 changes: 111 additions & 1 deletion src/sonic-yang-models/tests/yang_model_tests/tests_config/crm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,5 +218,115 @@
}
}
}
},
"MPLS_INSEG_WITH_CORRECT_FREE_VALUE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_inseg_high_threshold": 90,
"mpls_inseg_low_threshold": 70,
"mpls_inseg_threshold_type": "free"
}
}
}
},
"MPLS_INSEG_WITH_CORRECT_USED_VALUE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_inseg_high_threshold": 85,
"mpls_inseg_low_threshold": 25,
"mpls_inseg_threshold_type": "used"
}
}
}
},
"MPLS_INSEG_WITH_HIGH_THRESHOLD_ERR": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_inseg_high_threshold": 80,
"mpls_inseg_low_threshold": 81,
"mpls_inseg_threshold_type": "PERCENTAGE"
}
}
}
},
"MPLS_INSEG_WITH_WRONG_PERCENTAGE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_inseg_high_threshold": 110,
"mpls_inseg_low_threshold": 85,
"mpls_inseg_threshold_type": "PERCENTAGE"
}
}
}
},
"MPLS_INSEG_WITH_WRONG_THRESHOLD_TYPE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_inseg_high_threshold": 90,
"mpls_inseg_low_threshold": 70,
"mpls_inseg_threshold_type": "wrong"
}
}
}
},
"MPLS_NH_WITH_CORRECT_FREE_VALUE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_nexthop_high_threshold": 90,
"mpls_nexthop_low_threshold": 70,
"mpls_nexthop_threshold_type": "free"
}
}
}
},
"MPLS_NH_WITH_CORRECT_USED_VALUE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_nexthop_high_threshold": 85,
"mpls_nexthop_low_threshold": 25,
"mpls_nexthop_threshold_type": "used"
}
}
}
},
"MPLS_NH_WITH_HIGH_THRESHOLD_ERR": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_nexthop_high_threshold": 80,
"mpls_nexthop_low_threshold": 81,
"mpls_nexthop_threshold_type": "PERCENTAGE"
}
}
}
},
"MPLS_NH_WITH_WRONG_PERCENTAGE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_nexthop_high_threshold": 110,
"mpls_nexthop_low_threshold": 85,
"mpls_nexthop_threshold_type": "PERCENTAGE"
}
}
}
},
"MPLS_NH_WITH_WRONG_THRESHOLD_TYPE": {
"sonic-crm:sonic-crm": {
"sonic-crm:CRM": {
"Config": {
"mpls_nexthop_high_threshold": 90,
"mpls_nexthop_low_threshold": 70,
"mpls_nexthop_threshold_type": "wrong"
}
}
}
}
}
}
52 changes: 52 additions & 0 deletions src/sonic-yang-models/yang-models/sonic-crm.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,58 @@ module sonic-crm {
type threshold;
}

leaf mpls_inseg_threshold_type {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests for MPLS CRM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

description "CRM threshold support for MPLS in-segment entries";

must "(((current()='PERCENTAGE' or current()='percentage') and
../mpls_inseg_high_threshold<100 and
../mpls_inseg_low_threshold<100) or
(current()!='PERCENTAGE' and current()!='percentage'))";
type stypes:crm_threshold_type;
}

leaf mpls_inseg_high_threshold {
description "CRM threshold support for MPLS in-segment entries";

must "(current() > ../mpls_inseg_low_threshold)"
{
error-message "high_threshold should be more than low_threshold";
}
type threshold;
}

leaf mpls_inseg_low_threshold {
description "CRM threshold support for MPLS in-segment entries";

type threshold;
}

leaf mpls_nexthop_threshold_type {
description "CRM threshold support for MPLS next-hops";

must "(((current()='PERCENTAGE' or current()='percentage') and
../mpls_nexthop_high_threshold<100 and
../mpls_nexthop_low_threshold<100) or
(current()!='PERCENTAGE' and current()!='percentage'))";
type stypes:crm_threshold_type;
}

leaf mpls_nexthop_high_threshold {
description "CRM threshold support for MPLS next-hops";

must "(current() > ../mpls_nexthop_low_threshold)"
{
error-message "high_threshold should be more than low_threshold";
}
type threshold;
}

leaf mpls_nexthop_low_threshold {
description "CRM threshold support for MPLS next-hops";

type threshold;
}

}
/* end of Config */
}
Expand Down
7 changes: 7 additions & 0 deletions src/sonic-yang-models/yang-models/sonic-interface.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ module sonic-interface {
}
default "0";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

leaf mpls {
description "Enable/disable MPLS routing for the interface";
type enumeration {
enum enable;
enum disable;
}
}
}
/* end of INTERFACE_LIST */

Expand Down
7 changes: 7 additions & 0 deletions src/sonic-yang-models/yang-models/sonic-portchannel.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ module sonic-portchannel {
}
default "0";
}
leaf mpls {
description "Enable/disable MPLS routing for the portchannel interface";
type enumeration {
enum enable;
enum disable;
}
}
} /* end of list PORTCHANNEL_INTERFACE_LIST */

list PORTCHANNEL_INTERFACE_IPPREFIX_LIST {
Expand Down
7 changes: 7 additions & 0 deletions src/sonic-yang-models/yang-models/sonic-vlan.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ module sonic-vlan {
}
default "0";
}
leaf mpls {
description "Enable/disable MPLS routing for the vlan interface";
type enumeration {
enum enable;
enum disable;
Copy link
Collaborator

@venkatmahalingam venkatmahalingam Jun 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

}
}
}
/* end of VLAN_INTERFACE_LIST */

Expand Down