[Sonic yang models]: Added the sonic yang models for acl, port, portchannel, vlan, vrf, interface etc by anand-kumar-subramanian · Pull Request #4001 · sonic-net/sonic-buildimage

anand-kumar-subramanian · 2020-01-09T22:31:51Z

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

anand-kumar-subramanian · 2020-01-09T22:35:48Z

Since libyang is there after the following #3710 PR merge, we are fine without these exports

Merge master

…nterface etc

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

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

src/sonic-yang-models/sonic-acl.yang

renukamanavalan · 2020-01-31T17:29:21Z

src/sonic-yang-models/sonic-acl.yang

+					type string;
+				}
+
+				leaf PRIORITY {


Documenting is required. But that does not justify us to be random.
Let us follow a standard.
Agree with @praveen-li, let us go for all caps.
@anand-kumar-subramanian, please ensure new SONiC schema files in buzznic follows this.
Please update the doc too.

renukamanavalan · 2020-01-31T17:31:48Z

src/sonic-yang-models/sonic-acl.yang

+				}
+
+				leaf RULE_DESCRIPTION {
+					type string;


I see the length restriction dropped (compared to PR #18.
Is this intentional?

renukamanavalan · 2020-01-31T17:33:29Z

src/sonic-yang-models/sonic-acl.yang

+						enum FORWARD;
+						enum DROP;
+						enum REDIRECT;
+						enum INT_INSERT;


What are these new action types, "INT_INSERT" & "INT_DELETE"?

Does SONiC support today ?
Is this platform constrained?

renukamanavalan · 2020-01-31T20:08:37Z

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

+    description
+        "SONIC VLAN";
+
+    revision 2019-05-15 {


Not sure, if you would like to update revision date.

renukamanavalan · 2020-01-31T20:10:46Z

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

+                    }
+                }
+
+                leaf-list members {


Why do we need this field, when there is a separate table for VLAN_MEMBERS?

I think, this is for backward compatibility.
With change in SONIC Config, The challange is to ask NetOPS to change few tools which are responsible to deploy config on thousands of switches. So if we were using it in data Center previously then usually we try to be backward compatible.

renukamanavalan · 2020-01-31T20:11:25Z

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

+
+
+        container VLAN_TABLE {
+            config false;


Which DB is this from?

Yeah, same ques. We need VLAN VRF entry as below [I think, as per VRF HLD]. But VLAN_table, I am not sure.

"VLAN_INTERFACE": {
"Vlan100": {},
"Vlan777": {},
"Vlan100|2a04:....::1/64": {
"family": "IPv6",
"scope": "global"
},
"Vlan100|10...../26": {
"family": "IPv4",
"scope": "global"
},
"Vlan100|fe80::..../10": {
"family": "IPv6",
"scope": "local"
},
"Vlan777|2a04:..../64": {
"family": "IPv6",
"scope": "global"
},

This is not Vlan table. Its similar to LAG/PORT Tables. L3 is binded to another table which is suffex-ed with _INTERFACE which has VRF info. In the current context, I think it is the VLAN table that's referenced here. @renukamanavalan , its in APPL_DB

renukamanavalan · 2020-01-31T20:12:31Z

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

+                key "name ifname";
+
+                leaf name {
+                    type leafref {


Not sure, you need to declare as leafref for a read-only data. Redundant ? More like, 'unused' ?

renukamanavalan · 2020-01-31T20:14:55Z

src/sonic-yang-models/sonic-vrf.yang

+
+                leaf vni {
+                    type uint32 {
+                        range "0..16777215" {


Constraint to ensure that this ID is not reused?

praveen-li

Without ietf yang models, it looks good to me. Approving,

@anand-kumar-subramanian : kindly address other comments from Renuka, So that we can merge this. Thanks.

praveen-li · 2020-02-04T19:47:05Z

@anand-kumar-subramanian

It is good to fill below :). Thx.

What I did
How I did it
How to verify it
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)

praveen-li · 2020-02-05T20:09:53Z

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

+                    }
+                }
+
+                leaf-list members {


I think, this is for backward compatibility.
With change in SONIC Config, The challange is to ask NetOPS to change few tools which are responsible to deploy config on thousands of switches. So if we were using it in data Center previously then usually we try to be backward compatible.

praveen-li · 2020-02-05T20:13:35Z

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

+
+
+        container VLAN_TABLE {
+            config false;


Yeah, same ques. We need VLAN VRF entry as below [I think, as per VRF HLD]. But VLAN_table, I am not sure.

"VLAN_INTERFACE": {
"Vlan100": {},
"Vlan777": {},
"Vlan100|2a04:....::1/64": {
"family": "IPv6",
"scope": "global"
},
"Vlan100|10...../26": {
"family": "IPv4",
"scope": "global"
},
"Vlan100|fe80::..../10": {
"family": "IPv6",
"scope": "local"
},
"Vlan777|2a04:..../64": {
"family": "IPv6",
"scope": "global"
},

praveen-li · 2020-02-05T20:17:07Z

src/sonic-yang-models/sonic-mirror-session.yang

+                    }
+                }
+
+                must "not (src_port) or not (dst_port) or (dst_port != src_port)" {


Due to many must and pattern condition we thought of writing test cases which make sure that these conditions are working as expected.
I strongly think, we should have some test case results for these Or we should remove them right now. When I put test case infra then put them back with appropriate test cases.

prsunny · 2020-02-12T00:14:52Z

src/sonic-yang-models/sonic-acl.yang

+					}
+				}
+
+				leaf MIRROR_ACTION {


prsunny · 2020-02-12T00:19:22Z

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

+                                        }
+				}
+
+                                leaf nat_zone {


nat supported in latest master

prsunny · 2020-02-12T00:19:59Z

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

+                                    }
+                                }
+
+                leaf unnumbered {


unnumbered is not supported in Sonic now

prsunny · 2020-02-12T00:27:36Z

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

+                }
+
+                leaf ip_prefix {
+                    type inet:ip-prefix;


In general yes, but if they are in two different VRFs, it is ok to have same ip address

prsunny · 2020-02-12T00:29:22Z

src/sonic-yang-models/sonic-mirror-session.yang

+
+                leaf direction {
+                    type enumeration {
+                        enum rx;


a more common wording is "ingress" and "egress". E.g. mirror-ingress

prsunny · 2020-02-12T00:32:28Z

src/sonic-yang-models/sonic-mirror-session.yang

+                    type string;
+                }
+
+                leaf dst_mac {


how about src_mac?

prsunny · 2020-02-12T00:36:39Z

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

+
+
+        container VLAN_TABLE {
+            config false;


This is not Vlan table. Its similar to LAG/PORT Tables. L3 is binded to another table which is suffex-ed with _INTERFACE which has VRF info. In the current context, I think it is the VLAN table that's referenced here. @renukamanavalan , its in APPL_DB

prsunny · 2020-02-12T00:44:00Z

src/sonic-yang-models/sonic-vrf.yang

+                        "Enalbe/disable fallback feature which is useful for specified VRF user to access internet through global/main route.";
+                }
+
+                leaf vni {


what is VNI? is it for tunnel? can you give some more description?

…tabase, swss and syncd to After list

Removing the libYang exports which were pointing to the wrong directory

905b198

lguohan previously approved these changes Jan 9, 2020

View reviewed changes

anand-kumar-subramanian and others added 2 commits January 23, 2020 10:28

Merge pull request #1 from Azure/master

7c072fb

Merge master

Adding the sonic yang models for acl, port, portchannel, vlan, vrf, i…

1b84ebf

…nterface etc

anand-kumar-subramanian dismissed lguohan’s stale review via 1b84ebf January 23, 2020 19:16

anand-kumar-subramanian changed the title ~~[startup] Removing the libYang exports which were pointing to the wrong directory~~ Added the sonic yang models for acl, port, portchannel, vlan, vrf, interface etc Jan 23, 2020

anand-kumar-subramanian requested a review from renukamanavalan January 23, 2020 19:28

anand-kumar-subramanian changed the title ~~Added the sonic yang models for acl, port, portchannel, vlan, vrf, interface etc~~ [Sonic yang models] Added the sonic yang models for acl, port, portchannel, vlan, vrf, interface etc Jan 23, 2020

anand-kumar-subramanian changed the title ~~[Sonic yang models] Added the sonic yang models for acl, port, portchannel, vlan, vrf, interface etc~~ [Sonic yang models]: Added the sonic yang models for acl, port, portchannel, vlan, vrf, interface etc Jan 23, 2020

praveen-li reviewed Jan 24, 2020

View reviewed changes

anand-kumar-subramanian added 2 commits January 24, 2020 10:17

Removed the ietf yang models

1564008

Uncommented the needed checks in acl yang

5c75bc6

renukamanavalan requested a review from prsunny January 31, 2020 17:34

renukamanavalan reviewed Jan 31, 2020

View reviewed changes

praveen-li previously approved these changes Feb 3, 2020

View reviewed changes

praveen-li reviewed Feb 5, 2020

View reviewed changes

prsunny reviewed Feb 12, 2020

View reviewed changes

lguohan added the Port Breakout 🔱 label May 1, 2020

zhenggen-xu removed the Port Breakout 🔱 label May 4, 2020

Added database service as requirement and removed swss. Also added da…

7867e65

…tabase, swss and syncd to After list

anand-kumar-subramanian dismissed praveen-li’s stale review via 7867e65 May 6, 2020 20:12

anshuv-mfst added the YANG YANG model related changes label Feb 4, 2021

anand-kumar-subramanian closed this Mar 4, 2021

Conversation

anand-kumar-subramanian commented Jan 9, 2020

Uh oh!

anand-kumar-subramanian commented Jan 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praveen-li left a comment

Choose a reason for hiding this comment

Uh oh!

praveen-li commented Feb 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants