Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sub intf] Inherit parent port mtu changes #1479

Merged
merged 3 commits into from
Nov 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,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 @@ -2318,6 +2345,8 @@ void PortsOrch::doPortTask(Consumer &consumer)
{
gIntfsOrch->setRouterIntfsMtu(p);
}
// Sub interfaces inherit parent physical port mtu
updateChildPortsMtu(p, mtu);
}
else
{
Expand Down Expand Up @@ -2861,6 +2890,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
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
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