From 4374fa39ba02ef5372118e5381a244803f6a0e97 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Wed, 28 Feb 2024 16:07:48 +0200 Subject: [PATCH 01/10] Update sonic-vlan.yang to better constrain Vlan name ~ Signed-off-by: matiAlfaro --- .../tests/yang_model_tests/test_yang_model.py | 2 +- src/sonic-yang-models/yang-models/sonic-vlan.yang | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py b/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py index d776f479cd..d46d036232 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py +++ b/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py @@ -235,7 +235,7 @@ class Test_yang_models: self.logStartTest(desc) jInput = json.loads(self.readJsonInput(test)) # check all Vlan from 1 to 4094 - for i in range(4095): + for i in range(1,4095): vlan = 'Vlan'+str(i) jInput["sonic-vlan:sonic-vlan"]["sonic-vlan:VLAN"]["VLAN_LIST"]\ [0]["name"] = vlan diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index ad1ab8d743..1e2361adef 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -36,6 +36,10 @@ module sonic-vlan { description "VLAN yang Module for SONiC OS"; + revision 2021-04-28 { + description "Modify Vlan name constraint to allow only legal Vlan names"; + } + revision 2021-04-22 { description "Modify Vlan Member to include PortChannel along with Port"; } @@ -178,7 +182,7 @@ module sonic-vlan { leaf name { type string { - pattern 'Vlan([0-9]{1,3}|[1-3][0-9]{3}|[4][0][0-8][0-9]|[4][0][9][0-4])'; + pattern 'Vlan(409[0-5]|40[0-8][0-9]|[1-3][0-9]{3}|[1-9][0-9]{2}|[1-9][0-9]|[1-9])'; } } From 852d600289623eb2a52e4b05ca8f5b2293a08e29 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Thu, 29 Feb 2024 18:33:53 +0200 Subject: [PATCH 02/10] implement constraints in vlan yang model Signed-off-by: matiAlfaro --- .../yang-models/sonic-vlan.yang | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index 1e2361adef..5a17923257 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -34,11 +34,11 @@ module sonic-vlan { prefix vrf; } - description "VLAN yang Module for SONiC OS"; + import sonic-mirror-session { + prefix sms; + } - revision 2021-04-28 { - description "Modify Vlan name constraint to allow only legal Vlan names"; - } + description "VLAN yang Module for SONiC OS"; revision 2021-04-22 { description "Modify Vlan Member to include PortChannel along with Port"; @@ -184,12 +184,24 @@ module sonic-vlan { type string { pattern 'Vlan(409[0-5]|40[0-8][0-9]|[1-3][0-9]{3}|[1-9][0-9]{2}|[1-9][0-9]|[1-9])'; } + must "not(../../VLAN_INTERFACE_LIST[name=current()]/name)" + { + error-message "Vlan already exists"; + } + /* Not sure about this */ + must "not(../VLAN_INTERFACE/VLAN_INTERFACE_LIST/VLAN_INTERFACE_IPPREFIX_LIST) or ../config/mtu" { + error-message "Cannot remove VLAN with assigned IP address or non-default MTU."; + } } leaf vlanid { type uint16 { range 1..4094; } + must "(current() != 1)" + { + error-message "Vlan1 is default Vlan"; + } } leaf alias { @@ -254,6 +266,18 @@ module sonic-vlan { path "/lag:sonic-portchannel/lag:PORTCHANNEL/lag:PORTCHANNEL_LIST/lag:name"; } } + + must "not(/sms:sonic-mirror-session/sms:MIRROR_SESSION/sms:MIRROR_SESSION_LIST[sms:dst_port=current()])" + { + error-message "Port is used as a destination port in a mirror session."; + } + must "not(/vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_MEMBER/vlan:VLAN_MEMBER_LIST[vlan:port=current() and vlan:name=current()/../../name])" + { + error-message "Port is already a member of this VLAN."; + } + must "not(/lag:sonic-portchannel/lag:PORTCHANNEL/lag:PORTCHANNEL_MEMBER/lag:PORTCHANNEL_MEMBER_LIST[lag:port=current()]/lag:name)" { + error-message "Port is a member of a port channel."; + } } leaf tagging_mode { From a4f7f90afa74302114c47c9c3fbe5fa4488e4717 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Sun, 3 Mar 2024 12:35:25 +0200 Subject: [PATCH 03/10] Change interface in SPAN to obay Vlan constrains Signed-off-by: matiAlfaro --- src/sonic-yang-models/tests/files/sample_config_db.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index f54e9f5882..5403f6ee76 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -2260,7 +2260,7 @@ "span": { "direction": "RX", "type": "SPAN", - "dst_port": "Ethernet2", + "dst_port": "Ethernet7", "src_port": "Ethernet3,Ethernet4" } }, From efa4cfcf03440dc8aa1063211357a1a39081ffbd Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Tue, 5 Mar 2024 11:50:36 +0200 Subject: [PATCH 04/10] implement constraints defined in #12256 Signed-off-by: matiAlfaro --- .../tests/yang_model_tests/test_yang_model.py | 4 +- .../tests/yang_model_tests/tests/vlan.json | 32 +++ .../yang_model_tests/tests_config/vlan.json | 254 ++++++++++++++++++ .../yang-models/sonic-vlan.yang | 39 +-- 4 files changed, 308 insertions(+), 21 deletions(-) diff --git a/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py b/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py index d46d036232..d80293587a 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py +++ b/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py @@ -234,8 +234,8 @@ class Test_yang_models: desc = self.SpecialTests[test]['desc'] self.logStartTest(desc) jInput = json.loads(self.readJsonInput(test)) - # check all Vlan from 1 to 4094 - for i in range(1,4095): + # check all Vlan from 2 to 4094 + for i in range(2,4095): vlan = 'Vlan'+str(i) jInput["sonic-vlan:sonic-vlan"]["sonic-vlan:VLAN"]["VLAN_LIST"]\ [0]["name"] = vlan diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json index 7f1cb4eb35..b840a0b133 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json @@ -86,5 +86,37 @@ "VLAN_INTERFACE_INVALID_ENABLE_IPV6_LINK_LOCAL": { "desc": "Enable the ipv6 link-local as true.", "eStr": "Invalid value \"true\" in \"ipv6_use_link_local_only\" element." + }, + "VLAN_MEMBERS_WITHOUT_CREATING_VLAN":{ + "desc": "Vlan members without Creating Vlan", + "eStrKey": "LeafRef" + }, + "VLAN_CREATE_DEFAULT_VLAN":{ + "desc": "Try to create default Vlan (1)", + "eStr": "Vlan1 is default Vlan" + }, + "VLAN_CREATE_VLAN_WITH_MISSMATCHING_NAME": { + "desc": "Create Vlan so that name doesn't match id", + "eStr": "The vlanid must correspond to the VLAN name" + }, + "IP_PREFIX_WITHOUT_CREATNG_VLAN": { + "desc": "Create IP on not created Vlan", + "eStrKey": "LeafRef" + }, + "MIRROR_SESSION_ON_VLAN_MEMBER_PORT": { + "desc": "Set mirror dst port to a Vlan member", + "eStr": "Port is used as a destination port in a mirror session" + }, + "VLAN_ADD_PORT_CHANNEL_MEMBER": { + "desc": "Add port channel member to Vlan", + "eStr": "Port is a member of a port channel" + }, + "VLAN_ADD_PORT_THAT_IS_UNTAGGED": { + "desc": "Add port that is already untagged", + "eStr": "Port is an untagged member in another VLAN" + }, + "VLAN_ADD_PORT_THAT_IS_ROUTER_INTERFACE": { + "desc": "Add to Vlan port that is router interface", + "eStr": "Port is a router interface" } } diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json index 9f3086b60c..04bb54ed9f 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json @@ -719,5 +719,259 @@ ] } } + }, + + "VLAN_MEMBERS_WITHOUT_CREATING_VLAN": { + "sonic-port:sonic-port": { + "sonic-port:PORT": { + "PORT_LIST": [ + { + "admin_status": "up", + "name": "Ethernet0" + } + ] + } + }, + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN_MEMBER": { + "VLAN_MEMBER_LIST": [ + { + "port": "Ethernet0", + "tagging_mode": "tagged", + "name": "Vlan100" + } + ] + } + } + }, + "VLAN_CREATE_DEFAULT_VLAN": { + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "name": "Vlan1" + } + ] + } + } + }, + "VLAN_CREATE_VLAN_WITH_MISSMATCHING_NAME": { + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "name": "Vlan10", + "vlanid":11 + } + ] + } + } + }, + "IP_PREFIX_WITHOUT_CREATNG_VLAN": { + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "description": "server_vlan", + "name": "Vlan10" + } + ] + }, + "sonic-vlan:VLAN_INTERFACE": { + "VLAN_INTERFACE_IPPREFIX_LIST": [ + { + "family": "IPv4", + "ip-prefix": "20.0.0.1/24", + "scope": "global", + "name": "Vlan100" + } + ], + "VLAN_INTERFACE_LIST": [ + { + "name": "Vlan100" + } + ] + } + } + }, + "MIRROR_SESSION_ON_VLAN_MEMBER_PORT": { + "sonic-port:sonic-port": { + "sonic-port:PORT": { + "PORT_LIST": [ + { + "admin_status": "up", + "name": "Ethernet1", + "speed": 25000, + "lanes": "1" + }, + { + "admin_status": "up", + "name": "Ethernet2", + "speed": 25000, + "lanes": "1" + } + ] + } + }, + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "name": "Vlan10" + } + ] + }, + "sonic-vlan:VLAN_MEMBER": { + "VLAN_MEMBER_LIST": [ + { + "port": "Ethernet2", + "tagging_mode": "tagged", + "name": "Vlan10" + } + ] + } + }, + "sonic-mirror-session:sonic-mirror-session": { + "sonic-mirror-session:MIRROR_SESSION": { + "MIRROR_SESSION_LIST": [ { + "name":"mirror_session_dscp", + "type": "SPAN", + "src_port": "Ethernet1", + "dst_port": "Ethernet2" + }] + } + } + }, + "VLAN_ADD_PORT_CHANNEL_MEMBER": { + "sonic-port:sonic-port": { + "sonic-port:PORT": { + "PORT_LIST": [{ + "admin_status": "up", + "name": "Ethernet1", + "speed": 25000, + "lanes": "1" + }] + } + }, + "sonic-portchannel:sonic-portchannel": { + "sonic-portchannel:PORTCHANNEL": { + "PORTCHANNEL_LIST": [ + { + "admin_status": "up", + "min_links": "1", + "mtu": "9100", + "tpid": "0x8100", + "lacp_key": "auto", + "name": "PortChannel0001", + "fast_rate": "false", + "fallback" : "false", + "mode" : "routed" + } + ] + }, + "sonic-portchannel:PORTCHANNEL_MEMBER": { + "PORTCHANNEL_MEMBER_LIST": [ + { + "name": "PortChannel0001", + "port": "Ethernet1" + } + ] + } + }, + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "name": "Vlan10" + } + ] + }, + "sonic-vlan:VLAN_MEMBER": { + "VLAN_MEMBER_LIST": [ + { + "port": "Ethernet1", + "tagging_mode": "tagged", + "name": "Vlan10" + } + ] + } + } + }, + "VLAN_ADD_PORT_THAT_IS_UNTAGGED": { + "sonic-port:sonic-port": { + "sonic-port:PORT": { + "PORT_LIST": [{ + "admin_status": "up", + "name": "Ethernet1", + "speed": 25000, + "lanes": "1" + }] + } + }, + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "name": "Vlan10" + }, + { + "name": "Vlan11" + } + ] + }, + "sonic-vlan:VLAN_MEMBER": { + "VLAN_MEMBER_LIST": [ + { + "port": "Ethernet1", + "tagging_mode": "untagged", + "name": "Vlan10" + }, + { + "port": "Ethernet1", + "tagging_mode": "tagged", + "name": "Vlan11" + } + ] + } + } + }, + "VLAN_ADD_PORT_THAT_IS_ROUTER_INTERFACE": { + "sonic-interface:sonic-interface": { + "sonic-interface:INTERFACE": { + "INTERFACE_LIST": [ + { + "name": "Ethernet1" + } + ] + } + }, + "sonic-port:sonic-port": { + "sonic-port:PORT": { + "PORT_LIST": [{ + "admin_status": "up", + "name": "Ethernet1", + "speed": 25000, + "lanes": "1" + }] + } + }, + "sonic-vlan:sonic-vlan": { + "sonic-vlan:VLAN": { + "VLAN_LIST": [ + { + "name": "Vlan10" + } + ] + }, + "sonic-vlan:VLAN_MEMBER": { + "VLAN_MEMBER_LIST": [ + { + "port": "Ethernet1", + "tagging_mode": "untagged", + "name": "Vlan10" + } + ] + } + } } } diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index 5a17923257..d80d07c2ac 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -36,7 +36,10 @@ module sonic-vlan { import sonic-mirror-session { prefix sms; - } + } + import sonic-interface { + prefix intf; + } description "VLAN yang Module for SONiC OS"; @@ -66,7 +69,7 @@ module sonic-vlan { leaf name { type leafref { - path /vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_LIST/vlan:name; + path "/vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_LIST/vlan:name"; } } @@ -168,7 +171,7 @@ module sonic-vlan { type boolean; } } - /* end of VLAN_INTERFACE_LIST */ + /* end of VLAN_INTERFACE_IPPREFIX_LIST */ } /* end of VLAN_INTERFACE container */ @@ -184,24 +187,22 @@ module sonic-vlan { type string { pattern 'Vlan(409[0-5]|40[0-8][0-9]|[1-3][0-9]{3}|[1-9][0-9]{2}|[1-9][0-9]|[1-9])'; } - must "not(../../VLAN_INTERFACE_LIST[name=current()]/name)" + must "not(current() = 'Vlan1')" { - error-message "Vlan already exists"; + error-message "Vlan1 is default Vlan"; } - /* Not sure about this */ - must "not(../VLAN_INTERFACE/VLAN_INTERFACE_LIST/VLAN_INTERFACE_IPPREFIX_LIST) or ../config/mtu" { - error-message "Cannot remove VLAN with assigned IP address or non-default MTU."; - } - } - + } leaf vlanid { type uint16 { range 1..4094; } - must "(current() != 1)" + must "not(current() = 1)" { error-message "Vlan1 is default Vlan"; } + must '(../name = "Vlan" + current())' { + error-message "The vlanid must correspond to the VLAN name"; + } } leaf alias { @@ -255,7 +256,6 @@ module sonic-vlan { path "/vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_LIST/vlan:name"; } } - leaf port { /* key elements are mandatory by default */ type union { @@ -266,18 +266,19 @@ module sonic-vlan { path "/lag:sonic-portchannel/lag:PORTCHANNEL/lag:PORTCHANNEL_LIST/lag:name"; } } - must "not(/sms:sonic-mirror-session/sms:MIRROR_SESSION/sms:MIRROR_SESSION_LIST[sms:dst_port=current()])" { error-message "Port is used as a destination port in a mirror session."; } - must "not(/vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_MEMBER/vlan:VLAN_MEMBER_LIST[vlan:port=current() and vlan:name=current()/../../name])" - { - error-message "Port is already a member of this VLAN."; - } - must "not(/lag:sonic-portchannel/lag:PORTCHANNEL/lag:PORTCHANNEL_MEMBER/lag:PORTCHANNEL_MEMBER_LIST[lag:port=current()]/lag:name)" { + must "not(/lag:sonic-portchannel/lag:PORTCHANNEL_MEMBER/lag:PORTCHANNEL_MEMBER_LIST[lag:port=current()]/lag:name)" { error-message "Port is a member of a port channel."; } + must "not(/vlan:sonic-vlan/vlan:VLAN_MEMBER/vlan:VLAN_MEMBER_LIST[vlan:port=current() and vlan:tagging_mode='untagged' and vlan:name!=current()/../name])" { + error-message "Port is an untagged member in another VLAN."; + } + must "not(/intf:sonic-interface/intf:INTERFACE/intf:INTERFACE_LIST[intf:name=current()])" { + error-message "Port is a router interface."; + } } leaf tagging_mode { From 09a5f6303cb53f993da698467a87a25cebe07428 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Tue, 5 Mar 2024 13:54:48 +0200 Subject: [PATCH 05/10] fixes to sample_config_db.json so yang mgmt test will pass Signed-off-by: matiAlfaro --- src/sonic-yang-models/tests/files/sample_config_db.json | 4 ++-- src/sonic-yang-models/yang-models/sonic-vlan.yang | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index 5403f6ee76..1006dcfc7e 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -1183,10 +1183,10 @@ "Vlan111|Ethernet0": { "tagging_mode": "untagged" }, - "Vlan111|Ethernet1": { + "Vlan111|Ethernet10": { "tagging_mode": "untagged" }, - "Vlan111|Ethernet2": { + "Vlan111|Ethernet20": { "tagging_mode": "untagged" }, "Vlan111|Ethernet3": { diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index d80d07c2ac..5b89f82adb 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -200,7 +200,7 @@ module sonic-vlan { { error-message "Vlan1 is default Vlan"; } - must '(../name = "Vlan" + current())' { + must "(concat('Vlan', current()) = ../name)" { error-message "The vlanid must correspond to the VLAN name"; } } From 1b0779d28c48dce8c79df894ea1c978fb65be7a3 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Wed, 6 Mar 2024 13:21:46 +0200 Subject: [PATCH 06/10] config-engine tests fixes Fixed Problems/New Changes: src/sonic-yang-models/tests/files/sample_config_db.json Ethernet1 is member of PO3 which is already in Vlan Ethernet2 is member of PO4, so add it to Vlan src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml src/sonic-config-engine/tests/simple-sample-graph-case.xml fortyGigE0/4 is member of PortChannel01, so add port-channel to vlan src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml src/sonic-config-engine/tests/t0-sample-graph.xml fortyGigE0/112;fortyGigE0/116;fortyGigE0/120 are members of PortChannel01;PortChannel02;PortChannel03, so add port-channel. ~ Signed-off-by: matiAlfaro --- .../tests/simple-sample-graph-case-remap-disabled.xml | 2 +- ...-sample-graph-case-remap-enabled-no-tunnel-attributes.xml | 2 +- .../tests/simple-sample-graph-case-remap-enabled.xml | 2 +- src/sonic-config-engine/tests/simple-sample-graph-case.xml | 2 +- src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml | 2 +- src/sonic-config-engine/tests/t0-sample-graph.xml | 2 +- src/sonic-yang-models/tests/files/sample_config_db.json | 5 +---- src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json | 5 ++--- .../tests/yang_model_tests/tests_config/vlan.json | 2 +- 9 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml index 4ae1b33023..2d26bc9106 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml @@ -142,7 +142,7 @@ ab2 - fortyGigE0/4 + PortChannel01 192.0.0.1 fc02:2000::3;fc02:2000::4 2000 diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml index 6bc2a6db1b..650f56e4de 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml @@ -142,7 +142,7 @@ ab2 - fortyGigE0/4 + PortChannel01 192.0.0.1 fc02:2000::3;fc02:2000::4 2000 diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml index 8c69f04962..b290eb7c6c 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml @@ -142,7 +142,7 @@ ab2 - fortyGigE0/4 + PortChannel01 192.0.0.1 fc02:2000::3;fc02:2000::4 2000 diff --git a/src/sonic-config-engine/tests/simple-sample-graph-case.xml b/src/sonic-config-engine/tests/simple-sample-graph-case.xml index ddb5f03c58..008719c85b 100644 --- a/src/sonic-config-engine/tests/simple-sample-graph-case.xml +++ b/src/sonic-config-engine/tests/simple-sample-graph-case.xml @@ -143,7 +143,7 @@ ab2 - fortyGigE0/4 + PortChannel01 192.0.0.1 fc02:2000::3;fc02:2000::4 2000 diff --git a/src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml b/src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml index da1b56fe13..5d201cc3e2 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml @@ -281,7 +281,7 @@ Vlan2000 - fortyGigE0/112;fortyGigE0/116;fortyGigE0/120 + PortChannel01;PortChannel02;PortChannel03 False 0.0.0.0/0 diff --git a/src/sonic-config-engine/tests/t0-sample-graph.xml b/src/sonic-config-engine/tests/t0-sample-graph.xml index 05db8246ec..b343a8b5e3 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph.xml @@ -265,7 +265,7 @@ Vlan2000 - fortyGigE0/112;fortyGigE0/116;fortyGigE0/120 + PortChannel01;PortChannel02;PortChannel03 False 0.0.0.0/0 diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index 1006dcfc7e..2fef9dc4d9 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -1183,10 +1183,7 @@ "Vlan111|Ethernet0": { "tagging_mode": "untagged" }, - "Vlan111|Ethernet10": { - "tagging_mode": "untagged" - }, - "Vlan111|Ethernet20": { + "Vlan111|PortChannel0004": { "tagging_mode": "untagged" }, "Vlan111|Ethernet3": { diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json index b840a0b133..e653292efd 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json @@ -96,8 +96,7 @@ "eStr": "Vlan1 is default Vlan" }, "VLAN_CREATE_VLAN_WITH_MISSMATCHING_NAME": { - "desc": "Create Vlan so that name doesn't match id", - "eStr": "The vlanid must correspond to the VLAN name" + "desc": "Create Vlan so that name doesn't match id" }, "IP_PREFIX_WITHOUT_CREATNG_VLAN": { "desc": "Create IP on not created Vlan", @@ -113,7 +112,7 @@ }, "VLAN_ADD_PORT_THAT_IS_UNTAGGED": { "desc": "Add port that is already untagged", - "eStr": "Port is an untagged member in another VLAN" + "eStr": "Port can be untagged in at most one VLAN" }, "VLAN_ADD_PORT_THAT_IS_ROUTER_INTERFACE": { "desc": "Add to Vlan port that is router interface", diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json index 04bb54ed9f..642ec284b7 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json @@ -928,7 +928,7 @@ }, { "port": "Ethernet1", - "tagging_mode": "tagged", + "tagging_mode": "untagged", "name": "Vlan11" } ] From 168580a43a13f70ade785dd0ca725b25d3280317 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Wed, 6 Mar 2024 13:21:46 +0200 Subject: [PATCH 07/10] config-engine tests fixes Fixed Problems/New Changes: src/sonic-yang-models/tests/files/sample_config_db.json Ethernet1 is member of PO3 which is already in Vlan Ethernet2 is member of PO4, so add it to Vlan src/sonic-config-engine/tests/simple-sample-graph-case-remap-disabled.xml src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled-no-tunnel-attributes.xml src/sonic-config-engine/tests/simple-sample-graph-case-remap-enabled.xml src/sonic-config-engine/tests/simple-sample-graph-case.xml fortyGigE0/4 is member of PortChannel01, so add port-channel to vlan src/sonic-config-engine/tests/t0-sample-graph-two-mgmt.xml src/sonic-config-engine/tests/t0-sample-graph.xml fortyGigE0/112;fortyGigE0/116;fortyGigE0/120 are members of PortChannel01;PortChannel02;PortChannel03, so add port-channel. src/sonic-config-engine/tests/test_minigraph_case.py Ethernet4 is member of PortChannel01, update test_minigraph_portchannels ~ Signed-off-by: matiAlfaro --- .../tests/test_minigraph_case.py | 2 +- src/sonic-yang-models/yang-models/sonic-vlan.yang | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py index 70c5c41033..dbc19dd85f 100644 --- a/src/sonic-config-engine/tests/test_minigraph_case.py +++ b/src/sonic-config-engine/tests/test_minigraph_case.py @@ -129,7 +129,7 @@ class TestCfgGenCaseInsensitive(TestCase): output = self.run_script(argument) expected = { 'Vlan1000|Ethernet8': {'tagging_mode': 'untagged'}, - 'Vlan2000|Ethernet4': {'tagging_mode': 'untagged'} + 'Vlan2000|PortChannel01': {'tagging_mode': 'untagged'} } self.assertEqual( utils.to_dict(output.strip()), diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index 5b89f82adb..e20a9e2b08 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -191,6 +191,10 @@ module sonic-vlan { { error-message "Vlan1 is default Vlan"; } + /*must "(concat('Vlan', current()) = ../name) or ('Vlan' + current() = ../name)" + { + error-message "The vlanid must correspond to the VLAN name"; + }*/ } leaf vlanid { type uint16 { @@ -200,9 +204,6 @@ module sonic-vlan { { error-message "Vlan1 is default Vlan"; } - must "(concat('Vlan', current()) = ../name)" { - error-message "The vlanid must correspond to the VLAN name"; - } } leaf alias { @@ -273,12 +274,12 @@ module sonic-vlan { must "not(/lag:sonic-portchannel/lag:PORTCHANNEL_MEMBER/lag:PORTCHANNEL_MEMBER_LIST[lag:port=current()]/lag:name)" { error-message "Port is a member of a port channel."; } - must "not(/vlan:sonic-vlan/vlan:VLAN_MEMBER/vlan:VLAN_MEMBER_LIST[vlan:port=current() and vlan:tagging_mode='untagged' and vlan:name!=current()/../name])" { - error-message "Port is an untagged member in another VLAN."; - } must "not(/intf:sonic-interface/intf:INTERFACE/intf:INTERFACE_LIST[intf:name=current()])" { error-message "Port is a router interface."; } + must "count(../../vlan:VLAN_MEMBER_LIST[vlan:port=current() and vlan:tagging_mode='untagged']) <= 1" { + error-message "Port can be untagged in at most one VLAN."; + } } leaf tagging_mode { From e975e9e37a6b2e395f1b11728b672bf088547b4d Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Thu, 7 Mar 2024 08:49:37 +0200 Subject: [PATCH 08/10] fix vlan name & id match must Signed-off-by: matiAlfaro --- .../tests/yang_model_tests/tests/vlan.json | 3 ++- src/sonic-yang-models/yang-models/sonic-vlan.yang | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json index e653292efd..d7f1d551b0 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json @@ -96,7 +96,8 @@ "eStr": "Vlan1 is default Vlan" }, "VLAN_CREATE_VLAN_WITH_MISSMATCHING_NAME": { - "desc": "Create Vlan so that name doesn't match id" + "desc": "Create Vlan so that name doesn't match id", + "eStr": "The vlanid must correspond to the VLAN name" }, "IP_PREFIX_WITHOUT_CREATNG_VLAN": { "desc": "Create IP on not created Vlan", diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index e20a9e2b08..ad1b184b2d 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -191,10 +191,6 @@ module sonic-vlan { { error-message "Vlan1 is default Vlan"; } - /*must "(concat('Vlan', current()) = ../name) or ('Vlan' + current() = ../name)" - { - error-message "The vlanid must correspond to the VLAN name"; - }*/ } leaf vlanid { type uint16 { @@ -204,6 +200,9 @@ module sonic-vlan { { error-message "Vlan1 is default Vlan"; } + must "substring-after(../name, 'Vlan') = current()"{ + error-message "The vlanid must correspond to the VLAN name"; + } } leaf alias { From f5feebab288cfa0049feabf1b7374f292d6052d1 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Thu, 7 Mar 2024 09:19:03 +0200 Subject: [PATCH 09/10] update revision Signed-off-by: matiAlfaro --- src/sonic-yang-models/yang-models/sonic-vlan.yang | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index ad1b184b2d..09094a3fa8 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -43,6 +43,10 @@ module sonic-vlan { description "VLAN yang Module for SONiC OS"; + revision 2024-03-07 { + description "Add Constraints that are implemented in CLI"; + } + revision 2021-04-22 { description "Modify Vlan Member to include PortChannel along with Port"; } From c2c9a62130906e2f28828eab9f98fb1683142ed6 Mon Sep 17 00:00:00 2001 From: matiAlfaro Date: Mon, 25 Mar 2024 15:32:53 +0200 Subject: [PATCH 10/10] implement review comments Signed-off-by: matiAlfaro --- .../tests/yang_model_tests/tests/vlan.json | 4 -- .../yang_model_tests/tests_config/vlan.json | 11 ---- .../yang-models/sonic-vlan.yang | 51 ++++++++----------- 3 files changed, 21 insertions(+), 45 deletions(-) diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json index d7f1d551b0..a795a7e3b3 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests/vlan.json @@ -91,10 +91,6 @@ "desc": "Vlan members without Creating Vlan", "eStrKey": "LeafRef" }, - "VLAN_CREATE_DEFAULT_VLAN":{ - "desc": "Try to create default Vlan (1)", - "eStr": "Vlan1 is default Vlan" - }, "VLAN_CREATE_VLAN_WITH_MISSMATCHING_NAME": { "desc": "Create Vlan so that name doesn't match id", "eStr": "The vlanid must correspond to the VLAN name" diff --git a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json index 642ec284b7..b4288472cf 100644 --- a/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json +++ b/src/sonic-yang-models/tests/yang_model_tests/tests_config/vlan.json @@ -744,17 +744,6 @@ } } }, - "VLAN_CREATE_DEFAULT_VLAN": { - "sonic-vlan:sonic-vlan": { - "sonic-vlan:VLAN": { - "VLAN_LIST": [ - { - "name": "Vlan1" - } - ] - } - } - }, "VLAN_CREATE_VLAN_WITH_MISSMATCHING_NAME": { "sonic-vlan:sonic-vlan": { "sonic-vlan:VLAN": { diff --git a/src/sonic-yang-models/yang-models/sonic-vlan.yang b/src/sonic-yang-models/yang-models/sonic-vlan.yang index 09094a3fa8..f0f1aa603e 100644 --- a/src/sonic-yang-models/yang-models/sonic-vlan.yang +++ b/src/sonic-yang-models/yang-models/sonic-vlan.yang @@ -1,6 +1,6 @@ module sonic-vlan { - yang-version 1.1; + yang-version 1.1; namespace "http://github.com/sonic-net/sonic-vlan"; prefix vlan; @@ -91,7 +91,7 @@ module sonic-vlan { error-app-tag nat-zone-invalid; } } - default "0"; + default "0"; } leaf mpls { description "Enable/disable MPLS routing for the vlan interface"; @@ -120,10 +120,10 @@ module sonic-vlan { } - leaf loopback_action { - description "Packet action when a packet ingress and gets routed on the same IP interface"; - type stypes:loopback_action; - } + leaf loopback_action { + description "Packet action when a packet ingress and gets routed on the same IP interface"; + type stypes:loopback_action; + } } /* end of VLAN_INTERFACE_LIST */ @@ -160,9 +160,9 @@ module sonic-vlan { leaf family { /* family leaf needed for backward compatibility - Both ip4 and ip6 address are string in IETF RFC 6021, - so must statement can check based on : or ., family - should be IPv4 or IPv6 according. + Both ip4 and ip6 address are string in IETF RFC 6021, + so must statement can check based on : or ., family + should be IPv4 or IPv6 according. */ must "(contains(../ip-prefix, ':') and current()='IPv6') or @@ -189,24 +189,16 @@ module sonic-vlan { leaf name { type string { - pattern 'Vlan(409[0-5]|40[0-8][0-9]|[1-3][0-9]{3}|[1-9][0-9]{2}|[1-9][0-9]|[1-9])'; + pattern 'Vlan(409[0-5]|40[0-8][0-9]|[1-3][0-9]{3}|[1-9][0-9]{2}|[1-9][0-9]|[2-9])'; } - must "not(current() = 'Vlan1')" - { - error-message "Vlan1 is default Vlan"; - } - } + } leaf vlanid { type uint16 { - range 1..4094; - } - must "not(current() = 1)" - { - error-message "Vlan1 is default Vlan"; + range 2..4094; } must "substring-after(../name, 'Vlan') = current()"{ - error-message "The vlanid must correspond to the VLAN name"; - } + error-message "The vlanid must correspond to the VLAN name"; + } } leaf alias { @@ -272,28 +264,27 @@ module sonic-vlan { } must "not(/sms:sonic-mirror-session/sms:MIRROR_SESSION/sms:MIRROR_SESSION_LIST[sms:dst_port=current()])" { - error-message "Port is used as a destination port in a mirror session."; + error-message "Port is used as a destination port in a mirror session."; } must "not(/lag:sonic-portchannel/lag:PORTCHANNEL_MEMBER/lag:PORTCHANNEL_MEMBER_LIST[lag:port=current()]/lag:name)" { - error-message "Port is a member of a port channel."; + error-message "Port is a member of a port channel."; } must "not(/intf:sonic-interface/intf:INTERFACE/intf:INTERFACE_LIST[intf:name=current()])" { - error-message "Port is a router interface."; - } + error-message "Port is a router interface."; + } must "count(../../vlan:VLAN_MEMBER_LIST[vlan:port=current() and vlan:tagging_mode='untagged']) <= 1" { error-message "Port can be untagged in at most one VLAN."; } } - leaf tagging_mode { mandatory true; type stypes:vlan_tagging_mode; } } - /* end of list VLAN_MEMBER_LIST */ + /* end of list VLAN_MEMBER_LIST */ } - /* end of container VLAN_MEMBER */ + /* end of container VLAN_MEMBER */ } - /* end of container sonic-vlan */ + /* end of container sonic-vlan */ } /* end of module sonic-vlan */