add Tunnel yang model #12232

Ndancejic · 2022-09-30T01:10:27Z

Signed-off-by: Nikola Dancejic ndancejic@microsoft.com

Why I did it

Add yang model for TUNNEL config

How I did it

created sonic-tunnel.yang file and tests

How to verify it

make target/python-wheels/bullseye/sonic_yang_models-1.0-py3-none-any.whl

Which release branch to backport (provide reason below if selected)

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md#tunnel

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

zhangyanzhao · 2022-10-06T17:31:50Z

@praveen-li will add comments. @qiluo-msft will check the configuration part.

src/sonic-yang-models/yang-models/sonic-tunnel.yang

qiluo-msft · 2022-10-06T18:35:38Z

src/sonic-yang-models/yang-models/sonic-tunnel.yang

+                    description "optional source IPv4 address.";
+                }
+
+                leaf dst_ip {


dst_ip

Is there any constrain on dst_ip? For example, must it be a dualtor peer IP?

What would be the correct way to specify this? other than in the description

To clarify, I am not asking you to modify yang model. The point of accurate yang model, is to capture exact backend (in this case orchagent/etc) behavior. If backend has the implementation to validate dst_ip with some constrains, yang model need to state the constains. If backend has no validation at all, current yang model is good enough.

praveen-li · 2022-10-06T20:47:19Z

src/sonic-yang-models/doc/Configuration.md

+```
+{
+    "TUNNEL": {
+        "MuxTunnel0": {


Can we add couple of more Tunnel entries to show all possible configs. Such as "dscp_mode": "pipe" and ecn_mode values etc.

Sure, I'll add these and update!

praveen-li · 2022-10-06T20:49:19Z

src/sonic-yang-models/tests/files/sample_config_db.json

+            }
+        },
+        "TUNNEL": {
+            "MuxTunnel0": {


Same here, more entries to show\parse different possible configs.

I'll add this and update as well

praveen-li · 2022-10-06T20:51:09Z

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

+    },
+    "TUNNEL_INVALID_IP": {
+        "desc": "Load TUNNEL with invalid IPv4 Address.",
+        "eStrKey": "Pattern"


"estr": ["Exact field name"]
may be dst_ip ? in this case.

praveen-li · 2022-10-06T20:52:20Z

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

+    },
+    "TUNNEL_MISSING_NAME": {
+        "desc": "Load PEER_SWITCH missing PEER Device name.",
+        "eStrKey": "Mandatory"


"estr": ["Exact field name"]

praveen-li · 2022-10-06T20:52:47Z

src/sonic-yang-models/tests/yang_model_tests/tests_config/tunnel.json

+        }
+    },
+
+    "TUNNEL_INVALID_IP": {


TUNNEL_INVALID_IP -> TUNNEL_INVALID_FIELD_NAME

praveen-li · 2022-10-06T20:53:22Z

src/sonic-yang-models/tests/yang_model_tests/tests_config/tunnel.json

+        }
+    },
+
+    "TUNNEL_MISSING_NAME": {


TUNNEL_MISSING_NAME -> TUNNEL_MISSING_MUX_TUNNEL

praveen-li · 2022-10-06T20:53:52Z

src/sonic-yang-models/yang-models/sonic-tunnel.yang

+    }
+
+    container sonic-tunnel {
+        container TUNNEL {


description

praveen-li · 2022-10-06T20:56:53Z

src/sonic-yang-models/yang-models/sonic-tunnel.yang

+                leaf decap_dscp_to_tc_map {
+                    description "optional Decap DSCP to TC map";
+                    type string {
+                        pattern "AZURE_TUNNEL";


only 1 pattern needed?

As far as I can tell this was the only configuration supported, Is there a different way I should specify this?

src/sonic-yang-models/yang-models/sonic-tunnel.yang

zhangyanzhao · 2022-10-13T17:17:04Z

@Ndancejic can you please help to address the comments? Thanks.

neethajohn · 2022-10-21T20:31:56Z

src/sonic-yang-models/doc/Configuration.md

+            "dscp_mode": "uniform",
+            "dst_ip": "10.1.0.32",
+            "ecn_mode": "copy_from_outer",
+            "encap_ecn_mode": "standard",


please also include the encap and decap qos maps related attributes. I see that they have been added to the actual yang definition

Ndancejic · 2022-10-25T21:46:18Z

@praveen-li @qiluo-msft I had a few questions on some of your comments

dgsudharsan · 2022-10-27T17:18:08Z

src/sonic-yang-models/yang-models/sonic-tunnel.yang

+                leaf decap_tc_to_pg_map {
+                    description "optional Decap TC to PG map";
+                    type string {
+                        pattern "AZURE_TUNNEL";


Can we try to avoid using specific pattern names?

dgsudharsan · 2022-10-27T17:20:53Z

src/sonic-yang-models/yang-models/sonic-tunnel.yang

+                    }
+                }
+
+                leaf decap_dscp_to_tc_map {


Can you create reference to DSCP to TC map table?

ganglyu · 2022-11-03T03:13:45Z

why do you update submodule for frr?

Ndancejic · 2022-11-03T16:16:00Z

My mistake, it slipped past my diff. I'll update that

zhangyanzhao · 2022-11-03T17:09:16Z

@Ndancejic can you please help to address the comments? Thanks.

ganglyu · 2022-11-11T01:29:42Z

@Ndancejic Some unit tests failed, please let me know if you need any help.

prsunny · 2022-11-29T13:20:32Z

@Ndancejic , could you please check the failures?

zhangyanzhao · 2022-12-08T18:06:41Z

@Ndancejic please help to address the build failure. Thanks.

Ndancejic · 2022-12-12T21:25:33Z

Rebased and cleaned up the commits a bit. I would appreciate a second look - edit: nvm I need to take another look at this :/ my build is still going through without an issue for some reason... I think I'll have to clean and rebuild

ganglyu · 2022-12-12T23:50:49Z

Rebased and cleaned up the commits a bit. I would appreciate a second look - edit: nvm I need to take another look at this :/ my build is still going through without an issue for some reason... I think I'll have to clean and rebuild

sonic-config-engine will run yang validation for generated config_db, and it seems that test_minigraph_tunnel_table can't pass yang validation.

Ndancejic · 2022-12-12T23:56:24Z

Rebased and cleaned up the commits a bit. I would appreciate a second look - edit: nvm I need to take another look at this :/ my build is still going through without an issue for some reason... I think I'll have to clean and rebuild

sonic-config-engine will run yang validation for generated config_db, and it seems that test_minigraph_tunnel_table can't pass yang validation.

I can see the issue now, looks like it's because src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml is configured with DecapDscpToTcMap="AZURE_TUNNEL" DecapTcToPgMap="AZURE_TUNNEL" EncapTcToQueueMap="AZURE_TUNNEL" EncapTcToDscpMap="AZURE_TUNNEL" However there is no AZURE_TUNNEL qos map.

ganglyu · 2022-12-13T00:00:45Z

Rebased and cleaned up the commits a bit. I would appreciate a second look - edit: nvm I need to take another look at this :/ my build is still going through without an issue for some reason... I think I'll have to clean and rebuild

sonic-config-engine will run yang validation for generated config_db, and it seems that test_minigraph_tunnel_table can't pass yang validation.

I can see the issue now, looks like it's because src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml is configured with DecapDscpToTcMap="AZURE_TUNNEL" DecapTcToPgMap="AZURE_TUNNEL" EncapTcToQueueMap="AZURE_TUNNEL" EncapTcToDscpMap="AZURE_TUNNEL" However there is no AZURE_TUNNEL qos map.

Thanks! Would you please fix this unit test?

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>

ganglyu · 2022-12-23T00:47:58Z

src/sonic-yang-models/yang-models/sonic-tunnel.yang

+		prefix inet;
+	}
+
+    import sonic-peer-switch {


Would you please check the indent?

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>

Ndancejic requested a review from qiluo-msft as a code owner September 30, 2022 01:10

zhangyanzhao requested a review from praveen-li October 4, 2022 06:02

zhangyanzhao added the YANG YANG model related changes label Oct 4, 2022

qiluo-msft requested review from prsunny and ganglyu October 6, 2022 18:31

qiluo-msft reviewed Oct 6, 2022

View reviewed changes

src/sonic-yang-models/yang-models/sonic-tunnel.yang Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 6, 2022

View reviewed changes

src/sonic-yang-models/yang-models/sonic-tunnel.yang Outdated Show resolved Hide resolved

qiluo-msft reviewed Oct 6, 2022

View reviewed changes

praveen-li reviewed Oct 6, 2022

View reviewed changes

ganglyu reviewed Oct 6, 2022

View reviewed changes

src/sonic-yang-models/yang-models/sonic-tunnel.yang Outdated Show resolved Hide resolved

ganglyu linked an issue Oct 12, 2022 that may be closed by this pull request

[yang] Need Yang for TUNNEL table #10969

Closed

ganglyu removed a link to an issue Oct 12, 2022

[yang] Need Yang for TUNNEL table #10969

Closed

ganglyu linked an issue Oct 12, 2022 that may be closed by this pull request

[yang] Need Yang for TUNNEL table #10969

Closed

neethajohn reviewed Oct 21, 2022

View reviewed changes

dgsudharsan suggested changes Oct 27, 2022

View reviewed changes

Ndancejic requested a review from lguohan as a code owner November 2, 2022 20:03

ganglyu approved these changes Nov 11, 2022

View reviewed changes

ganglyu self-requested a review November 11, 2022 01:28

Ndancejic force-pushed the yang branch from 14c34df to d31eb52 Compare December 12, 2022 21:24

Adding Tunnel YANG model

be8a4eb

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>

Ndancejic force-pushed the yang branch from d31eb52 to be8a4eb Compare December 22, 2022 00:31

changing to sonic-net namespace

fa26df7

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>

ganglyu approved these changes Dec 23, 2022

View reviewed changes

ganglyu reviewed Dec 23, 2022

View reviewed changes

ganglyu self-requested a review December 23, 2022 00:48

Fixing indent

339a4d1

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>

ganglyu approved these changes Dec 24, 2022

View reviewed changes

qiluo-msft merged commit 86a5a7f into sonic-net:master Dec 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Tunnel yang model #12232

add Tunnel yang model #12232

Ndancejic commented Sep 30, 2022

zhangyanzhao commented Oct 6, 2022

qiluo-msft Oct 6, 2022 •

edited

Loading

Ndancejic Oct 18, 2022

qiluo-msft Oct 25, 2022 •

edited

Loading

praveen-li Oct 6, 2022

Ndancejic Oct 18, 2022

praveen-li Oct 6, 2022

Ndancejic Oct 18, 2022

praveen-li Oct 6, 2022

praveen-li Oct 6, 2022

praveen-li Oct 6, 2022

praveen-li Oct 6, 2022

praveen-li Oct 6, 2022

praveen-li Oct 6, 2022

Ndancejic Oct 18, 2022

zhangyanzhao commented Oct 13, 2022

neethajohn Oct 21, 2022

Ndancejic commented Oct 25, 2022

dgsudharsan Oct 27, 2022

dgsudharsan Oct 27, 2022

ganglyu commented Nov 3, 2022

Ndancejic commented Nov 3, 2022

zhangyanzhao commented Nov 3, 2022

ganglyu commented Nov 11, 2022

prsunny commented Nov 29, 2022

zhangyanzhao commented Dec 8, 2022

Ndancejic commented Dec 12, 2022 •

edited

Loading

ganglyu commented Dec 12, 2022

Ndancejic commented Dec 12, 2022

ganglyu commented Dec 13, 2022

ganglyu Dec 23, 2022

add Tunnel yang model #12232

add Tunnel yang model #12232

Conversation

Ndancejic commented Sep 30, 2022

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

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

zhangyanzhao commented Oct 6, 2022

qiluo-msft Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Oct 25, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhangyanzhao commented Oct 13, 2022

Choose a reason for hiding this comment

Ndancejic commented Oct 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganglyu commented Nov 3, 2022

Ndancejic commented Nov 3, 2022

zhangyanzhao commented Nov 3, 2022

ganglyu commented Nov 11, 2022

prsunny commented Nov 29, 2022

zhangyanzhao commented Dec 8, 2022

Ndancejic commented Dec 12, 2022 • edited Loading

ganglyu commented Dec 12, 2022

Ndancejic commented Dec 12, 2022

ganglyu commented Dec 13, 2022

Choose a reason for hiding this comment

qiluo-msft Oct 6, 2022 •

edited

Loading

qiluo-msft Oct 25, 2022 •

edited

Loading

Ndancejic commented Dec 12, 2022 •

edited

Loading