Skip to content

Commit

Permalink
[sub intf] Inherit parent port mtu changes (sonic-net#1479)
Browse files Browse the repository at this point in the history 
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 <wonda.ni@gmail.com>
  • Loading branch information
@wendani
wendani authored Nov 1, 2020
1 parent d7643f2 commit a89b8ed
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 22 deletions.
31 changes: 31 additions & 0 deletions orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2317,6 +2344,8 @@ void PortsOrch::doPortTask(Consumer &consumer)
{
gIntfsOrch->setRouterIntfsMtu(p);
}
// Sub interfaces inherit parent physical port mtu
updateChildPortsMtu(p, mtu);
}
else
{
Expand Down Expand Up @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion orchagent/portsorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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<Port> &portv);
void updateChildPortsMtu(const Port &p, const uint32_t mtu);

private:
unique_ptr<Table> m_counterTable;
unique_ptr<Table> m_counterLagTable;
Expand Down
85 changes: 64 additions & 21 deletions tests/test_sub_port_intf.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}
Expand All @@ -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'")
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -289,21 +292,21 @@ 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)

# 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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit a89b8ed

Please sign in to comment.