From a89b8ed58daa7ea6ec5f40631c926cdc03a89432 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Sun, 1 Nov 2020 02:11:02 -0800 Subject: [PATCH] [sub intf] Inherit parent port mtu changes (#1479) Sub interface inherits mtu from parent physical port or port channel according to https://github.com/Azure/SONiC/blob/master/doc/subport/sonic-sub-port-intf-hld.md#1-requirements This inheritance should include the scenario of mtu changes on parent physical port or port channel. Signed-off-by: Wenda Ni --- orchagent/portsorch.cpp | 31 ++++++++++++++ orchagent/portsorch.h | 4 +- tests/test_sub_port_intf.py | 85 ++++++++++++++++++++++++++++--------- 3 files changed, 98 insertions(+), 22 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 3a68dab4bf16..c6aa40ecc6bc 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -715,6 +715,33 @@ bool PortsOrch::removeSubPort(const string &alias) return true; } +void PortsOrch::updateChildPortsMtu(const Port &p, const uint32_t mtu) +{ + if (p.m_type != Port::PHY && p.m_type != Port::LAG) + { + return; + } + + for (const auto &child_port : p.m_child_ports) + { + Port subp; + if (!getPort(child_port, subp)) + { + SWSS_LOG_WARN("Sub interface %s Port object not found", child_port.c_str()); + continue; + } + + subp.m_mtu = mtu; + m_portList[child_port] = subp; + SWSS_LOG_NOTICE("Sub interface %s inherits mtu change %u from parent port %s", child_port.c_str(), mtu, p.m_alias.c_str()); + + if (subp.m_rif_id) + { + gIntfsOrch->setRouterIntfsMtu(subp); + } + } +} + void PortsOrch::setPort(string alias, Port p) { m_portList[alias] = p; @@ -2317,6 +2344,8 @@ void PortsOrch::doPortTask(Consumer &consumer) { gIntfsOrch->setRouterIntfsMtu(p); } + // Sub interfaces inherit parent physical port mtu + updateChildPortsMtu(p, mtu); } else { @@ -2860,6 +2889,8 @@ void PortsOrch::doLagTask(Consumer &consumer) { gIntfsOrch->setRouterIntfsMtu(l); } + // Sub interfaces inherit parent LAG mtu + updateChildPortsMtu(l, mtu); } if (!learn_mode.empty() && (l.m_learn_mode != learn_mode)) diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 806a63e53af3..4d7ebb25b15b 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -113,7 +113,7 @@ class PortsOrch : public Orch, public Subject acl_stage_type_t acl_stage); bool bindUnbindAclTableGroup(Port &port, bool ingress, - bool bind); + bool bind); bool getPortPfc(sai_object_id_t portId, uint8_t *pfc_bitmask); bool setPortPfc(sai_object_id_t portId, uint8_t pfc_bitmask); @@ -126,6 +126,8 @@ class PortsOrch : public Orch, public Subject bool addSubPort(Port &port, const string &alias, const bool &adminUp = true, const uint32_t &mtu = 0); bool removeSubPort(const string &alias); void getLagMember(Port &lag, vector &portv); + void updateChildPortsMtu(const Port &p, const uint32_t mtu); + private: unique_ptr m_counterTable; unique_ptr
m_counterLagTable; diff --git a/tests/test_sub_port_intf.py b/tests/test_sub_port_intf.py index e4d6adb95017..124514ec071c 100644 --- a/tests/test_sub_port_intf.py +++ b/tests/test_sub_port_intf.py @@ -3,6 +3,8 @@ from dvslib.dvs_common import wait_for_result from dvslib.dvs_database import DVSDatabase +DEFAULT_MTU = "9100" + CFG_VLAN_SUB_INTF_TABLE_NAME = "VLAN_SUB_INTERFACE" CFG_PORT_TABLE_NAME = "PORT" CFG_LAG_TABLE_NAME = "PORTCHANNEL" @@ -41,6 +43,7 @@ def connect_dbs(self, dvs): self.asic_db = dvs.get_asic_db() self.config_db = dvs.get_config_db() self.state_db = dvs.get_state_db() + dvs.setup_db() def set_parent_port_admin_status(self, dvs, port_name, status): fvs = {ADMIN_STATUS: status} @@ -52,7 +55,7 @@ def set_parent_port_admin_status(self, dvs, port_name, status): tbl_name = CFG_LAG_TABLE_NAME self.config_db.create_entry(tbl_name, port_name, fvs) - # follow the treatment in TestSubPortIntf::set_admin_status + # follow the treatment in TestRouterInterface::set_admin_status if tbl_name == CFG_LAG_TABLE_NAME: dvs.runcmd("bash -c 'echo " + ("1" if status == "up" else "0") + \ " > /sys/class/net/" + port_name + "/carrier'") @@ -76,14 +79,14 @@ def set_sub_port_intf_admin_status(self, sub_port_intf_name, status): def remove_sub_port_intf_profile(self, sub_port_intf_name): self.config_db.delete_entry(CFG_VLAN_SUB_INTF_TABLE_NAME, sub_port_intf_name) - def verify_sub_port_intf_removal(self, rif_oid): + def check_sub_port_intf_profile_removal(self, rif_oid): self.asic_db.wait_for_deleted_keys(ASIC_RIF_TABLE, [rif_oid]) def remove_sub_port_intf_ip_addr(self, sub_port_intf_name, ip_addr): key = "{}|{}".format(sub_port_intf_name, ip_addr) self.config_db.delete_entry(CFG_VLAN_SUB_INTF_TABLE_NAME, key) - def verify_sub_port_intf_ip_addr_removal(self, sub_port_intf_name, ip_addrs): + def check_sub_port_intf_ip_addr_removal(self, sub_port_intf_name, ip_addrs): interfaces = ["{}:{}".format(sub_port_intf_name, addr) for addr in ip_addrs] self.app_db.wait_for_deleted_keys(APP_INTF_TABLE_NAME, interfaces) @@ -161,14 +164,14 @@ def _test_sub_port_intf_creation(self, dvs, sub_port_intf_name): "SAI_ROUTER_INTERFACE_ATTR_OUTER_VLAN_ID": "{}".format(vlan_id), "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE": "true", "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE": "true", - "SAI_ROUTER_INTERFACE_ATTR_MTU": "9100", + "SAI_ROUTER_INTERFACE_ATTR_MTU": DEFAULT_MTU, } rif_oid = self.get_newly_created_oid(ASIC_RIF_TABLE, old_rif_oids) self.check_sub_port_intf_fvs(self.asic_db, ASIC_RIF_TABLE, rif_oid, fv_dict) # Remove a sub port interface self.remove_sub_port_intf_profile(sub_port_intf_name) - self.verify_sub_port_intf_removal(rif_oid) + self.check_sub_port_intf_profile_removal(rif_oid) def test_sub_port_intf_creation(self, dvs): self.connect_dbs(dvs) @@ -219,13 +222,13 @@ def _test_sub_port_intf_add_ip_addrs(self, dvs, sub_port_intf_name): # Remove IP addresses self.remove_sub_port_intf_ip_addr(sub_port_intf_name, self.IPV4_ADDR_UNDER_TEST) self.remove_sub_port_intf_ip_addr(sub_port_intf_name, self.IPV6_ADDR_UNDER_TEST) - self.verify_sub_port_intf_ip_addr_removal(sub_port_intf_name, - [self.IPV4_ADDR_UNDER_TEST, - self.IPV6_ADDR_UNDER_TEST]) + self.check_sub_port_intf_ip_addr_removal(sub_port_intf_name, + [self.IPV4_ADDR_UNDER_TEST, + self.IPV6_ADDR_UNDER_TEST]) # Remove a sub port interface self.remove_sub_port_intf_profile(sub_port_intf_name) - self.verify_sub_port_intf_removal(rif_oid) + self.check_sub_port_intf_profile_removal(rif_oid) def test_sub_port_intf_add_ip_addrs(self, dvs): self.connect_dbs(dvs) @@ -253,7 +256,7 @@ def _test_sub_port_intf_admin_status_change(self, dvs, sub_port_intf_name): fv_dict = { "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE": "true", "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE": "true", - "SAI_ROUTER_INTERFACE_ATTR_MTU": "9100", + "SAI_ROUTER_INTERFACE_ATTR_MTU": DEFAULT_MTU, } rif_oid = self.get_newly_created_oid(ASIC_RIF_TABLE, old_rif_oids) self.check_sub_port_intf_fvs(self.asic_db, ASIC_RIF_TABLE, rif_oid, fv_dict) @@ -271,7 +274,7 @@ def _test_sub_port_intf_admin_status_change(self, dvs, sub_port_intf_name): fv_dict = { "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE": "false", "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE": "false", - "SAI_ROUTER_INTERFACE_ATTR_MTU": "9100", + "SAI_ROUTER_INTERFACE_ATTR_MTU": DEFAULT_MTU, } rif_oid = self.get_newly_created_oid(ASIC_RIF_TABLE, old_rif_oids) self.check_sub_port_intf_fvs(self.asic_db, ASIC_RIF_TABLE, rif_oid, fv_dict) @@ -289,7 +292,7 @@ def _test_sub_port_intf_admin_status_change(self, dvs, sub_port_intf_name): fv_dict = { "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V4_STATE": "true", "SAI_ROUTER_INTERFACE_ATTR_ADMIN_V6_STATE": "true", - "SAI_ROUTER_INTERFACE_ATTR_MTU": "9100", + "SAI_ROUTER_INTERFACE_ATTR_MTU": DEFAULT_MTU, } rif_oid = self.get_newly_created_oid(ASIC_RIF_TABLE, old_rif_oids) self.check_sub_port_intf_fvs(self.asic_db, ASIC_RIF_TABLE, rif_oid, fv_dict) @@ -297,13 +300,13 @@ def _test_sub_port_intf_admin_status_change(self, dvs, sub_port_intf_name): # Remove IP addresses self.remove_sub_port_intf_ip_addr(sub_port_intf_name, self.IPV4_ADDR_UNDER_TEST) self.remove_sub_port_intf_ip_addr(sub_port_intf_name, self.IPV6_ADDR_UNDER_TEST) - self.verify_sub_port_intf_ip_addr_removal(sub_port_intf_name, - [self.IPV4_ADDR_UNDER_TEST, - self.IPV6_ADDR_UNDER_TEST]) + self.check_sub_port_intf_ip_addr_removal(sub_port_intf_name, + [self.IPV4_ADDR_UNDER_TEST, + self.IPV6_ADDR_UNDER_TEST]) # Remove a sub port interface self.remove_sub_port_intf_profile(sub_port_intf_name) - self.verify_sub_port_intf_removal(rif_oid) + self.check_sub_port_intf_profile_removal(rif_oid) def test_sub_port_intf_admin_status_change(self, dvs): self.connect_dbs(dvs) @@ -362,7 +365,7 @@ def _test_sub_port_intf_remove_ip_addrs(self, dvs, sub_port_intf_name): # Remove a sub port interface self.remove_sub_port_intf_profile(sub_port_intf_name) - self.verify_sub_port_intf_removal(rif_oid) + self.check_sub_port_intf_profile_removal(rif_oid) def test_sub_port_intf_remove_ip_addrs(self, dvs): self.connect_dbs(dvs) @@ -402,13 +405,13 @@ def _test_sub_port_intf_removal(self, dvs, sub_port_intf_name): # Remove IP addresses self.remove_sub_port_intf_ip_addr(sub_port_intf_name, self.IPV4_ADDR_UNDER_TEST) self.remove_sub_port_intf_ip_addr(sub_port_intf_name, self.IPV6_ADDR_UNDER_TEST) - self.verify_sub_port_intf_ip_addr_removal(sub_port_intf_name, - [self.IPV4_ADDR_UNDER_TEST, - self.IPV6_ADDR_UNDER_TEST]) + self.check_sub_port_intf_ip_addr_removal(sub_port_intf_name, + [self.IPV4_ADDR_UNDER_TEST, + self.IPV6_ADDR_UNDER_TEST]) # Remove a sub port interface self.remove_sub_port_intf_profile(sub_port_intf_name) - self.verify_sub_port_intf_removal(rif_oid) + self.check_sub_port_intf_profile_removal(rif_oid) # Verify that sub port interface state ok is removed from STATE_DB by Intfmgrd self.check_sub_port_intf_key_removal(self.state_db, state_tbl_name, sub_port_intf_name) @@ -425,6 +428,46 @@ def test_sub_port_intf_removal(self, dvs): self._test_sub_port_intf_removal(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST) self._test_sub_port_intf_removal(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST) + def _test_sub_port_intf_mtu(self, dvs, sub_port_intf_name): + substrs = sub_port_intf_name.split(VLAN_SUB_INTERFACE_SEPARATOR) + parent_port = substrs[0] + + old_rif_oids = self.get_oids(ASIC_RIF_TABLE) + + self.set_parent_port_admin_status(dvs, parent_port, "up") + self.create_sub_port_intf_profile(sub_port_intf_name) + + rif_oid = self.get_newly_created_oid(ASIC_RIF_TABLE, old_rif_oids) + + # Change parent port mtu + mtu = "8888" + dvs.set_mtu(parent_port, mtu) + + # Verify that sub port router interface entry in ASIC_DB has the updated mtu + fv_dict = { + "SAI_ROUTER_INTERFACE_ATTR_MTU": mtu, + } + self.check_sub_port_intf_fvs(self.asic_db, ASIC_RIF_TABLE, rif_oid, fv_dict) + + # Restore parent port mtu + dvs.set_mtu(parent_port, DEFAULT_MTU) + + # Verify that sub port router interface entry in ASIC_DB has the default mtu + fv_dict = { + "SAI_ROUTER_INTERFACE_ATTR_MTU": DEFAULT_MTU, + } + self.check_sub_port_intf_fvs(self.asic_db, ASIC_RIF_TABLE, rif_oid, fv_dict) + + # Remove a sub port interface + self.remove_sub_port_intf_profile(sub_port_intf_name) + self.check_sub_port_intf_profile_removal(rif_oid) + + def test_sub_port_intf_mtu(self, dvs): + self.connect_dbs(dvs) + + self._test_sub_port_intf_mtu(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST) + self._test_sub_port_intf_mtu(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST) + # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying